From d4c72697f21cd64be116f7ec6ebc762042a67890 Mon Sep 17 00:00:00 2001 From: Timothy Date: Sat, 22 Aug 2026 00:33:18 +0000 Subject: [PATCH] feat(780): commit a ruff config and enforce it in CI (#813) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Python lint here was a property of the operator's laptop: the global instructions say to run ruff, no workflow ran it, and with no committed config ruff fell back to whichever ~/.config/ruff/ruff.toml the machine happened to have. - ruff.toml at the root, pinned ruff==0.12.11 in the script-tests job. - Both lint steps pass an EXPLICIT population from `git ls-files` with `--no-force-exclude`, never `ruff check .` — an `exclude` empties a discovery-based run into a GREEN one (top level empties both commands, [lint] empties check, [format] empties format --check), and `ruff check .` over zero files exits 0 with only a stderr warning. Guarded by an empty-population arm. - Tree clean: 74 findings at 706674272, 57 fixed in code, 17 per-site noqa with reasons inline. S105 deliberately per-site, not a directory blanket. RUF100 selected so a suppression that suppresses nothing is itself a finding. - pyright stays ungated; reasoning in the record. Both steps witnessed red on the runner against the shipped bodies: run 2173 job 9176 (ruff check) and run 2170 job 9163 (ruff format --check). Docs: new record ci.python-lint-ruff-config-committed, ci.script-tests-job cross-ref, docs/ci-cd.md (also correcting a stale ~190-tests/~10s figure to the measured 773 tests / ~4.5 min), docs/defect-shapes-773.md §5.2 resolved. fixes #780 Co-authored-by: Timothy --- .gitea/workflows/pr-checks.yml | 100 +- docs/ci-cd.md | 32 +- docs/decisions/README.md | 3 +- .../ci/python-lint-ruff-config-committed.md | 97 ++ docs/decisions/records/ci/script-tests-job.md | 10 +- docs/defect-shapes-773.md | 38 + ruff.toml | 59 ++ scripts/decisions_validate.py | 9 +- scripts/generate-endpoint-index.py | 8 +- scripts/mcp_smoke.py | 48 +- scripts/scripted-schedules/entrypoint.py | 18 +- scripts/tests/conftest.py | 4 +- scripts/tests/test_bom_guard_detection.py | 41 +- scripts/tests/test_ci_dropped_step_guard.py | 72 +- scripts/tests/test_ci_image_pin_population.py | 6 +- .../tests/test_ci_release_path_scan_job.py | 48 +- scripts/tests/test_decisions_validate.py | 21 +- scripts/tests/test_guard_inventory.py | 42 +- scripts/tests/test_hook_fire_log.py | 526 ++++++---- scripts/tests/test_jq_preflight.py | 130 +-- .../tests/test_merge_consent_base_change.py | 47 +- scripts/tests/test_merge_consent_exemption.py | 116 ++- .../test_merge_consent_required_check.py | 167 ++-- scripts/tests/test_post_review_verdict.py | 23 +- scripts/tests/test_pr_changed_files.py | 944 +++++++++++------- scripts/tests/test_prove_fix.py | 73 +- scripts/tests/test_remote_state_inventory.py | 51 +- 27 files changed, 1737 insertions(+), 996 deletions(-) create mode 100644 docs/decisions/records/ci/python-lint-ruff-config-committed.md create mode 100644 ruff.toml diff --git a/.gitea/workflows/pr-checks.yml b/.gitea/workflows/pr-checks.yml index 50afae728..a667e9b59 100644 --- a/.gitea/workflows/pr-checks.yml +++ b/.gitea/workflows/pr-checks.yml @@ -359,7 +359,7 @@ jobs: [ "$failed" -eq 0 ] script-tests: - name: Script tests (pytest) + name: Script lint and tests (ruff + pytest) runs-on: small if: github.event_name == 'pull_request' steps: @@ -369,6 +369,90 @@ jobs: uses: actions/setup-python@v5 with: python-version: '3.x' + # Preflight, not an install (ersatztv#390 removed run-time `apt-get` from CI on purpose). + # Two consumers need `git`: the lint steps below derive their population from `git ls-files`, + # and test_post_review_verdict.py / test_merge_consent_exemption.py exec the REAL + # post-review-verdict.sh / pretooluse-merge-consent.sh. `curl` those tests shim on PATH; `jq` + # and `git` they do NOT. It stays AHEAD of the lint steps, not merely ahead of pytest: without + # it, a missing git reaches the lint steps as an empty population, which they report as a + # population problem. One actionable line beats a misdirected one, and beats the wall of + # unattributable assertion failures the suite produces without git. + - name: Preflight external tools + run: | + if ! command -v git >/dev/null 2>&1; then + echo "::error::script-tests needs git on PATH but it is absent. The lint steps derive" \ + "their population from it and the suite execs real shell scripts that use it." \ + "Bake it into the runner image rather than apt-get installing here (ersatztv#390)." + exit 1 + fi + echo "Preflight OK: $(git --version)" + # ersatztv#780. Lint runs EARLY — after the git preflight it depends on, but before the test + # dependencies, the jq preflight and the ~4-minute pytest run. A style red therefore arrives in + # seconds, and, more importantly, the lint does not sit behind `Preflight jq version`: that is + # an `--expect` tripwire, so a runner jq bump would take the lint dark for as long as the jq + # contract is broken, under a red that says "jq". + # + # The version is PINNED: an unpinned ruff makes the verdict a function of whenever the job ran + # — the same environment-divergence the committed ruff.toml exists to close. Bumping it is a + # deliberate PR (new rules may fire), exactly like the jq pin below. `pytest`/`pyyaml` are + # deliberately NOT pinned: a pytest release does not add assertions to your suite, a ruff + # release adds rules to your lint. + - name: Install ruff + run: python3 -m pip install --disable-pip-version-check --quiet 'ruff==0.12.11' + # POPULATION. Both steps lint an EXPLICIT list from `git ls-files`, never `ruff check .`, and + # pass `--no-force-exclude`. Measured with ruff 0.12.11 and `exclude = ["scripts/**"]` — a + # per-FILE pattern, because `exclude` matches per file: a bare `["scripts"]` still works at the + # top level but matches nothing under `[lint]`/`[format]`. The subject is a planted tracked file + # holding an unused import, a hardcoded credential and a formatting error. GREEN means the gate + # was silently off: + # + # DISCOVERY FORM EXPLICIT FORM (what ships) + # exclude scope check . format --check . check format --check + # top-level GREEN GREEN red red + # [lint] GREEN red red red + # [format] red GREEN red red + # top + force-exclude GREEN GREEN red red <- with the flag + # GREEN GREEN <- without it + # + # Only the top-level scope empties BOTH discovery commands; `[lint]` empties `check` and + # `[format]` empties `format --check`, so in those two the job would still redden on the other + # step. `[format]` is where a line appended to ruff.toml lands, by TOML rules. `include = []`, + # `extend-exclude` and a nested `scripts/ruff.toml` behave the same way and are equally inert + # against the explicit form. The last row is the whole reason for `--no-force-exclude`: + # `force-exclude = true` re-applies excludes to explicitly-passed paths, and is the one setting + # that reaches explicitly-passed paths at all. + # + # `ruff check .` over an empty tree exits **0** with only a stderr warning, so every GREEN above + # is a gate that was switched off without a red. + # + # This also derives the population from source rather than from the filesystem + # (docs/decisions/records/testing/guard-derives-population-from-source.md) and covers + # tracked-but-gitignored files, which `ruff check .` skips. The empty-population arm is the + # anti-vacuity check: a completeness check whose population is empty reports that it proved + # everything. What it does NOT cover: an emptied RULE set. `select = []` silences every selected + # rule, so the `ruff check` step goes green over any lint violation (a syntax error still reds) + # while printing a reassuring file count. + # `ruff format --check` is unaffected, because formatting is not rule-selected. So half the + # gate is killable by a config edit, and only a human reading that edit catches it. + - name: Lint scripts (ruff check) + run: | + mapfile -d '' -t PYFILES < <(git ls-files -z '*.py' '*.pyi' '*.ipynb') + if [ "${#PYFILES[@]}" -eq 0 ]; then + echo "::error::the lint population is EMPTY — git tracks no Python files. Either the" \ + "checkout is wrong or the glob is. A lint over nothing passes; see ersatztv#780." + exit 1 + fi + echo "Linting ${#PYFILES[@]} tracked Python files" + python3 -m ruff check --no-force-exclude -- "${PYFILES[@]}" + - name: Lint scripts (ruff format --check) + run: | + mapfile -d '' -t PYFILES < <(git ls-files -z '*.py' '*.pyi' '*.ipynb') + if [ "${#PYFILES[@]}" -eq 0 ]; then + echo "::error::the format population is EMPTY — git tracks no Python files. See ersatztv#780." + exit 1 + fi + echo "Format-checking ${#PYFILES[@]} tracked Python files" + python3 -m ruff format --check --no-force-exclude -- "${PYFILES[@]}" # pytest + PyYAML. PyYAML is NOT a contradiction of the dependency-free decisions READ path: # `decisions_lib._read_frontmatter` is hand-written precisely so validation runs where nothing # is installed, but the one-shot WRITE path `migrate_decisions_split.py` uses PyYAML by @@ -379,20 +463,6 @@ jobs: # went red in CI on a collection error. - name: Install test dependencies run: python3 -m pip install --disable-pip-version-check --quiet pytest pyyaml - # Preflight, not an install (ersatztv#390 removed run-time `apt-get` from CI on purpose). - # test_post_review_verdict.py and test_merge_consent_exemption.py exec the REAL - # post-review-verdict.sh / pretooluse-merge-consent.sh, which shell out to `jq` ~26 times. - # `curl` those tests shim on PATH; `jq` they do NOT. If it were missing, the suite would fail - # as ~20 opaque assertion errors — this turns that into one actionable line. - - name: Preflight external tools - run: | - if ! command -v git >/dev/null 2>&1; then - echo "::error::script-tests needs git on PATH but it is absent. The suite execs real" \ - "shell scripts that use it. Bake it into the runner image rather than apt-get" \ - "installing here (see ersatztv#390)." - exit 1 - fi - echo "Preflight OK: $(git --version)" # jq gets its OWN step because its VERSION, not merely its presence, is load-bearing # (ersatztv#648). `--expect` makes this a TRIPWIRE: scripts/tests exercises the jq 1.6 code path # only because this runner ships 1.6, so an upgrade would silently delete that coverage — and diff --git a/docs/ci-cd.md b/docs/ci-cd.md index 6def410e3..34f85f4ab 100644 --- a/docs/ci-cd.md +++ b/docs/ci-cd.md @@ -928,14 +928,14 @@ this job (`prove-fix.sh` exits 5 on a harness/git failure or a signal, and the j error), so what the exit-code discipline buys is the other way round — a **green** here means a claim was witnessed, never that a run was cancelled or broke. -### `script-tests` job (`Script tests (pytest)`, PR-only — in `pr-checks.yml`) +### `script-tests` job (`Script lint and tests (ruff + pytest)`, PR-only — in `pr-checks.yml`) > Reddens the run on failure, but like the other `pr-checks.yml` gates it is **not** one of the > three required status checks on `main` (`Build & test (.NET)`, `EF migration integrity`, > `review-verdict/h10`). Promoting it to required is a branch-protection change, tracked separately. Runs the repository's Python test suite: `PYTHONPATH=. python3 -m pytest scripts/tests -q` -(~190 tests at time of writing, ~10s; the suite grows, so treat the figure as indicative). It covers the decision-corpus parser/validator/catalog builder, the ersatztv#610 +(773 tests at `706674272`, ~4.5 min; the suite grows fast — it was ~190 tests / ~10s when this job landed in #631 — so treat the figure as a dated snapshot, not a budget). It covers the decision-corpus parser/validator/catalog builder, the ersatztv#610 migration-equivalence harness, the merge-consent exemption logic and the ersatztv#622 review-verdict poster. @@ -956,7 +956,33 @@ name keeps a real failure unambiguous. input set spans more than one directory — `test_post_review_verdict.py` and `test_merge_consent_exemption.py` execute the real `scripts/post-review-verdict.sh` and `.claude/hooks/pretooluse-merge-consent.sh` — so a `scripts/**` filter would silently miss a -`.claude/hooks/**` edit. At ~10s, a filter buys nothing but drift. +`.claude/hooks/**` edit. The reason is the input set, not the cost: the suite was ~10s when that was +decided and is now ~4.5 min, and it would still be wrong to filter on `scripts/**`. + +**It also lints (ersatztv#780).** Early in the job it installs a **pinned** `ruff==0.12.11` and runs +`ruff check` and `ruff format --check` against the repo-root `ruff.toml`. Five things are deliberate: + +- The config is **committed**. Without it ruff falls back to whatever `~/.config/ruff/ruff.toml` the + operator's machine has, so a second machine lints this repo differently or not at all. +- The version is **pinned** — an unpinned install makes the verdict a function of when the job ran, + the same divergence one layer up, and the same argument as the `jq` pin below. `pytest`/`pyyaml` + stay unpinned on purpose: a pytest release does not add assertions to your suite, a ruff release + adds rules to your lint. +- Lint runs **before the jq preflight**, and after the `git` one. `Preflight jq version` is a hard + `--expect` tripwire; a lint sitting behind it goes dark for as long as the jq contract is broken, + under a red that says "jq". `Preflight external tools` stays ahead, because the lint steps consume + `git` — without it a missing git reaches them as an empty population and they blame the glob. +- Neither step is `ruff check .`. Both pass an **explicit population** from + `git ls-files -z '*.py' '*.pyi' '*.ipynb'` with `--no-force-exclude`, and fail if that list is + empty. Discovery-based invocation is silently emptied by an `exclude` in the right config scope — + top level empties both commands, `[lint]` empties `check`, `[format]` empties `format --check` + (and `[format]` is where an appended line lands) — and + `ruff check .` over zero files exits **0** with only a stderr warning, so the failure mode is a + green gate. The measured matrix is in `ci.python-lint-ruff-config-committed`. +- `RUF100` is selected, so a `# noqa` that no longer matches anything is itself a finding. + +`pyright` is not gated; the reasoning and the exemption list are in +`ci.python-lint-ruff-config-committed`. **Dependencies: `pytest` and `pyyaml`** — the complete third-party set across `scripts/`, established by an AST import scan rather than by reading the files that looked relevant. PyYAML does **not** diff --git a/docs/decisions/README.md b/docs/decisions/README.md index 1aedeab2f..72f514d92 100644 --- a/docs/decisions/README.md +++ b/docs/decisions/README.md @@ -56,10 +56,11 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera | `ci.monitor-armed-at-pr-open` | Arm a CI monitor on the PR head sha the moment the PR opens, polling the commit-status endpoint — not at the end of the work. | 2026-07-21 | [link](records/ci/monitor-armed-at-pr-open.md) | | `ci.no-host-health-gating` | Push when your work is validated — never SSH to bumblebee to sample load/RAM first, and never hand-schedule around other sessions' runs. | 2026-07-21 | [link](records/ci/no-host-health-gating.md) | | `ci.peak-anon-measurement` | The `test` job's headline memory figure is a sampled high-water mark of cgroup `anon`, produced by `scripts/ci-peak-anon.sh`; `memory.peak` and the end-of-job `anon`/`file` split are kept only as a cache-inflated reference. | 2026-07-19 | [link](records/ci/peak-anon-measurement.md) | +| `ci.python-lint-ruff-config-committed` | The repo commits `ruff.toml`, and the `script-tests` job runs `ruff check` + `ruff format --check` under a PINNED ruff over an EXPLICIT population from `git ls-files`, never `ruff check .`. Never rely on `~/.config/ruff/ruff.toml`, and never add a lint rule to the config without making the tree clean against it in the same PR. | 2026-08-21 | [link](records/ci/python-lint-ruff-config-committed.md) | | `ci.required-job-step-execution-markers` | A step the runner declines to interpolate is DROPPED and the job still concludes `success` (`ci.workflow-run-body-no-expressions`). In `review-verdict.yml` that is fail-CLOSED — the required status is absent and the merge is blocked. In `docker-build.yml`'s `test` and `migrations` it is fail-OPEN: those are the other two required contexts on `main`, so the check reports green having done no work. So in those two jobs every `run:` step that is not `continue-on-error: true` calls `"$GITHUB_WORKSPACE/scripts/ci-step-ran.sh" mark ` as its FIRST act, and the job's LAST step calls `ci-step-ran.sh assert --always --gated `, which fails the job when an expected key was never recorded. PER STEP, not per job: a marker written by the first step only proves the job started, while the drop that costs something is `Test` or the migration replay. The guard carries NO `if:` — the default `success()` is the wanted condition, because a genuine failure in an early step legitimately skips every later one and an `always()` guard would announce a false "these steps never executed" on every ordinary red build; the invariant that makes the omission safe is that the guard is skipped only when an earlier step FAILED, which already fails the job, so guard-skipped implies job-red and every path to a green job runs the guard. Separately and independently, no `${{` OPENER may appear in any `run:` body of those two jobs OR of `build` — the drop mechanism requires the opener, so banning it makes the class unreachable rather than merely caught, and an UNCLOSED opener triggers the same rewrite as a well-formed pair. Pass values in through the step's `env:`, which is interpolated per value. The two halves have DIFFERENT scopes on purpose: markers cover the required pair, while the ban also covers `build`, whose `Smoke + IPTV E2E` step runs AFTER the image is pushed, so a drop there publishes a release candidate that was never booted and that `DeployStack jazz-media` then promotes. `functional-e2e` is delimiter-free but deliberately excluded (advisory by declaration), and `api-docs`/`format` keep one `github.base_ref` each and gate nothing that ships. The ban is enforced on the RELEASE PATH itself, not only in review (#767): a `scan` job runs the PyYAML-based ban test and `build` lists it in `needs:`, so a delimiter means `build` never runs and no image is published. A guard STEP inside `build` was tried first and is wrong — a step cannot protect the job it publishes from, and "my body has no opener so I cannot be dropped" is circular when only the PR-only test enforces that. The pytest in `script-tests` remains, but it is `on: pull_request` and not a required context, so it alone left the tag path unchecked. | 2026-08-10 | [link](records/ci/required-job-step-execution-markers.md) | | `ci.root-screenshot-guard` | The Husky `pre-commit` hook refuses a staged root-level `*.png` (belt-and-suspenders with the `.gitignore` rule); nested `*.png` real assets are unaffected. | 2026-07-12 | [link](records/ci/root-screenshot-guard.md) | | `ci.runner-placement` | No persistent Roslyn compiler server survives a CI build (`UseSharedCompilation=false` etc., runner env + Dockerfile `ENV`); every `services:` container gets its own explicit `--memory`/`--memory-swap`/`--cpus` cap (it does not inherit the job container's). | 2026-07-17 | [link](records/ci/runner-placement.md) | -| `ci.script-tests-job` | The `scripts/tests/` pytest suite runs on every PR as a dedicated `script-tests` job in `pr-checks.yml` (`runs-on: small`, `setup-python` + `pip install pytest`, `PYTHONPATH=. python3 -m pytest scripts/tests -q`), unconditionally rather than behind a `scripts/**` path filter, and **never as a step inside `decisions-guard`** — a job whose reds a standing rule instructs sessions to ignore must never host a gate whose reds are real. Any new CI gate must be reachable by a failure that is unambiguously attributable to it. | 2026-07-26 | [link](records/ci/script-tests-job.md) | +| `ci.script-tests-job` | The `scripts/tests/` pytest suite runs on every PR as a dedicated `script-tests` job in `pr-checks.yml` (`runs-on: small`, `setup-python` + `pip install pytest pyyaml`, `PYTHONPATH=. python3 -m pytest scripts/tests -q`; since #780 it also runs a pinned ruff over a `git ls-files` population first), unconditionally rather than behind a `scripts/**` path filter, and **never as a step inside `decisions-guard`** — a job whose reds a standing rule instructs sessions to ignore must never host a gate whose reds are real. Any new CI gate must be reachable by a failure that is unambiguously attributable to it. | 2026-07-26 | [link](records/ci/script-tests-job.md) | | `ci.shared-pr-file-enumeration` | A PR's complete set of changed file paths is computed by exactly one implementation, `scripts/pr-changed-files.sh`, called by both `.claude/hooks/pretooluse-merge-consent.sh` (advisory — a failure falls through to a human prompt) and `.gitea/workflows/review-verdict.yml` (enforced — a failure must fail closed, because a match here posts the branch-protection-required `review-verdict/h10` status with nobody in the loop). The script owns exhaustiveness (pagination, rename/path validation, head-sha binding, base-ref binding — see `ci.exemption-provenance` — and base-TIP binding, #707: the ref answers "did this PR RETARGET", the tip answers "did the base ADVANCE mid-enumeration", and only the second can see `/pulls/{n}/files` recomputing each offset-paged page against a moved base and dropping a path out of an already-consumed range; both ends of the window are bound, and an advance BEFORE the window is deliberately not an error, or ordinary churn on `main` would fail every open PR) and returns exit 0 only for a verified-complete list; it does NOT classify paths — each caller keeps its own docs-only allow-list, and the two allow-lists differ on purpose and stay separate. | 2026-07-26 | [link](records/ci/shared-pr-file-enumeration.md) | | `ci.small-lane-git-only` | `runs-on: small` is defined by what a job does (git-only), not its usual runtime; the two `docker build` jobs (docker-build.yml, ci-image.yml) move to `ubuntu-latest` because their worst-case memory, not median runtime, was pinning the small lane's per-slot cap. | 2026-07-20 | [link](records/ci/small-lane-git-only.md) | | `ci.ui-e2e-harness` | The UI-interactive E2E flows run as headless Playwright specs (`web/e2e/*.spec.ts`, driven by `scripts/e2e-ui.sh`) in a **second step of the existing advisory `functional-e2e` job**, never their own job; the browser is `chromium-headless-shell` **baked into the CI toolchain image** (`docker/ci/Dockerfile`, `PLAYWRIGHT_VERSION` kept equal to `web/package.json`'s EXACT `@playwright/test` pin), never installed per run; specs are `serial` with `retries: 0` and assert only contracts the curl harness structurally cannot reach. | 2026-07-25 | [link](records/ci/ui-e2e-harness.md) | diff --git a/docs/decisions/records/ci/python-lint-ruff-config-committed.md b/docs/decisions/records/ci/python-lint-ruff-config-committed.md new file mode 100644 index 000000000..998b8200b --- /dev/null +++ b/docs/decisions/records/ci/python-lint-ruff-config-committed.md @@ -0,0 +1,97 @@ +--- +key: ci.python-lint-ruff-config-committed +title: 2026-08-21 — Python lint is a committed ruff.toml enforced in CI, not the operator machine's global config (#780) +status: active +since: '2026-08-21' +supersedes: none +superseded-by: none +rule: The repo commits `ruff.toml`, and the `script-tests` job runs `ruff check` + `ruff format --check` under a PINNED ruff over an EXPLICIT population from `git ls-files`, never `ruff check .`. Never rely on `~/.config/ruff/ruff.toml`, and never add a lint rule to the config without making the tree clean against it in the same PR. +signals: 'ruff · pyright · python lint · `ruff format --check` · lint passes on my machine but not yours · no repo lint config · S105 on a test stub credential · paths: `ruff.toml`, `.gitea/workflows/pr-checks.yml` · issues: #780, #773, #648, #512' +mechanics: 'Config at repo root; `.gitea/workflows/pr-checks.yml` -> `script-tests` pins `ruff==0.12.11` via pip and runs both commands ahead of the jq preflight and pytest. Population is `git ls-files -z ''*.py'' ''*.pyi'' ''*.ipynb''` passed explicitly with `--no-force-exclude`, guarded by an empty-list arm; discovery-based invocation is defeated by an `exclude` in three config scopes, two of them per command. Bumping the pin is a deliberate PR because a new ruff release adds rules.' +--- + +The global instructions tell every session to run `ruff check`, `ruff format --check` and `pyright` +after touching Python. Before this, the repo enforced none of them and committed no config, so ruff +fell back to whichever `~/.config/ruff/ruff.toml` the operator's machine happened to have — **a second +machine lints this repo differently, or not at all.** That is the same shape as #643/#647/#648 (a +shell gate whose behaviour was a function of an untested interpreter version) and #512 (a test that +passed on a fast laptop and flaked on a starved CI VM): the verdict was a property of the environment +rather than of the repo. + +The committed config is the operator's global one apart from `per-file-ignores`, which is narrowed +to `scripts/tests/**`. That is what the tree was de-facto written against, so adopting it cost a +mechanical reformat rather than a rewrite: 74 findings against `706674272`, of which 57 were fixed in +code (mostly by the format pass) and 17 carry a per-site `# noqa` with its reason inline. `RUF100` is +selected so those suppressions stay honest — a `# noqa` that suppresses nothing is otherwise +invisible, and three were live the moment the rule was switched on: one whose rule had stopped firing, +one for a rule this config never enables, and one added mid-branch on a site the same branch had +already fixed in code. + +**One exemption is directory-wide, and it is the boring one.** `S101` for `scripts/tests/**`, because +a test suite asserts. **`S105` is deliberately NOT directory-wide.** All eight of its hits among +those 74 findings are stub credentials handed to the real hooks (`env["ETV_GITEA_TOKEN"] = "stub"`), +with no true positive in the tree today (a ninth `# noqa: S105` predates this and sits on a +commit-message marker in `decisions_validate.py`). A directory blanket would give up +hardcoded-credential coverage over the largest Python surface in the repo, permanently, to suppress +eight known lines — and this is the only Python lint the repo runs, so nothing else would catch a real +token pasted into a fixture next year. Per-site `# noqa: S105` costs the same and keeps the rule live. + +**The population comes from `git ls-files`, not from ruff's discovery, and that is the load-bearing +part.** `ruff check .` reports on what it *discovers*, and an `exclude` defeats discovery in three +different config scopes — including `[format]`, which is where an appended line lands by TOML rules (two of the +three defeat each command). Measured with ruff 0.12.11 and `exclude = ["scripts/**"]`, against a +tracked file holding an unused import, a hardcoded credential and a formatting error. The pattern +matters: `exclude` is matched per FILE, so a bare `["scripts"]` works at the top level but matches +nothing under `[lint]`/`[format]`. GREEN means the gate was silently off: + +| `exclude` in | `ruff check .` | explicit `check` | `ruff format --check .` | explicit `format` | +|---|---|---|---|---| +| top level | GREEN | red | GREEN | red | +| `[lint]` | GREEN | red | red | red | +| `[format]` | red | red | GREEN | red | +| top + `force-exclude` | GREEN | GREEN without `--no-force-exclude`, red with it | GREEN | same | + +Only the top-level scope empties both discovery commands; `[lint]` empties `check`, `[format]` empties +`format --check`, so in those two the job would still redden on the other step. `[format]` is where a +line appended to `ruff.toml` lands, by TOML rules. The last row is the whole reason for the flag. + +`include = []`, `extend-exclude` and a nested `scripts/ruff.toml` were tried too, and are equally +inert against the explicit form. The empty-list arm is the anti-vacuity +check — `ruff check .` over no files exits **0** with a stderr warning, so an emptied population is a +green gate, not a red one. Enumerating from git also covers tracked-but-gitignored files, which +discovery skips (`git add -f` under an ignored path is established practice here). + +**The rule set is not covered, and that is a stated limit rather than an oversight.** `select = []` +silences every selected rule, so the `ruff check` step goes green over any lint violation (a syntax +error still reds) while still printing a reassuring file count. `ruff format --check` is unaffected, because formatting is not rule-selected. So the population +arm makes an emptied *file* set loud, nothing makes an emptied *rule* set loud, and half the gate is +killable by a config edit only a reviewer catches. + +Both steps were witnessed red on the runner before merge, not argued to work — **on the body that +shipped**: run 2173 job 9176 (`❌ Failure - Main Lint scripts (ruff check)` on an `F401`) and run 2170 +job 9163 (`❌ Failure - Main Lint scripts (ruff format --check)`), printing `Linting 34 tracked +Python files` and `Format-checking 34 tracked Python files` — the population arm executing (34 = the +33 tracked files plus the probe; the merged tree has 33). Each came from a temporary probe commit +reverted before merge. Two probes are needed, not one: a check-dirty file stops the job +before the format step ever runs. Earlier reds against the previous, discovery-based bodies were +discarded rather than cited — a proof belongs to the code that ran, not to its predecessor. + +**Lint runs early in the job, ahead of the jq preflight.** `Preflight jq version` is a hard `--expect` +tripwire; a lint step behind it stops running for as long as the jq contract is broken, under a red +that names jq. Ordering is the difference between a gate that is skipped and one that is not. The +`git` half of `Preflight external tools` stays *ahead* of the lint steps, because they consume `git`: +without it, a missing git arrives as an empty population and both steps report a population problem +instead of the missing tool. + +**`pyright` is deliberately NOT gated.** Its only findings here are `reportMissingImports` for +`etv_client` in `scripts/scripted-schedules/entrypoint.py`, resolvable only inside that script's +deploy environment. Gating it would put a node toolchain on the git-only `small` lane to find nothing. +Revisit when this repo grows a typed Python surface — the reason is the cost/finding ratio today, not +a judgement that type checking does not belong. + +**The pin is the second half of the fix.** An unpinned `pip install ruff` re-introduces exactly the +divergence the config closes, one layer up: the verdict becomes a function of *when* the job ran. Same +argument as the `jq` pin in the same job (`ci.jq-version-contract`), and the same consequence — a bump +is a PR someone reads. `pytest` and `pyyaml` in the same job stay unpinned, and the +asymmetry is the point rather than an oversight: a pytest release does not add assertions to your +suite, a ruff release adds rules to your lint. diff --git a/docs/decisions/records/ci/script-tests-job.md b/docs/decisions/records/ci/script-tests-job.md index 8b5636fce..eb81d6b07 100644 --- a/docs/decisions/records/ci/script-tests-job.md +++ b/docs/decisions/records/ci/script-tests-job.md @@ -5,7 +5,7 @@ status: active since: '2026-07-26' supersedes: none superseded-by: none -rule: 'The `scripts/tests/` pytest suite runs on every PR as a dedicated `script-tests` job in `pr-checks.yml` (`runs-on: small`, `setup-python` + `pip install pytest`, `PYTHONPATH=. python3 -m pytest scripts/tests -q`), unconditionally rather than behind a `scripts/**` path filter, and **never as a step inside `decisions-guard`** — a job whose reds a standing rule instructs sessions to ignore must never host a gate whose reds are real. Any new CI gate must be reachable by a failure that is unambiguously attributable to it.' +rule: 'The `scripts/tests/` pytest suite runs on every PR as a dedicated `script-tests` job in `pr-checks.yml` (`runs-on: small`, `setup-python` + `pip install pytest pyyaml`, `PYTHONPATH=. python3 -m pytest scripts/tests -q`; since #780 it also runs a pinned ruff over a `git ls-files` population first), unconditionally rather than behind a `scripts/**` path filter, and **never as a step inside `decisions-guard`** — a job whose reds a standing rule instructs sessions to ignore must never host a gate whose reds are real. Any new CI gate must be reachable by a failure that is unambiguously attributable to it.' signals: 'scripts/tests never ran in CI, pytest not in any workflow, python test suite local-only, decorative test, decisions-guard runs the code not the tests, script-tests job, small lane pytest, negative control CI goes red · paths: `.gitea/workflows/pr-checks.yml`, `scripts/tests/`, `docs/ci-cd.md` · issues: #631, #610, #621, #622, #542' mechanics: '`.gitea/workflows/pr-checks.yml` -> `script-tests`; `docs/ci-cd.md` -> "`script-tests` job"' --- @@ -29,7 +29,7 @@ it lives in, so **a job under a standing ignore-rule can host no real gate.** This does not conflict with `ci.ui-e2e-harness` ("never their own job"). That record folds UI-E2E into `functional-e2e` because the specs need an app the job has *already booted* — sharing expensive -setup. Here there is no shared setup to reuse (a checkout plus `pip install pytest pyyaml`), and the +setup. Here there is no shared setup to reuse (a checkout plus `pip install pytest pyyaml` and a pinned ruff), and the sibling job carries an ignore-rule. Same question, opposite answers, for stated reasons. **Unconditional, not path-filtered.** The suite's real input set spans more than `scripts/`: @@ -104,3 +104,9 @@ from a proxy ("fewer than we asked for", "jq didn't complain"). It is not yet a *required* status check — `main` requires only `Build & test (.NET)`, `EF migration integrity` and `review-verdict/h10`. It reddens the run; promoting it to required is a branch-protection change left deliberately separate. + +Since #780 the job also lints Python before pytest, so its display name is +`Script lint and tests (ruff + pytest)`. It does **not** invoke `ruff check .` — the invocation and +the reasons for its exact shape are `ci.python-lint-ruff-config-committed`. Why the lint lives here +rather than in a job of its own: it needs the same `setup-python`, it costs seconds, and a second job would double the dispatch overhead +this file exists to keep small. diff --git a/docs/defect-shapes-773.md b/docs/defect-shapes-773.md index 9bcfcfd9c..fa0637f02 100644 --- a/docs/defect-shapes-773.md +++ b/docs/defect-shapes-773.md @@ -445,6 +445,44 @@ machine lints differently, or not at all. One of the two must move — either th committed ruff config and enforces it, or the global instruction stops claiming this repo enforces something it does not. +**Resolved 2026-08-21 (#780): the repo moved.** `ruff.toml` is committed at the root and the +`script-tests` job runs `ruff check` + `ruff format --check` under a pinned `ruff==0.12.11` over an +explicit population from `git ls-files` — not `ruff check .`, which an `exclude` in the right config +scope silently empties into a green run. The pre-fix state, reproducible rather than +asserted — against `706674272`, the base this landed on, with the committed config dropped in: + +``` +mkdir -p ~/scratch/m780 && git archive 706674272 | tar -x -C ~/scratch/m780 +git show 01f7a89e8:ruff.toml > ~/scratch/m780/ruff.toml # a sha: the file is not on main pre-merge +cd ~/scratch/m780 && ruff check . ; ruff format --check . # ruff 0.12.11 +# -> Found 74 errors. / 20 files would be reformatted, 13 files already formatted +``` + +(`;` not `&&` — `ruff check` exits 1, which would swallow the second command. Not `/tmp`: macOS purges +it. Redirect the config into place *before* running anything: an empty `ruff.toml` is valid, so a +failed `git show` leaves ruff silently using its own defaults and printing a different number.) + +The row above measured **47** eight days earlier against the operator's global config; the tree grew +and the configs differ, so the two numbers are not comparable and neither supersedes the other. Two +of the 74 are `RUF100` on suppressions that were already in the tree before this change — they exist +in this count only because the committed config enables that rule. + +Of the 74, **57 were fixed in code** (most of them by the `ruff format` pass itself, which splits the +40 semicolon statements) and **17 carry a per-site `# noqa` with its reason inline**: 8 `S105` on stub +credentials handed to the real hooks by `scripts/tests`, 9 `E501` on one-line JSON and shell fixtures. +The `S105`s are deliberately per-site rather than a directory exemption, so a real credential pasted +into a fixture later still reddens the gate. Only `S101` is exempted directory-wide for `scripts/tests/**`, +because a test suite asserts. + +`RUF100` is selected, which is what keeps that split honest: a `# noqa` that suppresses nothing still +reads as a suppression, and it is invisible without this rule. Three were live when it was switched +on: one on a site that had already been fixed in code, plus the two counted above — one whose rule had +stopped firing, one for a rule this config never enables. + +`pyright` stayed ungated: its only findings are the `etv_client` imports in the row above, and gating +it would put a node toolchain on the git-only `small` lane to find nothing. Rationale, the exemption +list and the measured exclude matrix: `ci.python-lint-ruff-config-committed`. + ### 5.3 Configured vs actually invoked Measured over the session transcript corpus (811 files under diff --git a/ruff.toml b/ruff.toml new file mode 100644 index 000000000..0ebc03316 --- /dev/null +++ b/ruff.toml @@ -0,0 +1,59 @@ +# Ruff configuration for this repo's Python surface (all of it lives under `scripts/`). +# +# WHY THIS FILE EXISTS (ersatztv#780). Without a committed config, ruff falls back to whatever +# `~/.config/ruff/ruff.toml` the operator's machine happens to have — so a second machine lints this +# repo differently, or not at all. That is the environment-divergence class #643/#647/#648 (a shell +# gate whose behaviour was a function of an untested interpreter version) and #512 (a test that +# passed on a fast laptop and flaked on a starved CI VM). The settings below are pinned HERE so the +# lint verdict is a property of the repo, not of the machine. +# +# It is enforced by the `script-tests` job (`Script lint and tests (ruff + pytest)`) in +# .gitea/workflows/pr-checks.yml. A config nobody runs is the same divergence one step later. +# +# That job does NOT invoke `ruff check .`: it passes an explicit population from `git ls-files` with +# `--no-force-exclude`. An `exclude` added to this file silently empties a discovery-based run into a +# GREEN one — a top-level `exclude` empties both commands, one under `[lint]` empties `check`, one +# under `[format]` (where an appended line lands, by TOML rules) empties `format --check`. Adding +# `exclude` here will therefore not do what you expect, which is the point. The measured matrix is in +# `ci.python-lint-ruff-config-committed`. +# +# `pyright` is deliberately NOT gated: its only findings here are `reportMissingImports` for +# `etv_client` in scripts/scripted-schedules/entrypoint.py, which resolves only inside that script's +# deploy environment, and gating it would put a node toolchain on the git-only `small` lane for zero +# real findings. Revisit if this repo grows a typed Python surface. + +target-version = "py311" +line-length = 120 + +[lint] +select = [ + "E", # pycodestyle errors + "W", # pycodestyle warnings + "F", # pyflakes + "I", # isort + "B", # flake8-bugbear + "UP", # pyupgrade + "SIM", # flake8-simplify + "S", # flake8-bandit (security) + # RUF100 is load-bearing, not tidiness: every `# noqa` below is an assertion that a real finding + # is being suppressed for a stated reason, and without this a suppression that suppresses nothing + # stays in the file reading as one. #780 did exactly that mid-branch — a `# noqa: UP031` on a site + # the same branch had already fixed in code — and found two more already in the tree: one whose + # rule had stopped firing, one for a rule this config never enables. + "RUF100", +] +ignore = [ + "S603", # subprocess call - check for execution of untrusted input (too noisy for scripts) + "S607", # starting a process with a partial executable path +] + +[lint.per-file-ignores] +# scripts/tests asserts, so S101 would fire on every test. S105 is deliberately NOT exempted here: +# the eight sites that trip it (`env["ETV_GITEA_TOKEN"] = "stub"`) carry a per-site `# noqa: S105` +# instead, so a real credential pasted into a fixture next year still reddens the gate. A directory +# blanket would have given up hardcoded-credential coverage over the largest Python surface in the +# repo, permanently, to suppress eight known lines. +"scripts/tests/**" = ["S101"] + +[format] +quote-style = "double" diff --git a/scripts/decisions_validate.py b/scripts/decisions_validate.py index 8ed4b1bc5..bf775b679 100644 --- a/scripts/decisions_validate.py +++ b/scripts/decisions_validate.py @@ -19,7 +19,7 @@ from datetime import date from pathlib import Path from typing import NamedTuple -import scripts.decisions_lib as dl # noqa: E402 (run with PYTHONPATH=. or as module) +import scripts.decisions_lib as dl # (run with PYTHONPATH=. or as module) SKIP_HEADINGS = dl.SKIP_HEADINGS # single source of truth # `signals` is required alongside the lifecycle fields: the `**Signals:**` line (plus `key:`) is what @@ -469,12 +469,7 @@ def _is_stripped_index(path: Path, archive_dir: Path, recs: list) -> bool: file there is exempt only if it actually LOOKS like a stripped index: exactly one keyless record whose heading is one of the known generated ones (`dl.SKIP_HEADINGS`). """ - return ( - path.parent == archive_dir - and len(recs) == 1 - and not recs[0].key - and recs[0].heading in dl.SKIP_HEADINGS - ) + return path.parent == archive_dir and len(recs) == 1 and not recs[0].key and recs[0].heading in dl.SKIP_HEADINGS def record_wing_faults(records_dir: Path | None = None, archive_dir: Path | None = None) -> list[str]: diff --git a/scripts/generate-endpoint-index.py b/scripts/generate-endpoint-index.py index 5aef63ce2..2f6461e92 100755 --- a/scripts/generate-endpoint-index.py +++ b/scripts/generate-endpoint-index.py @@ -62,9 +62,7 @@ def render(spec: dict) -> str: "`scripts/update-openapi.sh`.*" ) lines.append("") - lines.append( - f"{len(spec.get('paths', {}))} endpoints, {len(operations)} operations." - ) + lines.append(f"{len(spec.get('paths', {}))} endpoints, {len(operations)} operations.") lines.append("") for tag in tags: @@ -73,9 +71,7 @@ def render(spec: dict) -> str: lines.append("| Method | Path | Operation | Summary |") lines.append("|---|---|---|---|") for op in sorted(grouped[tag], key=lambda o: (o["path"], o["method"])): - lines.append( - f"| {op['method']} | `{op['path']}` | {op['operationId']} | {op['summary']} |" - ) + lines.append(f"| {op['method']} | `{op['path']}` | {op['operationId']} | {op['summary']} |") lines.append("") return "\n".join(lines).rstrip("\n") + "\n" diff --git a/scripts/mcp_smoke.py b/scripts/mcp_smoke.py index 62312b8e2..4099a20c5 100644 --- a/scripts/mcp_smoke.py +++ b/scripts/mcp_smoke.py @@ -37,6 +37,7 @@ explicitly NOT bounded; that is the accepted limit stated above. from __future__ import annotations +import contextlib import json import os import secrets @@ -47,6 +48,7 @@ import sys import threading import time + def fail(msg: str, code: int) -> int: print(f"FAIL: {msg}") return code @@ -60,15 +62,19 @@ def main() -> int: i = 0 while i < len(argv): if argv[i] == "--expect-server" and i + 1 < len(argv): - expect_server = argv[i + 1]; i += 2 + expect_server = argv[i + 1] + i += 2 elif argv[i] == "--expect-tool" and i + 1 < len(argv): - expect_tools.append(argv[i + 1]); i += 2 + expect_tools.append(argv[i + 1]) + i += 2 else: - positional.append(argv[i]); i += 1 + positional.append(argv[i]) + i += 1 if len(positional) < 2: - return fail("usage: mcp_smoke.py <.mcp.json> [timeout] " - "[--expect-server NAME] [--expect-tool NAME]...", 2) + return fail( + "usage: mcp_smoke.py <.mcp.json> [timeout] [--expect-server NAME] [--expect-tool NAME]...", 2 + ) cfg_path, server = positional[0], positional[1] if len(positional) > 2: try: @@ -136,8 +142,12 @@ def main() -> int: try: proc = subprocess.Popen( - [resolved, *args], stdin=subprocess.PIPE, stdout=subprocess.PIPE, - stderr=subprocess.DEVNULL, env=env, cwd=workdir, + [resolved, *args], + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.DEVNULL, + env=env, + cwd=workdir, start_new_session=True, # own process group, so children die with us ) except OSError as exc: @@ -222,10 +232,8 @@ def main() -> int: os.killpg(pgid, sig) except OSError: break # no group members left - try: + with contextlib.suppress(subprocess.TimeoutExpired): proc.wait(timeout=5) - except subprocess.TimeoutExpired: - pass time.sleep(0.2) else: for sig in (signal.SIGTERM, signal.SIGKILL): @@ -245,9 +253,20 @@ def main() -> int: id_tools = secrets.randbelow(2**31 - 1000) + 1000 while id_tools == id_init: id_tools = secrets.randbelow(2**31 - 1000) + 1000 - init = expect(id_init, {"jsonrpc": "2.0", "id": id_init, "method": "initialize", "params": { - "protocolVersion": "2024-11-05", "capabilities": {}, - "clientInfo": {"name": "mcp-smoke", "version": "0"}}}, deadline) + init = expect( + id_init, + { + "jsonrpc": "2.0", + "id": id_init, + "method": "initialize", + "params": { + "protocolVersion": "2024-11-05", + "capabilities": {}, + "clientInfo": {"name": "mcp-smoke", "version": "0"}, + }, + }, + deadline, + ) if init is None: return fail(f"no 'initialize' response within {budget}s (server did not start)", 9) if "error" in init: @@ -263,8 +282,7 @@ def main() -> int: return fail(f"wrong server: expected '{expect_server}', got '{actual}'", 13) send({"jsonrpc": "2.0", "method": "notifications/initialized", "params": {}}) - listed = expect(id_tools, {"jsonrpc": "2.0", "id": id_tools, - "method": "tools/list", "params": {}}, deadline) + listed = expect(id_tools, {"jsonrpc": "2.0", "id": id_tools, "method": "tools/list", "params": {}}, deadline) if listed is None: return fail(f"no 'tools/list' response within {budget}s", 10) lresult = listed.get("result") diff --git a/scripts/scripted-schedules/entrypoint.py b/scripts/scripted-schedules/entrypoint.py index 23b77a9e3..3728e5f5a 100755 --- a/scripts/scripted-schedules/entrypoint.py +++ b/scripts/scripted-schedules/entrypoint.py @@ -3,18 +3,18 @@ import argparse import importlib import sys - from uuid import UUID import etv_client from etv_client.api import ScriptedScheduleApi + def main(): parser = argparse.ArgumentParser(description="Run an ETV scripted schedule") - parser.add_argument('host', help="The ETV host (e.g., http://localhost:8409)") - parser.add_argument('build_id', type=UUID, help="The build ID for the playout") - parser.add_argument('mode', choices=['reset', 'continue'], help="The playout build mode") - parser.add_argument('script_name', help="The name of the script module to use (e.g., one)") + parser.add_argument("host", help="The ETV host (e.g., http://localhost:8409)") + parser.add_argument("build_id", type=UUID, help="The build ID for the playout") + parser.add_argument("mode", choices=["reset", "continue"], help="The playout build mode") + parser.add_argument("script_name", help="The name of the script module to use (e.g., one)") known_args, unknown_args = parser.parse_known_args() @@ -28,9 +28,9 @@ def main(): with etv_client.ApiClient(configuration) as api_client: try: - define_content = getattr(script_module, 'define_content') - reset_playout = getattr(script_module, 'reset_playout') - build_playout = getattr(script_module, 'build_playout') + define_content = script_module.define_content + reset_playout = script_module.reset_playout + build_playout = script_module.build_playout api_instance = ScriptedScheduleApi(api_client) @@ -48,6 +48,6 @@ def main(): except AttributeError as e: print(f"Error: the '{known_args.script_name}' script is missing a required function. {e}") + if __name__ == "__main__": main() - diff --git a/scripts/tests/conftest.py b/scripts/tests/conftest.py index 109c575fa..5aafcc955 100644 --- a/scripts/tests/conftest.py +++ b/scripts/tests/conftest.py @@ -24,6 +24,4 @@ import pytest @pytest.fixture(autouse=True) def isolate_hook_fire_log(tmp_path_factory, monkeypatch): - monkeypatch.setenv( - "ETV_HOOK_FIRE_LOG_DIR", str(tmp_path_factory.mktemp("hook-fire-log")) - ) + monkeypatch.setenv("ETV_HOOK_FIRE_LOG_DIR", str(tmp_path_factory.mktemp("hook-fire-log"))) diff --git a/scripts/tests/test_bom_guard_detection.py b/scripts/tests/test_bom_guard_detection.py index a44a96bb6..f8e846cdd 100644 --- a/scripts/tests/test_bom_guard_detection.py +++ b/scripts/tests/test_bom_guard_detection.py @@ -36,9 +36,17 @@ BOM = b"\xef\xbb\xbf" def _git(cwd: Path, *args: str) -> None: subprocess.run( - ["git", *args], cwd=str(cwd), check=True, capture_output=True, - env={**os.environ, "GIT_AUTHOR_NAME": "t", "GIT_AUTHOR_EMAIL": "t@e", - "GIT_COMMITTER_NAME": "t", "GIT_COMMITTER_EMAIL": "t@e"}, + ["git", *args], + cwd=str(cwd), + check=True, + capture_output=True, + env={ + **os.environ, + "GIT_AUTHOR_NAME": "t", + "GIT_AUTHOR_EMAIL": "t@e", + "GIT_COMMITTER_NAME": "t", + "GIT_COMMITTER_EMAIL": "t@e", + }, ) @@ -54,12 +62,18 @@ def _repo_with(tmp_path: Path, name: str, content: bytes) -> Path: def _run(hook: Path, repo: Path, env: dict) -> tuple[int, bytes]: - payload = json.dumps({ - "session_id": "s", "hook_event_name": "PreToolUse", "tool_name": "Bash", - "cwd": str(repo), "tool_input": {"command": "git commit -m x"}, - }) - p = subprocess.run(["bash", str(hook)], input=payload.encode(), capture_output=True, - cwd=str(repo), env=env, timeout=60) + payload = json.dumps( + { + "session_id": "s", + "hook_event_name": "PreToolUse", + "tool_name": "Bash", + "cwd": str(repo), + "tool_input": {"command": "git commit -m x"}, + } + ) + p = subprocess.run( + ["bash", str(hook)], input=payload.encode(), capture_output=True, cwd=str(repo), env=env, timeout=60 + ) return p.returncode, p.stdout @@ -105,8 +119,9 @@ def _path_without_xxd(tmp_path: Path) -> dict: def test_the_fixture_really_stages_a_BOM(tmp_path): repo = _repo_with(tmp_path, "Bad.cs", BOM + b"class A {}\n") assert (repo / "Bad.cs").read_bytes()[:3] == BOM - staged = subprocess.run(["git", "diff", "--name-only", "--cached"], cwd=str(repo), - capture_output=True, text=True).stdout.split() + staged = subprocess.run( + ["git", "diff", "--name-only", "--cached"], cwd=str(repo), capture_output=True, text=True + ).stdout.split() assert staged == ["Bad.cs"], f"nothing was staged, so the guard would have nothing to read: {staged}" @@ -179,6 +194,4 @@ def test_DISARMING_the_BOM_comparison_stops_detection(tmp_path): rc, out = _run(mutated, repo, dict(os.environ)) assert rc == 0 - assert out == b"", ( - f"disarming the BOM comparison did not stop detection, so it is not load-bearing: {out!r}" - ) + assert out == b"", f"disarming the BOM comparison did not stop detection, so it is not load-bearing: {out!r}" diff --git a/scripts/tests/test_ci_dropped_step_guard.py b/scripts/tests/test_ci_dropped_step_guard.py index 2f4a198f5..39fcd7b57 100644 --- a/scripts/tests/test_ci_dropped_step_guard.py +++ b/scripts/tests/test_ci_dropped_step_guard.py @@ -115,13 +115,13 @@ def _guard_buckets(job: str): argv = _guard(job)["run"].split() assert "--always" in argv and "--gated" in argv, argv a, g = argv.index("--always"), argv.index("--gated") - return argv[a + 1:g], argv[g + 1:] + return argv[a + 1 : g], argv[g + 1 :] # Mirrors the `if:` every gated step in these jobs carries. Compared as a normalised string rather # than by parsing the expression: what matters is that a step's gating and the guard's bucketing are # the SAME condition, and any rewrite of one that is not mirrored in the other should be loud. -SKIP_GATE = ("steps.detect.outputs.docs_only!='true'&&steps.revalidate.outputs.skip!='true'") +SKIP_GATE = "steps.detect.outputs.docs_only!='true'&&steps.revalidate.outputs.skip!='true'" def _is_gated(step) -> bool: @@ -211,17 +211,14 @@ def test_every_consequential_run_step_marks_itself_as_its_FIRST_act(job): # prefix as a preceding command and reddens every correctly-written step. lines = s["run"].splitlines() at = next(i for i, ln in enumerate(lines) if _MARK.search(ln)) - preceding = [ - ln.strip() for ln in lines[:at] - if ln.strip() and not ln.strip().startswith("#") - ] + preceding = [ln.strip() for ln in lines[:at] if ln.strip() and not ln.strip().startswith("#")] if [ln for ln in preceding if not ln.startswith("set -")]: late.append((s.get("name", "?"), preceding)) assert not missing, ( f"these run: steps of the REQUIRED job '{job}' do not record that they executed: {missing}. " "A step the runner drops concludes success, so without a marker its non-execution takes the " "whole required context green having done no work (ersatztv#756). Add " - '`\"${GITHUB_WORKSPACE:-.}/scripts/ci-step-ran.sh\" mark ` as the step\'s first line and ' + '`"${GITHUB_WORKSPACE:-.}/scripts/ci-step-ran.sh" mark ` as the step\'s first line and ' "the key to the guard step's --always/--gated list." ) assert not late, ( @@ -357,8 +354,10 @@ def test_the_guards_OWN_body_cannot_be_dropped_by_the_mechanism_it_guards_agains # a red here blocks every merge through the combined status, so brittleness is a real cost and # not a free strictness win. Whitespace inside the delimiters is normalised for the same reason. env = {k: re.sub(r"\s+", "", str(v)) for k, v in (guard.get("env") or {}).items()} - for name, want in (("ETV_DOCS_ONLY", "${{steps.detect.outputs.docs_only}}"), - ("ETV_REVALIDATE_SKIP", "${{steps.revalidate.outputs.skip}}")): + for name, want in ( + ("ETV_DOCS_ONLY", "${{steps.detect.outputs.docs_only}}"), + ("ETV_REVALIDATE_SKIP", "${{steps.revalidate.outputs.skip}}"), + ): assert env.get(name) == want, ( f"the '{job}' guard's env: has {name}={guard.get('env', {}).get(name)!r}, expected the " f"output the gated steps' own `if:` reads ({want}). A typo here is SILENT rather than " @@ -427,12 +426,10 @@ def _env(tmp_path, **extra): def _run(script: str, env): - return subprocess.run(["bash", "-c", script], cwd=REPO_ROOT, env=env, - capture_output=True, text=True) + return subprocess.run(["bash", "-c", script], cwd=REPO_ROOT, env=env, capture_output=True, text=True) -@pytest.mark.parametrize("gate", GATE_VALUES_IN_THE_WILD, - ids=["gate-false", "gate-empty", "gate-unset"]) +@pytest.mark.parametrize("gate", GATE_VALUES_IN_THE_WILD, ids=["gate-false", "gate-empty", "gate-unset"]) @pytest.mark.parametrize("job", MARKED_JOBS) def test_the_guard_PASSES_when_every_step_marked_itself(job, gate, tmp_path): """The positive control. Without it, a guard that always failed would satisfy every case below. @@ -458,7 +455,8 @@ def test_the_guard_PASSES_when_every_step_marked_itself(job, gate, tmp_path): # inverted provenance test would otherwise ship silently, and the operator reading this line to # settle the promotion question would read it wrong. assert f"Marker identity: job={job} run=424242 attempt=7 (from the runner)" in r.stdout, ( - f"the guard misreported its marker identity: {r.stdout!r}") + f"the guard misreported its marker identity: {r.stdout!r}" + ) @pytest.mark.parametrize("job", MARKED_JOBS) @@ -534,8 +532,7 @@ def test_an_EMPTY_gate_value_requires_the_gated_steps(job, tmp_path): what is required. """ guard = _guard(job)["run"] - r = _run("\n".join(["set -e", guard]), - _env(tmp_path, GITHUB_JOB=job, ETV_DOCS_ONLY="", ETV_REVALIDATE_SKIP="")) + r = _run("\n".join(["set -e", guard]), _env(tmp_path, GITHUB_JOB=job, ETV_DOCS_ONLY="", ETV_REVALIDATE_SKIP="")) assert r.returncode != 0 assert _guard_buckets(job)[1][-1] in (r.stdout + r.stderr), ( f"empty gate values were read as a skip, so the gated steps went unchecked: {r.stdout}" @@ -571,8 +568,7 @@ def test_a_STALE_marker_from_another_run_cannot_satisfy_the_guard(tmp_path): # ...nor may the OTHER job in the same run inherit them. sibling = _env(tmp_path, GITHUB_JOB="migrations", GITHUB_RUN_ID="111", GITHUB_RUN_ATTEMPT="1") assert _run(_guard("migrations")["run"], sibling).returncode != 0, ( - "the two required jobs share one marker file, so one job's markers answer for the other's " - "dropped steps" + "the two required jobs share one marker file, so one job's markers answer for the other's dropped steps" ) @@ -611,8 +607,7 @@ def test_a_key_is_matched_WHOLE_not_as_a_substring(tmp_path): assert _run(f"{SCRIPT} mark web-build && {SCRIPT} mark web-test", env).returncode == 0 r = _run(f"{SCRIPT} assert --always build", env) assert r.returncode != 0, ( - "the key 'build' was satisfied by a marker for 'web-build' — a dropped `dotnet build` would " - "pass unnoticed" + "the key 'build' was satisfied by a marker for 'web-build' — a dropped `dotnet build` would pass unnoticed" ) @@ -638,9 +633,11 @@ def test_a_degraded_run_IDENTITY_refuses_rather_than_sharing_a_marker_path(tmp_p f"{r.stdout}{r.stderr}" ) assert "cannot identify this run" in (r.stdout + r.stderr), ( - f"refused, but without naming the cause: {r.stdout!r} {r.stderr!r}") + f"refused, but without naming the cause: {r.stdout!r} {r.stderr!r}" + ) assert not list(tmp_path.iterdir()), ( - "a degraded-identity `mark` still created a marker file somewhere under RUNNER_TEMP") + "a degraded-identity `mark` still created a marker file somewhere under RUNNER_TEMP" + ) def test_the_marker_identity_is_REPORTED_on_stdout_every_run(tmp_path): @@ -661,12 +658,15 @@ def test_the_marker_identity_is_REPORTED_on_stdout_every_run(tmp_path): documented contract (the record's `mechanics:`), and a future reader is told to trust it. """ marks = [_mark_line(s) for s, _ in _marked("test")] - r = _run("\n".join(["set -e", *marks, _guard("test")["run"]]), - _env(tmp_path, GITHUB_RUN_ID="1916", GITHUB_RUN_ATTEMPT="4")) + r = _run( + "\n".join(["set -e", *marks, _guard("test")["run"]]), + _env(tmp_path, GITHUB_RUN_ID="1916", GITHUB_RUN_ATTEMPT="4"), + ) assert r.returncode == 0, r.stdout + r.stderr assert "Marker identity: job=test run=1916 attempt=4 (from the runner)" in r.stdout, ( "the guard did not report the identity its marker path was actually keyed on, so a reader " - f"cannot audit the keying from a run log: {r.stdout!r}") + f"cannot audit the keying from a run log: {r.stdout!r}" + ) def test_a_skip_gate_that_empties_the_expected_set_REFUSES(tmp_path): @@ -681,15 +681,16 @@ def test_a_skip_gate_that_empties_the_expected_set_REFUSES(tmp_path): that it proved everything while proving nothing" is the failure this whole file exists to remove. """ r = _run(f"{SCRIPT} assert --always --gated foo", _env(tmp_path, ETV_DOCS_ONLY="true")) - assert r.returncode != 0, ( - f"the guard passed with an empty post-gate expectation set: {r.stdout!r}") + assert r.returncode != 0, f"the guard passed with an empty post-gate expectation set: {r.stdout!r}" assert "no expected keys" in (r.stdout + r.stderr).lower() or "NO expected keys" in r.stderr -@pytest.mark.parametrize("revalidate", ["true", "false", "", None], - ids=lambda v: f"reval-{v if v is not None else 'unset'}") -@pytest.mark.parametrize("docs_only", ["true", "false", "", None], - ids=lambda v: f"docs-{v if v is not None else 'unset'}") +@pytest.mark.parametrize( + "revalidate", ["true", "false", "", None], ids=lambda v: f"reval-{v if v is not None else 'unset'}" +) +@pytest.mark.parametrize( + "docs_only", ["true", "false", "", None], ids=lambda v: f"docs-{v if v is not None else 'unset'}" +) def test_the_skip_gate_over_the_WHOLE_value_matrix(docs_only, revalidate, tmp_path): """Every combination of the two gate values, not just the diagonal — cold review's last finding. @@ -713,11 +714,14 @@ def test_the_skip_gate_over_the_WHOLE_value_matrix(docs_only, revalidate, tmp_pa job = "test" always, gated = _guard_buckets(job) marks = [_mark_line(s) for s, k in _marked(job) if k in always] - r = _run("\n".join(["set -e", *marks, _guard(job)["run"]]), - _env(tmp_path, ETV_DOCS_ONLY=docs_only, ETV_REVALIDATE_SKIP=revalidate)) + r = _run( + "\n".join(["set -e", *marks, _guard(job)["run"]]), + _env(tmp_path, ETV_DOCS_ONLY=docs_only, ETV_REVALIDATE_SKIP=revalidate), + ) should_skip = docs_only == "true" or revalidate == "true" assert (r.returncode == 0) is should_skip, ( f"with docs_only={docs_only!r} and revalidate={revalidate!r} the guard " f"{'passed' if r.returncode == 0 else 'failed'}, expected it to " f"{'skip the gated keys' if should_skip else 'require them'}. The gate must treat a value as " - "a skip if and only if it is exactly `true` in EITHER variable.\n" + r.stdout + r.stderr) + "a skip if and only if it is exactly `true` in EITHER variable.\n" + r.stdout + r.stderr + ) diff --git a/scripts/tests/test_ci_image_pin_population.py b/scripts/tests/test_ci_image_pin_population.py index 58e20e7aa..490759ccf 100644 --- a/scripts/tests/test_ci_image_pin_population.py +++ b/scripts/tests/test_ci_image_pin_population.py @@ -198,8 +198,7 @@ def test_the_registry_partitions_every_job_in_the_workflow(): ) assert not (TOOLCHAIN_JOBS & BARE_RUNNER_JOBS), "a job cannot be in both lists" assert all_jobs == TOOLCHAIN_JOBS | BARE_RUNNER_JOBS, ( - f"the registry names jobs that do not exist: " - f"{sorted((TOOLCHAIN_JOBS | BARE_RUNNER_JOBS) - all_jobs)}" + f"the registry names jobs that do not exist: {sorted((TOOLCHAIN_JOBS | BARE_RUNNER_JOBS) - all_jobs)}" ) @@ -242,8 +241,7 @@ def test_a_single_job_losing_its_pin_is_DETECTED(doc): empty list would satisfy the live assertion above and prove nothing — which is how #621 and #685 both shipped.""" assert pin_population_faults(doc), ( - "the population check accepted a workflow in which a container job no longer runs the " - "pinned toolchain image" + "the population check accepted a workflow in which a container job no longer runs the pinned toolchain image" ) diff --git a/scripts/tests/test_ci_release_path_scan_job.py b/scripts/tests/test_ci_release_path_scan_job.py index ebd3be911..d50e1f9ca 100644 --- a/scripts/tests/test_ci_release_path_scan_job.py +++ b/scripts/tests/test_ci_release_path_scan_job.py @@ -328,8 +328,7 @@ def _run_the_real_scan_body(repo: Path, tmp_path: Path): (repo / _NONCE_FILE).write_text(nonce) env[_FENCE] = nonce (tmp_path / "runner").mkdir(exist_ok=True) - return subprocess.run(["bash", "-c", body], cwd=repo, env=env, - capture_output=True, text=True) + return subprocess.run(["bash", "-c", body], cwd=repo, env=env, capture_output=True, text=True) @_nested @@ -358,7 +357,7 @@ def test_the_scan_step_REALLY_FAILS_on_a_poisoned_workflow(tmp_path): text = wf.read_text() anchor = ' IMG="${IMAGE}:${SMOKE_SHORT_SHA}"' assert anchor in text, "anchor for the poison is gone — rewrite this control" - wf.write_text(text.replace(anchor, ' # ${{ steps.meta.outputs.short }}\n' + anchor, 1)) + wf.write_text(text.replace(anchor, " # ${{ steps.meta.outputs.short }}\n" + anchor, 1)) res = _run_the_real_scan_body(repo, tmp_path) assert res.returncode != 0, ( @@ -403,8 +402,10 @@ def test_the_PROOF_SCRIPT_itself_refuses_when_the_ban_is_deselected(tmp_path): ) res = subprocess.run( ["bash", str(repo / "scripts" / "ci-prove-ban-detects.sh")], - cwd=repo, env={**os.environ, "GITHUB_WORKSPACE": str(repo)}, - capture_output=True, text=True, + cwd=repo, + env={**os.environ, "GITHUB_WORKSPACE": str(repo)}, + capture_output=True, + text=True, ) assert res.returncode != 0, ( "ci-prove-ban-detects.sh vouched for the gate while the ban test was deselected at the " @@ -446,19 +447,16 @@ def test_the_PROOF_SCRIPT_refuses_when_the_WRONG_test_fails(tmp_path): # landing on the "not enforcing" branch instead of the one under test. (First draft of this test # did exactly that and was red for the wrong reason.) ban = repo / BAN_TEST_FILE - ban.write_text( - ban.read_text() - + "\n\ndef test_an_unrelated_failure_for_this_probe():\n assert False\n" - ) + ban.write_text(ban.read_text() + "\n\ndef test_an_unrelated_failure_for_this_probe():\n assert False\n") res = subprocess.run( ["bash", str(repo / "scripts" / "ci-prove-ban-detects.sh")], - cwd=repo, env={**os.environ, "GITHUB_WORKSPACE": str(repo)}, - capture_output=True, text=True, + cwd=repo, + env={**os.environ, "GITHUB_WORKSPACE": str(repo)}, + capture_output=True, + text=True, ) combined = res.stdout + res.stderr - assert res.returncode != 0, ( - f"the script read an unrelated test's failure as proof.\n{combined}" - ) + assert res.returncode != 0, f"the script read an unrelated test's failure as proof.\n{combined}" # THE SPECIFIC branch. This scenario is built to land on "wrong test failed" (exit 1, no `[build]` # failure); accepting "could not prove anything" too would let it drift onto the exit-5 branch and # silently cover a branch it was not written for, while still looking green. @@ -474,15 +472,17 @@ def test_the_PROOF_SCRIPT_passes_on_a_clean_tree(tmp_path): repo = _repo_copy(tmp_path) res = subprocess.run( ["bash", str(repo / "scripts" / "ci-prove-ban-detects.sh")], - cwd=repo, env={**os.environ, "GITHUB_WORKSPACE": str(repo)}, - capture_output=True, text=True, + cwd=repo, + env={**os.environ, "GITHUB_WORKSPACE": str(repo)}, + capture_output=True, + text=True, ) assert res.returncode == 0, ( f"ci-prove-ban-detects.sh failed on a clean tree.\nstdout:\n{res.stdout}\nstderr:\n{res.stderr}" ) - assert (repo / ".gitea" / "workflows" / "docker-build.yml").read_text() == ( - WORKFLOW.read_text() - ), "the script did not restore the workflow file it poisoned" + assert (repo / ".gitea" / "workflows" / "docker-build.yml").read_text() == (WORKFLOW.read_text()), ( + "the script did not restore the workflow file it poisoned" + ) def test_the_pytest_invocation_cannot_DESELECT_or_swallow_its_result(): @@ -494,7 +494,7 @@ def test_the_pytest_invocation_cannot_DESELECT_or_swallow_its_result(): # Only the tokens AFTER `pytest` are pytest's own arguments. Checking the whole line would flag # the `-m` in `python3 -m pytest`, which is how the interpreter is invoked — a false positive # that would make this test red on the correct command. - args = tokens[tokens.index("pytest") + 1:] + args = tokens[tokens.index("pytest") + 1 :] banned = {"-k", "-m", "--deselect", "--ignore", "--collect-only", "--co"} assert not (banned & set(args)), f"pytest invocation may deselect tests: {line!r}" for op in ("||", "&&", ";", "|"): @@ -527,8 +527,7 @@ def test_no_step_in_the_scan_job_is_advisory(): @pytest.mark.parametrize( "step_name", - [s.get("name", "?") for s in yaml.safe_load(WORKFLOW.read_text())["jobs"][JOB]["steps"] - if s.get("run")], + [s.get("name", "?") for s in yaml.safe_load(WORKFLOW.read_text())["jobs"][JOB]["steps"] if s.get("run")], ) def test_every_run_body_in_the_scan_job_is_delimiter_free(step_name): """The guard must not be vulnerable to the defect it guards against. @@ -561,7 +560,7 @@ def test_the_guard_expectations_match_the_markers_exactly(): restated here, so adding a step without a marker is a red.""" argv = _guard()["run"].split() assert "--always" in argv, argv - always = argv[argv.index("--always") + 1:] + always = argv[argv.index("--always") + 1 :] assert "--gated" not in argv, "every step in this job is unconditional; there is nothing to gate" assert sorted(always) == sorted(k for _, k in _marked()) @@ -601,8 +600,7 @@ def _env(tmp_path, **extra): def _run(script: str, env): - return subprocess.run(["bash", "-c", script], cwd=REPO_ROOT, env=env, - capture_output=True, text=True) + return subprocess.run(["bash", "-c", script], cwd=REPO_ROOT, env=env, capture_output=True, text=True) def test_the_guard_PASSES_when_every_step_ran(tmp_path): diff --git a/scripts/tests/test_decisions_validate.py b/scripts/tests/test_decisions_validate.py index 8cf60d6f2..1310db8d8 100644 --- a/scripts/tests/test_decisions_validate.py +++ b/scripts/tests/test_decisions_validate.py @@ -744,7 +744,9 @@ def test_wing_faults_block_scalar_record_fails_loudly(tmp_path): this corpus's very long `rule:` values — makes the whole record silently invisible.""" records, archive = _wing(tmp_path) bad = records / "ci" / "blockscalar.md" - bad.write_text(_GOOD.replace("rule: 'a rule on one quoted line'\n", "rule: >-\n a long rule wrapped\n over two lines\n")) + bad.write_text( + _GOOD.replace("rule: 'a rule on one quoted line'\n", "rule: >-\n a long rule wrapped\n over two lines\n") + ) # Precondition: this really is the silent-vanish case, not some other parse error. assert dl.parse_file(bad) == [], "expected the reader to drop the record entirely" @@ -805,8 +807,10 @@ def test_wing_faults_exempts_stripped_legacy_archive_files(tmp_path): def test_validate_surfaces_wing_faults_as_errors(tmp_path): """Faults must arrive as validator ERRORS (exit 1), not warnings.""" - errs = _v([_rec(key="ci.a", source=Path("docs/decisions/records/ci/a.md"), heading="A")], - wing_faults=["docs/decisions/records/ci/x.md: parsed to 0 records, expected exactly 1"]) + errs = _v( + [_rec(key="ci.a", source=Path("docs/decisions/records/ci/a.md"), heading="A")], + wing_faults=["docs/decisions/records/ci/x.md: parsed to 0 records, expected exactly 1"], + ) assert any("x.md" in e for e in errs), errs @@ -826,7 +830,7 @@ def test_wing_faults_sees_a_DEEPER_nested_archive_record(tmp_path): one level down'.""" records, archive = _wing(tmp_path) (records / "ci" / "good.md").write_text(_GOOD) - (archive / "api.md").write_text("# api\n\n## Records formerly in this file\n") # still exempt + (archive / "api.md").write_text("# api\n\n## Records formerly in this file\n") # still exempt deep = archive / "ci" / "sub" deep.mkdir(parents=True) (deep / "broken.md").write_text("# not a record\n") @@ -887,8 +891,7 @@ def test_junk_frontmatter_key_from_a_split_value_is_faulted(tmp_path): PyYAML rejects this input, so the hand reader is more permissive than the writer.""" records, archive = _wing(tmp_path) bad = records / "ci" / "corrupt.md" - bad.write_text(_GOOD.replace("rule: 'a rule on one quoted line'\n", - "rule: >-\nthe real rule: with a colon\n")) + bad.write_text(_GOOD.replace("rule: 'a rule on one quoted line'\n", "rule: >-\nthe real rule: with a colon\n")) recs = dl.parse_file(bad) assert len(recs) == 1 and recs[0].key, "precondition: this parses to one KEYED record" assert recs[0].rule == ">-", f"precondition: the real value was truncated, got {recs[0].rule!r}" @@ -903,7 +906,7 @@ def test_an_empty_or_missing_record_wing_is_LOUD(tmp_path): missing = dv.record_wing_faults(tmp_path / "nope" / "records", tmp_path / "nope" / "archive") assert missing and "missing or contains no" in missing[0], missing - records, archive = _wing(tmp_path) # exists but holds no *.md + records, archive = _wing(tmp_path) # exists but holds no *.md empty = dv.record_wing_faults(records, archive) assert empty and "missing or contains no" in empty[0], empty @@ -1437,7 +1440,6 @@ def test_no_budget_flag_means_no_retirement_warning(capsys): assert "is RETIRED and was IGNORED" not in capsys.readouterr().err - def test_main_reports_ceiling_drift_as_a_NOTICE_and_still_exits_0(capsys): """The fine claim's live wiring (#688): the drift notice must fire, and must NOT turn the run red — the entire point of the v5 split. @@ -1511,8 +1513,7 @@ def test_main_actually_REPORTS_the_ceiling_and_the_trend(capsys): err = capsys.readouterr().err assert "prose lines across" in err, "the aggregate trend notice must always print" - over = [r.key for r in dl.all_active_records() - if r.key and dv.record_prose_lines(r) > dv.RECORD_CEILING_DEFAULT] + over = [r.key for r in dl.all_active_records() if r.key and dv.record_prose_lines(r) > dv.RECORD_CEILING_DEFAULT] warned = "exceed the" in err and "prose ceiling" in err assert warned is bool(over), f"ceiling warning printed={warned} but {len(over)} record(s) are over it" if over: diff --git a/scripts/tests/test_guard_inventory.py b/scripts/tests/test_guard_inventory.py index 34282ee8a..418ae7fb7 100644 --- a/scripts/tests/test_guard_inventory.py +++ b/scripts/tests/test_guard_inventory.py @@ -72,16 +72,17 @@ def derived_guard_files() -> set[str]: not a guard, and globbing would drag in every helper and make the inventory a chore that gets rubber-stamped. """ - found = { - str(p.relative_to(REPO_ROOT)) for p in HOOKS_DIR.glob("*.sh") - } | { - str(p.relative_to(REPO_ROOT)) for p in HUSKY_DIR.iterdir() if p.is_file() - } | { - # `pr-checks.yml` runs `pytest scripts/tests` as a directory, so every file in it is - # invoked and none is individually named anywhere. Globbing is the only derivation that - # matches how they actually run. - str(p.relative_to(REPO_ROOT)) for p in (REPO_ROOT / "scripts" / "tests").glob("test_*.py") - } + found = ( + {str(p.relative_to(REPO_ROOT)) for p in HOOKS_DIR.glob("*.sh")} + | {str(p.relative_to(REPO_ROOT)) for p in HUSKY_DIR.iterdir() if p.is_file()} + | { + # `pr-checks.yml` runs `pytest scripts/tests` as a directory, so every file in it is + # invoked and none is individually named anywhere. Globbing is the only derivation that + # matches how they actually run. + str(p.relative_to(REPO_ROOT)) + for p in (REPO_ROOT / "scripts" / "tests").glob("test_*.py") + } + ) callers = list(WORKFLOWS_DIR.glob("*.yml")) + list(HOOKS_DIR.glob("*.sh")) callers += [p for p in HUSKY_DIR.iterdir() if p.is_file()] @@ -113,13 +114,8 @@ def wired_hook_files() -> set[str]: text = (REPO_ROOT / ".claude" / "settings.json").read_text() for husky in HUSKY_DIR.iterdir(): if husky.is_file(): - text += "\n".join( - line for line in husky.read_text().splitlines() - if not line.lstrip().startswith("#") - ) - return { - str(p.relative_to(REPO_ROOT)) for p in HOOKS_DIR.glob("*.sh") if p.name in text - } + text += "\n".join(line for line in husky.read_text().splitlines() if not line.lstrip().startswith("#")) + return {str(p.relative_to(REPO_ROOT)) for p in HOOKS_DIR.glob("*.sh") if p.name in text} def inventory_rows() -> list[tuple[str, str, str, str]]: @@ -235,9 +231,7 @@ def test_proof_rows_do_not_themselves_claim_a_proof(): for guard, kind, proof, ref in inventory_rows(): if kind == "PROOF": assert proof == "NONE", f"{guard} is PROOF but claims Proof {proof}" - assert guard.startswith("scripts/tests/"), ( - f"{guard} is marked PROOF but does not live in scripts/tests/" - ) + assert guard.startswith("scripts/tests/"), f"{guard} is marked PROOF but does not live in scripts/tests/" assert ref in ("—", "-", ""), f"{guard}: PROOF rows carry no proof ref" @@ -283,8 +277,12 @@ def test_the_summary_counts_match_the_table(): ) claimed = tuple(int(g) for g in m.groups()) actual = ( - kinds["GUARD"], kinds["TOOLING"], kinds["PROOF"], - grades["MUTATION"], grades["BEHAVIOUR-ONLY"], grades["NONE"], + kinds["GUARD"], + kinds["TOOLING"], + kinds["PROOF"], + grades["MUTATION"], + grades["BEHAVIOUR-ONLY"], + grades["NONE"], ) assert claimed == actual, ( f"the summary claims (guards, tooling, proofs, mutation, behaviour-only, none) = {claimed} " diff --git a/scripts/tests/test_hook_fire_log.py b/scripts/tests/test_hook_fire_log.py index 125bbed88..904af8873 100644 --- a/scripts/tests/test_hook_fire_log.py +++ b/scripts/tests/test_hook_fire_log.py @@ -27,9 +27,9 @@ that is fine alone and lethal in context. from __future__ import annotations import os +import pty import re import subprocess -import pty from pathlib import Path import pytest @@ -103,9 +103,7 @@ def instrumentation_faults(text: str, name: str) -> list[str]: if not mode: faults.append(f"{name}: etv_hook_fire_begin names no stdout mode") elif mode.group(1) != want: - faults.append( - f"{name}: begins in {mode.group(1)!r} mode but its wiring implies {want!r}" - ) + faults.append(f"{name}: begins in {mode.group(1)!r} mode but its wiring implies {want!r}") # Order is the whole point: 8 of the hooks slurp stdin with `input=$(cat)`, and a begin # placed after that read would find the pipe already drained — recording a fire with no @@ -133,8 +131,9 @@ def strip_instrumentation(text: str) -> str: out.pop() continue if skipping: - if line.startswith(("#", "ETV_HOOK_FIRE_LIB=", "[ -r ", "type etv_hook_fire_begin", - "etv_hook_fire_begin ")): + if line.startswith( + ("#", "ETV_HOOK_FIRE_LIB=", "[ -r ", "type etv_hook_fire_begin", "etv_hook_fire_begin ") + ): continue skipping = False out.append(line) @@ -173,8 +172,15 @@ def test_the_stripper_removes_EXACTLY_the_preamble_and_nothing_else(): """ import difflib - allowed = ("# ersatztv#776", "# git hook:", "# Claude hook:", "ETV_HOOK_FIRE_LIB=", - "[ -r \"$ETV_HOOK_FIRE_LIB\"", "type etv_hook_fire_begin", "etv_hook_fire_begin ") + allowed = ( + "# ersatztv#776", + "# git hook:", + "# Claude hook:", + "ETV_HOOK_FIRE_LIB=", + '[ -r "$ETV_HOOK_FIRE_LIB"', + "type etv_hook_fire_begin", + "etv_hook_fire_begin ", + ) for hook in hook_scripts(): cur = hook.read_text().splitlines(keepends=True) stripped = strip_instrumentation("".join(cur)).splitlines(keepends=True) @@ -189,7 +195,6 @@ def test_the_stripper_removes_EXACTLY_the_preamble_and_nothing_else(): ) - def test_the_stripper_actually_strips(): """The A/B control and the mutation are the same function. If it were a no-op, the differential test would compare each hook against itself and pass on a wrapper that breaks everything.""" @@ -211,8 +216,9 @@ def test_every_hook_reports_that_it_fired(): for hook in hook_scripts(): faults += instrumentation_faults(hook.read_text(), hook.stem) assert not faults, ( - "these hooks do not report their own execution:\n " + "\n ".join(faults) + - "\n\nAdd the three-line preamble after the `set -` line and BEFORE any stdin read. A hook " + "these hooks do not report their own execution:\n " + + "\n ".join(faults) + + "\n\nAdd the three-line preamble after the `set -` line and BEFORE any stdin read. A hook " "that does not report is one whose firing we can only infer, which is ersatztv#776." ) @@ -235,7 +241,7 @@ def test_a_hook_that_LOSES_its_instrumentation_is_DETECTED(): def test_begin_placed_AFTER_the_stdin_read_is_DETECTED(): """The subtler mutation: present but too late. Ordering is the property that makes it work.""" - late = 'set -euo pipefail\ninput=$(cat)\nETV_HOOK_FIRE_LIB="x"\n[ -r "$ETV_HOOK_FIRE_LIB" ] && . "$ETV_HOOK_FIRE_LIB" || true\netv_hook_fire_begin demo "" capture || true\n' + late = 'set -euo pipefail\ninput=$(cat)\nETV_HOOK_FIRE_LIB="x"\n[ -r "$ETV_HOOK_FIRE_LIB" ] && . "$ETV_HOOK_FIRE_LIB" || true\netv_hook_fire_begin demo "" capture || true\n' # noqa: E501 - kept on one line so the fixture is greppable against the payload the hook receives faults = instrumentation_faults(late, "demo") assert any("reads stdin before" in f for f in faults), ( "a begin call placed after `input=$(cat)` was accepted. It would record a fire with no " @@ -257,15 +263,15 @@ def test_begin_placed_AFTER_the_stdin_read_is_DETECTED(): # issue `deny`. An A/B over silent allows proves transparency on the one path where there is nothing # to be transparent about. PAYLOADS = { - "bash-deny": '{"session_id":"s","hook_event_name":"PreToolUse","tool_name":"Bash","cwd":"%(cwd)s","tool_input":{"command":"ETV_UPDATE_GOLDENS=1 dotnet test"}}', - "bash-allow": '{"session_id":"s","hook_event_name":"PreToolUse","tool_name":"Bash","cwd":"%(cwd)s","tool_input":{"command":"ls -la"}}', - "git-commit": '{"session_id":"s","hook_event_name":"PreToolUse","tool_name":"Bash","cwd":"%(cwd)s","tool_input":{"command":"git commit -m x"}}', + "bash-deny": '{"session_id":"s","hook_event_name":"PreToolUse","tool_name":"Bash","cwd":"%(cwd)s","tool_input":{"command":"ETV_UPDATE_GOLDENS=1 dotnet test"}}', # noqa: E501 - kept on one line so the fixture is greppable against the payload the hook receives + "bash-allow": '{"session_id":"s","hook_event_name":"PreToolUse","tool_name":"Bash","cwd":"%(cwd)s","tool_input":{"command":"ls -la"}}', # noqa: E501 - kept on one line so the fixture is greppable against the payload the hook receives + "git-commit": '{"session_id":"s","hook_event_name":"PreToolUse","tool_name":"Bash","cwd":"%(cwd)s","tool_input":{"command":"git commit -m x"}}', # noqa: E501 - kept on one line so the fixture is greppable against the payload the hook receives "nav-deny": '{"session_id":"s","hook_event_name":"PreToolUse","tool_name":"mcp__x__navigate","cwd":"%(cwd)s","tool_input":{"url":"http://h/iptv/channels.m3u"}}', - "agent-no-model": '{"session_id":"s","hook_event_name":"PreToolUse","tool_name":"Agent","cwd":"%(cwd)s","tool_input":{"description":"d","prompt":"p"}}', - "agent-model": '{"session_id":"s","hook_event_name":"PreToolUse","tool_name":"Agent","cwd":"%(cwd)s","tool_input":{"description":"d","prompt":"p","model":"sonnet"}}', - "merge": '{"session_id":"s","hook_event_name":"PreToolUse","tool_name":"mcp__gitea__pull_request_write","cwd":"%(cwd)s","tool_input":{"method":"merge","owner":"timothy","repo":"ersatztv","pull_number":1}}', - "worktree-add": '{"session_id":"s","hook_event_name":"PostToolUse","tool_name":"Bash","cwd":"%(cwd)s","tool_input":{"command":"git worktree add /tmp/nope-%(nonce)s HEAD"}}', - "ui-edit": '{"session_id":"s","hook_event_name":"PreToolUse","tool_name":"Edit","cwd":"%(cwd)s","tool_input":{"file_path":"web/src/screens/Channels.tsx"}}', + "agent-no-model": '{"session_id":"s","hook_event_name":"PreToolUse","tool_name":"Agent","cwd":"%(cwd)s","tool_input":{"description":"d","prompt":"p"}}', # noqa: E501 - kept on one line so the fixture is greppable against the payload the hook receives + "agent-model": '{"session_id":"s","hook_event_name":"PreToolUse","tool_name":"Agent","cwd":"%(cwd)s","tool_input":{"description":"d","prompt":"p","model":"sonnet"}}', # noqa: E501 - kept on one line so the fixture is greppable against the payload the hook receives + "merge": '{"session_id":"s","hook_event_name":"PreToolUse","tool_name":"mcp__gitea__pull_request_write","cwd":"%(cwd)s","tool_input":{"method":"merge","owner":"timothy","repo":"ersatztv","pull_number":1}}', # noqa: E501 - kept on one line so the fixture is greppable against the payload the hook receives + "worktree-add": '{"session_id":"s","hook_event_name":"PostToolUse","tool_name":"Bash","cwd":"%(cwd)s","tool_input":{"command":"git worktree add /tmp/nope-%(nonce)s HEAD"}}', # noqa: E501 - kept on one line so the fixture is greppable against the payload the hook receives + "ui-edit": '{"session_id":"s","hook_event_name":"PreToolUse","tool_name":"Edit","cwd":"%(cwd)s","tool_input":{"file_path":"web/src/screens/Channels.tsx"}}', # noqa: E501 - kept on one line so the fixture is greppable against the payload the hook receives "empty": "", "garbage": "not json at all", } @@ -300,17 +306,23 @@ def sandbox(tmp_path_factory): def _run(script: Path, payload: str, env: dict, cwd: Path, arg: str | None = None): cmd = ["bash", str(script)] + ([arg] if arg else []) - p = subprocess.run( - cmd, input=payload.encode(), capture_output=True, cwd=str(cwd), env=env, timeout=60 - ) + p = subprocess.run(cmd, input=payload.encode(), capture_output=True, cwd=str(cwd), env=env, timeout=60) return p.returncode, p.stdout def _git(cwd: Path, *args: str) -> None: subprocess.run( - ["git", *args], cwd=str(cwd), check=True, capture_output=True, - env={**os.environ, "GIT_AUTHOR_NAME": "t", "GIT_AUTHOR_EMAIL": "t@e", - "GIT_COMMITTER_NAME": "t", "GIT_COMMITTER_EMAIL": "t@e"}, + ["git", *args], + cwd=str(cwd), + check=True, + capture_output=True, + env={ + **os.environ, + "GIT_AUTHOR_NAME": "t", + "GIT_AUTHOR_EMAIL": "t@e", + "GIT_COMMITTER_NAME": "t", + "GIT_COMMITTER_EMAIL": "t@e", + }, ) @@ -328,18 +340,23 @@ def positives(sandbox, tmp_path_factory): def pay(**kw) -> str: import json as _json - return _json.dumps({"session_id": "me", "hook_event_name": "PreToolUse", - "tool_name": "Bash", **kw}) + + return _json.dumps({"session_id": "me", "hook_event_name": "PreToolUse", "tool_name": "Bash", **kw}) # --- worktree-guard: a marker naming ANOTHER session, on a `git commit` ------------------- wt = w / "sibling" wt.mkdir() _git(wt, "init", "-q", ".") (wt / ".claude-worktree-owner").write_text("SOME-OTHER-SESSION\n") - cases["pretooluse-worktree-guard"] = [( - "foreign-marker-deny", - pay(cwd=str(wt), tool_input={"command": "git commit -m x"}), None, base_env, wt, - )] + cases["pretooluse-worktree-guard"] = [ + ( + "foreign-marker-deny", + pay(cwd=str(wt), tool_input={"command": "git commit -m x"}), + None, + base_env, + wt, + ) + ] # --- bom-guard: repo path must match `*ersatztv*`, with a staged BOM-carrying .cs ---------- br = w / "ersatztv-scratch" @@ -347,10 +364,15 @@ def positives(sandbox, tmp_path_factory): _git(br, "init", "-q", ".") (br / "Bad.cs").write_bytes(b"\xef\xbb\xbfclass A {}\n") _git(br, "add", "Bad.cs") - cases["pretooluse-bom-guard"] = [( - "staged-bom-deny", - pay(cwd=str(br), tool_input={"command": "git commit -m x"}), None, base_env, br, - )] + cases["pretooluse-bom-guard"] = [ + ( + "staged-bom-deny", + pay(cwd=str(br), tool_input={"command": "git commit -m x"}), + None, + base_env, + br, + ) + ] # --- agent-ram: both thresholds, via a stubbed `memory_pressure` --------------------------- ram_cases = [] @@ -362,11 +384,15 @@ def positives(sandbox, tmp_path_factory): stub.chmod(0o755) env = dict(base_env) env["PATH"] = f"{bindir}:{os.environ['PATH']}" - ram_cases.append(( - f"free-{pct}pct-{label}", - pay(tool_name="Agent", cwd=str(w), tool_input={"prompt": "p", "model": "sonnet"}), - None, env, w, - )) + ram_cases.append( + ( + f"free-{pct}pct-{label}", + pay(tool_name="Agent", cwd=str(w), tool_input={"prompt": "p", "model": "sonnet"}), + None, + env, + w, + ) + ) cases["pretooluse-agent-ram"] = ram_cases # --- decisions-guard: a validator that prints and blocks, and one that prints and passes --- @@ -409,12 +435,16 @@ def positives(sandbox, tmp_path_factory): _git(ahead, "add", "-A") _git(ahead, "commit", "-qm", "advance main") _git(ahead, "push", "-q", "origin", "main") - head = subprocess.run(["git", "rev-parse", "HEAD"], cwd=str(behind), - capture_output=True, text=True).stdout.strip() - cases["prepush-rebase-check"] = [( - "behind-origin-main-blocks", - f"refs/heads/feat {head} refs/heads/feat {'0' * 40}\n", None, base_env, behind, - )] + head = subprocess.run(["git", "rev-parse", "HEAD"], cwd=str(behind), capture_output=True, text=True).stdout.strip() + cases["prepush-rebase-check"] = [ + ( + "behind-origin-main-blocks", + f"refs/heads/feat {head} refs/heads/feat {'0' * 40}\n", + None, + base_env, + behind, + ) + ] # clean-worktree-check: a file both MODIFIED in the tree and present in the pushed set. dirty = clone("dirty") @@ -422,13 +452,16 @@ def positives(sandbox, tmp_path_factory): _git(dirty, "add", "-A") _git(dirty, "commit", "-qm", "change file") (dirty / "file.txt").write_text("uncommitted change\n") - dhead = subprocess.run(["git", "rev-parse", "HEAD"], cwd=str(dirty), - capture_output=True, text=True).stdout.strip() - cases["prepush-clean-worktree-check"] = [( - "dirty-file-in-pushed-set-blocks", - f"refs/heads/main {dhead} refs/heads/main {'0' * 40}\n", None, base_env, dirty, - )] - + dhead = subprocess.run(["git", "rev-parse", "HEAD"], cwd=str(dirty), capture_output=True, text=True).stdout.strip() + cases["prepush-clean-worktree-check"] = [ + ( + "dirty-file-in-pushed-set-blocks", + f"refs/heads/main {dhead} refs/heads/main {'0' * 40}\n", + None, + base_env, + dirty, + ) + ] # --- prepush-donewhen: a push to main closing an issue with an unticked Done-when box ------- # @@ -443,7 +476,7 @@ def positives(sandbox, tmp_path_factory): body = _json.dumps({"body": "## Done-when\n\n- [ ] not finished\n- [x] finished\n"}).encode() class _Stub(http.server.BaseHTTPRequestHandler): - def do_GET(self): # noqa: N802 + def do_GET(self): self.send_response(200) self.send_header("Content-Type", "application/json") self.send_header("Content-Length", str(len(body))) @@ -457,20 +490,23 @@ def positives(sandbox, tmp_path_factory): threading.Thread(target=srv.serve_forever, daemon=True).start() dw = clone("donewhen") - (dw / "src.cs").write_text("class A {}\n") # NOT docs-only, so the exemption does not apply + (dw / "src.cs").write_text("class A {}\n") # NOT docs-only, so the exemption does not apply _git(dw, "add", "-A") _git(dw, "commit", "-qm", "feat: thing\n\nfixes #999") - dw_head = subprocess.run(["git", "rev-parse", "HEAD"], cwd=str(dw), - capture_output=True, text=True).stdout.strip() - dw_prev = subprocess.run(["git", "rev-parse", "HEAD~1"], cwd=str(dw), - capture_output=True, text=True).stdout.strip() + dw_head = subprocess.run(["git", "rev-parse", "HEAD"], cwd=str(dw), capture_output=True, text=True).stdout.strip() + dw_prev = subprocess.run(["git", "rev-parse", "HEAD~1"], cwd=str(dw), capture_output=True, text=True).stdout.strip() dw_env = dict(base_env) dw_env["ETV_GITEA_URL"] = f"http://127.0.0.1:{srv.server_address[1]}" dw_env["ETV_GITEA_BASICAUTH"] = "stub:stub" - cases["prepush-donewhen"] = [( - "unticked-donewhen-blocks-push-to-main", - f"refs/heads/main {dw_head} refs/heads/main {dw_prev}\n", None, dw_env, dw, - )] + cases["prepush-donewhen"] = [ + ( + "unticked-donewhen-blocks-push-to-main", + f"refs/heads/main {dw_head} refs/heads/main {dw_prev}\n", + None, + dw_env, + dw, + ) + ] return cases @@ -482,10 +518,15 @@ def _ab_cases(hook: Path, sandbox, positives) -> list[tuple]: args = [None, "start", "finish"] if hook.stem == "design-sync-reminder" else [None] for name, template in PAYLOADS.items(): for arg in args: - cases.append(( - f"matrix:{name}:{arg}", template % {"cwd": str(root), "nonce": name}, - arg, env, root, - )) + cases.append( + ( + f"matrix:{name}:{arg}", + template % {"cwd": str(root), "nonce": name}, + arg, + env, + root, + ) + ) for label, payload, arg, penv, cwd in positives.get(hook.stem, []): cases.append((f"positive:{label}", payload, arg, penv, cwd)) return cases @@ -513,8 +554,7 @@ def _ab_run(hook: Path, sandbox, case, tag_root: str): def one(script: Path, t: str): cmd = ["bash", str(script)] + ([arg] if arg else []) - p = subprocess.run(cmd, input=payload.encode(), capture_output=True, - cwd=str(cwd), env=side(t), timeout=90) + p = subprocess.run(cmd, input=payload.encode(), capture_output=True, cwd=str(cwd), env=side(t), timeout=90) return p.returncode, p.stdout, p.stderr return one(control, "control"), one(hook, "instrumented") @@ -613,11 +653,16 @@ def test_the_worktree_markers_SIDE_EFFECT_is_unchanged(sandbox, tmp_path): script = tmp_path / "control-marker.sh" script.write_text(strip_instrumentation(hook.read_text())) import json as _json - payload = _json.dumps({ - "session_id": "SESSION-XYZ", "hook_event_name": "PostToolUse", "tool_name": "Bash", - "cwd": str(repo), - "tool_input": {"command": f"git worktree add {target} HEAD"}, - }) + + payload = _json.dumps( + { + "session_id": "SESSION-XYZ", + "hook_event_name": "PostToolUse", + "tool_name": "Bash", + "cwd": str(repo), + "tool_input": {"command": f"git worktree add {target} HEAD"}, + } + ) _git(repo, "worktree", "add", "-q", str(target), "HEAD") _run(script, payload, env, repo) marker = target / ".claude-worktree-owner" @@ -641,15 +686,13 @@ def test_stdout_is_replayed_BYTE_EXACT(sandbox): hook.write_text( "#!/usr/bin/env bash\nset -euo pipefail\n" f'. "{SINK}"\n' - "etv_hook_fire_begin trailing \"\" capture || true\n" + 'etv_hook_fire_begin trailing "" capture || true\n' "input=$(cat)\n" r'printf "{\"hookSpecificOutput\":{\"permissionDecision\":\"deny\"}}\n\n"' + "\n" ) rc, out = _run(hook, '{"tool_name":"Bash"}', env, root) assert rc == 0 - assert out == b'{"hookSpecificOutput":{"permissionDecision":"deny"}}\n\n', ( - f"trailing bytes were altered: {out!r}" - ) + assert out == b'{"hookSpecificOutput":{"permissionDecision":"deny"}}\n\n', f"trailing bytes were altered: {out!r}" @pytest.mark.parametrize("code", [0, 1, 2, 3]) @@ -704,8 +747,12 @@ def test_a_TTY_stdin_is_not_slurped(sandbox): master, slave = pty.openpty() try: p = subprocess.Popen( - ["bash", str(hook)], stdin=slave, stdout=subprocess.PIPE, - stderr=subprocess.DEVNULL, cwd=str(root), env=env, + ["bash", str(hook)], + stdin=slave, + stdout=subprocess.PIPE, + stderr=subprocess.DEVNULL, + cwd=str(root), + env=env, ) os.close(slave) try: @@ -751,11 +798,11 @@ def test_logging_failure_does_not_break_the_hook(sandbox): @pytest.mark.parametrize( "emit,mode,expected", [ - (r'{\"hookSpecificOutput\":{\"permissionDecision\":\"deny\"}}', "capture", "deny"), - (r'{\"hookSpecificOutput\":{\"permissionDecision\":\"ask\"}}', "capture", "ask"), - (r'{\"hookSpecificOutput\":{\"permissionDecision\":\"allow\"}}', "capture", "allow"), - (r'{\"hookSpecificOutput\":{\"additionalContext\":\"hi\"}}', "capture", "context"), - (r'{\"decision\":\"block\",\"reason\":\"r\"}', "capture", "block"), + (r"{\"hookSpecificOutput\":{\"permissionDecision\":\"deny\"}}", "capture", "deny"), + (r"{\"hookSpecificOutput\":{\"permissionDecision\":\"ask\"}}", "capture", "ask"), + (r"{\"hookSpecificOutput\":{\"permissionDecision\":\"allow\"}}", "capture", "allow"), + (r"{\"hookSpecificOutput\":{\"additionalContext\":\"hi\"}}", "capture", "context"), + (r"{\"decision\":\"block\",\"reason\":\"r\"}", "capture", "block"), ("", "capture", "no-op"), ("", "stream", "pass"), ], @@ -822,7 +869,10 @@ def test_the_report_names_hooks_that_NEVER_fired(sandbox): ) p = subprocess.run( ["bash", str(SINK), "report", "--all", "--dir", str(logdir)], - capture_output=True, cwd=str(REPO_ROOT), env=env, timeout=60, + capture_output=True, + cwd=str(REPO_ROOT), + env=env, + timeout=60, ) out = p.stdout.decode() assert p.returncode == 0, p.stderr.decode() @@ -859,7 +909,10 @@ def test_the_report_REFUSES_an_empty_population(tmp_path, sandbox): runenv.pop("CLAUDE_PROJECT_DIR", None) p = subprocess.run( ["bash", str(copied), "report", "--all", "--dir", str(tmp_path / "log")], - capture_output=True, cwd=str(tmp_path), env=runenv, timeout=60, + capture_output=True, + cwd=str(tmp_path), + env=runenv, + timeout=60, ) assert p.returncode == 2, ( "the report exited 0 over an empty hook population. It must refuse rather than print a " @@ -895,12 +948,11 @@ def test_stderr_is_NOT_silenced(sandbox): 'printf "IMPORTANT DIAGNOSTIC\\n" >&2\n' "exit 1\n" ) - p = subprocess.run(["bash", str(hook)], input=b'{"tool_name":"Bash"}', - capture_output=True, cwd=str(root), env=env, timeout=60) - assert p.returncode == 1 - assert b"IMPORTANT DIAGNOSTIC" in p.stderr, ( - f"the hook's stderr was swallowed by the instrumentation: {p.stderr!r}" + p = subprocess.run( + ["bash", str(hook)], input=b'{"tool_name":"Bash"}', capture_output=True, cwd=str(root), env=env, timeout=60 ) + assert p.returncode == 1 + assert b"IMPORTANT DIAGNOSTIC" in p.stderr, f"the hook's stderr was swallowed by the instrumentation: {p.stderr!r}" def test_output_SURVIVES_a_vanished_stdout_tempfile(sandbox): @@ -923,8 +975,14 @@ def test_output_SURVIVES_a_vanished_stdout_tempfile(sandbox): ) tmp = root / "vanish-tmp" tmp.mkdir(exist_ok=True) - p = subprocess.run(["bash", str(hook)], input=b'{"tool_name":"Bash"}', capture_output=True, - cwd=str(root), env={**env, "TMPDIR": str(tmp)}, timeout=60) + p = subprocess.run( + ["bash", str(hook)], + input=b'{"tool_name":"Bash"}', + capture_output=True, + cwd=str(root), + env={**env, "TMPDIR": str(tmp)}, + timeout=60, + ) assert p.returncode == 0 assert b'"permissionDecision":"deny"' in p.stdout, ( "the guard's deny was DISCARDED when its stdout scratch file vanished mid-run. The hook " @@ -932,24 +990,22 @@ def test_output_SURVIVES_a_vanished_stdout_tempfile(sandbox): ) - - @pytest.mark.parametrize( "emit,code,expected", [ # A NON-CANONICAL value is recorded as non-canonical, not laundered into a valid decision. # This case went both ways before settling: first filed as unclassified prose, then # lowercased into a clean `deny` (manufacturing a decision the harness may never honour). - (r'{\"permissionDecision\":\"Deny\"}', 0, "unrecognized"), + (r"{\"permissionDecision\":\"Deny\"}", 0, "unrecognized"), # A non-zero status neither ERASES a printed decision nor annotates it. The status has its # own field; the decision field states the decision and nothing else. - (r'{\"permissionDecision\":\"deny\"}', 1, "deny"), + (r"{\"permissionDecision\":\"deny\"}", 1, "deny"), # A failure with nothing classifiable IS an error. ("", 1, "error"), # Exit 2 is the harness's block channel and DOMINATES the printed JSON. Recording the # printed `allow` would report a permit for a call that was actually refused — the one # direction a log of security decisions must never be wrong in. - (r'{\"permissionDecision\":\"allow\"}', 2, "deny-exit2"), + (r"{\"permissionDecision\":\"allow\"}", 2, "deny-exit2"), ], ) def test_the_classifier_does_not_MISREPORT(sandbox, emit, code, expected): @@ -961,12 +1017,16 @@ def test_the_classifier_does_not_MISREPORT(sandbox, emit, code, expected): "#!/usr/bin/env bash\nset -uo pipefail\n" f'. "{SINK}"\n' 'etv_hook_fire_begin cls "" capture || true\n' - "input=$(cat)\n" - + (f'printf "{emit}"\n' if emit else "") - + f"exit {code}\n" + "input=$(cat)\n" + (f'printf "{emit}"\n' if emit else "") + f"exit {code}\n" + ) + subprocess.run( + ["bash", str(hook)], + input=b'{"session_id":"s1","tool_name":"Bash"}', + capture_output=True, + cwd=str(root), + env=runenv, + timeout=60, ) - subprocess.run(["bash", str(hook)], input=b'{"session_id":"s1","tool_name":"Bash"}', - capture_output=True, cwd=str(root), env=runenv, timeout=60) rec = [ln for ln in (logdir / "s1.jsonl").read_text().splitlines() if '"phase":"exit"' in ln] assert f'"decision":"{expected}"' in rec[0], f"expected {expected}, got {rec[0]}" @@ -983,9 +1043,14 @@ def test_the_word_additionalContext_in_PROSE_is_not_a_decision(sandbox): "input=$(cat)\n" 'printf "note: this hook does not use additionalContext at all\\n"\n' ) - subprocess.run(["bash", str(hook)], input=b'{"session_id":"s1","tool_name":"Bash"}', - capture_output=True, cwd=str(root), - env={**env, "ETV_HOOK_FIRE_LOG_DIR": str(logdir)}, timeout=60) + subprocess.run( + ["bash", str(hook)], + input=b'{"session_id":"s1","tool_name":"Bash"}', + capture_output=True, + cwd=str(root), + env={**env, "ETV_HOOK_FIRE_LOG_DIR": str(logdir)}, + timeout=60, + ) rec = [ln for ln in (logdir / "s1.jsonl").read_text().splitlines() if '"phase":"exit"' in ln] assert '"decision":"output"' in rec[0], f"prose was classified as a decision: {rec[0]}" @@ -1001,14 +1066,17 @@ def test_the_session_id_cannot_ESCAPE_the_log_directory(sandbox, tmp_path): logdir = tmp_path / "logs" hook = tmp_path / "esc.sh" hook.write_text( - "#!/usr/bin/env bash\nset -uo pipefail\n" - f'. "{SINK}"\n' - 'etv_hook_fire_begin esc "" capture || true\n' - "input=$(cat)\n" + f'#!/usr/bin/env bash\nset -uo pipefail\n. "{SINK}"\netv_hook_fire_begin esc "" capture || true\ninput=$(cat)\n' ) evil = '{"session_id":"../../escaped","tool_name":"Bash"}' - subprocess.run(["bash", str(hook)], input=evil.encode(), capture_output=True, - cwd=str(tmp_path), env={**env, "ETV_HOOK_FIRE_LOG_DIR": str(logdir)}, timeout=60) + subprocess.run( + ["bash", str(hook)], + input=evil.encode(), + capture_output=True, + cwd=str(tmp_path), + env={**env, "ETV_HOOK_FIRE_LOG_DIR": str(logdir)}, + timeout=60, + ) written = list(logdir.glob("*.jsonl")) assert written, "nothing was logged at all" for f in written: @@ -1040,8 +1108,14 @@ def test_the_suite_does_not_write_to_the_PRODUCTION_log(sandbox): 'etv_hook_fire_begin prodcheck "" capture || true\n' "input=$(cat)\n" ) - subprocess.run(["bash", str(hook)], input=b'{"session_id":"s1","tool_name":"Bash"}', - capture_output=True, cwd=str(root), env=env, timeout=60) + subprocess.run( + ["bash", str(hook)], + input=b'{"session_id":"s1","tool_name":"Bash"}', + capture_output=True, + cwd=str(root), + env=env, + timeout=60, + ) after = sorted(p.stat().st_mtime_ns for p in default.glob("*.jsonl")) if default.exists() else [] assert before == after, "a test run modified the production hook-fire log" @@ -1075,16 +1149,17 @@ def test_DELETING_the_replay_makes_the_differential_go_RED(sandbox, positives, t "input=$(cat)\n" r'printf "{\"hookSpecificOutput\":{\"permissionDecision\":\"deny\"}}"' + "\n" ) - p = subprocess.run(["bash", str(hook)], input=b'{"tool_name":"Bash"}', capture_output=True, - cwd=str(root), env={**env, "TMPDIR": str(tmp_path)}, timeout=60) - assert p.stdout == b"", ( - "the mutation did not actually disarm the replay, so this proof establishes nothing: " - f"stdout={p.stdout!r}" + p = subprocess.run( + ["bash", str(hook)], + input=b'{"tool_name":"Bash"}', + capture_output=True, + cwd=str(root), + env={**env, "TMPDIR": str(tmp_path)}, + timeout=60, + ) + assert p.stdout == b"", ( + f"the mutation did not actually disarm the replay, so this proof establishes nothing: stdout={p.stdout!r}" ) - - - - def test_no_exec_in_the_sink_carries_a_STDERR_REDIRECT(): @@ -1123,17 +1198,20 @@ def test_the_log_and_the_REPLAY_agree_when_the_hook_uses_fd_4(sandbox): f'. "{SINK}"\n' 'etv_hook_fire_begin fd4 "" capture || true\n' "input=$(cat)\n" - 'exec 4/dev/null 2>&1; then echo LEAKED_etv_field; fi\n' - 'if declare -F etv_hook_fire__field >/dev/null 2>&1; then echo LEAKED_namespaced; fi\n' + "if declare -F _etv_field >/dev/null 2>&1; then echo LEAKED_etv_field; fi\n" + "if declare -F etv_hook_fire__field >/dev/null 2>&1; then echo LEAKED_namespaced; fi\n" "echo CLEAN\n" ) - p = subprocess.run(["bash", str(hook)], input=b'{"session_id":"s1","tool_name":"Bash"}', - capture_output=True, cwd=str(root), env=env, timeout=60) + p = subprocess.run( + ["bash", str(hook)], + input=b'{"session_id":"s1","tool_name":"Bash"}', + capture_output=True, + cwd=str(root), + env=env, + timeout=60, + ) assert b"LEAKED" not in p.stdout, f"a helper leaked into the hook's namespace: {p.stdout!r}" assert b"CLEAN" in p.stdout diff --git a/scripts/tests/test_jq_preflight.py b/scripts/tests/test_jq_preflight.py index ccc1cfe32..e8d5d8590 100644 --- a/scripts/tests/test_jq_preflight.py +++ b/scripts/tests/test_jq_preflight.py @@ -51,12 +51,12 @@ def preflight(tmp_path): so every test passed. """ shim = bindir / "jq" - body = "#!/bin/sh\nif [ \"$1\" = \"--version\" ]; then\n" + body = '#!/bin/sh\nif [ "$1" = "--version" ]; then\n' if version_line: - body += ' printf "%%s\\n" %s\n' % _shq(version_line) + body += f' printf "%s\\n" {_shq(version_line)}\n' if stderr: - body += ' printf "%%s\\n" %s >&2\n' % _shq(stderr) - body += " exit %d\nfi\nexit 0\n" % exit_code + body += f' printf "%s\\n" {_shq(stderr)} >&2\n' + body += f" exit {exit_code}\nfi\nexit 0\n" shim.write_text(body) shim.chmod(0o755) @@ -73,8 +73,7 @@ def preflight(tmp_path): # but jq itself (`command -v` is a builtin, and bash is invoked by absolute path), so # there is nothing to keep. env["PATH"] = str(bindir) - return subprocess.run([BASH, str(SCRIPT), *args], - env=env, capture_output=True, text=True) + return subprocess.run([BASH, str(SCRIPT), *args], env=env, capture_output=True, text=True) def run_bytes(self, *args): """Same, but WITHOUT text mode. @@ -87,8 +86,7 @@ def preflight(tmp_path): """ env = dict(os.environ) env["PATH"] = str(bindir) - return subprocess.run([BASH, str(SCRIPT), *args], - env=env, capture_output=True) + return subprocess.run([BASH, str(SCRIPT), *args], env=env, capture_output=True) return Handle() @@ -172,11 +170,15 @@ def test_expect_without_a_value_is_a_usage_error_WITH_output(preflight): # the script exited 0 having asserted NOTHING. That is this script's own stated failure mode, # reproduced inside itself, which is why these cases are pinned rather than left to inspection. -@pytest.mark.parametrize("version_line", [ - "jq version 1.6", # some distro wrappers print this form - "JQ-1.6", - "jq-1.6-dirty", -]) + +@pytest.mark.parametrize( + "version_line", + [ + "jq version 1.6", # some distro wrappers print this form + "JQ-1.6", + "jq-1.6-dirty", + ], +) def test_unusual_but_parseable_version_forms_are_accepted(preflight, version_line): preflight.with_jq(version_line) r = preflight.run() @@ -190,7 +192,8 @@ def test_unparseable_version_fails_CLOSED_rather_than_asserting_nothing(prefligh r = preflight.run() assert r.returncode == 1, ( f"{version_line!r} exited {r.returncode}: an unparsed version must never reach — or " - "silently skip — the floor assertion") + "silently skip — the floor assertion" + ) assert "could not parse" in r.stderr @@ -202,20 +205,21 @@ def test_a_jq_that_cannot_START_fails_closed(preflight): discarding the exit status, so that message became the parse input, `2.34` matched, and the floor was certified green on a jq that cannot run at all. """ - preflight.with_jq( - "", stderr="jq: /lib/x86_64-linux-gnu/libc.so.6: version `GLIBC_2.34' not found", - exit_code=127) + preflight.with_jq("", stderr="jq: /lib/x86_64-linux-gnu/libc.so.6: version `GLIBC_2.34' not found", exit_code=127) r = preflight.run() assert r.returncode == 1 assert "cannot run" in r.stderr assert "parsed 2.34" not in r.stdout, "stderr must never be parsed as a version" -@pytest.mark.parametrize("version_line", [ - "warning: something 3.14", # a noise line carrying a plausible number - "2026.07.26 jq-1.6", # a date prefix, which outranks the real version if unanchored - "jq-master-v0.0.0-1.6", -]) +@pytest.mark.parametrize( + "version_line", + [ + "warning: something 3.14", # a noise line carrying a plausible number + "2026.07.26 jq-1.6", # a date prefix, which outranks the real version if unanchored + "jq-master-v0.0.0-1.6", + ], +) def test_a_number_that_is_not_the_VERSION_is_not_accepted_as_one(preflight, version_line): """Matching the first `.` ANYWHERE let a prefix win over the real version. `2026.07.26 jq-1.6` parsed as 2026.07 and sailed over the floor. The pattern is anchored to the @@ -226,10 +230,13 @@ def test_a_number_that_is_not_the_VERSION_is_not_accepted_as_one(preflight, vers assert "could not parse" in r.stderr -@pytest.mark.parametrize("version_line", [ - "jq-99999999999999999999999.0", - "jq-1.99999999999999999999999", -]) +@pytest.mark.parametrize( + "version_line", + [ + "jq-99999999999999999999999.0", + "jq-1.99999999999999999999999", + ], +) def test_an_OUT_OF_RANGE_digit_run_fails_closed(preflight, version_line): """The round-1 fail-open mechanism, resurrected via an over-long number. @@ -247,25 +254,28 @@ def test_an_OUT_OF_RANGE_digit_run_fails_closed(preflight, version_line): assert r.returncode == 1, f"{version_line!r} exited 0 — the floor was not asserted" -@pytest.mark.parametrize("version_line", [ - # Killed by the SEPARATOR restriction (a blank separator must be followed by `version`). - "jq\n2.34: cannot load shared library", - "jq\n\n\n99.9", - "jq -- 2.34 (real jq-1.6)", - "jq\t\t9.9", - # Killed ONLY by the first-line slice + `[[:blank:]]`. These carry the literal word `version`, - # so the separator restriction is satisfied and cannot save us — the newline must be excluded - # from the separator class AND the parse confined to line one. - # - # Without these, a round-5 mutation check found that reverting BOTH of those changes together - # (`[[:blank:]]`→`[[:space:]]` and parsing `$raw` instead of `$first`) left the whole suite - # GREEN: the four cases above are all killed by the separator alone, so they attributed the fix - # to the wrong layer. A test that passes for the wrong reason is how the previous three rounds - # each shipped a defect. - "jq\nversion\n9.9", - "jq\nversion 9.9", - "jq \n version \n 9.9", -]) +@pytest.mark.parametrize( + "version_line", + [ + # Killed by the SEPARATOR restriction (a blank separator must be followed by `version`). + "jq\n2.34: cannot load shared library", + "jq\n\n\n99.9", + "jq -- 2.34 (real jq-1.6)", + "jq\t\t9.9", + # Killed ONLY by the first-line slice + `[[:blank:]]`. These carry the literal word `version`, + # so the separator restriction is satisfied and cannot save us — the newline must be excluded + # from the separator class AND the parse confined to line one. + # + # Without these, a round-5 mutation check found that reverting BOTH of those changes together + # (`[[:blank:]]`→`[[:space:]]` and parsing `$raw` instead of `$first`) left the whole suite + # GREEN: the four cases above are all killed by the separator alone, so they attributed the fix + # to the wrong layer. A test that passes for the wrong reason is how the previous three rounds + # each shipped a defect. + "jq\nversion\n9.9", + "jq\nversion 9.9", + "jq \n version \n 9.9", + ], +) def test_a_number_AFTER_the_jq_token_is_not_reachable_across_filler(preflight, version_line): """Two independent layers keep a stray number from being read as the version, and both are pinned here: the separator must be one of the forms real jq emits (`jq-1.6` / `jq version 1.6`), @@ -306,14 +316,17 @@ def test_the_observability_line_stays_on_ONE_line(preflight): assert "trailing noise" not in r.stdout -@pytest.mark.parametrize("version_line,expected", [ - ("jq-1.6 (Debian 1.6-2.1)", "1.6"), # distro packaging suffix - ("jq-1.10", "1.10"), # two-digit minor: must compare numerically, not lexically - ("jq-1.7.1", "1.7"), - ("jq-1.6.0", "1.6"), - ("jq-v1.6", "1.6"), - ("JQ-1.6", "1.6"), -]) +@pytest.mark.parametrize( + "version_line,expected", + [ + ("jq-1.6 (Debian 1.6-2.1)", "1.6"), # distro packaging suffix + ("jq-1.10", "1.10"), # two-digit minor: must compare numerically, not lexically + ("jq-1.7.1", "1.7"), + ("jq-1.6.0", "1.6"), + ("jq-v1.6", "1.6"), + ("JQ-1.6", "1.6"), + ], +) def test_legitimate_forms_still_parse_to_the_right_version(preflight, version_line, expected): preflight.with_jq(version_line) r = preflight.run() @@ -323,6 +336,7 @@ def test_legitimate_forms_still_parse_to_the_right_version(preflight, version_li # --- Wiring guards: the preflight is worthless if a caller silently stops running it ------------ + def test_script_tests_pins_the_jq_version(): """The pin is the tripwire, so its presence is asserted rather than merely commented. @@ -333,8 +347,7 @@ def test_script_tests_pins_the_jq_version(): is `main`, which does not yet contain `scripts/jq-preflight.sh`. """ pr_checks = (WORKFLOWS / "pr-checks.yml").read_text() - assert "jq-preflight.sh --expect" in pr_checks, \ - "script-tests must pin the jq version — that pin is the tripwire" + assert "jq-preflight.sh --expect" in pr_checks, "script-tests must pin the jq version — that pin is the tripwire" def test_review_verdict_never_pins_a_jq_version(): @@ -344,6 +357,7 @@ def test_review_verdict_never_pins_a_jq_version(): follow-up PR adds the floor-only call — rather than being a comment someone can miss. """ review_verdict = (WORKFLOWS / "review-verdict.yml").read_text() - assert "jq-preflight.sh --expect" not in review_verdict, \ - ("review-verdict.yml must NOT pin a jq version: it writes the required review-verdict/h10 " - "status, so a pin would deadlock every merge on a jq bump (ersatztv#648)") + assert "jq-preflight.sh --expect" not in review_verdict, ( + "review-verdict.yml must NOT pin a jq version: it writes the required review-verdict/h10 " + "status, so a pin would deadlock every merge on a jq bump (ersatztv#648)" + ) diff --git a/scripts/tests/test_merge_consent_base_change.py b/scripts/tests/test_merge_consent_base_change.py index f7343ac3f..73ca25dbe 100644 --- a/scripts/tests/test_merge_consent_base_change.py +++ b/scripts/tests/test_merge_consent_base_change.py @@ -33,7 +33,7 @@ SHA = "a9e3e23abf337980ca4c05854f5b1e210099d08b" # The PR is deliberately NOT docs-only: the docs-only exemption short-circuits the whole gate, so a # docs PR would never reach the base check and the tests would pass without exercising it. -CURL_SHIM = r'''#!/usr/bin/env python3 +CURL_SHIM = r"""#!/usr/bin/env python3 import json, os, sys, pathlib, urllib.parse state = pathlib.Path(os.environ["STUB_DIR"]) @@ -75,14 +75,18 @@ if "/pulls/" in url: sys.exit(0) print("{}") -''' +""" @pytest.fixture def hook(tmp_path): - bindir = tmp_path / "bin"; bindir.mkdir() - curl = bindir / "curl"; curl.write_text(CURL_SHIM); curl.chmod(0o755) - state = tmp_path / "state"; state.mkdir() + bindir = tmp_path / "bin" + bindir.mkdir() + curl = bindir / "curl" + curl.write_text(CURL_SHIM) + curl.chmod(0o755) + state = tmp_path / "state" + state.mkdir() (state / "live_base").write_text("main") (state / "verdict_desc").write_text("Review-verdict: MERGEABLE @ a9e3e23 (base: main)") @@ -90,7 +94,7 @@ def hook(tmp_path): env["PATH"] = f"{bindir}{os.pathsep}{env['PATH']}" env["STUB_DIR"] = str(state) env["STUB_SHA"] = SHA - env["ETV_GITEA_TOKEN"] = "stub" + env["ETV_GITEA_TOKEN"] = "stub" # noqa: S105 - deliberately fake; the real credential comes from the environment env["ETV_GITEA_URL"] = "http://gitea.example" env.pop("ETV_GITEA_BASICAUTH", None) @@ -103,10 +107,8 @@ def hook(tmp_path): (state / "verdict_desc").write_text(desc) def decision(self): - payload = {"tool_input": {"method": "merge", "owner": "timothy", - "repo": "ersatztv", "pull_number": 42}} - r = subprocess.run(["bash", str(HOOK)], input=json.dumps(payload), - env=env, capture_output=True, text=True) + payload = {"tool_input": {"method": "merge", "owner": "timothy", "repo": "ersatztv", "pull_number": 42}} + r = subprocess.run(["bash", str(HOOK)], input=json.dumps(payload), env=env, capture_output=True, text=True) assert r.returncode == 0, r.stderr if not r.stdout.strip(): return None @@ -124,21 +126,24 @@ def test_a_retargeted_base_denies_a_verdict_formed_against_the_old_one(hook): reason = hook.reason() assert "deny" in reason, "a verdict formed against a different base was allowed to stand" assert "release/26.4" in reason and "main" in reason, ( - "the deny must name both bases; a reader cannot act on 'the base changed'") + "the deny must name both bases; a reader cannot act on 'the base changed'" + ) def test_positive_control_an_unchanged_base_does_not_trigger_the_base_deny(hook): """Without this, the test above could pass because the hook denies on every path — which it very nearly does, since this PR is non-docs and the rest of the gate is unstubbed.""" reason = hook.reason() - assert "ersatztv#632" not in reason, ( - "the base check fired on a PR whose base never moved") + assert "ersatztv#632" not in reason, "the base check fired on a PR whose base never moved" -@pytest.mark.parametrize("desc", [ - "Review-verdict: MERGEABLE @ a9e3e23", # posted before #632 - "NONE", # no verdict status on this head at all -]) +@pytest.mark.parametrize( + "desc", + [ + "Review-verdict: MERGEABLE @ a9e3e23", # posted before #632 + "NONE", # no verdict status on this head at all + ], +) def test_a_verdict_with_no_recorded_base_gets_no_opinion(hook, desc): """Graceful adoption. Denying here would block every in-flight PR the day this lands, and the window closes on its own: verdicts are per-head and short-lived, so every verdict posted after @@ -151,7 +156,8 @@ def test_a_verdict_with_no_recorded_base_gets_no_opinion(hook, desc): hook.set_live_base("release/26.4") hook.set_verdict_description(desc) assert "base" not in hook.reason(), ( - "a pre-#632 verdict drew a base-related decision for a field it could not have carried") + "a pre-#632 verdict drew a base-related decision for a field it could not have carried" + ) @pytest.mark.parametrize("failure", ["SCALAR-ROW", "NONSTRING-DESC"]) @@ -173,7 +179,7 @@ def test_a_malformed_status_MEMBER_asks_too(hook, failure): @pytest.mark.parametrize("failure", ["TRANSPORT-ERROR", "GARBAGE"]) def test_an_UNREADABLE_status_response_asks_rather_than_skipping_the_check(hook, failure): - """"Could not check" is a third outcome, not a quiet synonym for "no base recorded". + """ "Could not check" is a third outcome, not a quiet synonym for "no base recorded". The first draft collapsed the two: an unreadable status response produced an empty `recorded_base`, took the graceful-adoption path, and skipped validation in silence — after @@ -202,5 +208,4 @@ def test_the_comparator_is_the_base_REF_not_its_tip_sha(): A base branch that merely ADVANCES must be silent here; rebasing onto it moves the head sha, which the per-sha binding already covers.""" assert ".base.ref" in HOOK.read_text(), "the hook must compare the base BRANCH, not its tip sha" - assert ".base.sha" not in HOOK.read_text(), ( - "comparing base.sha deadlocks every open PR whenever main advances") + assert ".base.sha" not in HOOK.read_text(), "comparing base.sha deadlocks every open PR whenever main advances" diff --git a/scripts/tests/test_merge_consent_exemption.py b/scripts/tests/test_merge_consent_exemption.py index 63e293a6f..fdf209ee8 100644 --- a/scripts/tests/test_merge_consent_exemption.py +++ b/scripts/tests/test_merge_consent_exemption.py @@ -33,7 +33,7 @@ HOOK = REPO_ROOT / ".claude" / "hooks" / "pretooluse-merge-consent.sh" SHA = "a9e3e23abf337980ca4c05854f5b1e210099d08b" # Serves paged `pulls/N/files`, plus the minimal PR object the hook reads first. -CURL_SHIM = r'''#!/usr/bin/env python3 +CURL_SHIM = r"""#!/usr/bin/env python3 import json, os, sys, pathlib, urllib.parse state = pathlib.Path(os.environ["STUB_DIR"]) @@ -88,7 +88,7 @@ if "/pulls/" in url: sys.exit(0) print("{}") -''' +""" def _rows(paths): @@ -97,15 +97,19 @@ def _rows(paths): @pytest.fixture def hook(tmp_path): - bindir = tmp_path / "bin"; bindir.mkdir() - shim = bindir / "curl"; shim.write_text(CURL_SHIM); shim.chmod(0o755) - state = tmp_path / "state"; state.mkdir() + bindir = tmp_path / "bin" + bindir.mkdir() + shim = bindir / "curl" + shim.write_text(CURL_SHIM) + shim.chmod(0o755) + state = tmp_path / "state" + state.mkdir() env = dict(os.environ) env["PATH"] = f"{bindir}{os.pathsep}{env['PATH']}" env["STUB_DIR"] = str(state) env["STUB_SHA"] = SHA - env["ETV_GITEA_TOKEN"] = "stub" + env["ETV_GITEA_TOKEN"] = "stub" # noqa: S105 - deliberately fake; the real credential comes from the environment env["ETV_GITEA_URL"] = "http://gitea.example" env.pop("ETV_GITEA_BASICAUTH", None) @@ -114,10 +118,10 @@ def hook(tmp_path): (state / "pages.json").write_text(json.dumps(list(pages))) def run(self): - payload = {"tool_input": {"method": "merge", "owner": "timothy", - "repo": "ersatztv", "pull_number": 42}} - return subprocess.run(["bash", str(HOOK)], input=json.dumps(payload), - env=env, capture_output=True, text=True) + payload = {"tool_input": {"method": "merge", "owner": "timothy", "repo": "ersatztv", "pull_number": 42}} + return subprocess.run( + ["bash", str(HOOK)], input=json.dumps(payload), env=env, capture_output=True, text=True + ) def exempted(self): """Exempt == passthrough == exit 0 with no decision JSON.""" @@ -140,16 +144,14 @@ def test_code_pr_is_not_exempt(hook): def test_protected_path_on_a_LATER_page_is_still_seen(hook): """The #619 shape: 50 docs files on page 1, code hiding on page 2.""" - hook.set_pages(_rows([f"docs/f{i}.md" for i in range(50)]), - _rows(["scripts/decisions_lib.py"])) + hook.set_pages(_rows([f"docs/f{i}.md" for i in range(50)]), _rows(["scripts/decisions_lib.py"])) assert hook.exempted() is False def test_full_first_page_alone_does_not_end_enumeration(hook): """A full 50-row page must trigger a second fetch, not terminate the loop.""" - hook.set_pages(_rows([f"docs/f{i}.md" for i in range(50)]), - _rows(["docs/tail.md"])) - assert hook.exempted() is True # genuinely all docs, but only provable by reading page 2 + hook.set_pages(_rows([f"docs/f{i}.md" for i in range(50)]), _rows(["docs/tail.md"])) + assert hook.exempted() is True # genuinely all docs, but only provable by reading page 2 def test_rename_of_code_into_docs_is_not_exempt(hook): @@ -161,15 +163,15 @@ def test_rename_of_code_into_docs_is_not_exempt(hook): exempt. That is the hook's contract and differs from `review-verdict.yml`'s stricter PROTECTED list, which must never auto-post a green status for those paths. """ - hook.set_pages([{"filename": "docs/innocuous-note.md", "status": "renamed", - "previous_filename": "ErsatzTV/Program.cs"}]) + hook.set_pages( + [{"filename": "docs/innocuous-note.md", "status": "renamed", "previous_filename": "ErsatzTV/Program.cs"}] + ) assert hook.exempted() is False def test_rename_between_two_exempt_paths_stays_exempt(hook): """Guards the above: reading previous_filename must not over-trigger on legitimate moves.""" - hook.set_pages([{"filename": "docs/b.md", "status": "renamed", - "previous_filename": "docs/a.md"}]) + hook.set_pages([{"filename": "docs/b.md", "status": "renamed", "previous_filename": "docs/a.md"}]) assert hook.exempted() is True @@ -232,14 +234,15 @@ def test_malformed_rename_row_withholds_the_exemption(hook): Real Gitea always populates it (verified by constructing a rename), so this is the malformed-2xx class the guard claims to fail closed on; the claim should match the behaviour. """ - hook.set_pages(_rows([f"docs/f{i}.md" for i in range(50)]), - [{"filename": "docs/moved.md", "status": "renamed"}]) + hook.set_pages(_rows([f"docs/f{i}.md" for i in range(50)]), [{"filename": "docs/moved.md", "status": "renamed"}]) assert hook.exempted() is False def test_rename_row_with_empty_previous_filename_withholds_the_exemption(hook): - hook.set_pages(_rows([f"docs/f{i}.md" for i in range(50)]), - [{"filename": "docs/moved.md", "status": "renamed", "previous_filename": ""}]) + hook.set_pages( + _rows([f"docs/f{i}.md" for i in range(50)]), + [{"filename": "docs/moved.md", "status": "renamed", "previous_filename": ""}], + ) assert hook.exempted() is False @@ -254,9 +257,13 @@ def test_ordinary_row_without_previous_filename_is_still_valid(hook): `test_a_rename_disguised_by_an_unknown_status_is_rejected[None]`. The statusless row this test used to carry was incidental to what it is actually pinning. """ - hook.set_pages([{"filename": "docs/a.md", "status": "modified"}, - {"filename": "docs/b.md", "status": "added"}, - {"filename": "docs/c.md", "status": "changed"}]) + hook.set_pages( + [ + {"filename": "docs/a.md", "status": "modified"}, + {"filename": "docs/b.md", "status": "added"}, + {"filename": "docs/c.md", "status": "changed"}, + ] + ) assert hook.exempted() is True @@ -312,16 +319,20 @@ sys.exit(127) @pytest.fixture def hook_jq16(tmp_path): """Same harness as `hook`, plus a jq shim emulating jq 1.6's empty-input exit status.""" - bindir = tmp_path / "bin"; bindir.mkdir() - (bindir / "curl").write_text(CURL_SHIM); (bindir / "curl").chmod(0o755) - (bindir / "jq").write_text(_JQ16_SHIM); (bindir / "jq").chmod(0o755) - state = tmp_path / "state"; state.mkdir() + bindir = tmp_path / "bin" + bindir.mkdir() + (bindir / "curl").write_text(CURL_SHIM) + (bindir / "curl").chmod(0o755) + (bindir / "jq").write_text(_JQ16_SHIM) + (bindir / "jq").chmod(0o755) + state = tmp_path / "state" + state.mkdir() env = dict(os.environ) env["PATH"] = f"{bindir}{os.pathsep}{env['PATH']}" env["STUB_DIR"] = str(state) env["STUB_SHA"] = SHA - env["ETV_GITEA_TOKEN"] = "stub" + env["ETV_GITEA_TOKEN"] = "stub" # noqa: S105 - deliberately fake; the real credential comes from the environment env["ETV_GITEA_URL"] = "http://gitea.example" env.pop("ETV_GITEA_BASICAUTH", None) @@ -330,10 +341,8 @@ def hook_jq16(tmp_path): (state / "pages.json").write_text(json.dumps(list(pages))) def exempted(self): - payload = {"tool_input": {"method": "merge", "owner": "timothy", - "repo": "ersatztv", "pull_number": 42}} - r = subprocess.run(["bash", str(HOOK)], input=json.dumps(payload), - env=env, capture_output=True, text=True) + payload = {"tool_input": {"method": "merge", "owner": "timothy", "repo": "ersatztv", "pull_number": 42}} + r = subprocess.run(["bash", str(HOOK)], input=json.dumps(payload), env=env, capture_output=True, text=True) assert r.returncode == 0, r.stderr return r.stdout.strip() == "" @@ -394,8 +403,9 @@ def test_newline_in_previous_filename_is_also_rejected(hook): rejects on its own merits, so the test passed with the newline guard entirely removed — it asserted the outcome without ever exercising the mechanism. That is the same filter-hides-the-defect trap the guard itself is about.""" - hook.set_pages([{"filename": "docs/ok.md", "previous_filename": "safe.md\ndocs/Program.cs", - "status": "renamed"}], []) + hook.set_pages( + [{"filename": "docs/ok.md", "previous_filename": "safe.md\ndocs/Program.cs", "status": "renamed"}], [] + ) assert hook.exempted() is False @@ -406,8 +416,7 @@ def test_previous_filename_is_validated_on_NON_renamed_rows_too(hook, status): `chunk` emits `(.previous_filename // empty)` for EVERY row regardless of `.status`, but the field was validated only when `.status == "renamed"`. A row marked `modified` (or Gitea's distinct `copied`) carrying a newline in `previous_filename` was reproducibly exempted.""" - hook.set_pages([{"filename": "docs/ok.md", "status": status, - "previous_filename": "safe.md\ndocs/Program.cs"}], []) + hook.set_pages([{"filename": "docs/ok.md", "status": status, "previous_filename": "safe.md\ndocs/Program.cs"}], []) assert hook.exempted() is False @@ -421,21 +430,18 @@ def test_dotdot_path_component_is_rejected(hook): def test_legitimate_rename_within_docs_still_exempts(hook): """Positive control: the tightened row schema must not break a real docs-only rename.""" - hook.set_pages([{"filename": "docs/b.md", "status": "renamed", - "previous_filename": "docs/a.md"}], []) + hook.set_pages([{"filename": "docs/b.md", "status": "renamed", "previous_filename": "docs/a.md"}], []) assert hook.exempted() is True def test_short_NONTERMINAL_page_does_not_end_the_enumeration(hook): - """"Fewer rows than we asked for" must not be read as "last page". + """ "Fewer rows than we asked for" must not be read as "last page". Gitea caps `limit` at the server-wide MAX_RESPONSE_ITEMS (default 50, configurable) and may return fewer rows than requested. A 30-row docs page followed by a page of code would otherwise complete the enumeration over a PARTIAL list — the same fail-open, reached with no transport error at all. Only a validated EMPTY page may terminate it.""" - hook.set_pages(_rows([f"docs/f{i}.md" for i in range(30)]), - _rows(["ErsatzTV/Program.cs"]), - []) + hook.set_pages(_rows([f"docs/f{i}.md" for i in range(30)]), _rows(["ErsatzTV/Program.cs"]), []) assert hook.exempted() is False @@ -473,9 +479,14 @@ def test_gitea_real_status_values_are_accepted(hook): """Positive control for the closed set. The real Gitea 1.25.4 value for an edit is `changed`, NOT `modified` — a closed allow-list built from the wrong vocabulary would reject every real docs-only PR, which is a far worse failure than the hole it closes.""" - hook.set_pages([{"filename": "docs/a.md", "status": "changed"}, - {"filename": "docs/b.md", "status": "added"}, - {"filename": "docs/c.md", "status": "deleted"}], []) + hook.set_pages( + [ + {"filename": "docs/a.md", "status": "changed"}, + {"filename": "docs/b.md", "status": "added"}, + {"filename": "docs/c.md", "status": "deleted"}, + ], + [], + ) assert hook.exempted() is True @@ -483,6 +494,7 @@ def test_gitea_real_status_values_are_accepted(hook): # The round-3 `..` finding was an anchor subversion, and mutating the anchors showed no test # covered them: dropping `^` from the docs/ alternative, or `$` from `.md`, both survived. + def test_docs_must_be_a_PREFIX_not_a_substring(hook): """Dropping `^` would exempt `ErsatzTV/docs/Program.cs`.""" hook.set_pages([{"filename": "ErsatzTV/docs/Program.cs", "status": "changed"}], []) @@ -517,7 +529,6 @@ def test_object_valued_status_is_also_rejected(hook): assert hook.exempted() is False - # --- The `grep -q` / pipefail inversion, on the ADVISORY side (ersatztv#698) -------------------- # # Round-2 cross-family review noted the enforced gate gained large-input regression tests while the @@ -530,6 +541,7 @@ def test_object_valued_status_is_also_rejected(hook): # negated docs-only test. ~171KB is needed to cross the threshold; every other test in this file uses a # handful of short paths, which is exactly why the class was invisible here. + def _many_docs(n=1900): return [f"docs/{'d' * 40}-{i:040d}.md" for i in range(n)] @@ -539,12 +551,12 @@ def test_a_LARGE_pr_containing_a_code_file_is_NOT_exempt(hook): bulk of ~171KB still to write.""" hook.set_pages(_rows(["A.cs", *_many_docs()])) assert hook.exempted() is False, ( - "a large PR containing A.cs was granted the docs-only exemption — the predicate inverted") + "a large PR containing A.cs was granted the docs-only exemption — the predicate inverted" + ) def test_positive_control_a_LARGE_genuinely_docs_only_pr_IS_still_exempt(hook): """Guards the opposite failure: if large lists merely errored, the test above would pass while the hook prompted on every big docs PR. Without this, 'fixed' and 'broken' are indistinguishable.""" hook.set_pages(_rows(_many_docs())) - assert hook.exempted() is True, ( - "a large but genuinely docs-only PR lost its exemption") + assert hook.exempted() is True, "a large but genuinely docs-only PR lost its exemption" diff --git a/scripts/tests/test_merge_consent_required_check.py b/scripts/tests/test_merge_consent_required_check.py index cb9511992..b1cee216b 100644 --- a/scripts/tests/test_merge_consent_required_check.py +++ b/scripts/tests/test_merge_consent_required_check.py @@ -45,7 +45,7 @@ SHORT = SHA[:7] # The PR is deliberately NOT docs-only: the docs-only exemption short-circuits the whole gate before # the scheduled-merge branch is reached, so a docs PR would pass these tests without ever running # the code under test. -CURL_SHIM = r'''#!/usr/bin/env python3 +CURL_SHIM = r"""#!/usr/bin/env python3 import json, os, sys, pathlib, urllib.parse state = pathlib.Path(os.environ["STUB_DIR"]) @@ -242,14 +242,18 @@ if "/pulls/" in url: sys.exit(0) print("{}") -''' +""" @pytest.fixture def hook(tmp_path): - bindir = tmp_path / "bin"; bindir.mkdir() - curl = bindir / "curl"; curl.write_text(CURL_SHIM); curl.chmod(0o755) - state = tmp_path / "state"; state.mkdir() + bindir = tmp_path / "bin" + bindir.mkdir() + curl = bindir / "curl" + curl.write_text(CURL_SHIM) + curl.chmod(0o755) + state = tmp_path / "state" + state.mkdir() (state / "bp").write_text("GUARDED") env = dict(os.environ) @@ -257,7 +261,7 @@ def hook(tmp_path): env["STUB_DIR"] = str(state) env["STUB_SHA"] = SHA env["STUB_SHORT"] = SHORT - env["ETV_GITEA_TOKEN"] = "stub" + env["ETV_GITEA_TOKEN"] = "stub" # noqa: S105 - deliberately fake; the real credential comes from the environment env["ETV_GITEA_URL"] = "http://gitea.example" env["CLAUDE_PROJECT_DIR"] = str(REPO_ROOT) env.pop("ETV_GITEA_BASICAUTH", None) @@ -283,11 +287,16 @@ def hook(tmp_path): return f.read_text().splitlines() if f.exists() else [] def decision(self, scheduled=True): - payload = {"tool_input": {"method": "merge", "owner": "timothy", - "repo": "ersatztv", "pull_number": 42, - "merge_when_checks_succeed": scheduled}} - r = subprocess.run(["bash", str(HOOK)], input=json.dumps(payload), - env=env, capture_output=True, text=True) + payload = { + "tool_input": { + "method": "merge", + "owner": "timothy", + "repo": "ersatztv", + "pull_number": 42, + "merge_when_checks_succeed": scheduled, + } + } + r = subprocess.run(["bash", str(HOOK)], input=json.dumps(payload), env=env, capture_output=True, text=True) assert r.returncode == 0, r.stderr if not r.stdout.strip(): return None @@ -307,9 +316,11 @@ def test_a_base_without_the_required_check_denies_a_SCHEDULED_merge(hook): reason = hook.reason() assert "deny" in reason, ( "a scheduled auto-merge was armed with no 'review-verdict/h10' required check on the base — " - "that is ersatztv#622's hole reopened") + "that is ersatztv#622's hole reopened" + ) assert "review-verdict/h10" in reason, ( - "the deny must name the missing context; a reader cannot act on 'branch protection is wrong'") + "the deny must name the missing context; a reader cannot act on 'branch protection is wrong'" + ) def test_status_checks_disabled_wholesale_also_denies(hook): @@ -318,8 +329,7 @@ def test_status_checks_disabled_wholesale_also_denies(hook): check-the-label-not-the-capability shape (#697/#698) this repo has paid for twice.""" hook.set_branch_protection("STATUS-CHECK-OFF") reason = hook.reason() - assert "deny" in reason, ( - "status checks were disabled entirely and the listed context was read as protection anyway") + assert "deny" in reason, "status checks were disabled entirely and the listed context was read as protection anyway" def test_positive_control_a_guarded_base_REACHES_the_check_and_still_auto_grants(hook): @@ -334,7 +344,8 @@ def test_positive_control_a_guarded_base_REACHES_the_check_and_still_auto_grants decision = hook.decision() assert hook.branch_protection_urls(), ( "the guarded case never reached the branch-protection endpoint, so the other tests are not " - "exercising the code they claim to") + "exercising the code they claim to" + ) assert decision is not None, "the hook passed through instead of auto-granting" verdict = decision["hookSpecificOutput"]["permissionDecision"] assert verdict == "allow", f"a fully-satisfied gate did not auto-grant (got {verdict!r})" @@ -354,9 +365,11 @@ def test_a_context_name_that_merely_CONTAINS_the_required_one_does_not_satisfy_i hook.set_branch_protection(shape) reason = hook.reason() assert "allow" not in reason or "deny" in reason or "ask" in reason, ( - f"payload shape {shape} auto-granted a scheduled merge") + f"payload shape {shape} auto-granted a scheduled merge" + ) assert "satisfied" not in reason, ( - f"a context name that merely contains 'review-verdict/h10' ({shape}) was accepted as it") + f"a context name that merely contains 'review-verdict/h10' ({shape}) was accepted as it" + ) @pytest.mark.parametrize("shape", ["EMPTY-CONTEXTS", "NULL-CONTEXTS"]) @@ -366,8 +379,7 @@ def test_an_empty_or_null_contexts_list_DENIES_rather_than_asking(hook, shape): unknown-shape ask alongside genuinely unreadable payloads.""" hook.set_branch_protection(shape) reason = hook.reason() - assert "deny" in reason, ( - f"{shape} was treated as unreadable rather than as a confirmed absent required check") + assert "deny" in reason, f"{shape} was treated as unreadable rather than as a confirmed absent required check" def test_a_FALSE_contexts_value_asks_rather_than_denying(hook): @@ -400,7 +412,8 @@ def test_a_malformed_contexts_MEMBER_asks_rather_than_denying_with_the_wrong_rea reason = hook.reason() assert "ask" in reason, "a malformed contexts member produced a decision instead of a question" assert "NOT a required status check" not in reason, ( - "an unreadable payload was reported as a confirmed missing required check") + "an unreadable payload was reported as a confirmed missing required check" + ) def test_a_base_with_NO_branch_protection_at_all_denies_rather_than_asking(hook): @@ -417,11 +430,10 @@ def test_a_base_with_NO_branch_protection_at_all_denies_rather_than_asking(hook) """ hook.set_branch_protection("EMPTY-LIST") reason = hook.reason() - assert "deny" in reason, ( - "a base with no branch protection at all did not deny a scheduled auto-merge") + assert "deny" in reason, "a base with no branch protection at all did not deny a scheduled auto-merge" assert "none matches" in reason, ( - "the deny must distinguish 'the list was read and nothing governs this base' from " - "'could not read'") + "the deny must distinguish 'the list was read and nothing governs this base' from 'could not read'" + ) def test_a_403_asks_because_it_says_only_that_we_could_not_look(hook): @@ -431,13 +443,12 @@ def test_a_403_asks_because_it_says_only_that_we_could_not_look(hook): hook.set_branch_protection("FORBIDDEN") reason = hook.reason() assert "ask" in reason, "a 403 was treated as evidence about the protection" - assert "none matches" not in reason, ( - "a 403 was reported as a confirmed absence of branch protection") + assert "none matches" not in reason, "a 403 was reported as a confirmed absence of branch protection" @pytest.mark.parametrize("failure", ["TRANSPORT-ERROR", "GARBAGE", "EMPTY", "LIST-404"]) def test_an_UNREADABLE_branch_protection_asks_rather_than_denying_or_passing(hook, failure): - """"Could not check" is a third outcome, not a synonym for either neighbour. + """ "Could not check" is a third outcome, not a synonym for either neighbour. Denying would strand every scheduled merge on a Gitea hiccup or on credentials without the repo-admin scope this endpoint needs. Passing would be worse: it would restore the exact @@ -446,10 +457,8 @@ def test_an_UNREADABLE_branch_protection_asks_rather_than_denying_or_passing(hoo """ hook.set_branch_protection(failure) reason = hook.reason() - assert "ask" in reason, ( - f"an unreadable branch-protection response ({failure}) did not fall through to a human") - assert "branch-protection rules" in reason, ( - "the ask must name what could not be checked") + assert "ask" in reason, f"an unreadable branch-protection response ({failure}) did not fall through to a human" + assert "branch-protection rules" in reason, "the ask must name what could not be checked" def test_an_IMMEDIATE_merge_is_not_subjected_to_this_check(hook): @@ -465,7 +474,8 @@ def test_an_IMMEDIATE_merge_is_not_subjected_to_this_check(hook): hook.set_branch_protection("MISSING-CONTEXT") reason = hook.reason(scheduled=False) assert "NOT a required status check" not in reason, ( - "the required-check deny fired on an immediate merge, which has no post-scheduling window") + "the required-check deny fired on an immediate merge, which has no post-scheduling window" + ) def test_a_NON_BOOLEAN_enable_status_check_asks(hook): @@ -504,8 +514,8 @@ def test_a_SCALAR_status_row_asks_instead_of_killing_the_hook(hook): hook.set_branch_protection("SCALAR-STATUS-ROW") decision = hook.decision() assert decision is not None, ( - "the hook emitted no decision at all for a malformed statuses payload — it neither granted, " - "denied nor asked") + "the hook emitted no decision at all for a malformed statuses payload — it neither granted, denied nor asked" + ) assert decision["hookSpecificOutput"]["permissionDecision"] == "ask" @@ -521,8 +531,7 @@ def test_a_PERSISTENT_retarget_denies_on_the_IMMEDIATE_path_too(hook): hook.drop_recorded_base() hook.set_retarget("scratch") reason = hook.reason(scheduled=False) - assert "deny" in reason, ( - "an immediate merge was auto-granted after the PR was retargeted mid-evaluation") + assert "deny" in reason, "an immediate merge was auto-granted after the PR was retargeted mid-evaluation" assert "scratch" in reason and "main" in reason @@ -540,10 +549,10 @@ def test_a_PERSISTENT_retarget_after_the_first_read_denies(hook): hook.drop_recorded_base() hook.set_retarget("scratch") reason = hook.reason() - assert "deny" in reason, ( - "the PR was retargeted mid-evaluation and the gate still granted on the original base") + assert "deny" in reason, "the PR was retargeted mid-evaluation and the gate still granted on the original base" assert "scratch" in reason and "main" in reason, ( - "the deny must name both branches; a reader cannot act on 'the base changed'") + "the deny must name both branches; a reader cannot act on 'the base changed'" + ) def test_a_NON_STRING_status_reaches_the_scheduled_validator(hook): @@ -573,13 +582,12 @@ def test_a_base_that_a_GLOB_rule_could_govern_ASKS(hook): """ hook.set_branch_protection("GLOB-RULE") reason = hook.reason() - assert "ask" in reason, ( - "a base a glob rule could govern was decided rather than referred to a human") + assert "ask" in reason, "a base a glob rule could govern was decided rather than referred to a human" assert "could govern it" in reason, ( "the ask came from the generic could-not-read arm, not the undecidable-glob arm — those " - "are different outcomes and a crashed classifier must not pass as a correct classification") - assert "none matches" not in reason, ( - "a base a glob rule could govern was reported as having no protection at all") + "are different outcomes and a crashed classifier must not pass as a correct classification" + ) + assert "none matches" not in reason, "a base a glob rule could govern was reported as having no protection at all" def test_an_unreadable_rule_LIST_asks_rather_than_denying(hook): @@ -606,8 +614,7 @@ def test_the_protection_lookup_puts_NO_ref_in_the_url(hook): urls = hook.branch_protection_urls() assert urls, "the branch-protection endpoint was never requested" for u in urls: - assert u.rstrip("/").endswith("/branch_protections"), ( - f"a ref reached the branch-protection URL: {u}") + assert u.rstrip("/").endswith("/branch_protections"), f"a ref reached the branch-protection URL: {u}" def test_a_rule_name_with_REGEX_METACHARACTERS_does_not_match_a_different_base(hook): @@ -624,11 +631,11 @@ def test_a_rule_name_with_REGEX_METACHARACTERS_does_not_match_a_different_base(h """ hook.set_branch_protection("REGEX-META-RULE") reason = hook.reason() - assert "deny" in reason, ( - "a rule named 'mai.' was regex-matched against base 'main' and read as protection") + assert "deny" in reason, "a rule named 'mai.' was regex-matched against base 'main' and read as protection" assert "none matches" in reason, ( "'mai.' contains no GLOB metacharacter, so it is decidable: it simply does not govern " - "'main', and the base is genuinely unprotected") + "'main', and the base is genuinely unprotected" + ) def test_a_GLOB_rule_whose_literal_part_has_a_metacharacter_still_MATCHES(hook): @@ -645,17 +652,19 @@ def test_a_GLOB_rule_whose_literal_part_has_a_metacharacter_still_MATCHES(hook): rather than reporting the base as unprotected. """ hook.set_base("release/26.4") - hook.drop_recorded_base() # keep the #632 comparison out of this test's way + hook.drop_recorded_base() # keep the #632 comparison out of this test's way hook.set_branch_protection("GLOB-WITH-DOT-RULE") reason = hook.reason() assert "ask" in reason, ( - "the glob rule 'release/26.*' could govern base 'release/26.4', which is undecidable here " - "and must ask") + "the glob rule 'release/26.*' could govern base 'release/26.4', which is undecidable here and must ask" + ) assert "could govern it" in reason, ( - "the ask must come from the undecidable-glob arm, not from the classifier failing") + "the ask must come from the undecidable-glob arm, not from the classifier failing" + ) assert "none matches" not in reason, ( "a base a glob rule could govern was reported as entirely unprotected — the over-escaping " - "failure this test exists to catch") + "failure this test exists to catch" + ) def test_a_rule_name_containing_a_CHAR_CLASS_bracket_does_not_crash_the_matcher(hook): @@ -667,12 +676,13 @@ def test_a_rule_name_containing_a_CHAR_CLASS_bracket_does_not_crash_the_matcher( assert reason, "the hook emitted no decision at all" assert "ask" not in reason, ( "the exact-name rule was decidable and must have been honoured; an ask here means the " - "classifier failed rather than classified") + "classifier failed rather than classified" + ) assert "none matches" not in reason, ( "one rule with a bracket in its name discarded the whole list, including the exact-name " - "rule that actually protects this base") - assert "deny" not in reason, ( - "an exact-name rule requiring review-verdict/h10 was present and was not honoured") + "rule that actually protects this base" + ) + assert "deny" not in reason, "an exact-name rule requiring review-verdict/h10 was present and was not honoured" def test_a_plain_rule_name_is_matched_CASE_INSENSITIVELY(hook): @@ -683,9 +693,11 @@ def test_a_plain_rule_name_is_matched_CASE_INSENSITIVELY(hook): reason = hook.reason() assert "ask" not in reason, ( "the case-folded exact rule was decidable and must have been honoured; an ask means the " - "classifier failed rather than classified") + "classifier failed rather than classified" + ) assert "none matches" not in reason, ( - "a rule named 'MAIN' governs base 'main' in Gitea but was missed by a case-sensitive compare") + "a rule named 'MAIN' governs base 'main' in Gitea but was missed by a case-sensitive compare" + ) assert "deny" not in reason @@ -707,12 +719,13 @@ def test_a_BACKSLASH_ESCAPED_metacharacter_in_a_rule_name_is_undecidable_not_abs hook.set_branch_protection("ESCAPED-META-RULE") reason = hook.reason() assert "none matches" not in reason, ( - "a rule whose escaped brace governs this base was reported as unable to govern it") - assert "ask" in reason, ( - "an escaped-metacharacter rule is undecidable here and must ask") + "a rule whose escaped brace governs this base was reported as unable to govern it" + ) + assert "ask" in reason, "an escaped-metacharacter rule is undecidable here and must ask" assert "could govern it" in reason, ( "the ask must come from the undecidable-glob arm, not from the classifier failing — this " - "test hits the same arm as its two siblings and needs the same pin") + "test hits the same arm as its two siblings and needs the same pin" + ) def test_the_precedence_check_runs_even_when_an_exactly_named_rule_EXISTS(hook): @@ -732,13 +745,13 @@ def test_the_precedence_check_runs_even_when_an_exactly_named_rule_EXISTS(hook): """ hook.set_branch_protection("EXACT-PLUS-GLOB-RULE") reason = hook.reason() - assert "ask" in reason, ( - "an exactly-named rule was trusted without asking which rule Gitea would actually apply") + assert "ask" in reason, "an exactly-named rule was trusted without asking which rule Gitea would actually apply" assert "could govern it" in reason # And the by-name endpoint must not be consulted at all — its existence is what split the paths. for u in hook.branch_protection_urls(): assert u.rstrip("/").endswith("/branch_protections"), ( - f"the by-name lookup is back, and with it the unguarded path: {u}") + f"the by-name lookup is back, and with it the unguarded path: {u}" + ) def test_a_GLOB_rule_that_could_outrank_an_exact_one_wins_and_ASKS(hook): @@ -758,9 +771,11 @@ def test_a_GLOB_rule_that_could_outrank_an_exact_one_wins_and_ASKS(hook): reason = hook.reason() assert "ask" in reason, ( "an exact rule was trusted while a glob rule could outrank it — the gate granted on a base " - "whose enforced rule it never identified") + "whose enforced rule it never identified" + ) assert "could govern it" in reason, ( - "the ask must come from the undecidable-glob arm, not from the classifier failing") + "the ask must come from the undecidable-glob arm, not from the classifier failing" + ) def test_two_rules_differing_only_in_CASE_are_undecidable(hook): @@ -769,8 +784,7 @@ def test_two_rules_differing_only_in_CASE_are_undecidable(hook): whose enforced rule was never identified — the same defect as the arm order, one level down.""" hook.set_branch_protection("TWO-CASE-VARIANT-RULES") reason = hook.reason() - assert "ask" in reason, ( - "two fold-equal rules disagree about review-verdict/h10 and one was picked by list order") + assert "ask" in reason, "two fold-equal rules disagree about review-verdict/h10 and one was picked by list order" assert "could govern it" in reason @@ -784,7 +798,8 @@ def test_a_NON_ASCII_rule_or_base_is_undecidable_rather_than_fold_compared(hook) hook.set_branch_protection("NONASCII-RULE") reason = hook.reason() assert "none matches" not in reason, ( - "a rule that folds equal to this base under EqualFold was reported as unable to govern it") + "a rule that folds equal to this base under EqualFold was reported as unable to govern it" + ) assert "ask" in reason @@ -803,8 +818,7 @@ def test_an_HTTP_404_on_the_LIST_read_asks_and_does_not_claim_the_list_was_read( hook.set_branch_protection("LIST-404") reason = hook.reason() assert "ask" in reason, "an unreadable repo was treated as evidence about the base" - assert "none matches" not in reason, ( - "a 404 read claimed the full rule list had been read and matched nothing") + assert "none matches" not in reason, "a 404 read claimed the full rule list had been read and matched nothing" @pytest.mark.parametrize("shape", ["UNPARSEABLE-RULES", "GARBAGE", "EMPTY"]) @@ -825,6 +839,5 @@ def test_a_200_the_classifier_cannot_PARSE_asks_without_blaming_the_transport(ho hook.set_branch_protection(shape) reason = hook.reason() assert "ask" in reason, "an unparseable rule list produced a decision instead of a question" - assert "could not parse" in reason, ( - "the ask blamed the transport for a 200 the classifier simply could not read") + assert "could not parse" in reason, "the ask blamed the transport for a 200 the classifier simply could not read" assert "unreachable" not in reason diff --git a/scripts/tests/test_post_review_verdict.py b/scripts/tests/test_post_review_verdict.py index c09ceba6e..15d2d9b4f 100644 --- a/scripts/tests/test_post_review_verdict.py +++ b/scripts/tests/test_post_review_verdict.py @@ -104,7 +104,7 @@ def gitea(tmp_path): env = dict(os.environ) env["PATH"] = f"{bindir}{os.pathsep}{env['PATH']}" env["STUB_DIR"] = str(state) - env["ETV_GITEA_TOKEN"] = "stub-token" + env["ETV_GITEA_TOKEN"] = "stub-token" # noqa: S105 - deliberately fake; the real credential comes from the environment env["ETV_GITEA_URL"] = "http://gitea.example" env["ETV_GITEA_REPO"] = "timothy/ersatztv" env.pop("ETV_GITEA_BASICAUTH", None) @@ -127,7 +127,9 @@ def gitea(tmp_path): def run(self, *args): return subprocess.run( ["bash", str(SCRIPT), *args], - env=env, capture_output=True, text=True, + env=env, + capture_output=True, + text=True, ) def posts(self): @@ -229,6 +231,7 @@ def test_unreachable_pr_is_an_error_not_a_silent_success(gitea): # --- Cross-checks against the hook's own condition-(c) parser ----------------------------------- + def _classify(body: str, head: str) -> str: """Run the REAL H10 classifier over a comment body — no Python mirror of the grammar. @@ -239,9 +242,7 @@ def _classify(body: str, head: str) -> str: passing here while the shell drifted. """ payload = json.dumps([{"body": body}]) - p = subprocess.run( - ["bash", str(CLASSIFIER), "--head", head], input=payload, capture_output=True, text=True - ) + p = subprocess.run(["bash", str(CLASSIFIER), "--head", head], input=payload, capture_output=True, text=True) assert p.returncode == 0, f"classifier errored: {p.stderr}" return p.stdout.strip() @@ -280,6 +281,7 @@ def test_note_cannot_forge_a_second_verdict_line(gitea): # mirror case: the head sha and the status both hold still while the effective DIFF changes, so the # verdict keeps reading green for a review nobody performed against that base. + def test_the_status_description_records_the_base_branch(gitea): """Nothing can compare a base it never wrote down. This field is what the hook reads back.""" assert gitea.run("42", "MERGEABLE").returncode == 0 @@ -333,7 +335,8 @@ def test_a_failed_HEAD_RECHECK_writes_no_status(gitea): result = gitea.run("42", "MERGEABLE") assert result.returncode != 0 assert gitea.statuses() == [], ( - "a status was written even though the head/base re-read failed — nothing was confirmed") + "a status was written even though the head/base re-read failed — nothing was confirmed" + ) # --- write-side polarity, the half the #774 rescue initially missed ------------------------------ @@ -404,8 +407,8 @@ def test_a_reread_that_LOSES_a_field_refuses_instead_of_posting(head_seq, base_s gitea.set_base_sequence(*base_seq) result = gitea.run("42", "MERGEABLE") assert result.returncode != 0, ( - f"a re-read missing its {field} was accepted; the script posted a verdict having confirmed " - "nothing about it") + f"a re-read missing its {field} was accepted; the script posted a verdict having confirmed nothing about it" + ) assert not gitea.statuses(), ( - f"a status was written despite the re-read carrying no {field} — this is the fail-open " - "the -n conjunct created") + f"a status was written despite the re-read carrying no {field} — this is the fail-open the -n conjunct created" + ) diff --git a/scripts/tests/test_pr_changed_files.py b/scripts/tests/test_pr_changed_files.py index 787628492..5e3ed0ea2 100644 --- a/scripts/tests/test_pr_changed_files.py +++ b/scripts/tests/test_pr_changed_files.py @@ -39,7 +39,7 @@ SHA = "a9e3e23abf337980ca4c05854f5b1e210099d08b" OTHER_SHA = "b71c0d4e2f8a91b3c5d7e9f1a3b5c7d9e1f3a5b7" # Serves paged `pulls/N/files`, plus the PR object the script re-reads to bind the enumeration. -CURL_SHIM = r'''#!/usr/bin/env python3 +CURL_SHIM = r"""#!/usr/bin/env python3 import json, os, sys, pathlib, urllib.parse state = pathlib.Path(os.environ["STUB_DIR"]) @@ -98,7 +98,7 @@ if "/pulls/" in url: sys.exit(0) print("{}") -''' +""" def _rows(paths, status="modified"): @@ -107,15 +107,19 @@ def _rows(paths, status="modified"): @pytest.fixture def enumerate_files(tmp_path): - bindir = tmp_path / "bin"; bindir.mkdir() - shim = bindir / "curl"; shim.write_text(CURL_SHIM); shim.chmod(0o755) - state = tmp_path / "state"; state.mkdir() + bindir = tmp_path / "bin" + bindir.mkdir() + shim = bindir / "curl" + shim.write_text(CURL_SHIM) + shim.chmod(0o755) + state = tmp_path / "state" + state.mkdir() env = dict(os.environ) env["PATH"] = f"{bindir}{os.pathsep}{env['PATH']}" env["STUB_DIR"] = str(state) env["STUB_SHA"] = SHA - env["ETV_GITEA_TOKEN"] = "stub" + env["ETV_GITEA_TOKEN"] = "stub" # noqa: S105 - deliberately fake; the real credential comes from the environment env["ETV_GITEA_URL"] = "http://gitea.example" env.pop("ETV_GITEA_BASICAUTH", None) env.pop("GITEA_TOKEN", None) @@ -154,8 +158,7 @@ def enumerate_files(tmp_path): """The base tip sha is absent/unparseable on the post-paging re-read.""" (state / "pr_base_sha_after.txt").write_text("MISSING") - def run(self, expected_sha=SHA, args=("timothy", "ersatztv", "42"), - expected_base="main"): + def run(self, expected_sha=SHA, args=("timothy", "ersatztv", "42"), expected_base="main"): argv = ["bash", str(SCRIPT), *args, expected_sha] if expected_base is not None: argv.append(expected_base) @@ -177,6 +180,7 @@ def enumerate_files(tmp_path): # --- The happy path, so the failure-path assertions below cannot pass vacuously ---------------- + def test_complete_enumeration_returns_every_path(enumerate_files): enumerate_files.set_pages(_rows(["docs/a.md", "ErsatzTV/Program.cs", "README.md"])) assert enumerate_files.paths() == ["docs/a.md", "ErsatzTV/Program.cs", "README.md"] @@ -184,25 +188,30 @@ def test_complete_enumeration_returns_every_path(enumerate_files): def test_a_rename_contributes_BOTH_sides(enumerate_files): """One row, two paths — the `git mv` hole. Reading `.filename` alone hides the source.""" - enumerate_files.set_pages([{"filename": "docs/innocuous-note.md", "status": "renamed", - "previous_filename": ".gitea/workflows/renovate.yml"}]) - assert sorted(enumerate_files.paths()) == [".gitea/workflows/renovate.yml", - "docs/innocuous-note.md"] + enumerate_files.set_pages( + [ + { + "filename": "docs/innocuous-note.md", + "status": "renamed", + "previous_filename": ".gitea/workflows/renovate.yml", + } + ] + ) + assert sorted(enumerate_files.paths()) == [".gitea/workflows/renovate.yml", "docs/innocuous-note.md"] def test_paths_on_a_later_page_are_included(enumerate_files): - enumerate_files.set_pages(_rows([f"docs/f{i}.md" for i in range(50)]), - _rows(["scripts/decisions_lib.py"])) + enumerate_files.set_pages(_rows([f"docs/f{i}.md" for i in range(50)]), _rows(["scripts/decisions_lib.py"])) assert "scripts/decisions_lib.py" in enumerate_files.paths() # --- Fail-closed contract: each of these MUST be non-zero, by design --------------------------- + def test_transport_failure_mid_pagination_fails_closed(enumerate_files): """The defect that started all of this: an errored page counted as zero rows and read as 'end of list', completing the enumeration over a PARTIAL list.""" - enumerate_files.set_pages(_rows([f"docs/f{i}.md" for i in range(50)]), "ERROR", - _rows(["docs/tail.md"])) + enumerate_files.set_pages(_rows([f"docs/f{i}.md" for i in range(50)]), "ERROR", _rows(["docs/tail.md"])) assert enumerate_files.fails_closed() @@ -262,8 +271,9 @@ def test_CRLF_in_filename_fails_closed(enumerate_files, evil): def test_CRLF_in_previous_filename_on_a_NON_renamed_row_fails_closed(enumerate_files): """The hole one predicate wide: `previous_filename` is CONSUMED on every row, so it must be VALIDATED on every row — not only where `.status == "renamed"` makes it semantically expected.""" - enumerate_files.set_pages([{"filename": "docs/a.md", "status": "modified", - "previous_filename": "safe.md\ndocs/Program.cs"}]) + enumerate_files.set_pages( + [{"filename": "docs/a.md", "status": "modified", "previous_filename": "safe.md\ndocs/Program.cs"}] + ) assert enumerate_files.fails_closed() @@ -277,8 +287,7 @@ def test_dotdot_path_component_fails_closed(enumerate_files): def test_status_outside_the_closed_allow_list_fails_closed(enumerate_files, status): """Without a closed set, the `renamed => previous_filename REQUIRED` clause is dodgeable by any other value, letting a `git mv` drop its source path and read as docs-only.""" - enumerate_files.set_pages([{"filename": "docs/a.md", "status": status, - "previous_filename": "ErsatzTV/Program.cs"}]) + enumerate_files.set_pages([{"filename": "docs/a.md", "status": status, "previous_filename": "ErsatzTV/Program.cs"}]) assert enumerate_files.fails_closed() @@ -313,8 +322,11 @@ def test_missing_credentials_fails_closed(enumerate_files): @pytest.mark.parametrize("args", [("timothy", "ersatztv"), ("timothy", "ersatztv", "")]) def test_usage_errors_exit_2(enumerate_files, args): enumerate_files.set_pages(_rows(["docs/a.md"])) - r = enumerate_files.run(args=args) if len(args) == 3 else subprocess.run( - ["bash", str(SCRIPT), *args], env=enumerate_files.env, capture_output=True, text=True) + r = ( + enumerate_files.run(args=args) + if len(args) == 3 + else subprocess.run(["bash", str(SCRIPT), *args], env=enumerate_files.env, capture_output=True, text=True) + ) assert r.returncode == 2, r.stderr @@ -325,6 +337,7 @@ def test_usage_errors_exit_2(enumerate_files, args): # mid-run, enumerated docs-only, granted `review-verdict/h10=success` while its diff against `main` # carried a C# file. + def test_a_FOUR_argument_call_is_a_usage_error_not_an_unbound_enumeration(enumerate_files): """The binding is REQUIRED, not optional. @@ -337,7 +350,8 @@ def test_a_FOUR_argument_call_is_a_usage_error_not_an_unbound_enumeration(enumer r = enumerate_files.run(expected_base=None) assert r.returncode == 2, ( "a 4-argument call was accepted, so the base binding is effectively optional and any caller " - f"that forgets it silently enumerates against a mutable base (stdout={r.stdout!r})") + f"that forgets it silently enumerates against a mutable base (stdout={r.stdout!r})" + ) def test_an_EMPTY_base_ref_argument_fails_closed(enumerate_files): @@ -385,17 +399,16 @@ def test_a_BASE_ADVANCE_DURING_enumeration_fails_closed(enumerate_files): assert r.returncode != 0, f"expected fail-closed, got {r.returncode}: stdout={r.stdout!r}" assert not r.stdout.strip(), f"stdout must be meaningless on a failed enumeration: {r.stdout!r}" assert "base" in r.stderr and "advanced" in r.stderr, ( - f"stderr should name the base-advance failure distinctly, got: {r.stderr!r}") - assert re.search(r"[0-9a-f]{7}", r.stderr), ( - f"stderr should name both short shas involved, got: {r.stderr!r}") + f"stderr should name the base-advance failure distinctly, got: {r.stderr!r}" + ) + assert re.search(r"[0-9a-f]{7}", r.stderr), f"stderr should name both short shas involved, got: {r.stderr!r}" def test_positive_control_an_UNMOVED_base_sha_still_enumerates_across_pages(enumerate_files): """Without this, the fail-closed tests around it could be passing only because the new guard broke the ordinary path outright rather than because it correctly distinguishes movement from no movement. Deliberately multi-page, to prove the guard survives several round trips.""" - enumerate_files.set_pages(_rows([f"docs/f{i}.md" for i in range(50)]), - _rows(["ErsatzTV/Program.cs"])) + enumerate_files.set_pages(_rows([f"docs/f{i}.md" for i in range(50)]), _rows(["ErsatzTV/Program.cs"])) paths = enumerate_files.paths() assert len(paths) == 51 assert "ErsatzTV/Program.cs" in paths @@ -412,7 +425,8 @@ def test_the_707_WINDOW_base_ref_unchanged_but_TIP_advanced_still_fails(enumerat r = enumerate_files.run() # expected_base="main" throughout; ref never moves assert r.returncode != 0, ( "base ref stayed 'main' but its tip advanced mid-enumeration; must still fail closed " - f"(got {r.returncode}, stdout={r.stdout!r}, stderr={r.stderr!r})") + f"(got {r.returncode}, stdout={r.stdout!r}, stderr={r.stderr!r})" + ) def test_a_base_that_ADVANCED_BEFORE_the_enumeration_STILL_SUCCEEDS(enumerate_files): @@ -433,7 +447,8 @@ def test_a_base_that_ADVANCED_BEFORE_the_enumeration_STILL_SUCCEEDS(enumerate_fi enumerate_files.set_pages(_rows(["docs/a.md", "docs/b.md"]), []) assert enumerate_files.paths() == ["docs/a.md", "docs/b.md"], ( "a PR whose base advanced BEFORE this enumeration began was failed closed; the guard is " - "comparing against a stale expectation instead of the tip it actually started from") + "comparing against a stale expectation instead of the tip it actually started from" + ) def test_an_UNREADABLE_base_sha_on_the_AFTER_read_fails_closed(enumerate_files): @@ -458,12 +473,12 @@ def test_the_base_comparator_is_the_BRANCH_NAME_never_the_TIP_SHA(): """ src = SCRIPT.read_text() assert ".base.ref" in src, "the enumeration no longer reads .base.ref" - binding = [ln for ln in src.splitlines() - if "base_before=" in ln or "base_after=" in ln] + binding = [ln for ln in src.splitlines() if "base_before=" in ln or "base_after=" in ln] assert binding, "no base binding assignments found" for ln in binding: assert ".base.ref" in ln and ".base.sha" not in ln, ( - f"the base binding compares a tip sha, which deadlocks on ordinary churn: {ln.strip()!r}") + f"the base binding compares a tip sha, which deadlocks on ordinary churn: {ln.strip()!r}" + ) def test_a_SHORT_page_does_not_end_the_enumeration(enumerate_files): @@ -471,8 +486,7 @@ def test_a_SHORT_page_does_not_end_the_enumeration(enumerate_files): 'Fewer than 50 rows means last page' would complete over a partial list without any transport error — so termination requires a validated EMPTY page. A 30-row page followed by code must be seen.""" - enumerate_files.set_pages(_rows([f"docs/f{i}.md" for i in range(30)]), - _rows(["ErsatzTV/Program.cs"])) + enumerate_files.set_pages(_rows([f"docs/f{i}.md" for i in range(30)]), _rows(["ErsatzTV/Program.cs"])) assert "ErsatzTV/Program.cs" in enumerate_files.paths() @@ -502,7 +516,7 @@ def enumerate_files_jq16(enumerate_files, tmp_path): def test_jq16_shim_actually_reproduces_the_quirk(enumerate_files_jq16, tmp_path): """Verify the verifier. A shim that failed to install would make the test below pass vacuously, reporting the guard safe on jq 1.6 without ever exercising the quirk.""" - assert enumerate_files_jq16 is not None # the fixture is what installs the shim + assert enumerate_files_jq16 is not None # the fixture is what installs the shim jq = str(tmp_path / "bin" / "jq") empty = subprocess.run([jq, "-e", "."], input="", capture_output=True, text=True) assert empty.returncode == 0, "the shim does not reproduce jq 1.6's empty-input exit 0" @@ -514,8 +528,7 @@ def test_jq16_shim_actually_reproduces_the_quirk(enumerate_files_jq16, tmp_path) def test_transport_failure_mid_pagination_fails_closed_on_jq16(enumerate_files_jq16): """The property, asserted on the interpreter that actually runs it in CI.""" - enumerate_files_jq16.set_pages(_rows([f"docs/f{i}.md" for i in range(50)]), "ERROR", - _rows(["docs/tail.md"])) + enumerate_files_jq16.set_pages(_rows([f"docs/f{i}.md" for i in range(50)]), "ERROR", _rows(["docs/tail.md"])) assert enumerate_files_jq16.fails_closed() @@ -541,14 +554,18 @@ def test_a_docs_only_pr_still_enumerates_cleanly_on_jq16(enumerate_files_jq16): # the redundancy cannot absorb. The positive control immediately after it proves the harness can # actually observe the difference, rather than reporting "not exempt" for some unrelated reason. -HOOK_STUB_CURL = r'''#!/usr/bin/env python3 +_HOOK_STUB_CURL_TEMPLATE = r"""#!/usr/bin/env python3 import json, sys url = [a for a in sys.argv[1:] if a.startswith("http")][-1] if "/pulls/" in url and "/files" not in url: print(json.dumps({"head": {"sha": "%s"}, "body": "no linked issue"})) else: print("{}") -''' % SHA +""" + +# Percent formatting is required: the template is Python source containing literal `{}`, +# which .format() would eat. +HOOK_STUB_CURL = _HOOK_STUB_CURL_TEMPLATE % SHA def _mirror_tree(tmp_path, stub_body): @@ -566,19 +583,26 @@ def _mirror_tree(tmp_path, stub_body): stub.write_text(stub_body) stub.chmod(0o755) - bindir = tmp_path / "bin"; bindir.mkdir() - curl = bindir / "curl"; curl.write_text(HOOK_STUB_CURL); curl.chmod(0o755) + bindir = tmp_path / "bin" + bindir.mkdir() + curl = bindir / "curl" + curl.write_text(HOOK_STUB_CURL) + curl.chmod(0o755) env = dict(os.environ) env["PATH"] = f"{bindir}{os.pathsep}{env['PATH']}" - env["ETV_GITEA_TOKEN"] = "stub" + env["ETV_GITEA_TOKEN"] = "stub" # noqa: S105 - deliberately fake; the real credential comes from the environment env["ETV_GITEA_URL"] = "http://gitea.example" env.pop("ETV_GITEA_BASICAUTH", None) - payload = {"tool_input": {"method": "merge", "owner": "timothy", - "repo": "ersatztv", "pull_number": 42}} - r = subprocess.run(["bash", str(hooks / "pretooluse-merge-consent.sh")], - input=json.dumps(payload), env=env, capture_output=True, text=True) + payload = {"tool_input": {"method": "merge", "owner": "timothy", "repo": "ersatztv", "pull_number": 42}} + r = subprocess.run( + ["bash", str(hooks / "pretooluse-merge-consent.sh")], + input=json.dumps(payload), + env=env, + capture_output=True, + text=True, + ) assert r.returncode == 0, r.stderr # Exempt == passthrough == exit 0 with no decision JSON on stdout. return r.stdout.strip() == "" @@ -591,7 +615,8 @@ def test_a_FAILING_script_withholds_the_exemption_even_when_stdout_looks_docs_on exempt = _mirror_tree(tmp_path, "#!/usr/bin/env bash\n" + DOCS_ONLY_OUTPUT + "exit 1\n") assert exempt is False, ( "the hook granted a docs-only exemption from the stdout of a script that FAILED — the exit " - "status is not being checked") + "status is not being checked" + ) def test_positive_control_the_same_output_with_exit_0_DOES_exempt(tmp_path): @@ -599,13 +624,13 @@ def test_positive_control_the_same_output_with_exit_0_DOES_exempt(tmp_path): exempt = _mirror_tree(tmp_path, "#!/usr/bin/env bash\n" + DOCS_ONLY_OUTPUT + "exit 0\n") assert exempt is True, ( "the positive control failed, so the negative test above cannot be trusted to be measuring " - "the exit status at all") + "the exit status at all" + ) def test_a_script_that_crashes_also_withholds_the_exemption(tmp_path): """Not every failure is a clean `exit 1` — a crash must not read as success either.""" - exempt = _mirror_tree( - tmp_path, "#!/usr/bin/env bash\n" + DOCS_ONLY_OUTPUT + "kill -TERM $$\n") + exempt = _mirror_tree(tmp_path, "#!/usr/bin/env bash\n" + DOCS_ONLY_OUTPUT + "kill -TERM $$\n") assert exempt is False @@ -615,16 +640,23 @@ def test_a_MISSING_script_withholds_the_exemption(tmp_path): hooks.mkdir(parents=True) (hooks / "pretooluse-merge-consent.sh").write_text(HOOK.read_text()) (tmp_path / "scripts").mkdir() - bindir = tmp_path / "bin"; bindir.mkdir() - curl = bindir / "curl"; curl.write_text(HOOK_STUB_CURL); curl.chmod(0o755) + bindir = tmp_path / "bin" + bindir.mkdir() + curl = bindir / "curl" + curl.write_text(HOOK_STUB_CURL) + curl.chmod(0o755) env = dict(os.environ) env["PATH"] = f"{bindir}{os.pathsep}{env['PATH']}" - env["ETV_GITEA_TOKEN"] = "stub" + env["ETV_GITEA_TOKEN"] = "stub" # noqa: S105 - deliberately fake; the real credential comes from the environment env["ETV_GITEA_URL"] = "http://gitea.example" - payload = {"tool_input": {"method": "merge", "owner": "timothy", - "repo": "ersatztv", "pull_number": 42}} - r = subprocess.run(["bash", str(hooks / "pretooluse-merge-consent.sh")], - input=json.dumps(payload), env=env, capture_output=True, text=True) + payload = {"tool_input": {"method": "merge", "owner": "timothy", "repo": "ersatztv", "pull_number": 42}} + r = subprocess.run( + ["bash", str(hooks / "pretooluse-merge-consent.sh")], + input=json.dumps(payload), + env=env, + capture_output=True, + text=True, + ) assert r.returncode == 0, r.stderr assert r.stdout.strip() != "", "a missing shared script must not grant an exemption" @@ -637,8 +669,7 @@ def test_a_MISSING_script_withholds_the_exemption(tmp_path): # the entire subject of the issue, so a guard that covered only the advisory side would have been # the same mistake one level up. -CALLERS = pytest.mark.parametrize( - "caller", [HOOK, WORKFLOW], ids=["advisory-hook", "enforced-workflow"]) +CALLERS = pytest.mark.parametrize("caller", [HOOK, WORKFLOW], ids=["advisory-hook", "enforced-workflow"]) def _code_lines(path: Path) -> str: @@ -650,14 +681,14 @@ def _code_lines(path: Path) -> str: a piece of prose. `#`-prefixed works for both files: YAML comments and the shell comments inside the workflow's `run:` block share the marker. """ - return "\n".join( - ln for ln in path.read_text().splitlines() if not ln.lstrip().startswith("#")) + return "\n".join(ln for ln in path.read_text().splitlines() if not ln.lstrip().startswith("#")) @CALLERS def test_the_caller_uses_the_shared_script(caller): assert "scripts/pr-changed-files.sh" in _code_lines(caller), ( - f"{caller.relative_to(REPO_ROOT)} no longer calls the shared enumeration") + f"{caller.relative_to(REPO_ROOT)} no longer calls the shared enumeration" + ) @CALLERS @@ -677,24 +708,24 @@ def test_the_caller_passes_the_BASE_REF_argument(caller): """ code = _code_lines(caller) if '"$ENUM"' in code: - assert re.search(r'^\s*ENUM=\S*scripts/pr-changed-files\.sh\s*$', code, re.M), ( - f"{caller.relative_to(REPO_ROOT)} invokes \"$ENUM\" but ENUM is not assigned the shared " - "enumeration script") - invocations = [ln for ln in code.splitlines() - if re.search(r'\$\(\s*"(\$ENUM|[^"]*pr-changed-files\.sh)"', ln)] - assert invocations, ( - f"{caller.relative_to(REPO_ROOT)} has no executable call to the shared enumeration") + assert re.search(r"^\s*ENUM=\S*scripts/pr-changed-files\.sh\s*$", code, re.M), ( + f'{caller.relative_to(REPO_ROOT)} invokes "$ENUM" but ENUM is not assigned the shared enumeration script' + ) + invocations = [ln for ln in code.splitlines() if re.search(r'\$\(\s*"(\$ENUM|[^"]*pr-changed-files\.sh)"', ln)] + assert invocations, f"{caller.relative_to(REPO_ROOT)} has no executable call to the shared enumeration" for ln in invocations: after = ln.split('"', 2)[2] if '"$ENUM"' in ln else ln.split("pr-changed-files.sh", 1)[1] args = re.findall(r'"[^"]*\$[^"]*"', after) assert len(args) >= 5, ( f"{caller.relative_to(REPO_ROOT)} calls the enumeration with {len(args)} quoted " - f"arguments, expected 5 including the expected base ref: {ln.strip()!r}") + f"arguments, expected 5 including the expected base ref: {ln.strip()!r}" + ) # Counting five arguments is not enough — cold review caught that passing `"$SHA"` twice # satisfied the count while stalling every real exemption. Name the fifth. - assert re.search(r'(?i)base', args[4]), ( + assert re.search(r"(?i)base", args[4]), ( f"{caller.relative_to(REPO_ROOT)} passes {args[4]} as the 5th argument; it must be the " - f"expected BASE ref: {ln.strip()!r}") + f"expected BASE ref: {ln.strip()!r}" + ) @CALLERS @@ -710,13 +741,16 @@ def test_the_caller_does_not_reimplement_the_enumeration(caller): # gap rather than closing it. assert not re.search(r"pulls/\$?\{?\w+\}?/files\?", _code_lines(caller)), ( f"{caller.relative_to(REPO_ROOT)} appears to enumerate PR files inline again — " - "that is the duplication ersatztv#649 removed") + "that is the duplication ersatztv#649 removed" + ) # --- The ENFORCED caller's own preconditions --------------------------------------------------- + def _workflow_steps(): import yaml + wf = yaml.safe_load(WORKFLOW.read_text()) return wf["jobs"]["set-verdict-status"]["steps"] @@ -737,6 +771,7 @@ def _workflow_triggers(): broken" rather than "the trigger changed", which is the wrong failure to hand a maintainer. """ import yaml + wf = yaml.safe_load(WORKFLOW.read_text()) on = wf.get("on", wf.get(True)) assert isinstance(on, dict), f"review-verdict.yml has no parseable `on:` mapping (got {on!r})" @@ -776,13 +811,15 @@ def test_the_workflow_trigger_is_pull_request_TARGET_scoped_to_main(): f"review-verdict.yml must trigger on `pull_request_target` and NOTHING else; got " f"{sorted(map(str, on))}. Plain `pull_request` takes the workflow DEFINITION from the PR " "head (ersatztv#672), and `push`/`workflow_dispatch` resolve it from an arbitrary ref — " - "any of them, ADDED ALONGSIDE rather than replacing, reopens the hole") + "any of them, ADDED ALONGSIDE rather than replacing, reopens the hole" + ) branches = (on["pull_request_target"] or {}).get("branches") assert branches == ["main"], ( f"`pull_request_target.branches` is {branches!r}; it must be exactly ['main']. Base " "resolution means the BASE branch supplies the gate, so an unfiltered trigger lets a PR " - "into an attacker-pushed base run that branch's rewritten copy (ersatztv#672)") + "into an attacker-pushed base run that branch's rewritten copy (ersatztv#672)" + ) def test_no_OTHER_workflow_writes_the_review_verdict_status(): @@ -817,7 +854,8 @@ def test_no_OTHER_workflow_writes_the_review_verdict_status(): assert not offenders, ( f"{offenders} reference `review-verdict/h10` in executable lines. Only review-verdict.yml " "may write the gate's own status; another workflow doing so is a forgery path (see " - "ersatztv#697) or, at best, a second implementation of the gate that will drift") + "ersatztv#697) or, at best, a second implementation of the gate that will drift" + ) def test_the_workflow_checks_out_the_BASE_ref_never_the_head(): @@ -833,16 +871,18 @@ def test_the_workflow_checks_out_the_BASE_ref_never_the_head(): checkouts = [s for s in _workflow_steps() if "actions/checkout" in (s.get("uses") or "")] assert len(checkouts) == 1, ( f"expected exactly one checkout step, found {len(checkouts)} — a second checkout can " - "silently overwrite the base ref with the PR head") + "silently overwrite the base ref with the PR head" + ) with_ = checkouts[0].get("with") or {} ref = str(with_.get("ref", "")) assert "pull_request.base.sha" in ref, ( - f"the checkout ref is {ref!r}; it must be the PR's BASE sha, so a PR cannot rewrite the " - "gate that judges it") + f"the checkout ref is {ref!r}; it must be the PR's BASE sha, so a PR cannot rewrite the gate that judges it" + ) assert "head" not in ref, f"the checkout ref {ref!r} references the PR head" assert with_.get("persist-credentials") is False, ( "persist-credentials must be false — nothing here pushes, and a token left in .git/config " - "is handed to every script the job runs") + "is handed to every script the job runs" + ) def test_the_workflow_runs_the_jq_preflight_in_FLOOR_mode_only(): @@ -861,13 +901,16 @@ def test_the_workflow_runs_the_jq_preflight_in_FLOOR_mode_only(): steps = [s for s in _workflow_steps() if "jq-preflight.sh" in (s.get("run") or "")] assert steps, ( "review-verdict.yml no longer runs the jq preflight, so the version its shell gates run " - "under is unobservable again (ersatztv#648)") - invocations = [ln.strip() for ln in steps[0]["run"].splitlines() - if re.match(r"^\s*(\./)?scripts/jq-preflight\.sh(\s|$)", ln)] + "under is unobservable again (ersatztv#648)" + ) + invocations = [ + ln.strip() for ln in steps[0]["run"].splitlines() if re.match(r"^\s*(\./)?scripts/jq-preflight\.sh(\s|$)", ln) + ] assert invocations, "the jq preflight is referenced but never actually invoked" assert "--expect" not in code, ( "review-verdict.yml must run jq-preflight.sh in floor-only mode; --expect here deadlocks " - "every merge on `main` the day the runner's jq changes") + "every merge on `main` the day the runner's jq changes" + ) assert all("--expect" not in ln for ln in invocations) @@ -883,7 +926,7 @@ def test_the_workflow_runs_the_jq_preflight_in_FLOOR_mode_only(): # behavioural test of the shipped text — not a paraphrase of it — at the cost of not exercising the # runner's step wiring, which no local test can reach anyway. -WORKFLOW_STUB_CURL = r'''#!/usr/bin/env python3 +WORKFLOW_STUB_CURL = r"""#!/usr/bin/env python3 import json, os, pathlib, sys args = sys.argv[1:] @@ -1172,29 +1215,41 @@ if "/status" in url: sys.exit(0) print("{}") -''' +""" -def _run_classify(tmp_path, enum_stub: str | None, author: str = "timothy", - status_mode: str = "none", jq16: bool = False, - status_creator: str | None = "timothy", - status_desc: str = "Review-verdict: MERGEABLE @ a9e3e23 (base: main)", - timeline_mode: str = "none", history_mode: str = "none", - timeline_terminator: str = "null", - status_empty_shape: str = "null"): +def _run_classify( + tmp_path, + enum_stub: str | None, + author: str = "timothy", + status_mode: str = "none", + jq16: bool = False, + status_creator: str | None = "timothy", + status_desc: str = "Review-verdict: MERGEABLE @ a9e3e23 (base: main)", + timeline_mode: str = "none", + history_mode: str = "none", + timeline_terminator: str = "null", + status_empty_shape: str = "null", +): """Execute the workflow's classify `run:` block with a stubbed enumeration script. Returns the status payload the job POSTed, or None if it posted nothing. """ - tmp_path.mkdir(parents=True, exist_ok=True) # the chained sentinel test passes sub-paths - bindir = tmp_path / "bin"; bindir.mkdir() - curl = bindir / "curl"; curl.write_text(WORKFLOW_STUB_CURL); curl.chmod(0o755) + tmp_path.mkdir(parents=True, exist_ok=True) # the chained sentinel test passes sub-paths + bindir = tmp_path / "bin" + bindir.mkdir() + curl = bindir / "curl" + curl.write_text(WORKFLOW_STUB_CURL) + curl.chmod(0o755) if jq16: # Reproduce the runner's jq 1.6 (`-e` over EMPTY input exits 0, not 4). The shim is the one # already used for pr-changed-files.sh in this file, so its fidelity is covered by that # suite's own verify-the-verifier test. - jq = bindir / "jq"; jq.write_text(_JQ16_SHIM); jq.chmod(0o755) - scripts = tmp_path / "scripts"; scripts.mkdir() + jq = bindir / "jq" + jq.write_text(_JQ16_SHIM) + jq.chmod(0o755) + scripts = tmp_path / "scripts" + scripts.mkdir() if enum_stub is not None: enum = scripts / "pr-changed-files.sh" enum.write_text(enum_stub) @@ -1210,26 +1265,27 @@ def _run_classify(tmp_path, enum_stub: str | None, author: str = "timothy", env["STUB_TIMELINE_TERMINATOR"] = timeline_terminator env["STUB_STATUS_EMPTY_SHAPE"] = status_empty_shape env["STUB_HISTORY_MODE"] = history_mode - env.update({ - "GITEA_TOKEN": "stub", - "BASE_URL": "http://gitea.example/api/v1", - "GITEA_BASE_URL": "http://gitea.example/api/v1", - "REPO": "timothy/ersatztv", - "PR": "42", - "SHA": SHA, - "BASE_SHA": OTHER_SHA, - # The base branch the event was raised for, threaded to the enumeration (ersatztv#698). - # Omitting it is not a soft failure: `set -u` kills the step, no status is posted, and the - # absent required check blocks the merge — fail-closed, but it would take every PR with it. - "BASE_REF": "main", - "AUTHOR": author, - "PR_URL": "http://gitea.example/timothy/ersatztv/pulls/42", - }) + env.update( + { + "GITEA_TOKEN": "stub", + "BASE_URL": "http://gitea.example/api/v1", + "GITEA_BASE_URL": "http://gitea.example/api/v1", + "REPO": "timothy/ersatztv", + "PR": "42", + "SHA": SHA, + "BASE_SHA": OTHER_SHA, + # The base branch the event was raised for, threaded to the enumeration (ersatztv#698). + # Omitting it is not a soft failure: `set -u` kills the step, no status is posted, and the + # absent required check blocks the merge — fail-closed, but it would take every PR with it. + "BASE_REF": "main", + "AUTHOR": author, + "PR_URL": "http://gitea.example/timothy/ersatztv/pulls/42", + } + ) script = tmp_path / "step.sh" script.write_text(_classify_step()["run"]) - r = subprocess.run(["bash", str(script)], cwd=tmp_path, env=env, - capture_output=True, text=True) + r = subprocess.run(["bash", str(script)], cwd=tmp_path, env=env, capture_output=True, text=True) # Assert the WIRING, not only the classification. The stub accepts every POST, so a status aimed # at the wrong endpoint, sha, host or repo would otherwise leave these tests green while the real # required check was never written. @@ -1243,11 +1299,12 @@ def _run_classify(tmp_path, enum_stub: str | None, author: str = "timothy", url_file = tmp_path / "posted_url.txt" if posted.exists(): assert url_file.exists(), ( - "a status was POSTed but its URL was not recorded — the wiring assertion below would " - "have silently skipped") + "a status was POSTed but its URL was not recorded — the wiring assertion below would have silently skipped" + ) expected = f"{env['BASE_URL']}/repos/{env['REPO']}/statuses/{SHA}" assert url_file.read_text().strip() == expected, ( - f"the status was POSTed to {url_file.read_text().strip()!r}, expected {expected!r}") + f"the status was POSTed to {url_file.read_text().strip()!r}, expected {expected!r}" + ) return (json.loads(posted.read_text()) if posted.exists() else None), r @@ -1261,7 +1318,8 @@ def test_a_FAILING_enumeration_withholds_the_exemption_even_when_stdout_looks_do assert posted["state"] == "pending", ( f"a docs-only exemption was granted from the stdout of a script that FAILED " f"(state={posted['state']}, desc={posted['description']!r}) — the exit status is not being " - "checked") + "checked" + ) def test_workflow_positive_control_the_same_output_with_exit_0_DOES_exempt(tmp_path): @@ -1270,7 +1328,8 @@ def test_workflow_positive_control_the_same_output_with_exit_0_DOES_exempt(tmp_p assert posted is not None, f"the job posted no status at all: {r.stderr}" assert posted["state"] == "success", ( "the positive control failed, so the negative test above cannot be trusted to be measuring " - f"the exit status at all (state={posted['state']}, desc={posted['description']!r})") + f"the exit status at all (state={posted['state']}, desc={posted['description']!r})" + ) def test_a_crashing_enumeration_also_withholds_the_exemption(tmp_path): @@ -1316,21 +1375,22 @@ def test_a_BOT_pr_touching_a_protected_path_is_NOT_exempt(tmp_path): merge gate could actually merge itself. """ posted, _ = _run_classify( - tmp_path, _emitting("ErsatzTV/Program.cs", "scripts/pr-changed-files.sh"), - author="renovate") + tmp_path, _emitting("ErsatzTV/Program.cs", "scripts/pr-changed-files.sh"), author="renovate" + ) assert posted is not None assert posted["state"] == "pending", ( "a bot PR editing the shared enumeration was auto-exempted — a PR that weakens the merge " - "gate must never be able to exempt itself from the merge gate") + "gate must never be able to exempt itself from the merge gate" + ) def test_bot_positive_control_a_plain_bot_pr_IS_exempt(tmp_path): """Proves the test above measures `PROTECTED` and not merely 'bot PRs are never exempt'.""" - posted, _ = _run_classify( - tmp_path, _emitting("Directory.Packages.props"), author="renovate") + posted, _ = _run_classify(tmp_path, _emitting("Directory.Packages.props"), author="renovate") assert posted is not None assert posted["state"] == "success", ( - "the bot exemption never fires at all, so the protected-path test above proves nothing") + "the bot exemption never fires at all, so the protected-path test above proves nothing" + ) # --- The STATUS READ: the ersatztv#647 guard and the never-overwrite short-circuit ------------- @@ -1338,6 +1398,7 @@ def test_bot_positive_control_a_plain_bot_pr_IS_exempt(tmp_path): # These were unreachable until the stub's status response became configurable. Four mutations # survived the full suite without them, including re-introducing the literal ersatztv#647 fail-open. + def test_a_transport_failure_on_the_STATUS_READ_posts_NOTHING(tmp_path): """`gh()` is `curl -sf`, so an HTTP error yields exit 22 and EMPTY stdout. Reading that as "no verdict exists" would let the job post over a real human verdict. It must fail WITHOUT posting. @@ -1363,12 +1424,16 @@ def test_a_GARBAGE_status_response_posts_NOTHING(tmp_path): # That is fail-closed by REDUNDANCY. The guard's own behaviour is pinned by the jq-1.6 test below, # which is where it actually matters. + @pytest.mark.parametrize("existing", ["success", "failure"]) -@pytest.mark.parametrize("paths,author", [ - (("ErsatzTV/Program.cs",), "timothy"), # non-exempt: only the short-circuit can stop it - (("docs/a.md",), "timothy"), # docs-only EXEMPT - (("Directory.Packages.props",), "renovate"), # bot EXEMPT -]) +@pytest.mark.parametrize( + "paths,author", + [ + (("ErsatzTV/Program.cs",), "timothy"), # non-exempt: only the short-circuit can stop it + (("docs/a.md",), "timothy"), # docs-only EXEMPT + (("Directory.Packages.props",), "renovate"), # bot EXEMPT + ], +) def test_an_existing_verdict_on_this_head_is_NEVER_overwritten(tmp_path, existing, paths, author): """A human verdict for this exact head may already exist — the reviewer ran post-review-verdict.sh before this job finished, or the job re-ran. Re-posting would un-approve @@ -1377,15 +1442,14 @@ def test_an_existing_verdict_on_this_head_is_NEVER_overwritten(tmp_path, existin `failure` is the sharp case: that is a human saying NO, and an exemption posted over it would turn a rejection into a merge. """ - posted, r = _run_classify(tmp_path, _emitting(*paths), author=author, - status_mode=f"existing:{existing}") + posted, r = _run_classify(tmp_path, _emitting(*paths), author=author, status_mode=f"existing:{existing}") assert r.returncode == 0, r.stderr - assert posted is None, ( - f"overwrote an existing '{existing}' verdict on this head with {paths} as {author}") + assert posted is None, f"overwrote an existing '{existing}' verdict on this head with {paths} as {author}" # --- The `count -eq 0` guard, on the path where it is the ONLY guard --------------------------- + def test_an_EMPTY_enumeration_is_not_exempt_even_for_a_BOT(tmp_path): """The author must be a BOT for this to test anything. @@ -1401,11 +1465,13 @@ def test_an_EMPTY_enumeration_is_not_exempt_even_for_a_BOT(tmp_path): assert posted is not None, f"the job posted no status at all: {r.stderr}" assert posted["state"] == "pending", ( "an empty file list was treated as a bot exemption — the enumeration returning nothing is " - "not evidence that nothing was changed") + "not evidence that nothing was changed" + ) # --- Anchors in the classifier predicates ------------------------------------------------------ + def test_the_BOT_match_is_whole_line_not_substring(tmp_path): """`grep -qxF` is anchored; plain `grep -qF` would exempt any author whose name CONTAINS a bot name. `ova` is a substring of `renovate`.""" @@ -1438,11 +1504,10 @@ def test_a_transport_failure_under_jq_1_6_STILL_posts_nothing(tmp_path): This test is strictly stronger: it catches that mutant, needs no comment-stripping, and fails for the right reason. Verified by mutation under both jq versions. """ - posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), - status_mode="transport-error", jq16=True) + posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), status_mode="transport-error", jq16=True) assert r.returncode != 0, ( - "under jq 1.6 an unreadable status read must still fail the job — this is the exact " - "ersatztv#647 fail-open") + "under jq 1.6 an unreadable status read must still fail the job — this is the exact ersatztv#647 fail-open" + ) assert posted is None, "nothing may be posted when the existing verdict state is unknown" @@ -1467,25 +1532,27 @@ def test_DOCS_ONLY_anchors_the_START_of_the_path_too(tmp_path): # historical outlier, PR #20, touched a `.csproj` AND two C# files — and received an unattended bot # exemption for a source change. + def test_a_BOT_pr_carrying_a_CODE_file_is_NOT_exempt(tmp_path): """Route 2, stated as a behaviour: the hijacked-branch case.""" posted, _ = _run_classify(tmp_path, _emitting("ErsatzTV/Program.cs"), author="renovate") assert posted is not None assert posted["state"] == "pending", ( "a PR authored by the renovate account was exempted while changing a C# file — the bot " - "identity is being read as attribution for code it did not write") + "identity is being read as attribution for code it did not write" + ) def test_a_BOT_pr_mixing_a_manifest_WITH_code_is_NOT_exempt(tmp_path): """The realistic shape of the attack: keep the manifest edit so the PR still looks like a bump, and smuggle the code alongside it. A rule that asked 'does it touch a manifest' rather than 'is EVERY path a manifest' would exempt this.""" - posted, _ = _run_classify( - tmp_path, _emitting("Directory.Packages.props", "ErsatzTV/Program.cs"), author="renovate") + posted, _ = _run_classify(tmp_path, _emitting("Directory.Packages.props", "ErsatzTV/Program.cs"), author="renovate") assert posted is not None assert posted["state"] == "pending", ( "a manifest edit was enough to carry a C# file through the bot exemption — the allow-list " - "is being applied as 'any' rather than 'all'") + "is being applied as 'any' rather than 'all'" + ) def test_bot_positive_control_a_REAL_dependency_bump_IS_still_exempt(tmp_path): @@ -1495,7 +1562,8 @@ def test_bot_positive_control_a_REAL_dependency_bump_IS_still_exempt(tmp_path): assert posted is not None assert posted["state"] == "success", ( f"a plain dependency bump lost its exemption ({posted['description']!r}) — every Renovate PR " - "would now stall waiting on a human verdict") + "would now stall waiting on a human verdict" + ) @pytest.mark.parametrize("path", ["Directory.Packages.props", ".config/dotnet-tools.json"]) @@ -1510,8 +1578,7 @@ def test_every_manifest_in_the_allow_list_is_actually_exempt(tmp_path, path): def test_the_manifest_allow_list_is_ANCHORED(tmp_path): """Unanchored, `Directory.Packages.props` would match `evil/Directory.Packages.props.cs`. Same class as the two DOCS_ONLY anchoring tests above, which is why it is tested the same way.""" - posted, _ = _run_classify(tmp_path, _emitting("evil/Directory.Packages.props.cs"), - author="renovate") + posted, _ = _run_classify(tmp_path, _emitting("evil/Directory.Packages.props.cs"), author="renovate") assert posted is not None assert posted["state"] == "pending", "the manifest allow-list matched mid-path" @@ -1528,13 +1595,13 @@ def test_a_BOT_docs_only_pr_is_STILL_exempt_via_the_docs_rule(tmp_path): assert posted is not None assert posted["state"] == "success", ( "a docs-only PR lost its docs-only exemption merely because its author is a bot — the " - "exemptions are chained rather than composed") + "exemptions are chained rather than composed" + ) def test_a_BOT_pr_touching_a_protected_path_is_still_NOT_exempt(tmp_path): """PROTECTED must keep outranking both exemptions, including the manifest one.""" - posted, _ = _run_classify(tmp_path, _emitting("Directory.Packages.props", "scripts/evil.sh"), - author="renovate") + posted, _ = _run_classify(tmp_path, _emitting("Directory.Packages.props", "scripts/evil.sh"), author="renovate") assert posted is not None assert posted["state"] == "pending" @@ -1552,19 +1619,24 @@ def test_a_BOT_pr_touching_a_protected_path_is_still_NOT_exempt(tmp_path): # short-circuit only on something identified as a human verdict — so an unrecognised shape is # re-derived rather than trusted. + def test_a_MACHINE_written_exemption_success_is_RE_DERIVED_not_inherited(tmp_path): """Route 3. The status looks exactly like one this job writes: creator null, `Exempt:` wording. The PR now carries a C# file, so re-deriving must downgrade it to `pending`. Inheriting it would leave a forged exemption standing forever. """ - posted, _ = _run_classify(tmp_path, _emitting("ErsatzTV/Program.cs"), - status_mode="existing:success", - status_creator=None, - status_desc="Exempt: docs-only change (no code, no protected path)") + posted, _ = _run_classify( + tmp_path, + _emitting("ErsatzTV/Program.cs"), + status_mode="existing:success", + status_creator=None, + status_desc="Exempt: docs-only change (no code, no protected path)", + ) assert posted is not None, ( "an existing machine-written success was inherited unchanged — this is route 3, and it is " - "how a forgery obtained once survives every subsequent run") + "how a forgery obtained once survives every subsequent run" + ) assert posted["state"] == "pending" @@ -1572,20 +1644,26 @@ def test_a_success_with_NO_creator_but_a_VERDICT_LOOKING_description_is_re_deriv """Isolates the creator half. Both conditions are required; either alone is forgeable by the other party. If a future Gitea populates `creator` for Actions, the description half still fails — the guard degrades toward re-deriving, never toward trusting.""" - posted, _ = _run_classify(tmp_path, _emitting("ErsatzTV/Program.cs"), - status_mode="existing:success", - status_creator=None, - status_desc="Review-verdict: MERGEABLE @ a9e3e23 (base: main)") + posted, _ = _run_classify( + tmp_path, + _emitting("ErsatzTV/Program.cs"), + status_mode="existing:success", + status_creator=None, + status_desc="Review-verdict: MERGEABLE @ a9e3e23 (base: main)", + ) assert posted is not None, "a creatorless status was trusted on the strength of its description" assert posted["state"] == "pending" def test_a_success_with_a_creator_but_an_EXEMPT_description_is_re_derived(tmp_path): """Isolates the description half, the mirror of the test above.""" - posted, _ = _run_classify(tmp_path, _emitting("ErsatzTV/Program.cs"), - status_mode="existing:success", - status_creator="timothy", - status_desc="Exempt: docs-only change (no code, no protected path)") + posted, _ = _run_classify( + tmp_path, + _emitting("ErsatzTV/Program.cs"), + status_mode="existing:success", + status_creator="timothy", + status_desc="Exempt: docs-only change (no code, no protected path)", + ) assert posted is not None, "a status was trusted on the strength of its creator alone" assert posted["state"] == "pending" @@ -1594,9 +1672,9 @@ def test_an_UNRECOGNISED_success_shape_is_re_derived_rather_than_trusted(tmp_pat """The direction-of-test check. Written as 'skip if it looks machine-written', anything novel would fall through to TRUSTED. Written as 'skip only if positively identified as human', novel shapes are re-derived. This test fails under the first spelling and passes under the second.""" - posted, _ = _run_classify(tmp_path, _emitting("ErsatzTV/Program.cs"), - status_mode="existing:success", - status_creator="", status_desc="") + posted, _ = _run_classify( + tmp_path, _emitting("ErsatzTV/Program.cs"), status_mode="existing:success", status_creator="", status_desc="" + ) assert posted is not None assert posted["state"] == "pending" @@ -1610,21 +1688,30 @@ def test_a_REAL_human_verdict_is_still_NEVER_overwritten(tmp_path, existing): have posted an exemption `success` had it not short-circuited — without that, a passing test would prove only that nothing was posted for some unrelated reason. """ - posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), - status_mode=f"existing:{existing}", - status_creator="timothy", - status_desc=f"Review-verdict: MERGEABLE @ a9e3e23 (base: main)") + posted, r = _run_classify( + tmp_path, + _emitting("docs/a.md"), + status_mode=f"existing:{existing}", + status_creator="timothy", + status_desc="Review-verdict: MERGEABLE @ a9e3e23 (base: main)", + ) assert r.returncode == 0, r.stderr assert posted is None, ( f"overwrote a human '{existing}' verdict written by timothy — the never-overwrite property " - "has been lost while fixing route 3") + "has been lost while fixing route 3" + ) def test_a_PENDING_status_from_a_previous_run_is_replaced_normally(tmp_path): """`pending` was never short-circuited and must stay that way, or a PR that becomes exempt after an earlier pending run could never reach `success`.""" - posted, _ = _run_classify(tmp_path, _emitting("docs/a.md"), status_mode="existing:pending", - status_creator=None, status_desc="Awaiting review verdict for a9e3e23") + posted, _ = _run_classify( + tmp_path, + _emitting("docs/a.md"), + status_mode="existing:pending", + status_creator=None, + status_desc="Awaiting review verdict for a9e3e23", + ) assert posted is not None assert posted["state"] == "success" @@ -1642,8 +1729,8 @@ def test_the_npm_manifests_are_NOT_exempt(tmp_path, path): posted, _ = _run_classify(tmp_path, _emitting(path), author="renovate") assert posted is not None assert posted["state"] == "pending", ( - f"{path} was exempted; package.json scripts execute in CI, and npm is not a managed ecosystem " - "in this repo") + f"{path} was exempted; package.json scripts execute in CI, and npm is not a managed ecosystem in this repo" + ) # --- The `grep -q` + `pipefail` inversion (found by cross-family review of this PR) --------------- @@ -1657,6 +1744,7 @@ def test_the_npm_manifests_are_NOT_exempt(tmp_path, path): # earlier case used a handful of short paths, far below the buffer, so the whole class was invisible. # The construct PREDATES #698, so `main` carried this hole with no retarget or bot account required. + def _many_docs(n=1900): return [f"docs/{'d' * 40}-{i:040d}.md" for i in range(n)] @@ -1668,7 +1756,8 @@ def test_a_LARGE_pr_with_a_code_file_is_not_classified_docs_only(tmp_path): assert posted is not None, f"the job posted nothing: {r.stderr[-2000:]}" assert posted["state"] == "pending", ( "a PR containing A.cs was exempted as docs-only because the classification pipeline was " - f"inverted by SIGPIPE on a large file list (desc={posted['description']!r})") + f"inverted by SIGPIPE on a large file list (desc={posted['description']!r})" + ) def test_a_LARGE_pr_touching_a_PROTECTED_path_still_voids_the_exemptions(tmp_path): @@ -1678,7 +1767,8 @@ def test_a_LARGE_pr_touching_a_PROTECTED_path_still_voids_the_exemptions(tmp_pat assert posted is not None, f"the job posted nothing: {r.stderr[-2000:]}" assert posted["state"] == "pending", ( "a PR editing .gitea/workflows/ was exempted — the protected-path guard was skipped on a " - f"large file list (desc={posted['description']!r})") + f"large file list (desc={posted['description']!r})" + ) def test_a_LARGE_bot_pr_with_a_code_file_is_not_manifest_exempt(tmp_path): @@ -1694,7 +1784,8 @@ def test_positive_control_a_LARGE_genuinely_docs_only_pr_IS_still_exempt(tmp_pat posted, r = _run_classify(tmp_path, _emitting(*_many_docs())) assert posted is not None, f"the job posted nothing: {r.stderr[-2000:]}" assert posted["state"] == "success", ( - f"a large but genuinely docs-only PR lost its exemption (desc={posted['description']!r})") + f"a large but genuinely docs-only PR lost its exemption (desc={posted['description']!r})" + ) def test_a_human_verdict_landing_MID_RUN_is_not_overwritten(tmp_path): @@ -1709,12 +1800,12 @@ def test_a_human_verdict_landing_MID_RUN_is_not_overwritten(tmp_path): landing between the re-read and the POST is still lost — there is no compare-and-set on Gitea's status API. That remainder is #706, not a claim made here. """ - posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), - status_mode="appears-on-read:2") + posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), status_mode="appears-on-read:2") assert r.returncode == 0, r.stderr assert posted is None, ( "an exemption success was posted over a human BLOCKED verdict that landed while the job was " - "classifying — the pre-POST re-read is missing or ineffective") + "classifying — the pre-POST re-read is missing or ineffective" + ) def test_positive_control_no_late_verdict_still_posts_normally(tmp_path): @@ -1734,6 +1825,7 @@ def test_positive_control_no_late_verdict_still_posts_normally(tmp_path): # The lesson generalises: when several branches produce the same outcome, asserting the outcome cannot # tell you which branch ran. Assert the DISCRIMINATOR — here the reason string the branch writes. + def test_a_protected_path_is_rejected_BY_THE_PROTECTED_BRANCH(tmp_path): posted, r = _run_classify(tmp_path, _emitting("scripts/evil.sh", "docs/a.md")) assert posted is not None, f"the job posted nothing: {r.stderr[-2000:]}" @@ -1744,7 +1836,8 @@ def test_a_protected_path_is_rejected_BY_THE_PROTECTED_BRANCH(tmp_path): # guard — the assertion has to be aimed at something that actually differs per branch. assert "protected" in r.stdout.lower(), ( "the PR was not exempted, but NOT via the protected-path branch — it reached the same verdict " - f"by another route, so that guard may be dead. Decision log:\n{r.stdout[-800:]}") + f"by another route, so that guard may be dead. Decision log:\n{r.stdout[-800:]}" + ) def test_a_protected_path_defeats_the_BOT_exemption_by_the_protected_branch(tmp_path): @@ -1752,20 +1845,24 @@ def test_a_protected_path_defeats_the_BOT_exemption_by_the_protected_branch(tmp_ branch this still lands on `pending` (the protected file is not a manifest), so again only the reason string distinguishes a working guard from a dead one.""" posted, r = _run_classify( - tmp_path, _emitting("Directory.Packages.props", ".gitea/workflows/renovate.yml"), - author="renovate") + tmp_path, _emitting("Directory.Packages.props", ".gitea/workflows/renovate.yml"), author="renovate" + ) assert posted is not None assert posted["state"] == "pending" assert "protected" in r.stdout.lower(), ( - f"the bot exemption was refused, but not by the PROTECTED branch:\n{r.stdout[-800:]}") + f"the bot exemption was refused, but not by the PROTECTED branch:\n{r.stdout[-800:]}" + ) -@pytest.mark.parametrize("paths,author", [ - (("docs/a.md",), "timothy"), - (("Directory.Packages.props",), "renovate"), - (("ErsatzTV/Program.cs",), "timothy"), - (("scripts/evil.sh",), "timothy"), -]) +@pytest.mark.parametrize( + "paths,author", + [ + (("docs/a.md",), "timothy"), + (("Directory.Packages.props",), "renovate"), + (("ErsatzTV/Program.cs",), "timothy"), + (("scripts/evil.sh",), "timothy"), + ], +) def test_the_classify_step_runs_without_SHELL_ERRORS(tmp_path, paths, author): """A cheap, general trap-catcher for the whole step. @@ -1779,11 +1876,14 @@ def test_the_classify_step_runs_without_SHELL_ERRORS(tmp_path, paths, author): which is the actual liveness guard; this one is the cheap net for the whole error-emitting family. """ _, r = _run_classify(tmp_path, _emitting(*paths), author=author) - bad = [ln for ln in r.stderr.splitlines() - if "command not found" in ln - or "integer expression expected" in ln - or "unbound variable" in ln - or "syntax error" in ln] + bad = [ + ln + for ln in r.stderr.splitlines() + if "command not found" in ln + or "integer expression expected" in ln + or "unbound variable" in ln + or "syntax error" in ln + ] assert not bad, f"the classification step emitted shell errors, so a guard is not running: {bad}" @@ -1799,23 +1899,30 @@ def test_a_human_verdict_formed_against_ANOTHER_BASE_is_not_inherited(tmp_path): `post-review-verdict.sh` already records the base it reviewed (`(base: …)`, ersatztv#632); this asserts the gate actually READS it. """ - posted, r = _run_classify(tmp_path, _emitting("ErsatzTV/Program.cs"), - status_mode="existing:success", - status_creator="timothy", - status_desc="Review-verdict: MERGEABLE @ a9e3e23 (base: probe/scratch-base)") + posted, r = _run_classify( + tmp_path, + _emitting("ErsatzTV/Program.cs"), + status_mode="existing:success", + status_creator="timothy", + status_desc="Review-verdict: MERGEABLE @ a9e3e23 (base: probe/scratch-base)", + ) assert posted is not None, ( "a human verdict formed against a DIFFERENT base was inherited unchanged — the reviewed diff " - f"is not this PR's diff. Log:\n{r.stdout[-600:]}") + f"is not this PR's diff. Log:\n{r.stdout[-600:]}" + ) assert posted["state"] == "pending" def test_a_human_verdict_for_THIS_base_is_still_honoured(tmp_path): """The positive control the test above needs: matching bases must still short-circuit, or the check has simply broken every verdict.""" - posted, _ = _run_classify(tmp_path, _emitting("ErsatzTV/Program.cs"), - status_mode="existing:success", - status_creator="timothy", - status_desc="Review-verdict: MERGEABLE @ a9e3e23 (base: main)") + posted, _ = _run_classify( + tmp_path, + _emitting("ErsatzTV/Program.cs"), + status_mode="existing:success", + status_creator="timothy", + status_desc="Review-verdict: MERGEABLE @ a9e3e23 (base: main)", + ) assert posted is None, "a verdict formed against THIS base was re-derived; the base check is too strict" @@ -1823,10 +1930,13 @@ def test_a_LEGACY_verdict_with_no_recorded_base_is_still_honoured(tmp_path): """Verdicts predating ersatztv#632 carry no `(base: …)`. Absent is deliberately not treated as a mismatch: re-deriving over one would un-approve a genuinely reviewed head. Only a base that is PRESENT and DIFFERENT is rejected.""" - posted, _ = _run_classify(tmp_path, _emitting("ErsatzTV/Program.cs"), - status_mode="existing:success", - status_creator="timothy", - status_desc="Review-verdict: MERGEABLE @ a9e3e23") + posted, _ = _run_classify( + tmp_path, + _emitting("ErsatzTV/Program.cs"), + status_mode="existing:success", + status_creator="timothy", + status_desc="Review-verdict: MERGEABLE @ a9e3e23", + ) assert posted is None @@ -1841,12 +1951,16 @@ def test_an_APPENDED_base_cannot_override_the_real_one(tmp_path): appended marker makes the count 2 and is rejected outright. """ posted, r = _run_classify( - tmp_path, _emitting("ErsatzTV/Program.cs"), - status_mode="existing:success", status_creator="timothy", - status_desc="Review-verdict: MERGEABLE @ a9e3e23 (base: probe/scratch) (base: main)") + tmp_path, + _emitting("ErsatzTV/Program.cs"), + status_mode="existing:success", + status_creator="timothy", + status_desc="Review-verdict: MERGEABLE @ a9e3e23 (base: probe/scratch) (base: main)", + ) assert posted is not None, ( "an appended '(base: main)' overrode the real recorded base, so a verdict formed elsewhere was " - f"inherited. Log:\n{r.stdout[-600:]}") + f"inherited. Log:\n{r.stdout[-600:]}" + ) assert posted["state"] == "pending" @@ -1854,9 +1968,12 @@ def test_an_EMPTY_recorded_base_is_treated_as_a_mismatch(tmp_path): """`(base: )` is not 'absent' — it is present and not equal to the PR's base, so it fails closed rather than being waved through by the legacy-verdict allowance.""" posted, _ = _run_classify( - tmp_path, _emitting("ErsatzTV/Program.cs"), - status_mode="existing:success", status_creator="timothy", - status_desc="Review-verdict: MERGEABLE @ a9e3e23 (base: )") + tmp_path, + _emitting("ErsatzTV/Program.cs"), + status_mode="existing:success", + status_creator="timothy", + status_desc="Review-verdict: MERGEABLE @ a9e3e23 (base: )", + ) assert posted is not None and posted["state"] == "pending" @@ -1874,12 +1991,16 @@ def test_a_branch_name_containing_a_PAREN_cannot_truncate_into_the_current_base( the exact expected literal, never parse a value out of attacker- or user-influenced text. """ posted, r = _run_classify( - tmp_path, _emitting("ErsatzTV/Program.cs"), - status_mode="existing:success", status_creator="timothy", - status_desc="Review-verdict: MERGEABLE @ a9e3e23 (base: main)evil)") + tmp_path, + _emitting("ErsatzTV/Program.cs"), + status_mode="existing:success", + status_creator="timothy", + status_desc="Review-verdict: MERGEABLE @ a9e3e23 (base: main)evil)", + ) assert posted is not None, ( "a verdict recorded against branch 'main)evil' was inherited by a PR targeting 'main' — the " - f"base value is being truncated at the first ')'. Log:\n{r.stdout[-600:]}") + f"base value is being truncated at the first ')'. Log:\n{r.stdout[-600:]}" + ) assert posted["state"] == "pending" @@ -1899,10 +2020,11 @@ def test_a_branch_name_containing_a_PAREN_cannot_truncate_into_the_current_base( # The fence keys on the timeline's `change_target_branch` COUNT because the branch NAME is # ABA-vulnerable (`main → S → main` reads `main` at both ends), while the count is monotonic. + def _posted_sequence(tmp_path): """Every status POST the job made, in order — not just the last one.""" f = tmp_path / "posted_all.jsonl" - return [json.loads(l) for l in f.read_text().splitlines() if l.strip()] if f.exists() else [] + return [json.loads(line) for line in f.read_text().splitlines() if line.strip()] if f.exists() else [] def test_a_RETARGET_DURING_the_run_posts_NOTHING(tmp_path): @@ -1913,18 +2035,21 @@ def test_a_RETARGET_DURING_the_run_posts_NOTHING(tmp_path): assert r.returncode == 0, r.stderr assert posted is None, ( "a stale run posted its classification after the PR was retargeted underneath it — the " - f"retarget fence did not fire. Log:\n{r.stdout[-900:]}") + f"retarget fence did not fire. Log:\n{r.stdout[-900:]}" + ) # Assert the DISCRIMINATOR, not just the outcome: several unrelated failures also end in "posted # nothing", so the state alone cannot tell a working fence from a broken job. assert "retargeted while this job was classifying" in r.stdout, ( - f"nothing was posted, but NOT via the retarget fence. Log:\n{r.stdout[-900:]}") + f"nothing was posted, but NOT via the retarget fence. Log:\n{r.stdout[-900:]}" + ) def test_positive_control_a_QUIET_run_still_posts_its_exemption(tmp_path): """Without this, the test above passes against a job that simply never posts.""" posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), timeline_mode="none") assert posted is not None and posted["state"] == "success", ( - f"an undisturbed docs-only PR lost its exemption. Log:\n{r.stdout[-900:]}") + f"an undisturbed docs-only PR lost its exemption. Log:\n{r.stdout[-900:]}" + ) def test_a_PR_retargeted_BEFORE_the_run_but_QUIET_during_it_is_STILL_exempt(tmp_path): @@ -1937,7 +2062,8 @@ def test_a_PR_retargeted_BEFORE_the_run_but_QUIET_during_it_is_STILL_exempt(tmp_ posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), timeline_mode="stable:2") assert posted is not None and posted["state"] == "success", ( "a PR with a settled, non-zero retarget history was refused its exemption — the fence is " - f"testing the count's VALUE instead of its MOVEMENT. Log:\n{r.stdout[-900:]}") + f"testing the count's VALUE instead of its MOVEMENT. Log:\n{r.stdout[-900:]}" + ) @pytest.mark.parametrize("mode", ["unreadable", "transport-error"]) @@ -1947,7 +2073,8 @@ def test_an_UNTRUSTED_retarget_count_withholds_the_EXEMPTION(tmp_path, mode): posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), timeline_mode=mode) assert posted is None, ( "an exemption success was posted even though the retarget count could not be established " - f"({mode}). Log:\n{r.stdout[-900:]}") + f"({mode}). Log:\n{r.stdout[-900:]}" + ) @pytest.mark.parametrize("mode", ["unreadable", "transport-error"]) @@ -1960,7 +2087,8 @@ def test_an_UNTRUSTED_retarget_count_STILL_LETS_PENDING_THROUGH(tmp_path, mode): posted, r = _run_classify(tmp_path, _emitting("ErsatzTV/Program.cs"), timeline_mode=mode) assert posted is not None and posted["state"] == "pending", ( "an ordinary PR was left with NO status because the timeline was unreadable; only the " - f"exemption success should be gated on the count. Log:\n{r.stdout[-900:]}") + f"exemption success should be gated on the count. Log:\n{r.stdout[-900:]}" + ) def test_a_human_verdict_landing_AFTER_the_POST_is_repaired_to_pending(tmp_path): @@ -1977,7 +2105,8 @@ def test_a_human_verdict_landing_AFTER_the_POST_is_repaired_to_pending(tmp_path) seq = _posted_sequence(tmp_path) assert len(seq) == 2, ( "expected the exemption write followed by a repair; the post-write verification did not " - f"fire. Posts: {seq}. Log:\n{r.stdout[-900:]}") + f"fire. Posts: {seq}. Log:\n{r.stdout[-900:]}" + ) assert seq[0]["state"] == "success" assert seq[1]["state"] == "pending", f"the raced exemption was not repaired: {seq}" assert posted is not None and posted["state"] == "pending" @@ -1992,17 +2121,18 @@ def test_a_PRE_EXISTING_human_row_does_NOT_trigger_a_repair(tmp_path): on EVERY later run, repair each exemption to `pending`, and permanently deadlock a PR that had one mismatched verdict once. Only rows newer than the high-water mark count. """ - posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), - history_mode="stale-human-already-present") + posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), history_mode="stale-human-already-present") seq = _posted_sequence(tmp_path) assert len(seq) == 1, ( "an old, already-present human verdict row was mistaken for one that raced this run's write, " - f"so the exemption was repaired away. Posts: {seq}. Log:\n{r.stdout[-900:]}") + f"so the exemption was repaired away. Posts: {seq}. Log:\n{r.stdout[-900:]}" + ) assert posted is not None and posted["state"] == "success" # --- ersatztv#711: the PROTECTED list must cover every mirror of the enforcement hooks ----------- + def test_a_CODEX_hook_copy_is_a_PROTECTED_path(tmp_path): """`.codex/hooks/` is a byte-identical mirror of `.claude/hooks/`, including `pretooluse-merge-consent.sh`. Editing the `.claude/` copy correctly voids both exemptions; @@ -2017,11 +2147,13 @@ def test_a_CODEX_hook_copy_is_a_PROTECTED_path(tmp_path): assert posted is not None, f"the job posted nothing: {r.stderr[-2000:]}" assert posted["state"] == "pending", ( "a PR editing a .codex/ copy of the merge-consent hook was granted the docs-only exemption " - f"(desc={posted['description']!r})") + f"(desc={posted['description']!r})" + ) # Assert the DISCRIMINATOR: a non-exempt outcome is reachable by several routes, so the state # alone cannot show the PROTECTED branch is what rejected it. assert "protected" in r.stdout.lower(), ( - f"rejected, but NOT by the protected-path branch. Decision log:\n{r.stdout[-800:]}") + f"rejected, but NOT by the protected-path branch. Decision log:\n{r.stdout[-800:]}" + ) def test_BOTH_hook_directories_are_in_the_PROTECTED_pattern(): @@ -2029,16 +2161,18 @@ def test_BOTH_hook_directories_are_in_the_PROTECTED_pattern(): today; this one states the INVARIANT — the two hook directories are mirrors, so any future edit that drops one while keeping the other is the bug, not a simplification.""" src = _code_lines(WORKFLOW) - protected = [l for l in src.splitlines() if l.strip().startswith("PROTECTED=")] + protected = [line for line in src.splitlines() if line.strip().startswith("PROTECTED=")] assert len(protected) == 1, f"expected exactly one PROTECTED definition, found {protected}" for d in (r"\.claude/", r"\.codex/"): assert d in protected[0], ( f"{d} is missing from PROTECTED ({protected[0].strip()!r}) — a directory carrying a copy " - "of the enforcement hooks can exempt itself from the gate it enforces") + "of the enforcement hooks can exempt itself from the gate it enforces" + ) # --- ersatztv#706, round 2: findings from the cold adversarial review ---------------------------- + def test_the_high_water_MARK_is_captured_BEFORE_the_last_moment_re_read(): """The High finding of round 2, pinned as the ORDERING property it actually is. @@ -2063,7 +2197,8 @@ def test_the_high_water_MARK_is_captured_BEFORE_the_last_moment_re_read(): assert len(calls) >= 2, f"expected two read_existing_verdict call sites, found {len(calls)}" assert mark < calls[-1], ( "the high-water mark is captured AFTER the last-moment re-read, reopening the blind window " - "in which a human verdict is neither seen by the re-read nor repaired by the post-write check") + "in which a human verdict is neither seen by the re-read nor repaired by the post-write check" + ) def test_a_previously_REPAIRED_head_is_never_re_exempted(tmp_path): @@ -2075,15 +2210,20 @@ def test_a_previously_REPAIRED_head_is_never_re_exempted(tmp_path): again one event later. The repair description is now a sentinel the classification recognises. """ posted, r = _run_classify( - tmp_path, _emitting("docs/a.md"), - status_mode="existing:pending", status_creator=None, - status_desc="Human verdict raced this exemption write — re-post the verdict") + tmp_path, + _emitting("docs/a.md"), + status_mode="existing:pending", + status_creator=None, + status_desc="Human verdict raced this exemption write — re-post the verdict", + ) assert posted is not None, f"the job posted nothing: {r.stderr[-2000:]}" assert posted["state"] == "pending", ( "a head whose human verdict was previously overwritten was granted a FRESH exemption, burying " - f"the rejection again (desc={posted['description']!r})") + f"the rejection again (desc={posted['description']!r})" + ) assert "raced a previous exemption" in r.stdout, ( - f"rejected, but not via the repair-sentinel branch. Log:\n{r.stdout[-800:]}") + f"rejected, but not via the repair-sentinel branch. Log:\n{r.stdout[-800:]}" + ) def test_positive_control_an_ORDINARY_machine_pending_is_still_re_derived(tmp_path): @@ -2091,11 +2231,15 @@ def test_positive_control_an_ORDINARY_machine_pending_is_still_re_derived(tmp_pa a pre-existing `pending`. An ordinary machine `pending` — no sentinel — must still re-derive to `success` for a docs-only PR.""" posted, r = _run_classify( - tmp_path, _emitting("docs/a.md"), - status_mode="existing:pending", status_creator=None, - status_desc="Awaiting review verdict for a9e3e23") + tmp_path, + _emitting("docs/a.md"), + status_mode="existing:pending", + status_creator=None, + status_desc="Awaiting review verdict for a9e3e23", + ) assert posted is not None and posted["state"] == "success", ( - f"an ordinary pending was not re-derived to an exemption. Log:\n{r.stdout[-800:]}") + f"an ordinary pending was not re-derived to an exemption. Log:\n{r.stdout[-800:]}" + ) def test_the_fence_gates_PENDING_TOO_not_only_the_exemption(tmp_path): @@ -2109,7 +2253,10 @@ def test_the_fence_gates_PENDING_TOO_not_only_the_exemption(tmp_path): posted, r = _run_classify(tmp_path, _emitting("ErsatzTV/Program.cs"), timeline_mode="moves:0,1") assert posted is None, ( "the fence let a stale `pending` through; it is gating only the exemption path " - f"(desc={posted['description']!r})" if posted else "") + f"(desc={posted['description']!r})" + if posted + else "" + ) assert "retargeted while this job was classifying" in r.stdout @@ -2120,7 +2267,8 @@ def test_an_UNTRUSTED_count_withholds_the_exemption_BY_THAT_BRANCH(tmp_path): assert posted is None assert r.returncode == 0, f"the job died rather than declining cleanly: {r.stderr[-800:]}" assert "Could not establish a trusted retarget count" in r.stdout, ( - f"nothing posted, but not via the untrusted-count branch. Log:\n{r.stdout[-800:]}") + f"nothing posted, but not via the untrusted-count branch. Log:\n{r.stdout[-800:]}" + ) def test_the_repair_sentinel_is_a_FIXED_POINT_across_consecutive_runs(tmp_path): @@ -2136,23 +2284,31 @@ def test_the_repair_sentinel_is_a_FIXED_POINT_across_consecutive_runs(tmp_path): is that the sentinel branch's own output re-triggers the sentinel branch, forever. """ first, r1 = _run_classify( - tmp_path / "run1", _emitting("docs/a.md"), - status_mode="existing:pending", status_creator=None, - status_desc="Human verdict raced this exemption write — re-post the verdict") + tmp_path / "run1", + _emitting("docs/a.md"), + status_mode="existing:pending", + status_creator=None, + status_desc="Human verdict raced this exemption write — re-post the verdict", + ) assert first is not None and first["state"] == "pending", f"run 1: {r1.stdout[-600:]}" second, r2 = _run_classify( - tmp_path / "run2", _emitting("docs/a.md"), - status_mode="existing:pending", status_creator=None, - status_desc=first["description"]) # <-- the chain + tmp_path / "run2", + _emitting("docs/a.md"), + status_mode="existing:pending", + status_creator=None, + status_desc=first["description"], + ) # <-- the chain assert second is not None, f"run 2 posted nothing: {r2.stderr[-800:]}" assert second["state"] == "pending", ( "the repair decayed: run 1's own posted description did not re-trigger the sentinel, so run 2 " f"re-derived the exemption and buried the human verdict again (run1 desc={first['description']!r}, " - f"run2 state={second['state']}, run2 desc={second['description']!r})") + f"run2 state={second['state']}, run2 desc={second['description']!r})" + ) assert second["description"] == first["description"], ( "the sentinel is not a fixed point — run 2 wrote a different description than run 1, so run 3 " - f"would not recognise it ({first['description']!r} -> {second['description']!r})") + f"would not recognise it ({first['description']!r} -> {second['description']!r})" + ) def test_a_sentinel_APPEARING_MID_RUN_stops_a_stale_run_overwriting_it(tmp_path): @@ -2167,14 +2323,15 @@ def test_a_sentinel_APPEARING_MID_RUN_stops_a_stale_run_overwriting_it(tmp_path) The re-read recomputes `ex_repair`; the bug was that nothing downstream consulted it. """ - posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), - status_mode="sentinel-appears-on-read:2") + posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), status_mode="sentinel-appears-on-read:2") assert r.returncode == 0, r.stderr assert posted is None, ( "a stale run posted its exemption over a repair sentinel written mid-run, burying the human " - f"verdict it records (posted={posted})") + f"verdict it records (posted={posted})" + ) assert "repair sentinel was written" in r.stdout, ( - f"nothing posted, but not via the mid-run sentinel guard. Log:\n{r.stdout[-800:]}") + f"nothing posted, but not via the mid-run sentinel guard. Log:\n{r.stdout[-800:]}" + ) def test_positive_control_a_sentinel_present_from_the_START_still_posts_pending(tmp_path): @@ -2185,9 +2342,12 @@ def test_positive_control_a_sentinel_present_from_the_START_still_posts_pending( what makes the mid-run guard's condition exact rather than conservative. """ posted, r = _run_classify( - tmp_path, _emitting("docs/a.md"), - status_mode="existing:pending", status_creator=None, - status_desc="Human verdict raced this exemption write — re-post the verdict") + tmp_path, + _emitting("docs/a.md"), + status_mode="existing:pending", + status_creator=None, + status_desc="Human verdict raced this exemption write — re-post the verdict", + ) assert posted is not None, f"the repaired head stopped posting entirely: {r.stdout[-800:]}" assert posted["state"] == "pending" assert posted["description"] == "Human verdict raced this exemption write — re-post the verdict" @@ -2207,14 +2367,17 @@ def test_a_PENDING_path_run_also_refuses_to_clobber_a_mid_run_sentinel(tmp_path) """ posted, r = _run_classify( tmp_path, - "#!/usr/bin/env bash\n" + DOCS_ONLY + "exit 1\n", # enumeration fails -> pending, generic desc - status_mode="sentinel-appears-on-read:2") + "#!/usr/bin/env bash\n" + DOCS_ONLY + "exit 1\n", # enumeration fails -> pending, generic desc + status_mode="sentinel-appears-on-read:2", + ) assert r.returncode == 0, r.stderr assert posted is None, ( "a pending-path run overwrote a repair sentinel written mid-run with the generic description; " - f"the next run would re-derive the exemption and bury the human verdict (posted={posted})") + f"the next run would re-derive the exemption and bury the human verdict (posted={posted})" + ) assert "repair sentinel was written" in r.stdout, ( - f"nothing posted, but not via the mid-run sentinel guard. Log:\n{r.stdout[-800:]}") + f"nothing posted, but not via the mid-run sentinel guard. Log:\n{r.stdout[-800:]}" + ) def test_a_SENTINEL_landing_above_the_mark_also_triggers_the_repair(tmp_path): @@ -2236,11 +2399,13 @@ def test_a_SENTINEL_landing_above_the_mark_also_triggers_the_repair(tmp_path): seq = _posted_sequence(tmp_path) assert len(seq) == 2, ( "the run posted its exemption on top of a sentinel written by an overlapping run and did not " - f"repair, leaving a human rejection permanently green. Posts: {seq}\n{r.stdout[-800:]}") + f"repair, leaving a human rejection permanently green. Posts: {seq}\n{r.stdout[-800:]}" + ) assert seq[0]["state"] == "success" assert seq[1]["state"] == "pending" assert seq[1]["description"] == "Human verdict raced this exemption write — re-post the verdict", ( - f"repaired, but not to the sentinel, so the fixed point is broken: {seq[1]}") + f"repaired, but not to the sentinel, so the fixed point is broken: {seq[1]}" + ) # --- The expression-delimiter class (ersatztv#751) ---------------------------------------------- @@ -2270,14 +2435,40 @@ _EXPR = re.compile(r"\$\{\{(.*?)\}\}", re.S) # Roots of a dotted context path, and the callable functions. Both lists are what the runner # actually accepts; anything outside them cannot evaluate, and an expression that cannot evaluate # does not fail loudly — it silently removes the step it appears in. -_EXPR_CONTEXTS = frozenset({ - "github", "env", "vars", "secrets", "inputs", "runner", "steps", "needs", "matrix", "job", - "jobs", "strategy", -}) -_EXPR_FUNCTIONS = frozenset({ - "always", "success", "failure", "cancelled", "hashFiles", "format", "toJSON", "toJson", - "fromJSON", "fromJson", "contains", "startsWith", "endsWith", "join", -}) +_EXPR_CONTEXTS = frozenset( + { + "github", + "env", + "vars", + "secrets", + "inputs", + "runner", + "steps", + "needs", + "matrix", + "job", + "jobs", + "strategy", + } +) +_EXPR_FUNCTIONS = frozenset( + { + "always", + "success", + "failure", + "cancelled", + "hashFiles", + "format", + "toJSON", + "toJson", + "fromJSON", + "fromJson", + "contains", + "startsWith", + "endsWith", + "join", + } +) _EXPR_LITERALS = frozenset({"true", "false", "null"}) @@ -2294,6 +2485,7 @@ def _yaml_string_scalars(path: Path): every workflow while ignoring the one place a delimiter is genuinely harmless. """ import yaml + out: list[str] = [] def walk(node): @@ -2385,21 +2577,25 @@ def test_the_verdict_workflow_has_NO_expression_delimiter_in_any_run_body(): # That catches the failure the count existed for (a helper looking at the wrong key, or dropping # steps) without breaking when a step is legitimately added or removed. import yaml as _yaml - _steps = (_yaml.safe_load(WORKFLOW.read_text())["jobs"]["set-verdict-status"]["steps"] or []) + + _steps = _yaml.safe_load(WORKFLOW.read_text())["jobs"]["set-verdict-status"]["steps"] or [] declared = sum(1 for s in _steps if isinstance(s, dict) and s.get("run")) assert len(bodies) == declared, ( f"the YAML walk reached {len(bodies)} run: bodies but the job declares {declared} — the " - "assertion above did not examine every body, so a green here proves nothing") + "assertion above did not examine every body, so a green here proves nothing" + ) assert max(len(b) for b in bodies) > 5000, ( "the YAML walk did not reach a substantial run: body — the ~700-line classifier is the one " - "that must be scanned, so this test would be vacuous") + "that must be scanned, so this test would be vacuous" + ) def _iter_workflow_steps(path: Path): import yaml + doc = yaml.safe_load(path.read_text()) or {} for job_name, job in (doc.get("jobs") or {}).items(): - for step in (job.get("steps") or []): + for step in job.get("steps") or []: yield job_name, step @@ -2431,16 +2627,16 @@ def test_every_workflow_expression_names_a_REAL_context_or_function(): offenders = [] for wf in _workflow_files(): for scalar in _yaml_string_scalars(wf): - for m in _EXPR.finditer(scalar): - payload = m.group(1).strip() - # Strip string literals first: a path inside `hashFiles('web/package-lock.json')` is - # data, not an identifier, and would otherwise read as an unknown context. - bare = re.sub(r"'[^']*'", "''", payload) - for ident in re.finditer(r"(? unknown '{name}'") + for m in _EXPR.finditer(scalar): + payload = m.group(1).strip() + # Strip string literals first: a path inside `hashFiles('web/package-lock.json')` is + # data, not an identifier, and would otherwise read as an unknown context. + bare = re.sub(r"'[^']*'", "''", payload) + for ident in re.finditer(r"(? unknown '{name}'") assert not offenders, ( "these workflow expressions name something the runner cannot resolve, so they will fail to " f"interpolate — which DROPS THE STEP and still reports the job green (ersatztv#751): " @@ -2488,23 +2684,26 @@ def test_a_dropped_classify_step_FAILS_the_job_instead_of_going_green(): marker_write = re.search(r'(\w+)="(\$\{RUNNER_TEMP[^"]*)"', classify["run"]) assert marker_write, ( "the classify step no longer assigns a start-marker path under RUNNER_TEMP, so a step the " - "runner drops goes green again (ersatztv#751)") + "runner drops goes green again (ersatztv#751)" + ) var, marker_name = marker_write.group(1), marker_write.group(2) assert re.search(rf':\s*>\s*"\${var}"', classify["run"]), ( f"the classify step defines {var} but never creates the marker, so the guard below will " - "fail on every run and read as broken rather than as a real dropped step") + "fail on every run and read as broken rather than as a real dropped step" + ) - guard_idx = [i for i, s in enumerate(steps) - if i != classifiers[0] and marker_name in (s.get("run") or "")] + guard_idx = [i for i, s in enumerate(steps) if i != classifiers[0] and marker_name in (s.get("run") or "")] assert guard_idx, ( f"no step checks for the {marker_name!r} start marker. Without it a dropped classify step " - "concludes success and the merge gate is silently dead (ersatztv#751)") + "concludes success and the merge gate is silently dead (ersatztv#751)" + ) # AFTER the classifier, not merely present. A guard placed before it would read a marker that # has not been written yet and fail on every run — fail-closed, but it would deadlock `main` and # read as this guard being broken, which is how a correct-looking guard gets deleted. assert guard_idx[0] > classifiers[0], ( f"the dropped-step guard is step {guard_idx[0]} but the classifier is step " - f"{classifiers[0]} — a guard that runs first always fails") + f"{classifiers[0]} — a guard that runs first always fails" + ) guard = steps[guard_idx[0]] # `always()` and `${{ always() }}` are the same condition; the runner accepts both and the second # is the more common spelling. Pinning the bare form EXACTLY would red the repo over a @@ -2513,7 +2712,8 @@ def test_a_dropped_classify_step_FAILS_the_job_instead_of_going_green(): guard_if = re.sub(r"\s+", "", str(guard.get("if", ""))) assert guard_if in ("always()", "${{always()}}"), ( f"the dropped-step guard's `if:` is {guard.get('if')!r}; it must be `always()` (bare or " - "wrapped), or it will be skipped on exactly the runs where the classifier failed") + "wrapped), or it will be skipped on exactly the runs where the classifier failed" + ) # INSIDE the missing-marker branch, not merely somewhere in the body. Cold review pointed out # that a bare `exit 1` substring is satisfied by an unreachable `if false; then exit 1; fi` while # the real branch says `exit 0` — the test passes and a dropped classifier goes green again. The @@ -2521,14 +2721,17 @@ def test_a_dropped_classify_step_FAILS_the_job_instead_of_going_green(): # by dead code. missing_branch = re.search(r'if \[ ! -f "\$marker" \]; then(.*?)\bfi\b', guard["run"], re.S) assert missing_branch, ( - "the dropped-step guard no longer tests for a MISSING marker with `if [ ! -f \"$marker\" ]`, " - "so the assertion below cannot locate the branch that must fail the job") + 'the dropped-step guard no longer tests for a MISSING marker with `if [ ! -f "$marker" ]`, ' + "so the assertion below cannot locate the branch that must fail the job" + ) assert re.search(r"exit\s+1", missing_branch.group(1)), ( "the dropped-step guard detects the missing marker but does not `exit 1` inside that branch, " - "so it observes the failure and still lets the job go green — which is the whole defect") + "so it observes the failure and still lets the job go green — which is the whole defect" + ) assert not _EXPR.search(guard["run"]), ( "the dropped-step guard's own run body contains an expression delimiter, so the mechanism " - "it guards against can drop the guard too — and that absence would be silent as well") + "it guards against can drop the guard too — and that absence would be silent as well" + ) @pytest.mark.parametrize("terminator", ["null", "[]"], ids=["null-page", "empty-array-page"]) @@ -2553,18 +2756,21 @@ def test_the_fence_TRUSTS_the_count_and_POSTS_when_the_timeline_terminates(tmp_p real probe run and the job still posted nothing — the decision and the write are different events, and only the write is what a merge reads. """ - posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), timeline_mode="stable:0", - timeline_terminator=terminator) + posted, r = _run_classify( + tmp_path, _emitting("docs/a.md"), timeline_mode="stable:0", timeline_terminator=terminator + ) assert r.returncode == 0, r.stderr assert posted is not None, ( f"a docs-only PR whose timeline terminates with `{terminator}` got NO status at all. The " "fence could not establish a trusted retarget count, so it withheld the exemption — which " "leaves the required review-verdict/h10 absent and the PR unmergeable with no bypass " - f"(ersatztv#751).\n{r.stdout[-1200:]}") + f"(ersatztv#751).\n{r.stdout[-1200:]}" + ) assert posted["state"] == "success", f"expected the docs-only exemption, got {posted}" assert "trusted=yes" in r.stdout, ( "the exemption was posted but the fence did not report a trusted count — the two must agree, " - f"or this test is passing for a different reason than it claims.\n{r.stdout[-800:]}") + f"or this test is passing for a different reason than it claims.\n{r.stdout[-800:]}" + ) def _marker_steps(): @@ -2587,11 +2793,9 @@ def _marker_steps(): classify = steps[idx[0]] lines = classify["run"].splitlines() write_at = [i for i, ln in enumerate(lines) if re.match(r'\s*:\s*>\s*"\$RAN_MARKER"', ln)] - assert len(write_at) == 1, ( - f"expected exactly one `: > \"$RAN_MARKER\"` in the classify body, found {len(write_at)}") - prefix = "\n".join(lines[:write_at[0] + 1]) - guard = next(s for i, s in enumerate(steps) - if i != idx[0] and "h10-classifier-started" in (s.get("run") or "")) + assert len(write_at) == 1, f'expected exactly one `: > "$RAN_MARKER"` in the classify body, found {len(write_at)}' + prefix = "\n".join(lines[: write_at[0] + 1]) + guard = next(s for i, s in enumerate(steps) if i != idx[0] and "h10-classifier-started" in (s.get("run") or "")) return prefix, guard["run"] @@ -2611,54 +2815,63 @@ def test_the_dropped_step_guard_BEHAVIOURALLY_fails_without_the_marker_and_passe """ prologue, guard = _marker_steps() assert prologue.count("RAN_MARKER=") == 1 and ': > "$RAN_MARKER"' in prologue, ( - f"could not extract the classify step's marker prologue; got {prologue!r}") + f"could not extract the classify step's marker prologue; got {prologue!r}" + ) # The prefix must be the REAL top of the body, so a write hidden in an uncalled function is not # executed by this test either. assert prologue.lstrip().startswith("set -euo pipefail"), ( "the extracted prefix does not start at the top of the classify body, so it is a " - f"reconstruction rather than the production path: {prologue[:120]!r}") - env = {"PATH": os.environ["PATH"], "RUNNER_TEMP": str(tmp_path), - "GITHUB_RUN_ID": "424242", "GITHUB_RUN_ATTEMPT": "7"} + f"reconstruction rather than the production path: {prologue[:120]!r}" + ) + env = { + "PATH": os.environ["PATH"], + "RUNNER_TEMP": str(tmp_path), + "GITHUB_RUN_ID": "424242", + "GITHUB_RUN_ATTEMPT": "7", + } # A — the step was DROPPED: no marker exists. The job must fail. a = subprocess.run(["bash", "-c", guard], env=env, capture_output=True, text=True) assert a.returncode != 0, ( "the guard exited 0 with NO start marker present — a dropped classify step would go green " - f"again, which is the whole defect (ersatztv#751).\nstdout: {a.stdout}\nstderr: {a.stderr}") + f"again, which is the whole defect (ersatztv#751).\nstdout: {a.stdout}\nstderr: {a.stderr}" + ) assert "did not execute" in (a.stdout + a.stderr), ( - f"the guard failed but without an actionable message: {a.stdout!r} {a.stderr!r}") + f"the guard failed but without an actionable message: {a.stdout!r} {a.stderr!r}" + ) assert not list(tmp_path.glob("h10-classifier-started*")), ( - "the guard itself created the marker it is supposed to be checking for") + "the guard itself created the marker it is supposed to be checking for" + ) # B — the classifier RAN: its own prologue created the marker. The guard must pass. - b = subprocess.run(["bash", "-c", prologue + "\n" + guard], env=env, - capture_output=True, text=True) + b = subprocess.run(["bash", "-c", prologue + "\n" + guard], env=env, capture_output=True, text=True) assert b.returncode == 0, ( "the guard rejected a marker written by the classify step's OWN prologue — the two steps " f"disagree on the path, so this guard would fail on every run.\nstdout: {b.stdout}\n" - f"stderr: {b.stderr}") + f"stderr: {b.stderr}" + ) # C — the write must be reached by STRAIGHT-LINE code. Guarding the prefix trick itself: if the # write were wrapped in a function or an `if`, the prefix would still contain it but production # might not reach it. Executing the prefix with the function/conditional intact is the test; this # assertion makes the intent explicit and fails loudly rather than subtly. - body_before = "\n".join(ln for ln in prologue.splitlines() - if not ln.lstrip().startswith("#")) + body_before = "\n".join(ln for ln in prologue.splitlines() if not ln.lstrip().startswith("#")) # Covers all four bash spellings: `mk() {`, `mk(){`, `function mk {`, `function mk() {`. The third # was added when review found the first regex missed it, and the FOURTH still slipped that fix — # the union form is the natural next spelling once `function mk {` is caught. Budget three rounds # for any string-matching predicate. assert not re.search(r"^\s*(function\s+)?\w+\s*(\(\s*\))?\s*\{", body_before, re.M), ( "a function is defined before the marker write, so the write may be inside it and unreached " - f"in production while this test still passes:\n{body_before}") + f"in production while this test still passes:\n{body_before}" + ) written = [q.name for q in tmp_path.glob("h10-classifier-started*")] assert written == ["h10-classifier-started-424242-7"], ( f"the marker is not keyed on the run id/attempt as intended; found {written}. A fixed name in " - "a shared RUNNER_TEMP lets a stale marker satisfy this guard on a run whose step was dropped") + "a shared RUNNER_TEMP lets a stale marker satisfy this guard on a run whose step was dropped" + ) -@pytest.mark.parametrize("shape", ["null", "array"], - ids=["statuses-null", "statuses-empty-array"]) +@pytest.mark.parametrize("shape", ["null", "array"], ids=["statuses-null", "statuses-empty-array"]) def test_a_head_with_NO_statuses_YET_is_readable_and_still_gets_its_exemption(tmp_path, shape): """The twin of the timeline terminator, found by cold review of the fix for that one (#751). @@ -2675,10 +2888,11 @@ def test_a_head_with_NO_statuses_YET_is_readable_and_still_gets_its_exemption(tm posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), status_empty_shape=shape) assert r.returncode == 0, ( f"the classifier died reading a `statuses: {shape}` body instead of treating it as " - f"'no verdict yet', so nothing was posted at all.\n{r.stdout[-1000:]}\n{r.stderr[-600:]}") + f"'no verdict yet', so nothing was posted at all.\n{r.stdout[-1000:]}\n{r.stderr[-600:]}" + ) assert posted is not None, ( - f"a docs-only PR whose head has no statuses yet (shape: {shape}) got NO status at " - f"all.\n{r.stdout[-1000:]}") + f"a docs-only PR whose head has no statuses yet (shape: {shape}) got NO status at all.\n{r.stdout[-1000:]}" + ) assert posted["state"] == "success", f"expected the docs-only exemption, got {posted}" @@ -2701,10 +2915,12 @@ def test_the_GOVERNANCE_docs_are_protected_and_get_no_docs_only_exemption(tmp_pa assert posted is not None, f"the job posted nothing: {r.stderr[-1500:]}" assert posted["state"] == "pending", ( f"{path} was granted an exemption ({posted}) — the document that DEFINES the merge gate must " - f"not be able to exempt itself from it.\n{r.stdout[-800:]}") + f"not be able to exempt itself from it.\n{r.stdout[-800:]}" + ) assert "protected" in r.stdout.lower(), ( f"{path} was not exempted, but not via the protected-path branch either, so that guard may be " - f"dead for it.\n{r.stdout[-800:]}") + f"dead for it.\n{r.stdout[-800:]}" + ) def test_a_terminator_on_PAGE_ONE_does_not_certify_a_zero_retarget_count(tmp_path): @@ -2726,9 +2942,11 @@ def test_a_terminator_on_PAGE_ONE_does_not_certify_a_zero_retarget_count(tmp_pat assert r.returncode == 0, r.stderr assert posted is None, ( "an exemption `success` was posted from a timeline whose FIRST page was already the " - f"terminator, so no page of events was ever actually read: {posted}\n{r.stdout[-800:]}") + f"terminator, so no page of events was ever actually read: {posted}\n{r.stdout[-800:]}" + ) assert "trusted=no" in r.stdout, ( - f"the exemption was withheld, but not because the count was untrusted:\n{r.stdout[-800:]}") + f"the exemption was withheld, but not because the count was untrusted:\n{r.stdout[-800:]}" + ) def test_a_SECOND_PAGE_of_statuses_refuses_to_conclude_that_no_verdict_exists(tmp_path): @@ -2751,9 +2969,11 @@ def test_a_SECOND_PAGE_of_statuses_refuses_to_conclude_that_no_verdict_exists(tm posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), status_mode="twopage") assert posted is None, ( "an exemption was posted while the status list ran to a second page, so an existing verdict " - f"beyond page 1 would have been silently overwritten: {posted}\n{r.stdout[-800:]}") + f"beyond page 1 would have been silently overwritten: {posted}\n{r.stdout[-800:]}" + ) assert "page 2" in (r.stdout + r.stderr).lower(), ( - f"nothing was posted, but not because of the page-2 completeness probe:\n{r.stdout[-800:]}") + f"nothing was posted, but not because of the page-2 completeness probe:\n{r.stdout[-800:]}" + ) def test_a_SINGLE_page_of_statuses_reads_normally(tmp_path): @@ -2764,8 +2984,8 @@ def test_a_SINGLE_page_of_statuses_reads_normally(tmp_path): posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), status_mode="rows:40") assert r.returncode == 0, r.stderr assert posted is not None and posted["state"] == "success", ( - f"a single page of statuses should read normally and still exempt; got {posted}\n" - f"{r.stdout[-800:]}") + f"a single page of statuses should read normally and still exempt; got {posted}\n{r.stdout[-800:]}" + ) def test_a_STRING_total_count_is_not_accepted_as_numeric_zero(tmp_path): @@ -2779,13 +2999,18 @@ def test_a_STRING_total_count_is_not_accepted_as_numeric_zero(tmp_path): posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), status_mode="total-count-string") assert posted is None, ( f"a string total_count was accepted as numeric zero, so a body that merely lost its statuses " - f"array reads as 'no verdict exists': {posted}\n{r.stdout[-800:]}") + f"array reads as 'no verdict exists': {posted}\n{r.stdout[-800:]}" + ) -@pytest.mark.parametrize("mode,why", [ - ("page2-garbage", "a non-JSON page 2"), - ("page2-error", "an HTTP error on page 2"), -], ids=["garbage", "transport-error"]) +@pytest.mark.parametrize( + "mode,why", + [ + ("page2-garbage", "a non-JSON page 2"), + ("page2-error", "an HTTP error on page 2"), + ], + ids=["garbage", "transport-error"], +) def test_an_UNREADABLE_page_2_refuses_to_conclude_that_no_verdict_exists(tmp_path, mode, why): """The two refuse branches of the completeness probe, which cold review found untested. @@ -2800,9 +3025,11 @@ def test_an_UNREADABLE_page_2_refuses_to_conclude_that_no_verdict_exists(tmp_pat posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), status_mode=mode) assert posted is None, ( f"an exemption was posted despite {why}, so 'no verdict exists' was concluded without " - f"evidence: {posted}\n{r.stdout[-800:]}") + f"evidence: {posted}\n{r.stdout[-800:]}" + ) assert "page 2" in (r.stdout + r.stderr).lower(), ( - f"nothing was posted, but not because of the page-2 probe:\n{r.stdout[-800:]}") + f"nothing was posted, but not because of the page-2 probe:\n{r.stdout[-800:]}" + ) def test_a_status_history_RUNNING_PAST_PAGE_1_repairs_rather_than_leaving_green(tmp_path): @@ -2823,17 +3050,24 @@ def test_a_status_history_RUNNING_PAST_PAGE_1_repairs_rather_than_leaving_green( seq = _posted_sequence(tmp_path) assert len(seq) == 2, ( "the exemption was posted and left standing even though the status history ran past page 1, so " - f"a raced verdict beyond it would be buried. Posts: {seq}\n{r.stdout[-900:]}") + f"a raced verdict beyond it would be buried. Posts: {seq}\n{r.stdout[-900:]}" + ) assert seq[0]["state"] == "success" and seq[1]["state"] == "pending", ( - f"expected an exemption then a repair to pending; got {seq}") + f"expected an exemption then a repair to pending; got {seq}" + ) assert seq[1]["description"] == "Human verdict raced this exemption write — re-post the verdict", ( - f"repaired, but not to the sentinel, so the fixed point is broken: {seq[1]}") + f"repaired, but not to the sentinel, so the fixed point is broken: {seq[1]}" + ) -@pytest.mark.parametrize("mode,why", [ - ("second-page-garbage", "a non-JSON page 2"), - ("second-page-error", "an HTTP error on page 2"), -], ids=["garbage", "transport-error"]) +@pytest.mark.parametrize( + "mode,why", + [ + ("second-page-garbage", "a non-JSON page 2"), + ("second-page-error", "an HTTP error on page 2"), + ], + ids=["garbage", "transport-error"], +) def test_an_UNREADABLE_history_page_2_also_repairs_rather_than_leaving_green(tmp_path, mode, why): """Found by mutating the branch and watching nothing go red — my own coverage gap, in the same class the review had just flagged twice. @@ -2847,6 +3081,8 @@ def test_an_UNREADABLE_history_page_2_also_repairs_rather_than_leaving_green(tmp seq = _posted_sequence(tmp_path) assert len(seq) == 2, ( f"the exemption was left standing despite {why} — a raced verdict beyond page 1 would be " - f"buried. Posts: {seq}\n{r.stdout[-900:]}") + f"buried. Posts: {seq}\n{r.stdout[-900:]}" + ) assert seq[0]["state"] == "success" and seq[1]["state"] == "pending", ( - f"expected an exemption then a repair to pending; got {seq}") + f"expected an exemption then a repair to pending; got {seq}" + ) diff --git a/scripts/tests/test_prove_fix.py b/scripts/tests/test_prove_fix.py index a29954631..bbd14ac74 100644 --- a/scripts/tests/test_prove_fix.py +++ b/scripts/tests/test_prove_fix.py @@ -35,14 +35,18 @@ PROVE_FIX = Path(os.environ.get("PROVE_FIX_PATH") or (REPO_ROOT / "scripts" / "p def _git(repo: Path, *args: str) -> str: return subprocess.run( ["git", "-C", str(repo), *args], - check=True, capture_output=True, text=True, + check=True, + capture_output=True, + text=True, ).stdout.strip() def _run(repo: Path, *args: str) -> subprocess.CompletedProcess[str]: return subprocess.run( ["bash", str(PROVE_FIX), "--repo", str(repo), *args], - capture_output=True, text=True, cwd=str(repo), + capture_output=True, + text=True, + cwd=str(repo), ) @@ -61,9 +65,7 @@ def fixrepo(tmp_path: Path) -> Path: # --- commit 1: the bug, plus a test that cannot see it (repo / "calc.py").write_text("def add(a, b):\n return a - b # bug\n") - (repo / "scripts" / "tests" / "test_unrelated.py").write_text( - "def test_unrelated():\n assert 1 + 1 == 2\n" - ) + (repo / "scripts" / "tests" / "test_unrelated.py").write_text("def test_unrelated():\n assert 1 + 1 == 2\n") _git(repo, "add", "-A") _git(repo, "commit", "-q", "-m", "initial: buggy add, unrelated test") @@ -73,8 +75,7 @@ def fixrepo(tmp_path: Path) -> Path: "from calc import add\n\n\ndef test_add():\n assert add(2, 3) == 5\n" ) _git(repo, "add", "-A") - _git(repo, "commit", "-q", "-m", - "fix: add() returned a difference\n\nProves: scripts/tests/test_add.py") + _git(repo, "commit", "-q", "-m", "fix: add() returned a difference\n\nProves: scripts/tests/test_add.py") return repo @@ -143,7 +144,8 @@ def test_added_code_file_is_removed_not_checked_out(tmp_path: Path) -> None: _git(repo, "config", "user.email", "t@example.com") _git(repo, "config", "user.name", "T") (repo / "scripts" / "tests" / "keep.py").write_text("# placeholder\n") - _git(repo, "add", "-A"); _git(repo, "commit", "-q", "-m", "initial") + _git(repo, "add", "-A") + _git(repo, "commit", "-q", "-m", "initial") # the fix ADDS helper.py (it has no parent version) and a test that needs it (repo / "helper.py").write_text("def shout(s):\n return s.upper()\n") @@ -165,7 +167,8 @@ def test_root_commit_REFUSES(tmp_path: Path) -> None: _git(repo, "config", "user.email", "t@example.com") _git(repo, "config", "user.name", "T") (repo / "a.py").write_text("x = 1\n") - _git(repo, "add", "-A"); _git(repo, "commit", "-q", "-m", "root") + _git(repo, "add", "-A") + _git(repo, "commit", "-q", "-m", "root") r = _run(repo, "HEAD", "scripts/tests") assert r.returncode == 5 assert "root commit" in r.stderr @@ -185,9 +188,10 @@ def test_runs_under_the_system_bash(fixrepo: Path) -> None: # developer Mac. The 3.2 hazard (mapfile yielding an empty array) is what motivated it. ver = subprocess.run([str(system_bash), "--version"], capture_output=True, text=True).stdout r = subprocess.run( - [str(system_bash), str(PROVE_FIX), "--repo", str(fixrepo), "HEAD", - "scripts/tests/test_add.py"], - capture_output=True, text=True, cwd=str(fixrepo), + [str(system_bash), str(PROVE_FIX), "--repo", str(fixrepo), "HEAD", "scripts/tests/test_add.py"], + capture_output=True, + text=True, + cwd=str(fixrepo), ) assert r.returncode == 0, ( f"prove-fix.sh must work under the system bash ({ver.splitlines()[0] if ver else '?'}); " @@ -204,8 +208,7 @@ def test_control_failure_REFUSES(fixrepo: Path) -> None: ) (fixrepo / "calc.py").write_text("def add(a, b):\n return a + b # unchanged\n") _git(fixrepo, "add", "-A") - _git(fixrepo, "commit", "-q", "-m", - "fix: with an already-failing test\n\nProves: scripts/tests/test_broken.py") + _git(fixrepo, "commit", "-q", "-m", "fix: with an already-failing test\n\nProves: scripts/tests/test_broken.py") r = _run(fixrepo, "HEAD") assert r.returncode == 6, f"expected 6 (control failed), got {r.returncode}\n{r.stdout}\n{r.stderr}" assert "control FAILED" in r.stderr @@ -248,9 +251,19 @@ def test_MUTATION_disarming_the_UNPROVEN_clause_reddens_the_refusal_test( env = {**os.environ, "PROVE_FIX_PATH": str(mutant), "PROVE_FIX_MUTATION_RUN": "1"} nested = subprocess.run( - ["python3", "-m", "pytest", f"{Path(__file__).name}::test_unrelated_test_is_UNPROVEN", - "-q", "-p", "no:cacheprovider"], - cwd=str(Path(__file__).parent), env=env, capture_output=True, text=True, + [ + "python3", + "-m", + "pytest", + f"{Path(__file__).name}::test_unrelated_test_is_UNPROVEN", + "-q", + "-p", + "no:cacheprovider", + ], + cwd=str(Path(__file__).parent), + env=env, + capture_output=True, + text=True, ) out = nested.stdout + nested.stderr assert nested.returncode == 1, ( @@ -282,17 +295,20 @@ def test_SIGTERM_mid_run_never_reports_PROVEN(tmp_path: Path) -> None: _git(repo, "config", "user.name", "T") (repo / "mod.py").write_text("VALUE = 1\n") (repo / "scripts" / "tests" / "test_slow.py").write_text( - "import time\nfrom mod import VALUE\n\n\n" - "def test_slow():\n time.sleep(20)\n assert VALUE == 2\n" + "import time\nfrom mod import VALUE\n\n\ndef test_slow():\n time.sleep(20)\n assert VALUE == 2\n" ) - _git(repo, "add", "-A"); _git(repo, "commit", "-q", "-m", "initial") + _git(repo, "add", "-A") + _git(repo, "commit", "-q", "-m", "initial") (repo / "mod.py").write_text("VALUE = 2\n") _git(repo, "add", "-A") _git(repo, "commit", "-q", "-m", "fix: bump\n\nProves: scripts/tests/test_slow.py") proc = subprocess.Popen( ["bash", str(PROVE_FIX), "--repo", str(repo), "HEAD"], - stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, cwd=str(repo), + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + cwd=str(repo), ) time.sleep(4) # inside the control run, which sleeps 20s assert proc.poll() is None, "the run finished before it could be signalled; test is void" @@ -305,9 +321,7 @@ def test_SIGTERM_mid_run_never_reports_PROVEN(tmp_path: Path) -> None: # bugs landing on the same observable is not a witnessed fix. Cold review measured that # pair failing to separate old from new; rc==5 AND the handler's own message do separate # them. - assert proc.returncode == 5, ( - f"a signalled run must exit 5 from on_signal, got {proc.returncode}\n{out}\n{err}" - ) + assert proc.returncode == 5, f"a signalled run must exit 5 from on_signal, got {proc.returncode}\n{out}\n{err}" assert "interrupted by signal" in err, ( f"expected the signal handler's own message, so this test cannot be satisfied by an " f"unrelated later failure:\n{err}" @@ -335,7 +349,8 @@ def test_a_harness_failure_is_NOT_reported_as_PROVEN(tmp_path: Path) -> None: _git(repo, "config", "user.email", "t@example.com") _git(repo, "config", "user.name", "T") (repo / "scripts" / "tests" / "keep.py").write_text("# placeholder\n") - _git(repo, "add", "-A"); _git(repo, "commit", "-q", "-m", "initial") + _git(repo, "add", "-A") + _git(repo, "commit", "-q", "-m", "initial") (repo / "added.py").write_text("def val():\n return 7\n") (repo / "scripts" / "tests" / "test_added.py").write_text( "from added import val\n\n\ndef test_val():\n assert val() == 7\n" @@ -343,7 +358,8 @@ def test_a_harness_failure_is_NOT_reported_as_PROVEN(tmp_path: Path) -> None: _git(repo, "add", "-A") _git(repo, "commit", "-q", "-m", "feat: add val\n\nProves: scripts/tests/test_added.py") - shim = tmp_path / "bin"; shim.mkdir() + shim = tmp_path / "bin" + shim.mkdir() counter = tmp_path / "count" (shim / "git").write_text( "#!/bin/sh\n" @@ -358,7 +374,10 @@ def test_a_harness_failure_is_NOT_reported_as_PROVEN(tmp_path: Path) -> None: env = {**os.environ, "PATH": f"{shim}:{os.environ['PATH']}"} r = subprocess.run( ["bash", str(PROVE_FIX), "--repo", str(repo), "HEAD"], - capture_output=True, text=True, cwd=str(repo), env=env, + capture_output=True, + text=True, + cwd=str(repo), + env=env, ) assert "PROVEN" not in r.stdout, ( "a phase whose worktree does not exist cannot witness anything; pre-fix this " diff --git a/scripts/tests/test_remote_state_inventory.py b/scripts/tests/test_remote_state_inventory.py index 7270a1537..b65a363e6 100644 --- a/scripts/tests/test_remote_state_inventory.py +++ b/scripts/tests/test_remote_state_inventory.py @@ -109,7 +109,8 @@ def _tracked_files() -> list[str]: """ proc = subprocess.run( ["git", "-C", str(REPO_ROOT), "ls-files", "-z"], - capture_output=True, check=True, + capture_output=True, + check=True, ) return [p for p in proc.stdout.decode().split("\0") if p] @@ -150,7 +151,8 @@ def _inventory_section(text: str) -> str: raise AssertionError( f"{INVENTORY.name} has no {heading!r} heading. Row parsing is bounded by " "'## The inventory' and '## Limits'; renaming or reordering either one would " - "silently change which rows are checked, so it fails here instead.") + "silently change which rows are checked, so it fails here instead." + ) start = text.index("## The inventory") try: end = text.index("## Limits", start) @@ -161,7 +163,8 @@ def _inventory_section(text: str) -> str: # either returns an index >= start or raises. A guard that cannot execute proves nothing. raise AssertionError( f"{INVENTORY.name}: '## Limits' precedes '## The inventory', so the parsed window " - "would be empty and every completeness assertion would pass vacuously.") from None + "would be empty and every completeness assertion would pass vacuously." + ) from None return text[start:end] @@ -188,10 +191,11 @@ def test_anti_vacuity_the_derivation_and_the_table_both_found_something(): sites = inventory_sites() assert len(population) >= 40, ( f"derived only {len(population)} in-scope files — the globs are broken, not the repo " - "(the scope held 59 files on 2026-08-16, and it only grows)") + "(the scope held 59 files on 2026-08-16, and it only grows)" + ) assert len(sites) >= 40, ( - f"parsed only {len(sites)} rows out of the inventory — the row regex has drifted from the " - "table format") + f"parsed only {len(sites)} rows out of the inventory — the row regex has drifted from the table format" + ) def test_every_in_scope_file_has_a_row_and_every_row_names_a_real_file(): @@ -208,9 +212,9 @@ def test_every_in_scope_file_has_a_row_and_every_row_names_a_real_file(): phantom = sorted(sites - population) assert not missing, ( "in scope but absent from docs/remote-state-inventory.md (classify each as " - f"PINNED / CAS / UNSAFE-KNOWN / N/A): {missing}") - assert not phantom, ( - f"listed in docs/remote-state-inventory.md but no such in-scope file exists: {phantom}") + f"PINNED / CAS / UNSAFE-KNOWN / N/A): {missing}" + ) + assert not phantom, f"listed in docs/remote-state-inventory.md but no such in-scope file exists: {phantom}" def test_MUTATION_PROOF_a_dropped_row_and_a_phantom_row_are_both_detected(): @@ -228,27 +232,28 @@ def test_MUTATION_PROOF_a_dropped_row_and_a_phantom_row_are_both_detected(): population = derived_population() victim = sorted(population)[0] - dropped = "\n".join( - line for line in text.splitlines() if not line.startswith(f"| `{victim}`")) + dropped = "\n".join(line for line in text.splitlines() if not line.startswith(f"| `{victim}`")) assert victim not in inventory_sites(dropped), ( - f"the mutation did not actually remove {victim}; the proof below would be vacuous") + f"the mutation did not actually remove {victim}; the proof below would be vacuous" + ) assert population - inventory_sites(dropped), ( - "a row was removed from the inventory and the comparison still reported complete coverage") + "a row was removed from the inventory and the comparison still reported complete coverage" + ) # Inserted INSIDE the inventory section, not appended to the file: rows are parsed only between # "## The inventory" and "## Limits", so appending at the end would test nothing. - phantom = text.replace( - "## Limits", - "| `scripts/does-not-exist.sh` — invented | `PINNED` | n/a |\n\n## Limits", 1) + phantom = text.replace("## Limits", "| `scripts/does-not-exist.sh` — invented | `PINNED` | n/a |\n\n## Limits", 1) assert inventory_sites(phantom) - population == {"scripts/does-not-exist.sh"}, ( - "a row naming a file that does not exist was not reported as phantom") + "a row naming a file that does not exist was not reported as phantom" + ) def test_every_class_cell_comes_from_the_closed_vocabulary(): bad = sorted({cls for _, cls in inventory_rows() if cls not in CLASSES}) assert not bad, ( f"unknown classification(s) {bad}; allowed: {sorted(CLASSES)}. A typo here would silently " - "create a state nobody reviews.") + "create a state nobody reviews." + ) def test_every_unsafe_row_states_why_the_residual_is_accepted(): @@ -268,8 +273,7 @@ def test_every_unsafe_row_states_why_the_residual_is_accepted(): site, note = cells[0], cells[-1] if len(note) < 120: thin.append(site[:60]) - assert not thin, ( - f"UNSAFE-KNOWN row(s) with no stated justification: {thin}") + assert not thin, f"UNSAFE-KNOWN row(s) with no stated justification: {thin}" def test_the_population_never_includes_a_file_git_does_not_track(monkeypatch): @@ -296,14 +300,15 @@ def test_the_population_never_includes_a_file_git_does_not_track(monkeypatch): ) # Patch the module object this test is running inside, whatever name it was imported under. import sys + mod = sys.modules[__name__] monkeypatch.setattr(mod, "_tracked_files", lambda: tracked) - assert (REPO_ROOT / victim).is_file(), ( - f"{victim} must still exist on disk for this proof to mean anything") + assert (REPO_ROOT / victim).is_file(), f"{victim} must still exist on disk for this proof to mean anything" assert victim not in derived_population(), ( f"{victim} is on disk and matches the scope, but git no longer tracks it — it must not enter " - "the population, or untracked build output can redden this guard again") + "the population, or untracked build output can redden this guard again" + ) def test_every_derived_member_is_tracked():