test(785): mutation proofs for the three unproven ranked guards (#810)
Build ErsatzTV Image / Delimiter ban (release path) (push) Successful in 26s
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 8m48s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 6m14s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 6m17s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Skipped
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 4m10s
Build ErsatzTV Image / Delimiter ban (release path) (push) Successful in 26s
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 8m48s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 6m14s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 6m17s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Skipped
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 4m10s
Closes the remaining three entries on #785's ranked list with clause-level mutation proofs, each witnessed red against the real subject in place: * the `pretooluse-worktree-guard.sh` + `posttooluse-worktree-marker.sh` PAIR — four clauses, including the cross-file seam (a clause in the marker hook, asserted against the guard's decision) that could not exist while the halves were tested apart; * `.husky/pre-push:11`'s `unset GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE` — git exports `GIT_DIR` to `pre-push` only from a worktree, which `process.shared-tree-readonly` makes the mandated way to work here, so the guarded case is the normal one; * `scripts/build_decisions_catalog.py --check` — including the `__main__` wiring, which can print "is stale" on stderr and still exit 0. Nine ways the catalog guard can stop gating are detected, judged by executing the step's whole `run` script rather than by matching lines out of it. Two channels are undecidable outside the runner and are stated as uncovered rather than guessed at. Inventory regraded to 12 MUTATION / 6 BEHAVIOUR-ONLY / 16 NONE, with a stated reason for every remaining NONE row, verified member-for-member against the derived set. Five cold review rounds; findings closed include production-hook-fire-log corruption, a tautological assertion, a guard asserting on its helper rather than on the effect, and two false greens in the workflow extractor. Follow-up: #809. fixes #785 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>
This commit was merged in pull request #810.
This commit is contained in:
Claude Opus 5
+122
-30
@@ -75,7 +75,7 @@ all.
|
||||
| --- | --- | --- | --- | --- |
|
||||
| `.claude/hooks/decisions-guard.sh` | a commit | GUARD | NONE | — |
|
||||
| `.claude/hooks/design-sync-reminder.sh` | the first Stop after a UI change (one-shot, then allows) | GUARD | NONE | — |
|
||||
| `.claude/hooks/posttooluse-worktree-marker.sh` | nothing (writes the marker the worktree guard reads) | GUARD | NONE | — |
|
||||
| `.claude/hooks/posttooluse-worktree-marker.sh` | nothing (writes the marker the worktree guard reads) | GUARD | MUTATION | `test_worktree_ownership_guard.py::test_MUTATION_a_marker_hook_that_stops_WRITING_makes_the_guard_go_quiet` |
|
||||
| `.claude/hooks/prepush-clean-worktree-check.sh` | a push with uncommitted changes in the pushed set | GUARD | NONE | — |
|
||||
| `.claude/hooks/prepush-donewhen.sh` | a direct push to `main` with unticked Done-when boxes | GUARD | NONE | — |
|
||||
| `.claude/hooks/prepush-rebase-check.sh` | a push from a branch behind `origin/main` | GUARD | BEHAVIOUR-ONLY | `test_prepush_rebase_check_tag_exemption.py::test_zero_ref_lines_does_not_exempt` |
|
||||
@@ -85,11 +85,11 @@ all.
|
||||
| `.claude/hooks/pretooluse-bom-guard.sh` | a commit/push carrying a BOM in a touched `.cs` | GUARD | MUTATION | `test_bom_guard_detection.py::test_DISARMING_the_BOM_comparison_stops_detection` |
|
||||
| `.claude/hooks/pretooluse-merge-consent.sh` | a PR merge without derived consent | GUARD | BEHAVIOUR-ONLY | `test_merge_consent_exemption.py::test_protected_path_on_a_LATER_page_is_still_seen` |
|
||||
| `.claude/hooks/pretooluse-nav-guard.sh` | a browser navigate to a streaming URL | GUARD | NONE | — |
|
||||
| `.claude/hooks/pretooluse-worktree-guard.sh` | a commit/merge in a foreign worktree | GUARD | NONE | — |
|
||||
| `.claude/hooks/pretooluse-worktree-guard.sh` | a commit/merge in a foreign worktree | GUARD | MUTATION | `test_worktree_ownership_guard.py::test_MUTATION_disarming_the_guards_MARKER_READ_stops_the_deny` |
|
||||
| `.husky/commit-msg` | a commit with no `Co-Authored-By` trailer | GUARD | NONE | — |
|
||||
| `.husky/pre-commit` | a commit failing lint-staged, decisions, root-PNG or format | GUARD | NONE | — |
|
||||
| `.husky/pre-push` | a push failing any pre-push hook or the SPA gate | GUARD | NONE | — |
|
||||
| `scripts/build_decisions_catalog.py` | the `decisions-guard` job, on a stale catalog | GUARD | NONE | — |
|
||||
| `.husky/pre-push` | a push failing any pre-push hook or the SPA gate | GUARD | MUTATION | `test_prepush_unsets_git_env.py::test_MUTATION_DELETING_the_unset_lets_drift_through_silently` |
|
||||
| `scripts/build_decisions_catalog.py` | the `decisions-guard` job, on a stale catalog | GUARD | MUTATION | `test_build_catalog_check_path.py::test_MUTATION_disarming_the_stale_comparison_stops_detection` |
|
||||
| `scripts/check-kickoff-guard.sh` | the `decisions-guard` job, on a revived #237 reference | GUARD | NONE | — |
|
||||
| `scripts/check-review-verdict.sh` | the merge-consent hook's verdict classification | GUARD | BEHAVIOUR-ONLY | `test_check_review_verdict.py::test_falseopen_token_must_be_a_whole_word` |
|
||||
| `scripts/ci-detect-already-validated.sh` | nothing directly (feeds the skip gate) | GUARD | NONE | — |
|
||||
@@ -109,6 +109,7 @@ all.
|
||||
| `scripts/update-openapi.sh` | nothing (regenerates the spec) | TOOLING | NONE | — |
|
||||
| `scripts/tests/test_bom_guard_detection.py` | the `script-tests` job | PROOF | NONE | — |
|
||||
| `scripts/tests/test_build_catalog.py` | the `script-tests` job | PROOF | NONE | — |
|
||||
| `scripts/tests/test_build_catalog_check_path.py` | the `script-tests` job | PROOF | NONE | — |
|
||||
| `scripts/tests/test_check_review_verdict.py` | the `script-tests` job | PROOF | NONE | — |
|
||||
| `scripts/tests/test_ci_dropped_step_guard.py` | the `script-tests` job | PROOF | NONE | — |
|
||||
| `scripts/tests/test_ci_image_pin_population.py` | the `script-tests` job, when a container job loses its pin | GUARD | MUTATION | `test_ci_image_pin_population.py::test_a_single_job_losing_its_pin_is_DETECTED` |
|
||||
@@ -123,15 +124,17 @@ all.
|
||||
| `scripts/tests/test_merge_consent_required_check.py` | the `script-tests` job | PROOF | NONE | — |
|
||||
| `scripts/tests/test_migration_equivalence.py` | the `script-tests` job | PROOF | NONE | — |
|
||||
| `scripts/tests/test_post_review_verdict.py` | the `script-tests` job | PROOF | NONE | — |
|
||||
| `scripts/tests/test_prepush_unsets_git_env.py` | the `script-tests` job | PROOF | NONE | — |
|
||||
| `scripts/tests/test_pr_changed_files.py` | the `script-tests` job | PROOF | NONE | — |
|
||||
| `scripts/tests/test_prepush_rebase_check_tag_exemption.py` | the `script-tests` job | PROOF | NONE | — |
|
||||
| `scripts/tests/test_prove_fix.py` | the `script-tests` job | PROOF | NONE | — |
|
||||
| `scripts/tests/test_remote_state_inventory.py` | the `script-tests` job, on an executable that talks to a remote service with no row in `docs/remote-state-inventory.md` | GUARD | MUTATION | `test_remote_state_inventory.py::test_MUTATION_PROOF_a_dropped_row_and_a_phantom_row_are_both_detected` |
|
||||
| `scripts/tests/test_worktree_ownership_guard.py` | the `script-tests` job | PROOF | NONE | — |
|
||||
|
||||
## What the numbers say
|
||||
|
||||
34 guards, 5 tooling scripts, 15 proof files. **8 guards carry a mutation proof; 6 are
|
||||
behaviour-only; 20 have none.** These figures are asserted against the table by
|
||||
34 guards, 5 tooling scripts, 18 proof files. **12 guards carry a mutation proof; 6 are
|
||||
behaviour-only; 16 have none.** These figures are asserted against the table by
|
||||
`test_the_summary_counts_match_the_table` — they were wrong in the first draft (28/4/6/3/19 against
|
||||
a table holding 27/5/6/3/18), because a hand-maintained summary of a table is a second copy of it,
|
||||
which is the duplication family this change argues against. Both cold reviewers found the error
|
||||
@@ -186,41 +189,130 @@ lines flagged wrongly — and cold review then constructed more of both (`[[ "$x
|
||||
a backslash continuation, a `>` inside a quoted string). Deleted rather than patched a fifth time,
|
||||
on the same reasoning as the vocabulary-parity withdrawal above.
|
||||
|
||||
**`test_hook_fire_log.py` asserts TWO clauses
|
||||
**`test_hook_fire_log.py` proves THREE clauses, and its row claims two of them.** Coverage (every
|
||||
hook reports that it fired) via `test_a_hook_that_LOSES_its_instrumentation_is_DETECTED`;
|
||||
transparency (the wrapper changes nothing the harness can see) via
|
||||
`test_instrumentation_changes_NOTHING_the_harness_can_see` with its mutation proof
|
||||
`test_DELETING_the_replay_makes_the_differential_go_RED`; and placement (`etv_hook_fire_begin` must
|
||||
precede the stdin read) via `test_begin_placed_AFTER_the_stdin_read_is_DETECTED`. The row's `Blocks`
|
||||
column covers the first two — "stops reporting that it fired, **or whose reporting changes what the
|
||||
harness sees**" — and the `Proof ref` column holds one ref because the column holds one, not because
|
||||
the second is unproven. Placement is proved and unclaimed.
|
||||
|
||||
**Its `test_the_suite_does_not_write_to_the_PRODUCTION_log` is narrower than its docstring**, which
|
||||
says `conftest.py` "must isolate every test, not just this file's". What it checks is that the
|
||||
fixture set `ETV_HOOK_FIRE_LOG_DIR` *for the test currently running* and that a hook it drives in
|
||||
its own sandbox does not touch the real log. It cannot see another suite that snapshots
|
||||
`os.environ` at IMPORT time — before the autouse fixture runs — and hands that stale mapping to its
|
||||
subprocesses. That suite's hooks then write to `$HOME/.cache/ersatztv/hook-fire/` while every
|
||||
assertion stays green, because the fire-log library is fail-open. #785 shipped exactly that defect,
|
||||
inside the file added to prove those very hooks.
|
||||
|
||||
The reproduction is the part worth keeping: restore a module-level `{**os.environ}` snapshot — or
|
||||
leave the helper correct and point a single `env=` argument at one — then run that file and count
|
||||
records for its synthetic session ids. **58 per run**, measured identically on macOS/git 2.55 and
|
||||
Linux/git 2.47.3. (The accumulated total observed before the fix ran to four figures across many
|
||||
runs; that is an observation rather than a reproducible measurement, so check the per-run figure.)
|
||||
|
||||
The pin is `test_worktree_ownership_guard.py::test_driving_a_hook_LANDS_its_records_in_the_ISOLATED_dir`,
|
||||
which asserts the EFFECT — records land in the fixture's dir — rather than the shape of the fix. Its
|
||||
predecessor asserted the helper's return value, and cold review showed that leaves the call site
|
||||
unguarded: `_env()` correct, one `env=` reverted, all tests green, records still leaking. Pinning to
|
||||
a hand-written revert rather than to the property is `verify-against-the-REAL-predecessor`.
|
||||
|
||||
It is still narrower than the property: it guards the launch path it drives. A second launcher in
|
||||
the same file that passed a stale environment would leave it green — measured, 18 records — because
|
||||
the hooks it drives would still log correctly. Every hook in that file goes through one helper
|
||||
today, which is what makes it sufficient there. Generalising it is #809, and the reason that is hard
|
||||
is that the obvious version races against a real session's hooks firing during the run.
|
||||
|
||||
**That unproven set carries no number here on purpose.** It restated the count, drifted the moment
|
||||
the BOM guard was regraded, and `test_the_summary_counts_match_the_table` cannot see it — the parser
|
||||
checks the formatted summary sentence and nothing else. A second hand-maintained copy of a number is
|
||||
the duplication family this file argues against, so the copy is removed rather than corrected: the
|
||||
set is every `PreToolUse` hook except merge-consent and the BOM guard, and the count is in the
|
||||
summary above.
|
||||
set is the eight `.claude/hooks/` guards enumerated in the standing list below, and the count is in
|
||||
the summary above. Naming them by event is what made the earlier wording wrong twice — they do not
|
||||
share one event, and `design-sync-reminder.sh` is registered on **both** `PreToolUse` and `Stop` in
|
||||
`.claude/settings.json`, so any "every X hook" phrasing double-counts it.
|
||||
|
||||
**They are now observable but still unproven, and the two words carry different weight.** Observable:
|
||||
every hook records its own execution through `scripts/hook-fire-log.sh`, so "did this hook fire, and
|
||||
what did it decide" is a measurement — run `scripts/hook-fire-log.sh report` (#776). Unproven:
|
||||
nobody has demonstrated any of them is load-bearing, which is what #785 tracks. Observability tells
|
||||
you a guard ran; only a mutation tells you it would have caught anything. The BOM guard is the case
|
||||
that shows why the distinction matters — it was firing on every commit the whole time it was
|
||||
fail-open.
|
||||
nobody has demonstrated any of them is load-bearing — the first group in the standing list below.
|
||||
Observability tells you a guard ran; only a mutation tells you it would have caught anything. The
|
||||
BOM guard is the case that shows why the distinction matters — it was firing on every commit the
|
||||
whole time it was fail-open.
|
||||
|
||||
The gaps are not uniform in cost, and the ranking that matters is *what a silent failure would let
|
||||
through*, not test count:
|
||||
through*, not test count. **All four ranked entries now carry clause-level mutation proofs (#785);
|
||||
they are kept here with what each mutation established, because the ranking is the reusable part and
|
||||
because two of them turned out to be worse than the ranking predicted.**
|
||||
|
||||
1. ~~`pretooluse-bom-guard.sh`~~ — **now proven, and it was fail-open the whole time.** Ranked
|
||||
first here because the defect it guards has recurred three times (#311, #402, #405); that
|
||||
ranking turned out to be right for a worse reason than intended. It detected a BOM with
|
||||
`xxd -p`, and `xxd` ships with vim and is **absent on the Linux CI runner**, so the comparison
|
||||
never matched and every BOM was allowed in silence. `od` now, with a clause-level mutation proof.
|
||||
The lesson for the rows below: an unproven guard is not merely untested, it is a guard whose
|
||||
*current* behaviour nobody has established.
|
||||
2. `pretooluse-worktree-guard.sh` + `posttooluse-worktree-marker.sh` — a two-file mechanism guarding
|
||||
#289, where a regression in either half is invisible and the two halves have never been tested
|
||||
together.
|
||||
3. `.husky/pre-push:11`'s `unset GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE` — a one-line fix for a real
|
||||
bug (a nested `git diff --exit-code` silently reporting no diff) that nothing pins. Reordering it
|
||||
after the nested git calls reintroduces the bug silently.
|
||||
4. `scripts/build_decisions_catalog.py` — its `--check` path is what CI runs and no test calls
|
||||
`main()` at all; the tests exercise `render_catalog()` directly.
|
||||
1. ~~`pretooluse-bom-guard.sh`~~ — **proven, and it was fail-open the whole time.** Ranked first
|
||||
because the defect it guards has recurred three times (#311, #402, #405); that ranking turned out
|
||||
to be right for a worse reason than intended. It detected a BOM with `xxd -p`, and `xxd` ships
|
||||
with vim and is **absent on the Linux CI runner**, so the comparison never matched and every BOM
|
||||
was allowed in silence. `od` now. The lesson for the rows below: an unproven guard is not merely
|
||||
untested, it is a guard whose *current* behaviour nobody has established.
|
||||
2. ~~`pretooluse-worktree-guard.sh` + `posttooluse-worktree-marker.sh`~~ — **proven as a pair.**
|
||||
Four clauses were disarmed and witnessed red: the guard's marker read; the guard's ownership
|
||||
comparison (inverted, because disarming it the other way only makes the guard deny more and every
|
||||
deny assertion stays green); the `commit|merge` alternation, whose `merge` half every other case
|
||||
in the file left untested while guarding the plumbing-merge path; and — the one that could not
|
||||
exist while the halves were tested apart — the *marker hook's write*, asserted against the
|
||||
*guard's* decision. Both hooks are
|
||||
deliberately fail-open, so an absent mechanism and a working one produce the identical "commit
|
||||
allowed".
|
||||
3. ~~`.husky/pre-push:11`'s `unset GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE`~~ — **proven, and the case
|
||||
is the normal one rather than an edge.** Git exports `GIT_DIR` to `pre-push` **when the push
|
||||
comes from a worktree** and not from the main tree; `process.shared-tree-readonly` makes the
|
||||
worktree the mandated way to work here, so every push takes the exposed path. With `GIT_DIR` set
|
||||
and `GIT_WORK_TREE` unset git stops discovering the repo and treats the *current directory* as
|
||||
the work tree, so `cd web && npm run check:api`'s `git diff --exit-code` compares against index
|
||||
paths that do not exist and reports no diff. Both the deletion and the relocation are proved.
|
||||
4. ~~`scripts/build_decisions_catalog.py`~~ — **proven, including the wiring.** The `--check`
|
||||
comparison is mutated directly, and a separate subprocess case runs the command *derived from
|
||||
`pr-checks.yml`* against a copied corpus. That second case is not redundant: replacing
|
||||
`raise SystemExit(main())` with a bare `main()` leaves the script printing
|
||||
`docs/decisions/README.md is stale` on stderr while exiting **0**, and the workflow step reads
|
||||
nothing but the exit code. Only the subprocess case reddens — the #751/#719 shape.
|
||||
|
||||
Filling these is tracked rather than done in one pass, deliberately: a mutation proof written to
|
||||
**What a file-level `MUTATION` grade does and does not claim, because three of these four rows are
|
||||
multi-purpose files.** The grade covers *the clause the cited case actually mutates*, per
|
||||
`testing.guard-ships-with-mutation-proof`; it is not a statement about every line in the file.
|
||||
`.husky/pre-push` runs four other things, and its row asserts only that line 11 is load-bearing —
|
||||
the three hooks it invokes carry their own rows and their own grades. Reading the row as "pre-push
|
||||
is tested" is the same relabelling error the withdrawn-guard table above exists to prevent.
|
||||
Clause-level grading is tracked in #790.
|
||||
|
||||
**The remaining unproven guards, and why each is still `NONE` rather than merely unattended** — the
|
||||
third `## Done-when` box of #785, answered here rather than left implicit. They fall into three
|
||||
groups, and the split is the point: the first two are backlogs, the third is not.
|
||||
|
||||
- **The eight remaining hook guards** — genuinely unproven, and a real backlog. Four are
|
||||
`PreToolUse` (`pretooluse-agent-model.sh`, `pretooluse-agent-ram.sh`, `pretooluse-bash-guard.sh`,
|
||||
`pretooluse-nav-guard.sh`), two are pre-push (`prepush-clean-worktree-check.sh`,
|
||||
`prepush-donewhen.sh`), one is pre-commit (`decisions-guard.sh`) and one is registered on both
|
||||
`PreToolUse` and `Stop` (`design-sync-reminder.sh`); they are grouped by their status, not by
|
||||
their event, because the events do not partition them. They are now
|
||||
*observable* (`scripts/hook-fire-log.sh report`, #776), which is a weaker claim than proven and is
|
||||
stated as such above.
|
||||
- **`.husky/commit-msg` and `.husky/pre-commit`** — unproven, and each carries its own clause, so
|
||||
they are a backlog too rather than "covered by what they call". `pre-commit` dispatches to
|
||||
`lint-staged` and `decisions-guard.sh`, but the root-level-`*.png` refusal and the
|
||||
`dotnet format whitespace --verify-no-changes` block are its own. `commit-msg` is not a dispatcher
|
||||
at all: it is one `grep -q '^Co-Authored-By:'` plus a `MERGE_HEAD` exemption, and nothing tests
|
||||
that the exemption fires only for merges. Neither is covered by what it calls — the trap is to read
|
||||
"it dispatches to guards" as "its own clauses are guarded".
|
||||
- **`ci-detect-already-validated.sh`, `ci-detect-docs-only.sh`, `ci-prove-ban-detects.sh`,
|
||||
`e2e-functional.sh`, `check-kickoff-guard.sh`, `test_ci_release_path_scan_job.py`** — not
|
||||
unattended either, but each needs its own harness rather than a proof written to clear a row.
|
||||
`ci-prove-ban-detects.sh` is the exception noted in `testing.guard-ships-with-mutation-proof`: it
|
||||
runs its own mutation at CI time, because what it proves is disarmable from inside pytest.
|
||||
|
||||
That list is prose and nothing checks it, which is the honest limit — a guard moving out of a group
|
||||
will not redden anything. It is here so the next session inherits *why* a row is `NONE`, which is
|
||||
the distinction #785 asked for; the machine-checked half remains the table.
|
||||
|
||||
Filling the rest is tracked rather than done in one pass, deliberately: a mutation proof written to
|
||||
close a row is the kind of test that passes for the wrong reason.
|
||||
|
||||
@@ -0,0 +1,546 @@
|
||||
"""`scripts/build_decisions_catalog.py`'s `--check` path — the one CI actually runs.
|
||||
|
||||
`decisions-guard` (`.gitea/workflows/pr-checks.yml`, the "Active catalog in sync" step) runs
|
||||
`python3 scripts/build_decisions_catalog.py --check`. `scripts/tests/test_build_catalog.py` covers
|
||||
`render_catalog()` directly and never calls `main()` at all, so nothing there proves:
|
||||
|
||||
* that `main()`'s stale-detection comparison (`want.strip() != have.strip()`) is load-bearing —
|
||||
a version that always agreed would pass every existing test;
|
||||
* that the `if __name__ == "__main__": raise SystemExit(main())` wiring actually turns a stale
|
||||
catalog into a non-zero process exit code, which is the only thing CI's `run:` step reads.
|
||||
|
||||
`testing.guard-ships-with-mutation-proof` (#775) is explicit that a guard is not tested because a
|
||||
test *involving* it passes: it ships with a mutation proof — disarm the guard's own clause, alone,
|
||||
and a named test must go red. This file is that proof for the catalog-guard, plus the subprocess
|
||||
proof that the `__main__` wiring is connected (the #751/#719 shape the decision record names: a
|
||||
green `main()` behind dead wiring).
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import importlib.util
|
||||
import re
|
||||
import subprocess
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
import yaml
|
||||
|
||||
import scripts.build_decisions_catalog as bc
|
||||
import scripts.decisions_lib as dl
|
||||
|
||||
REPO_ROOT = Path(__file__).resolve().parents[2]
|
||||
CATALOG_SCRIPT = REPO_ROOT / "scripts" / "build_decisions_catalog.py"
|
||||
DECISIONS_LIB = REPO_ROOT / "scripts" / "decisions_lib.py"
|
||||
SCRIPTS_INIT = REPO_ROOT / "scripts" / "__init__.py"
|
||||
|
||||
|
||||
def _current_catalog_text() -> str:
|
||||
return bc.render_catalog(dl.all_active_records())
|
||||
|
||||
|
||||
# ------------------------------------------------------------------------------------------------
|
||||
# ANTI-VACUITY — if the real corpus is empty, every assertion below passes for nothing.
|
||||
# ------------------------------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_the_real_corpus_is_non_empty_and_renders_a_real_catalog():
|
||||
records = dl.all_active_records()
|
||||
active = [r for r in records if r.status == "active" and r.key]
|
||||
assert active, "no active decision records were parsed — every test below would be vacuous"
|
||||
|
||||
text = _current_catalog_text()
|
||||
assert bc.BANNER in text, "render_catalog produced no banner — not a real catalog document"
|
||||
assert f"`{active[0].key}`" in text, (
|
||||
"render_catalog produced no row for a known active record — not a real catalog document"
|
||||
)
|
||||
|
||||
|
||||
# ------------------------------------------------------------------------------------------------
|
||||
# `main(["--check"])` — the comparison CI reads
|
||||
# ------------------------------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_check_returns_0_when_OUTPUT_matches_render_catalog(tmp_path, monkeypatch):
|
||||
fresh = _current_catalog_text()
|
||||
output = tmp_path / "README.md"
|
||||
output.write_text(fresh.rstrip("\n") + "\n", encoding="utf-8")
|
||||
monkeypatch.setattr(bc, "OUTPUT", output)
|
||||
|
||||
assert bc.main(["--check"]) == 0
|
||||
|
||||
|
||||
def test_check_returns_1_when_OUTPUT_is_stale(tmp_path, monkeypatch):
|
||||
fresh = _current_catalog_text()
|
||||
output = tmp_path / "README.md"
|
||||
# Append a line: the on-disk file no longer matches what render_catalog would produce.
|
||||
output.write_text(fresh.rstrip("\n") + "\nEXTRA STALE LINE\n", encoding="utf-8")
|
||||
monkeypatch.setattr(bc, "OUTPUT", output)
|
||||
|
||||
assert bc.main(["--check"]) == 1
|
||||
|
||||
|
||||
def test_generate_then_check_round_trips(tmp_path, monkeypatch):
|
||||
"""The no-argument path WRITES the catalog, and a subsequent --check must then pass.
|
||||
|
||||
This pins the property CI depends on: generate and check agree. If they ever diverged, `main([])`
|
||||
would produce a file that `main(["--check"])` immediately rejects — a self-contradiction that
|
||||
would make the generator useless for fixing the exact problem `--check` reports.
|
||||
"""
|
||||
output = tmp_path / "README.md"
|
||||
assert not output.exists()
|
||||
monkeypatch.setattr(bc, "OUTPUT", output)
|
||||
|
||||
assert bc.main([]) == 0
|
||||
assert output.exists(), "main([]) with no --check must write OUTPUT"
|
||||
written = output.read_text(encoding="utf-8")
|
||||
assert written.strip() == _current_catalog_text().strip()
|
||||
|
||||
assert bc.main(["--check"]) == 0, "the file main([]) just wrote must satisfy main(['--check'])"
|
||||
|
||||
|
||||
# ------------------------------------------------------------------------------------------------
|
||||
# THE `__main__` WIRING — proof CI's subprocess invocation actually surfaces staleness
|
||||
# ------------------------------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _seed_decisions_copy(root: Path) -> None:
|
||||
"""Copy only what build_decisions_catalog.py + decisions_lib.py need to resolve a real corpus."""
|
||||
(root / "scripts").mkdir(parents=True)
|
||||
(root / "scripts" / "__init__.py").write_bytes(SCRIPTS_INIT.read_bytes())
|
||||
(root / "scripts" / "build_decisions_catalog.py").write_bytes(CATALOG_SCRIPT.read_bytes())
|
||||
(root / "scripts" / "decisions_lib.py").write_bytes(DECISIONS_LIB.read_bytes())
|
||||
|
||||
docs = root / "docs"
|
||||
docs.mkdir()
|
||||
(docs / "decisions.md").write_bytes((REPO_ROOT / "docs" / "decisions.md").read_bytes())
|
||||
|
||||
dst_decisions = docs / "decisions"
|
||||
src_decisions = REPO_ROOT / "docs" / "decisions"
|
||||
dst_decisions.mkdir()
|
||||
for item in src_decisions.iterdir():
|
||||
if item.is_dir():
|
||||
_copy_tree(item, dst_decisions / item.name)
|
||||
else:
|
||||
(dst_decisions / item.name).write_bytes(item.read_bytes())
|
||||
|
||||
|
||||
def _copy_tree(src: Path, dst: Path) -> None:
|
||||
dst.mkdir(parents=True, exist_ok=True)
|
||||
for item in src.rglob("*"):
|
||||
rel = item.relative_to(src)
|
||||
target = dst / rel
|
||||
if item.is_dir():
|
||||
target.mkdir(parents=True, exist_ok=True)
|
||||
else:
|
||||
target.parent.mkdir(parents=True, exist_ok=True)
|
||||
target.write_bytes(item.read_bytes())
|
||||
|
||||
|
||||
WORKFLOW = REPO_ROOT / ".gitea" / "workflows" / "pr-checks.yml"
|
||||
|
||||
|
||||
def _active_runs(workflow: Path | None = None) -> list[str]:
|
||||
"""The `run` script of every step the `decisions-guard` job would ACTUALLY execute.
|
||||
|
||||
Parsed with `yaml.safe_load`, and returned WHOLE — not split into lines. Both choices are
|
||||
scar tissue.
|
||||
|
||||
Text-scanning for `run:` was round one, and cold review broke it three ways: a `run: |` block
|
||||
scalar was invisible; a job or step switched off still read as wired; and `run:` inside
|
||||
block-scalar *text* was extracted and executed. Round two parsed the YAML and matched a LINE
|
||||
beginning with `PYTHONPATH=.` — and review broke that too, with a heredoc:
|
||||
|
||||
run: |
|
||||
cat <<'EOF' > /dev/null
|
||||
PYTHONPATH=. python3 scripts/build_decisions_catalog.py --check
|
||||
EOF
|
||||
|
||||
The matched line is heredoc DATA. The extractor reported the guard as running, and the proof
|
||||
executed a command CI does not. Deciding which lines of a shell script are executed requires
|
||||
parsing shell, and `fixing-a-parser-bug-introduces-the-next-one` is explicit that this repo has
|
||||
lost that argument repeatedly — a regex over shell is not a parser, and round four would find
|
||||
round five.
|
||||
|
||||
So the line-level heuristic is WITHDRAWN. The whole `run` script is handed to `bash`, exactly as
|
||||
the runner does. The heredoc above then runs, writes to `/dev/null`, checks nothing, and exits
|
||||
0 — so the stale-catalog case fails to redden and the proof reports the defect instead of
|
||||
stepping around it. No shell parsing, and the ambiguous cases resolve by execution.
|
||||
"""
|
||||
doc = yaml.safe_load((workflow or WORKFLOW).read_text())
|
||||
|
||||
# The workflow's TRIGGERS, before its jobs. A `decisions-guard` job that is perfectly healthy
|
||||
# gates nothing if the workflow stopped running on pull requests, and starting at `jobs:` cannot
|
||||
# see that. Note `on` parses to the boolean True in YAML 1.1 (the Norway problem's cousin), so
|
||||
# the key is looked up both ways rather than assumed.
|
||||
triggers = (doc or {}).get("on", (doc or {}).get(True)) or {}
|
||||
names = set(triggers) if isinstance(triggers, dict) else {triggers} if isinstance(triggers, str) else set(triggers)
|
||||
assert "pull_request" in names, (
|
||||
"pr-checks.yml no longer runs on `pull_request`, so NO gate in it — including the catalog "
|
||||
f"guard — fires on a PR. Triggers found: {sorted(str(n) for n in names)}"
|
||||
)
|
||||
|
||||
jobs = (doc or {}).get("jobs") or {}
|
||||
job = jobs.get("decisions-guard")
|
||||
assert job is not None, (
|
||||
"no `decisions-guard` job in pr-checks.yml. Either it was renamed or it was removed — the "
|
||||
f"second is the far more serious finding. Jobs present: {sorted(jobs)}"
|
||||
)
|
||||
assert not _disabled(job), (
|
||||
"the `decisions-guard` job is disabled at the job level "
|
||||
f"(if: {job.get('if')!r}, continue-on-error: {job.get('continue-on-error')!r}), so nothing "
|
||||
"in it runs — including the catalog guard"
|
||||
)
|
||||
return [str(step["run"]) for step in (job.get("steps") or []) if step.get("run") and not _disabled(step)]
|
||||
|
||||
|
||||
def _unwrap(value: str) -> str:
|
||||
"""Strip an `${{ ... }}` expression wrapper, if present, and lowercase.
|
||||
|
||||
Written as a regex over the WHOLE value rather than `.strip("${{ }}")`, which strips a character
|
||||
SET — it would turn `"false}"` into `"false"` and reads as though it removed a wrapper it never
|
||||
checked for.
|
||||
"""
|
||||
inner = value.strip()
|
||||
m = re.fullmatch(r"\$\{\{(.*)\}\}", inner, flags=re.DOTALL)
|
||||
if m:
|
||||
inner = m.group(1)
|
||||
return inner.strip().lower()
|
||||
|
||||
|
||||
def _falsey(value) -> bool:
|
||||
"""A literal false, however this workflow dialect spells it.
|
||||
|
||||
`if: false`, `if: "false"` and `if: ${{ false }}` all mean never. The middle and last are the
|
||||
ones a text comparison misses; the last was a live false green — `${{ false }}` is the ordinary
|
||||
spelling in Actions-flavoured YAML, and it read as wired.
|
||||
|
||||
An expression that is merely falsy AT RUN TIME (`if: ${{ github.event_name == 'x' }}`) is not
|
||||
decidable here and is deliberately not guessed at.
|
||||
"""
|
||||
if value is False:
|
||||
return True
|
||||
if not isinstance(value, str):
|
||||
return False
|
||||
return _unwrap(value) == "false"
|
||||
|
||||
|
||||
def _truthy_literal(value) -> bool:
|
||||
if value is True:
|
||||
return True
|
||||
if not isinstance(value, str):
|
||||
return False
|
||||
return _unwrap(value) == "true"
|
||||
|
||||
|
||||
def _disabled(node: dict) -> bool:
|
||||
"""A job or step that cannot fail the run: switched off, or allowed to fail.
|
||||
|
||||
`continue-on-error: true` is the subtle one — the step still runs and still reports, but its
|
||||
failure does not fail the job, so it is not a gate.
|
||||
"""
|
||||
return _falsey(node.get("if", True)) or _truthy_literal(node.get("continue-on-error", False))
|
||||
|
||||
|
||||
def _ci_check_command(workflow: Path | None = None) -> list[str]:
|
||||
"""The catalog step's script, DERIVED from the workflow and executed whole.
|
||||
|
||||
`testing.guard-derives-population-from-source`: a hand-copied command is a second copy of the
|
||||
workflow that drifts silently, and this test's whole value is that it runs what CI runs.
|
||||
"""
|
||||
runs = _active_runs(workflow)
|
||||
matches = [r for r in runs if "build_decisions_catalog.py" in r and "--check" in r]
|
||||
assert matches, (
|
||||
"the `decisions-guard` job has no ACTIVE step mentioning "
|
||||
"`build_decisions_catalog.py --check`. The catalog guard has stopped running in CI — that "
|
||||
f"is the finding, not this test's failure. Active step scripts in that job: {runs}"
|
||||
)
|
||||
assert len(matches) == 1, f"expected exactly one such step, found {matches}"
|
||||
script = matches[0]
|
||||
assert "${{" not in script, (
|
||||
"the catalog step's script interpolates an Actions expression, which cannot be expanded "
|
||||
f"outside the runner — this proof would be executing something else: {script!r}"
|
||||
)
|
||||
# Substitute the interpreter only where `python3` is a bare command word. A plain
|
||||
# `str.replace` rewrites EVERY occurrence, including inside a path — `/usr/bin/python3` would
|
||||
# become `/usr/bin/<venv>/bin/python3` and fail with ENOENT, a red blaming the catalog guard for
|
||||
# something this line did.
|
||||
return ["bash", "-c", re.sub(r"(?<![\w/])python3\b", sys.executable, script)]
|
||||
|
||||
|
||||
def _proof_holds(workflow: Path, repo: Path) -> bool:
|
||||
"""Does the whole guarantee hold — fresh corpus passes AND stale corpus fails?
|
||||
|
||||
The disablement cases below assert on THIS rather than on whether extraction raises, because
|
||||
the ways a guard can stop gating do not all surface at the same place. Deletion and disablement
|
||||
surface as a failed extraction; a heredoc or an `echo` surfaces only when the script is run and
|
||||
reports success over a stale catalog. One predicate covers both.
|
||||
"""
|
||||
try:
|
||||
cmd = _ci_check_command(workflow)
|
||||
except AssertionError:
|
||||
return False
|
||||
|
||||
readme = repo / "docs" / "decisions" / "README.md"
|
||||
original = readme.read_text(encoding="utf-8")
|
||||
try:
|
||||
readme.write_text(original, encoding="utf-8")
|
||||
fresh = subprocess.run(cmd, cwd=str(repo), capture_output=True, text=True, timeout=60)
|
||||
if fresh.returncode != 0:
|
||||
return False
|
||||
readme.write_text(original + "\nSTALE INJECTED LINE\n", encoding="utf-8")
|
||||
stale = subprocess.run(cmd, cwd=str(repo), capture_output=True, text=True, timeout=60)
|
||||
return stale.returncode != 0
|
||||
finally:
|
||||
readme.write_text(original, encoding="utf-8")
|
||||
|
||||
|
||||
_STEP = (
|
||||
" - name: Active catalog in sync\n"
|
||||
" run: PYTHONPATH=. python3 scripts/build_decisions_catalog.py --check\n"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"label,mutate",
|
||||
[
|
||||
(
|
||||
"the step is COMMENTED OUT",
|
||||
lambda s: s.replace(
|
||||
_STEP,
|
||||
" # - name: Active catalog in sync\n"
|
||||
" # run: PYTHONPATH=. python3 scripts/build_decisions_catalog.py --check\n",
|
||||
1,
|
||||
),
|
||||
),
|
||||
("the step is DELETED", lambda s: s.replace(_STEP, "", 1)),
|
||||
(
|
||||
"the step is switched off with `if: false`",
|
||||
lambda s: s.replace(
|
||||
_STEP,
|
||||
" - name: Active catalog in sync\n if: false\n"
|
||||
" run: PYTHONPATH=. python3 scripts/build_decisions_catalog.py --check\n",
|
||||
1,
|
||||
),
|
||||
),
|
||||
(
|
||||
"the step is switched off with `if: ${{ false }}`",
|
||||
lambda s: s.replace(
|
||||
_STEP,
|
||||
" - name: Active catalog in sync\n if: ${{ false }}\n"
|
||||
" run: PYTHONPATH=. python3 scripts/build_decisions_catalog.py --check\n",
|
||||
1,
|
||||
),
|
||||
),
|
||||
(
|
||||
"the step is allowed to fail with `continue-on-error: true`",
|
||||
lambda s: s.replace(
|
||||
_STEP,
|
||||
" - name: Active catalog in sync\n continue-on-error: true\n"
|
||||
" run: PYTHONPATH=. python3 scripts/build_decisions_catalog.py --check\n",
|
||||
1,
|
||||
),
|
||||
),
|
||||
(
|
||||
"the whole JOB is switched off with `if: false`",
|
||||
lambda s: s.replace(
|
||||
" decisions-guard:\n name: decisions lifecycle\n runs-on: small\n"
|
||||
" if: github.event_name == 'pull_request'\n",
|
||||
" decisions-guard:\n name: decisions lifecycle\n runs-on: small\n if: false\n",
|
||||
1,
|
||||
),
|
||||
),
|
||||
(
|
||||
"the command survives only as TEXT in another step's `echo`",
|
||||
lambda s: s.replace(
|
||||
_STEP,
|
||||
" - name: Note\n run: |\n"
|
||||
" echo we no longer run: PYTHONPATH=. python3 "
|
||||
"scripts/build_decisions_catalog.py --check\n",
|
||||
1,
|
||||
),
|
||||
),
|
||||
(
|
||||
"the WORKFLOW no longer runs on pull requests",
|
||||
lambda s: s.replace("on:\n pull_request:", "on:\n workflow_dispatch:", 1),
|
||||
),
|
||||
(
|
||||
"the command survives only as HEREDOC DATA",
|
||||
lambda s: s.replace(
|
||||
_STEP,
|
||||
" - name: Note\n run: |\n"
|
||||
" cat <<'EOF' > /dev/null\n"
|
||||
" PYTHONPATH=. python3 scripts/build_decisions_catalog.py --check\n"
|
||||
" EOF\n",
|
||||
1,
|
||||
),
|
||||
),
|
||||
],
|
||||
)
|
||||
def test_a_guard_that_stopped_RUNNING_is_DETECTED(tmp_path, label, mutate):
|
||||
"""Eight ways the catalog guard can stop gating, each of which must be caught.
|
||||
|
||||
Commenting out is one of them and was the only one the first version detected. The last two
|
||||
leave the command in the file, parseable and even matchable — as `echo` argument and as heredoc
|
||||
data — which is why the proof executes the step's whole script instead of a line lifted out of it.
|
||||
"""
|
||||
raw = WORKFLOW.read_text()
|
||||
mutated = mutate(raw)
|
||||
assert mutated != raw, f"the mutation for {label!r} matched nothing; RETARGET it"
|
||||
|
||||
alt = tmp_path / "pr-checks.yml"
|
||||
alt.write_text(mutated)
|
||||
repo = tmp_path / "repo-copy"
|
||||
repo.mkdir()
|
||||
_seed_decisions_copy(repo)
|
||||
|
||||
assert not _proof_holds(alt, repo), (
|
||||
f"the catalog guard still reported as gating when {label}. CI would run nothing and this "
|
||||
"file would report full coverage."
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"label,replacement",
|
||||
[
|
||||
(
|
||||
"block scalar",
|
||||
" - name: Active catalog in sync\n run: |\n"
|
||||
" PYTHONPATH=. python3 scripts/build_decisions_catalog.py --check\n",
|
||||
),
|
||||
(
|
||||
"backslash continuation",
|
||||
" - name: Active catalog in sync\n run: |\n"
|
||||
" PYTHONPATH=. python3 \\\n"
|
||||
" scripts/build_decisions_catalog.py --check\n",
|
||||
),
|
||||
(
|
||||
"a leading `set -euo pipefail`",
|
||||
" - name: Active catalog in sync\n run: |\n"
|
||||
" set -euo pipefail\n"
|
||||
" PYTHONPATH=. python3 scripts/build_decisions_catalog.py --check\n",
|
||||
),
|
||||
(
|
||||
"wrapped in a shell block",
|
||||
" - name: Active catalog in sync\n run: |\n"
|
||||
" if true; then\n"
|
||||
" PYTHONPATH=. python3 scripts/build_decisions_catalog.py --check\n"
|
||||
" fi\n",
|
||||
),
|
||||
],
|
||||
)
|
||||
def test_the_step_may_be_REFORMATTED_without_being_flagged(tmp_path, label, replacement):
|
||||
"""The negative controls for the eight above: a legitimate rewrite must NOT be flagged.
|
||||
|
||||
`run: |` is idiomatic in this very job (the sibling "Validate decision lifecycle" step uses it),
|
||||
and the text-scanning version reported that form as "the guard has stopped running in CI" — a
|
||||
false red whose message asserts a regression that has not happened. A detector that cannot tell
|
||||
a reformat from a removal trains its readers to ignore it, which is the whole subject of #806.
|
||||
|
||||
A line-matching version had to be taught each of these shapes one at a time, and the
|
||||
continuation case defeated two rounds of it. Executing the script gets all four for free: the
|
||||
question "is the guard still gating" is answered by running it, not by recognising how it was
|
||||
written.
|
||||
"""
|
||||
raw = WORKFLOW.read_text()
|
||||
mutated = raw.replace(_STEP, replacement, 1)
|
||||
assert mutated != raw, "retarget this reformatting; the step's text has changed"
|
||||
|
||||
alt = tmp_path / "pr-checks.yml"
|
||||
alt.write_text(mutated)
|
||||
repo = tmp_path / "repo-copy"
|
||||
repo.mkdir()
|
||||
_seed_decisions_copy(repo)
|
||||
|
||||
assert _proof_holds(alt, repo), f"a legitimate reformat ({label}) was reported as a removal"
|
||||
|
||||
|
||||
def test_CLI_subprocess_exits_nonzero_on_a_stale_catalog(tmp_path):
|
||||
"""The real CI invocation, as a subprocess, against a real corpus.
|
||||
|
||||
This is the `__main__` → `SystemExit(main())` wiring proof: `main()` returning 1 is worthless if
|
||||
the process still exits 0, and the workflow step reads nothing but the exit code. It is the
|
||||
#751/#719 shape the decision record names — a green result behind wiring that is not connected.
|
||||
|
||||
The FRESH half is not optional decoration. Without it this test passes whenever the subprocess
|
||||
dies for any reason at all — an import error, a missing file in the copy, a syntax error — none
|
||||
of which is the guard detecting anything. `arbitrary-sample-gives-false-negatives` in reverse:
|
||||
a non-zero exit is only evidence when the same harness is shown to exit zero on a clean corpus.
|
||||
"""
|
||||
repo = tmp_path / "repo-copy"
|
||||
repo.mkdir()
|
||||
_seed_decisions_copy(repo)
|
||||
cmd = _ci_check_command()
|
||||
|
||||
fresh = subprocess.run(cmd, cwd=str(repo), capture_output=True, text=True, timeout=60)
|
||||
assert fresh.returncode == 0, (
|
||||
"the copied corpus does not even pass --check when untouched, so a non-zero exit below "
|
||||
f"would be the harness failing rather than the guard firing: {fresh.stdout!r} {fresh.stderr!r}"
|
||||
)
|
||||
|
||||
readme = repo / "docs" / "decisions" / "README.md"
|
||||
readme.write_text(readme.read_text(encoding="utf-8") + "\nSTALE INJECTED LINE\n", encoding="utf-8")
|
||||
|
||||
stale = subprocess.run(cmd, cwd=str(repo), capture_output=True, text=True, timeout=60)
|
||||
assert stale.returncode != 0, (
|
||||
"the CLI wiring did not surface a stale catalog as a non-zero exit — CI would report "
|
||||
f"green over a stale README.md: stdout={stale.stdout!r} stderr={stale.stderr!r}"
|
||||
)
|
||||
|
||||
|
||||
# ------------------------------------------------------------------------------------------------
|
||||
# THE MUTATION PROOF — disarm `main()`'s stale-detection clause alone, --check must stop detecting
|
||||
# ------------------------------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _load_mutated_module(tmp_path: Path, mutated_source: str):
|
||||
mutated_path = tmp_path / "mutated_build_decisions_catalog.py"
|
||||
mutated_path.write_text(mutated_source, encoding="utf-8")
|
||||
spec = importlib.util.spec_from_file_location("mutated_build_decisions_catalog", mutated_path)
|
||||
assert spec and spec.loader
|
||||
module = importlib.util.module_from_spec(spec)
|
||||
spec.loader.exec_module(module)
|
||||
return module
|
||||
|
||||
|
||||
def test_MUTATION_disarming_the_stale_comparison_stops_detection(tmp_path, monkeypatch):
|
||||
"""`testing.guard-ships-with-mutation-proof` (#775).
|
||||
|
||||
`main()`'s ONLY stale-detection logic is the clause `want.strip() != have.strip()`. Disarm that
|
||||
clause alone — replace it with the constant `False` in an isolated copy of the module, changing
|
||||
nothing else — and `--check` must stop reporting staleness. If it still returns 1, the deny is
|
||||
coming from somewhere other than the clause the guard is supposed to hang on, and every test
|
||||
above proves nothing about it.
|
||||
"""
|
||||
text = CATALOG_SCRIPT.read_text(encoding="utf-8")
|
||||
clause = "want.strip() != have.strip()"
|
||||
assert clause in text, (
|
||||
"the stale-detection clause has moved or been reworded; RETARGET this mutation at its new "
|
||||
"location rather than loosening the string match — a mutation that silently stops mutating "
|
||||
"is the exact failure this file exists to catch"
|
||||
)
|
||||
mutated_source = text.replace(clause, "False", 1)
|
||||
assert mutated_source != text and clause not in mutated_source, (
|
||||
"the replacement did not change the source, so the mutant is the subject"
|
||||
)
|
||||
|
||||
fresh = _current_catalog_text()
|
||||
output = tmp_path / "README.md"
|
||||
output.write_text(fresh.rstrip("\n") + "\nEXTRA STALE LINE\n", encoding="utf-8")
|
||||
|
||||
# POSITIVE CONTROL FIRST. Without this, the mutation assertion below would pass just as well if
|
||||
# `main(["--check"])` never detected anything at all on this input — "the mutant is silent"
|
||||
# proves nothing unless the real guard is first shown to be loud on the exact same input.
|
||||
monkeypatch.setattr(bc, "OUTPUT", output)
|
||||
assert bc.main(["--check"]) == 1, (
|
||||
"the UNMUTATED guard did not detect the staleness on this input, so a silent mutant below "
|
||||
"would prove nothing about the clause"
|
||||
)
|
||||
|
||||
mutant = _load_mutated_module(tmp_path, mutated_source)
|
||||
mutant.OUTPUT = output
|
||||
assert mutant.main(["--check"]) == 0, (
|
||||
"disarming `want.strip() != have.strip()` alone did not stop --check from reporting "
|
||||
"staleness, so that clause is not what the guard's exit code hangs on"
|
||||
)
|
||||
@@ -0,0 +1,330 @@
|
||||
"""`.husky/pre-push` line 11 — `unset GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE` — is load-bearing.
|
||||
|
||||
That one line is the fix for a real, silent defect, and ersatztv#785 ranks it third because nothing
|
||||
pinned it: reordering it after the nested git calls, or dropping it in a tidy-up, reintroduces the
|
||||
bug with no symptom at all. The failure is a check reporting SUCCESS, which is the family
|
||||
`testing.guard-ships-with-mutation-proof` exists for.
|
||||
|
||||
**The mechanism, measured rather than asserted.** Measured on both platforms this suite runs on,
|
||||
because the guard it replaces was fail-open for months on exactly the platform nobody measured:
|
||||
macOS/git 2.55 (development) and Linux/git 2.47.3 (the `script-tests` runner host). Identical on
|
||||
both — `GIT_DIR` exported, `show-toplevel` answering `<wt>/web`, the nested diff reporting exit 0.
|
||||
The whole file was run green there, and the deletion mutation was witnessed red there:
|
||||
|
||||
* Git exports `GIT_DIR` to `pre-push` **when the push comes from a worktree** — e.g.
|
||||
`GIT_DIR=/repo/.git/worktrees/wt`. From the main tree it exports nothing, which is why this
|
||||
never bites in a plain checkout and why it bites here constantly: `process.shared-tree-readonly`
|
||||
makes working in a worktree the mandated path, so the exported-`GIT_DIR` case is the NORMAL one.
|
||||
* With `GIT_DIR` set and `GIT_WORK_TREE` unset, git stops discovering the repo and takes the
|
||||
**current directory** as the work tree. `git rev-parse --show-toplevel` from `web/` answers
|
||||
`/repo/wt/web`.
|
||||
* So `pre-push`'s last line — `cd web && npm run check:api`, whose `check:api` ends in
|
||||
`git diff --exit-code` — compares against index paths that do not exist under that root. It
|
||||
reports **no diff and exits 0**. Generated-API drift ships, and the gate that exists to catch it
|
||||
prints success.
|
||||
|
||||
The test drives the REAL `.husky/pre-push` file, unedited, in a real worktree, with the environment
|
||||
git really exports. What is substituted is only what surrounds it: the three `.claude/hooks` calls
|
||||
are stubs (they are separately guarded and are not the subject), and `npm` is a stub on PATH whose
|
||||
`run check:api` performs the nested `git diff --exit-code` that the real one ends in. The subject —
|
||||
the ordering of the `unset` against the nested git call — is untouched.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import shutil
|
||||
import stat
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
|
||||
REPO_ROOT = Path(__file__).resolve().parents[2]
|
||||
PRE_PUSH = REPO_ROOT / ".husky" / "pre-push"
|
||||
|
||||
UNSET_CLAUSE = "unset GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE"
|
||||
|
||||
|
||||
# The hooks `pre-push` calls before the CI-parity block. Stubbed because each is guarded on its own
|
||||
# terms and none of them is what this file is about — but DERIVED from the real file rather than
|
||||
# hand-listed, so a hook added to `pre-push` tomorrow cannot leave this harness silently running a
|
||||
# `pre-push` that dies at a missing script and calling that a red.
|
||||
def _hook_calls(text: str) -> list[str]:
|
||||
"""Every `.claude/hooks/` script `pre-push` actually invokes.
|
||||
|
||||
Two refinements over a plain substring scan, each closing a way the harness would misreport:
|
||||
|
||||
* **comment lines are skipped.** A comment naming a hook that no longer exists would otherwise
|
||||
redden `test_the_stub_hooks_are_derived_from_the_real_file` for a file that is perfectly
|
||||
correct.
|
||||
* **the prefix is not assumed to be `./`.** A hook invoked as `bash .claude/hooks/x.sh` would
|
||||
be missed, left unstubbed, and exit 127 — a NON-ZERO status that two tests here read as
|
||||
"drift was caught". That is a false green in the direction that matters, so the match is on
|
||||
the path segment rather than on `./`.
|
||||
"""
|
||||
names = []
|
||||
for line in text.splitlines():
|
||||
# INLINE comments too, not just whole-line ones. Broadening the marker from `./.claude/hooks/`
|
||||
# to the path segment made a trailing `# ... .claude/hooks/removed-helper.sh` match, which
|
||||
# would redden this file for a `pre-push` that is perfectly correct — a false red the
|
||||
# narrower marker did not have. The fix for one over-match must not introduce another.
|
||||
stripped = line.split("#", 1)[0].strip()
|
||||
marker = ".claude/hooks/"
|
||||
if marker in stripped:
|
||||
rest = stripped.split(marker, 1)[1]
|
||||
names.append(rest.split()[0].rstrip("|&;\"'"))
|
||||
return names
|
||||
|
||||
|
||||
def _git(cwd: Path, *args: str) -> str:
|
||||
return subprocess.run(
|
||||
["git", *args],
|
||||
cwd=str(cwd),
|
||||
check=True,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
env={
|
||||
**os.environ,
|
||||
"GIT_AUTHOR_NAME": "t",
|
||||
"GIT_AUTHOR_EMAIL": "t@e",
|
||||
"GIT_COMMITTER_NAME": "t",
|
||||
"GIT_COMMITTER_EMAIL": "t@e",
|
||||
},
|
||||
).stdout.strip()
|
||||
|
||||
|
||||
def _exe(path: Path, body: str) -> None:
|
||||
path.write_text(body)
|
||||
path.chmod(path.stat().st_mode | stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH)
|
||||
|
||||
|
||||
class Bench:
|
||||
"""A real repo, a real worktree, the real `pre-push`, and the env git really exports."""
|
||||
|
||||
def __init__(self, tmp_path: Path, pre_push_text: str, *, drift: bool):
|
||||
tmp_path.mkdir(parents=True, exist_ok=True)
|
||||
self.root = tmp_path / "main-tree"
|
||||
self.root.mkdir()
|
||||
_git(self.root, "init", "-q", "-b", "main", ".")
|
||||
(self.root / "seed").write_text("seed\n")
|
||||
_git(self.root, "add", "seed")
|
||||
_git(self.root, "commit", "-qm", "init")
|
||||
|
||||
self.wt = tmp_path / "wt"
|
||||
_git(self.root, "worktree", "add", "-q", str(self.wt), "-b", "feature")
|
||||
|
||||
web = self.wt / "web"
|
||||
web.mkdir()
|
||||
(web / "gen.txt").write_text("generated\n")
|
||||
_git(self.wt, "add", "web/gen.txt")
|
||||
_git(self.wt, "commit", "-qm", "add generated file")
|
||||
if drift:
|
||||
# The condition `check:api` exists to catch: the committed generated artifact no longer
|
||||
# matches what regeneration produces.
|
||||
(web / "gen.txt").write_text("generated\nDRIFT\n")
|
||||
|
||||
(self.wt / ".husky").mkdir()
|
||||
self.pre_push = self.wt / ".husky" / "pre-push"
|
||||
_exe(self.pre_push, pre_push_text)
|
||||
|
||||
hooks = self.wt / ".claude" / "hooks"
|
||||
hooks.mkdir(parents=True)
|
||||
for name in _hook_calls(pre_push_text):
|
||||
_exe(hooks / name, "#!/usr/bin/env bash\ncat >/dev/null\nexit 0\n")
|
||||
|
||||
# The npm stand-in. `run check:api` performs the nested `git diff --exit-code` the real
|
||||
# script ends in; everything else is a no-op. It records the nested diff's OWN exit code,
|
||||
# not merely that it ran. That distinction is load-bearing: one of the mutations below moves
|
||||
# the `unset` to the end of the file, which also makes the script's terminal status 0 (a
|
||||
# bare `unset` succeeds) — so asserting on pre-push's exit code alone would be satisfied for
|
||||
# a reason that has nothing to do with the nested git call. The recorded diff verdict IS the
|
||||
# defect; the script's exit code is downstream of it.
|
||||
self.witness = tmp_path / "check-api-ran"
|
||||
bindir = tmp_path / "bin"
|
||||
bindir.mkdir()
|
||||
_exe(
|
||||
bindir / "npm",
|
||||
"#!/usr/bin/env bash\n"
|
||||
'if [ "$1" = "run" ] && [ "$2" = "check:api" ]; then\n'
|
||||
" git diff --exit-code -- gen.txt >/dev/null\n"
|
||||
" _rc=$?\n"
|
||||
f' echo "$PWD $_rc" >> "{self.witness}"\n'
|
||||
" exit $_rc\n"
|
||||
"fi\n"
|
||||
"exit 0\n",
|
||||
)
|
||||
self.bindir = bindir
|
||||
# `GIT_DIR` exactly as git exports it for a push from this worktree, verified against a real
|
||||
# push in the investigation that produced this file.
|
||||
self.git_dir = _git(self.wt, "rev-parse", "--absolute-git-dir")
|
||||
|
||||
def run(self, *, export_git_dir: bool) -> subprocess.CompletedProcess:
|
||||
env = {k: v for k, v in os.environ.items() if k not in ("GIT_DIR", "GIT_WORK_TREE", "GIT_INDEX_FILE")}
|
||||
env["PATH"] = f"{self.bindir}:{env['PATH']}"
|
||||
if export_git_dir:
|
||||
env["GIT_DIR"] = self.git_dir
|
||||
return subprocess.run(
|
||||
["bash", str(self.pre_push), "origin", "file:///dev/null"],
|
||||
input=b"refs/heads/feature abc refs/heads/feature def\n",
|
||||
capture_output=True,
|
||||
cwd=str(self.wt),
|
||||
env=env,
|
||||
timeout=120,
|
||||
)
|
||||
|
||||
def check_api_ran(self) -> bool:
|
||||
return self.witness.exists()
|
||||
|
||||
def nested_diff_rc(self) -> int:
|
||||
"""The exit code the nested `git diff --exit-code` actually reported.
|
||||
|
||||
0 means it saw NO diff. With a drifted tree that answer is the bug.
|
||||
"""
|
||||
assert self.witness.exists(), "check:api never ran, so there is no nested diff verdict"
|
||||
return int(self.witness.read_text().split()[-1])
|
||||
|
||||
|
||||
# ------------------------------------------------------------------------------------------------
|
||||
# ANTI-VACUITY — the clause exists, and the harness reaches the check that depends on it
|
||||
# ------------------------------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_the_unset_clause_is_still_in_pre_push():
|
||||
text = PRE_PUSH.read_text()
|
||||
assert UNSET_CLAUSE in text, (
|
||||
f"`{UNSET_CLAUSE}` is gone from .husky/pre-push. If it was removed deliberately, this file "
|
||||
"must be removed with it and docs/guard-inventory.md updated — do not delete this assertion "
|
||||
"on its own, it is the only thing pinning the ordering"
|
||||
)
|
||||
lines = text.splitlines()
|
||||
unset_at = next(i for i, ln in enumerate(lines) if UNSET_CLAUSE in ln)
|
||||
nested_at = next(i for i, ln in enumerate(lines) if "npm run check:api" in ln)
|
||||
assert unset_at < nested_at, (
|
||||
"the unset now comes AFTER the nested git call it exists to protect — that ordering is the "
|
||||
"regression, and it is silent"
|
||||
)
|
||||
|
||||
|
||||
def test_the_harness_actually_reaches_check_api(tmp_path):
|
||||
bench = Bench(tmp_path, PRE_PUSH.read_text(), drift=True)
|
||||
bench.run(export_git_dir=False)
|
||||
assert bench.check_api_ran(), (
|
||||
"the CI-parity block never ran, so every exit code below would be reporting on the hooks "
|
||||
"before it rather than on the nested git call this file is about"
|
||||
)
|
||||
|
||||
|
||||
# ------------------------------------------------------------------------------------------------
|
||||
# THE GUARD DECIDES
|
||||
# ------------------------------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_drift_is_CAUGHT_with_no_git_env_exported(tmp_path):
|
||||
"""Positive control: the harness detects real drift when nothing is in the way."""
|
||||
bench = Bench(tmp_path, PRE_PUSH.read_text(), drift=True)
|
||||
p = bench.run(export_git_dir=False)
|
||||
assert bench.nested_diff_rc() != 0, "the nested diff saw no drift even with a clean env"
|
||||
assert p.returncode != 0, f"the harness did not detect drift even with a clean env: {p.stderr!r}"
|
||||
|
||||
|
||||
def test_drift_is_CAUGHT_when_git_exports_GIT_DIR_from_a_worktree(tmp_path):
|
||||
"""THE REAL CASE. Every push from a worktree — the mandated way to work here — runs this."""
|
||||
bench = Bench(tmp_path, PRE_PUSH.read_text(), drift=True)
|
||||
p = bench.run(export_git_dir=True)
|
||||
assert bench.nested_diff_rc() != 0, (
|
||||
"the nested `git diff --exit-code` reported NO DIFF on a drifted file. That is the silent "
|
||||
"fail-open the `unset` exists to prevent, and it is invisible from the exit code alone"
|
||||
)
|
||||
assert p.returncode != 0, (
|
||||
"pre-push reported success on a drifted generated file while git had exported GIT_DIR: "
|
||||
f"{p.stdout!r} {p.stderr!r}"
|
||||
)
|
||||
|
||||
|
||||
def test_a_CLEAN_tree_is_allowed_through(tmp_path):
|
||||
"""Negative control. A harness that always failed would pass both assertions above."""
|
||||
bench = Bench(tmp_path, PRE_PUSH.read_text(), drift=False)
|
||||
p = bench.run(export_git_dir=True)
|
||||
assert p.returncode == 0, f"a clean tree was blocked: {p.stdout!r} {p.stderr!r}"
|
||||
assert bench.nested_diff_rc() == 0, "the nested diff invented a diff on a clean tree"
|
||||
|
||||
|
||||
# ------------------------------------------------------------------------------------------------
|
||||
# MUTATION PROOFS — the two ways the clause stops protecting anything
|
||||
# ------------------------------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _positive_control(tmp_path: Path, label: str) -> None:
|
||||
bench = Bench(tmp_path / f"pc-{label}", PRE_PUSH.read_text(), drift=True)
|
||||
p = bench.run(export_git_dir=True)
|
||||
assert bench.nested_diff_rc() != 0 and p.returncode != 0, (
|
||||
"the UNMUTATED pre-push did not catch the drift, so 'the mutant lets it through' proves "
|
||||
f"nothing about the clause: {p.stdout!r} {p.stderr!r}"
|
||||
)
|
||||
|
||||
|
||||
def test_MUTATION_DELETING_the_unset_lets_drift_through_silently(tmp_path):
|
||||
_positive_control(tmp_path, "delete")
|
||||
|
||||
text = PRE_PUSH.read_text()
|
||||
assert UNSET_CLAUSE in text, "retarget this mutation; the clause has moved"
|
||||
mutated = text.replace(UNSET_CLAUSE, "# clause removed by the mutation proof", 1)
|
||||
|
||||
bench = Bench(tmp_path / "mut", mutated, drift=True)
|
||||
p = bench.run(export_git_dir=True)
|
||||
assert bench.check_api_ran(), "the mutant died before check:api, so its exit code says nothing"
|
||||
assert bench.nested_diff_rc() == 0, (
|
||||
"removing the unset did NOT blind the nested diff, so the clause is not what protects it "
|
||||
"and this whole file is pinning the wrong thing"
|
||||
)
|
||||
assert p.returncode == 0, f"the drift was still caught somehow: {p.stdout!r}"
|
||||
|
||||
|
||||
def test_MUTATION_REORDERING_the_unset_after_the_nested_git_call_lets_drift_through(tmp_path):
|
||||
"""The regression ersatztv#785 names by hand: not deletion, relocation.
|
||||
|
||||
Deleting a line is a conspicuous diff. Moving it — during a tidy-up, or when a new check is
|
||||
appended above it — reads as a no-op and is not.
|
||||
|
||||
This asserts on the NESTED DIFF's verdict, not on pre-push's exit code. Relocating the `unset`
|
||||
to the end of the file also makes it the script's last statement, and a bare `unset` succeeds —
|
||||
so `returncode == 0` would hold here even if the nested git call had worked perfectly. That is a
|
||||
test passing for the wrong reason, and it was written that way in this file's first draft.
|
||||
"""
|
||||
_positive_control(tmp_path, "reorder")
|
||||
|
||||
lines = PRE_PUSH.read_text().splitlines()
|
||||
kept = [ln for ln in lines if UNSET_CLAUSE not in ln]
|
||||
assert len(kept) == len(lines) - 1, "expected exactly one unset line to relocate"
|
||||
mutated = "\n".join(kept + [UNSET_CLAUSE, ""])
|
||||
|
||||
bench = Bench(tmp_path / "mut", mutated, drift=True)
|
||||
bench.run(export_git_dir=True)
|
||||
assert bench.check_api_ran(), "the mutant died before check:api, so it reports on nothing"
|
||||
assert bench.nested_diff_rc() == 0, (
|
||||
"moving the unset below the nested git call did NOT blind it, so the ORDERING is not "
|
||||
"load-bearing and the ordering assertion in the anti-vacuity test is decoration"
|
||||
)
|
||||
|
||||
|
||||
def test_the_stub_hooks_are_derived_from_the_real_file():
|
||||
"""If `pre-push` gains a hook call, the harness must stub it rather than die at a missing file.
|
||||
|
||||
A `pre-push` that exits 127 at a missing script produces a non-zero exit — indistinguishable
|
||||
from 'drift was caught' in two of the tests above.
|
||||
"""
|
||||
calls = _hook_calls(PRE_PUSH.read_text())
|
||||
assert calls, "no ./.claude/hooks/ call found in pre-push; the extractor has stopped matching"
|
||||
for name in calls:
|
||||
assert (REPO_ROOT / ".claude" / "hooks" / name).is_file(), (
|
||||
f"pre-push calls {name}, which does not exist in .claude/hooks/"
|
||||
)
|
||||
|
||||
|
||||
def test_shutil_which_npm_is_not_what_the_harness_used(tmp_path):
|
||||
"""Anti-vacuity for the stand-in: a real `npm` on PATH would run the real scripts and pass."""
|
||||
bench = Bench(tmp_path, PRE_PUSH.read_text(), drift=True)
|
||||
resolved = shutil.which("npm", path=f"{bench.bindir}:{os.environ['PATH']}")
|
||||
assert resolved == str(bench.bindir / "npm"), (
|
||||
f"the harness would have used {resolved}, not its stand-in — the check it performs would "
|
||||
"then be whatever the real package.json says, not the nested git call under test"
|
||||
)
|
||||
@@ -0,0 +1,490 @@
|
||||
"""The worktree-ownership mechanism is TWO files, and this drives both halves as one thing.
|
||||
|
||||
`pretooluse-worktree-guard.sh` denies a `git commit`/`git merge` inside a worktree another session
|
||||
created. It can only do that because `posttooluse-worktree-marker.sh` wrote the
|
||||
`.claude-worktree-owner` marker at `git worktree add` time. Neither file had a test, and — the part
|
||||
that makes this rank second in ersatztv#785 — **the halves had never been exercised together**, so a
|
||||
regression in either one is invisible: the marker hook silently writing nothing and the guard hook
|
||||
silently reading nothing produce the identical outcome, which is *the commit is allowed*, which is
|
||||
also what a correct fail-open looks like.
|
||||
|
||||
Both hooks are deliberately fail-open (`docs/decisions` — the main tree is never marked, and
|
||||
pre-convention worktrees have no marker), and that is exactly why an absent mechanism is
|
||||
indistinguishable from a working one from the outside. It is the shape
|
||||
`testing.guard-ships-with-mutation-proof` was written for: "in every case a human had read the guard
|
||||
and believed it worked. The guard was not subtly wrong, it was *absent*."
|
||||
|
||||
So this file:
|
||||
|
||||
* drives the REAL pair end to end over a REAL `git worktree add`, in the real payload shape —
|
||||
marker hook first as the harness would fire it, then the guard hook;
|
||||
* carries a negative control (a non-mutating git command) and a fail-open control (an unmarked
|
||||
worktree), because a guard that denied everything would satisfy the deny assertions;
|
||||
* and performs FOUR clause-level mutations: the guard's marker read, the guard's ownership
|
||||
comparison, the command-detection alternation (`commit|merge`), and the *other file's* marker
|
||||
write. The last is the one that could not exist while the halves were tested apart.
|
||||
|
||||
These four are the clauses whose disarm this file detects. They are not every line in either hook —
|
||||
the `git -C` / `cd` redirection extraction and the marker hook's argument parsing are exercised
|
||||
behaviourally but not mutated, and the grade in `docs/guard-inventory.md` covers the clause its
|
||||
cited case mutates, not the whole file.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import os
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
|
||||
REPO_ROOT = Path(__file__).resolve().parents[2]
|
||||
GUARD = REPO_ROOT / ".claude" / "hooks" / "pretooluse-worktree-guard.sh"
|
||||
MARKER_HOOK = REPO_ROOT / ".claude" / "hooks" / "posttooluse-worktree-marker.sh"
|
||||
MARKER_NAME = ".claude-worktree-owner"
|
||||
|
||||
SESSION_A = "session-aaaa-1111"
|
||||
SESSION_B = "session-bbbb-2222"
|
||||
|
||||
|
||||
def _env() -> dict:
|
||||
"""The subprocess environment, built PER CALL — never snapshotted at import.
|
||||
|
||||
Two things it must get right.
|
||||
|
||||
`CLAUDE_PROJECT_DIR` is pinned because the hooks resolve `scripts/hook-fire-log.sh` from it,
|
||||
falling back to a path relative to their own location; a MUTATED copy lives in tmp_path, where
|
||||
that fallback finds nothing. Without the pin the mutant differs from the subject in a second way
|
||||
and the comparison stops being about the mutated clause.
|
||||
|
||||
And it is a FUNCTION rather than a module-level dict because `conftest.py`'s autouse
|
||||
`isolate_hook_fire_log` fixture monkeypatches `ETV_HOOK_FIRE_LOG_DIR` into `os.environ` at test
|
||||
setup — which happens AFTER this module is imported. A `{**os.environ}` snapshot taken at import
|
||||
time captures the environment as it was before the fixture ran, so every hook subprocess writes
|
||||
to the REAL `$HOME/.cache/ersatztv/hook-fire/` log instead of the fixture's tmp dir. That is not
|
||||
untidiness: it is #776's defect reintroduced in the file that is meant to prove #776's hooks,
|
||||
and it corrupts the `hook-fire-log.sh report` surface this repo cites as the observability claim
|
||||
for every guard still graded NONE. The reproduction, rather than a figure whose evidence has
|
||||
since been deleted: reintroduce the snapshot and run this file, then count records for the two
|
||||
synthetic session ids below — 58 per run, on macOS and Linux alike.
|
||||
"""
|
||||
return {**os.environ, "CLAUDE_PROJECT_DIR": str(REPO_ROOT)}
|
||||
|
||||
|
||||
def _git(cwd: Path, *args: str) -> str:
|
||||
p = subprocess.run(
|
||||
["git", *args],
|
||||
cwd=str(cwd),
|
||||
check=True,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
env={
|
||||
**os.environ,
|
||||
"GIT_AUTHOR_NAME": "t",
|
||||
"GIT_AUTHOR_EMAIL": "t@e",
|
||||
"GIT_COMMITTER_NAME": "t",
|
||||
"GIT_COMMITTER_EMAIL": "t@e",
|
||||
},
|
||||
)
|
||||
return p.stdout
|
||||
|
||||
|
||||
def _scratch_repo(tmp_path: Path) -> Path:
|
||||
repo = tmp_path / "main-tree"
|
||||
repo.mkdir()
|
||||
_git(repo, "init", "-q", "-b", "main", ".")
|
||||
(repo / "f.txt").write_text("one\n")
|
||||
_git(repo, "add", "f.txt")
|
||||
_git(repo, "commit", "-qm", "init")
|
||||
return repo
|
||||
|
||||
|
||||
def _run(hook: Path, payload: dict, cwd: Path) -> tuple[int, bytes]:
|
||||
p = subprocess.run(
|
||||
["bash", str(hook)],
|
||||
input=json.dumps(payload).encode(),
|
||||
capture_output=True,
|
||||
cwd=str(cwd),
|
||||
env=_env(),
|
||||
timeout=60,
|
||||
)
|
||||
return p.returncode, p.stdout
|
||||
|
||||
|
||||
def _add_worktree(repo: Path, name: str, marker_hook: Path | None, session: str) -> Path:
|
||||
"""`git worktree add` exactly as a session does it, then fire the PostToolUse marker hook.
|
||||
|
||||
The marker hook is driven with the payload the harness would hand it AFTER the command
|
||||
succeeded, which is when PostToolUse fires — not a hand-planted marker file. A hand-planted
|
||||
marker would make every deny below a test of the guard alone, and the untested seam is the
|
||||
handoff between the two files.
|
||||
"""
|
||||
wt = repo.parent / name
|
||||
_git(repo, "worktree", "add", "-q", str(wt))
|
||||
if marker_hook is not None:
|
||||
rc, _ = _run(
|
||||
marker_hook,
|
||||
{
|
||||
"session_id": session,
|
||||
"hook_event_name": "PostToolUse",
|
||||
"tool_name": "Bash",
|
||||
"cwd": str(repo),
|
||||
"tool_input": {"command": f"git worktree add {wt}"},
|
||||
},
|
||||
repo,
|
||||
)
|
||||
assert rc == 0, "the marker hook must always exit 0"
|
||||
return wt
|
||||
|
||||
|
||||
def _commit_payload(session: str, cwd: Path, command: str = "git commit -m x") -> dict:
|
||||
return {
|
||||
"session_id": session,
|
||||
"hook_event_name": "PreToolUse",
|
||||
"tool_name": "Bash",
|
||||
"cwd": str(cwd),
|
||||
"tool_input": {"command": command},
|
||||
}
|
||||
|
||||
|
||||
def _denied(out: bytes) -> bool:
|
||||
return b'"permissionDecision": "deny"' in out or b'"permissionDecision":"deny"' in out
|
||||
|
||||
|
||||
# ------------------------------------------------------------------------------------------------
|
||||
# ANTI-VACUITY — if the fixture never produces a marked worktree, every deny below is meaningless
|
||||
# ------------------------------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_the_subprocess_env_CARRIES_the_isolated_hook_fire_log_dir():
|
||||
"""The hooks these tests drive must log to the fixture's dir, never the production one.
|
||||
|
||||
`conftest.py`'s autouse `isolate_hook_fire_log` monkeypatches `ETV_HOOK_FIRE_LOG_DIR` into
|
||||
`os.environ` at test setup. Anything that snapshots `os.environ` at IMPORT time captures the
|
||||
value from before the fixture ran and silently defeats it — the hooks then append to
|
||||
`$HOME/.cache/ersatztv/hook-fire/`, which is #776's defect reintroduced inside the file that
|
||||
proves #776's hooks, corrupting the one surface this repo cites as the observability claim for
|
||||
every guard still graded NONE.
|
||||
|
||||
It is invisible from the outside: the tests pass either way, because the fire-log library is
|
||||
fail-open by design. So it needs its own assertion.
|
||||
"""
|
||||
env = _env()
|
||||
isolated = os.environ.get("ETV_HOOK_FIRE_LOG_DIR")
|
||||
assert isolated, "the autouse isolation fixture did not run; conftest.py is not being loaded"
|
||||
assert env.get("ETV_HOOK_FIRE_LOG_DIR") == isolated, (
|
||||
"the subprocess environment does not carry the isolated log dir, so every hook driven by "
|
||||
"this file is writing into the production hook-fire log. Build the env per call; do not "
|
||||
f"snapshot os.environ at import time. env has {env.get('ETV_HOOK_FIRE_LOG_DIR')!r}"
|
||||
)
|
||||
# The path `hook-fire-log.sh` falls back to when ETV_HOOK_FIRE_LOG_DIR is unset, derived the
|
||||
# same way it derives it rather than restated as a literal.
|
||||
production = Path(os.environ.get("HOME", "/tmp")) / ".cache" / "ersatztv" / "hook-fire" # noqa: S108 — mirrors hook-fire-log.sh's own ${HOME:-/tmp}
|
||||
assert Path(isolated).resolve() != production.resolve(), (
|
||||
f"the 'isolated' log dir IS the production one ({production}), so the fixture is isolating "
|
||||
"nothing and this test would pass while the leak continued"
|
||||
)
|
||||
|
||||
|
||||
def test_driving_a_hook_LANDS_its_records_in_the_ISOLATED_dir(tmp_path):
|
||||
"""The invariant, asserted at the EFFECT rather than at the helper that is supposed to produce it.
|
||||
|
||||
`test_the_subprocess_env_CARRIES_...` above checks `_env()`'s return value, and that is not the
|
||||
same claim: `_env()` can be perfectly correct while a call site passes something else. Cold
|
||||
review demonstrated exactly that — restore the module-level snapshot and change one `env=_env()`
|
||||
back to `env=_ENV`, and all thirteen tests pass while 54 records leak into the real log. The
|
||||
guard was pinned to the shape of the fix instead of to the property, which is
|
||||
`verify-against-the-REAL-predecessor`: a hand-written revert is not the code a future tidy-up
|
||||
produces.
|
||||
|
||||
So this drives a real hook through the real `_run()` and asserts the records landed where the
|
||||
fixture put them.
|
||||
|
||||
ITS SCOPE, stated because the first version of this docstring claimed more than it delivers: it
|
||||
guards THE LAUNCH PATH IT DRIVES, not the file. Cold review demonstrated the gap — add a second
|
||||
launcher alongside `_run()` that passes a stale snapshot and point the mutation tests at it, and
|
||||
this test stays green while 18 records leak, because the hooks IT drives still log correctly.
|
||||
Every hook in this file goes through `_run()` today, which is what makes the guard sufficient
|
||||
HERE and not a general property. The general form is a suite-level check, tracked in #809; the
|
||||
reason it is hard is that the obvious version — diff the production log around each test — races
|
||||
against a real session's hooks firing concurrently.
|
||||
"""
|
||||
isolated = Path(os.environ["ETV_HOOK_FIRE_LOG_DIR"])
|
||||
before = {p.name for p in isolated.glob("*.jsonl")} if isolated.exists() else set()
|
||||
|
||||
repo = _scratch_repo(tmp_path)
|
||||
wt = _add_worktree(repo, "wt-a", MARKER_HOOK, SESSION_A)
|
||||
rc, out = _run(GUARD, _commit_payload(SESSION_B, wt), wt)
|
||||
|
||||
# Anti-vacuity: if the hook decided nothing, it may simply have had nothing to log.
|
||||
assert rc == 0 and _denied(out), f"the hook reached no decision, so 'records landed' would prove nothing: {out!r}"
|
||||
|
||||
after = {p.name for p in isolated.glob("*.jsonl")} if isolated.exists() else set()
|
||||
assert after > before, (
|
||||
f"driving two hooks added no record to the isolated log dir {isolated}. Either the "
|
||||
"instrumentation stopped firing, or these hooks are logging somewhere else — and the only "
|
||||
"somewhere else is the production log this file must never touch"
|
||||
)
|
||||
|
||||
|
||||
def test_the_marker_hook_really_marks_the_worktree_it_was_told_about(tmp_path):
|
||||
repo = _scratch_repo(tmp_path)
|
||||
wt = _add_worktree(repo, "wt-a", MARKER_HOOK, SESSION_A)
|
||||
|
||||
marker = wt / MARKER_NAME
|
||||
assert marker.is_file(), (
|
||||
f"no {MARKER_NAME} in {wt}. Without it the guard has nothing to read and every 'denied' "
|
||||
"assertion in this file would be testing a mechanism that is not there"
|
||||
)
|
||||
assert marker.read_text().strip() == SESSION_A, (
|
||||
f"the marker names {marker.read_text().strip()!r}, not the session that created the "
|
||||
"worktree — ownership would be attributed to the wrong session"
|
||||
)
|
||||
|
||||
|
||||
def test_the_marker_hook_ignores_a_command_that_is_not_a_worktree_add(tmp_path):
|
||||
"""The write side's own negative control: a hook that marked on any command would pass above."""
|
||||
repo = _scratch_repo(tmp_path)
|
||||
wt = repo.parent / "wt-unrelated"
|
||||
_git(repo, "worktree", "add", "-q", str(wt))
|
||||
rc, _ = _run(
|
||||
MARKER_HOOK,
|
||||
{
|
||||
"session_id": SESSION_A,
|
||||
"hook_event_name": "PostToolUse",
|
||||
"tool_name": "Bash",
|
||||
"cwd": str(repo),
|
||||
"tool_input": {"command": f"ls {wt}"},
|
||||
},
|
||||
repo,
|
||||
)
|
||||
assert rc == 0
|
||||
assert not (wt / MARKER_NAME).exists(), "the marker hook stamped a worktree it never created"
|
||||
|
||||
|
||||
# ------------------------------------------------------------------------------------------------
|
||||
# THE PAIR DECIDES — both halves, in sequence, as the harness fires them
|
||||
# ------------------------------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_a_commit_in_ANOTHER_sessions_worktree_is_DENIED(tmp_path):
|
||||
"""The #289 case the mechanism exists for, end to end across both files."""
|
||||
repo = _scratch_repo(tmp_path)
|
||||
wt = _add_worktree(repo, "wt-a", MARKER_HOOK, SESSION_A)
|
||||
|
||||
rc, out = _run(GUARD, _commit_payload(SESSION_B, wt), wt)
|
||||
assert rc == 0, "the hook communicates by printing, and must always exit 0"
|
||||
assert _denied(out), f"a commit into session A's worktree was not denied from session B: {out!r}"
|
||||
assert SESSION_A.encode() in out, (
|
||||
"the deny reason must name the owning session — without it the operator cannot tell whether "
|
||||
f"the marker is stale or the worktree is genuinely foreign: {out!r}"
|
||||
)
|
||||
|
||||
|
||||
def test_a_commit_in_MY_OWN_worktree_is_ALLOWED(tmp_path):
|
||||
"""Negative control. A guard that denied every marked worktree would pass the test above."""
|
||||
repo = _scratch_repo(tmp_path)
|
||||
wt = _add_worktree(repo, "wt-a", MARKER_HOOK, SESSION_A)
|
||||
|
||||
rc, out = _run(GUARD, _commit_payload(SESSION_A, wt), wt)
|
||||
assert rc == 0
|
||||
assert out == b"", f"the owning session was blocked from committing in its own worktree: {out!r}"
|
||||
|
||||
|
||||
def test_an_UNMARKED_worktree_is_ALLOWED(tmp_path):
|
||||
"""The deliberate fail-open: pre-convention worktrees and the main tree carry no marker."""
|
||||
repo = _scratch_repo(tmp_path)
|
||||
wt = _add_worktree(repo, "wt-none", None, SESSION_A)
|
||||
|
||||
rc, out = _run(GUARD, _commit_payload(SESSION_B, wt), wt)
|
||||
assert rc == 0
|
||||
assert out == b"", f"an unmarked worktree was blocked, which breaks the main tree too: {out!r}"
|
||||
|
||||
|
||||
def test_a_NON_MUTATING_git_command_in_a_foreign_worktree_is_ALLOWED(tmp_path):
|
||||
"""Only `commit`/`merge` are guarded; `git status` in a sibling worktree is normal work."""
|
||||
repo = _scratch_repo(tmp_path)
|
||||
wt = _add_worktree(repo, "wt-a", MARKER_HOOK, SESSION_A)
|
||||
|
||||
rc, out = _run(GUARD, _commit_payload(SESSION_B, wt, "git status"), wt)
|
||||
assert rc == 0
|
||||
assert out == b"", f"a read-only git command was denied: {out!r}"
|
||||
|
||||
|
||||
def test_a_MERGE_in_a_foreign_worktree_is_DENIED(tmp_path):
|
||||
"""The other half of the guarded alternation.
|
||||
|
||||
Every other deny case here uses `git commit`, so `merge` could be dropped from the detection
|
||||
regex and this file would stay green — the mechanism guards the plumbing-merge path
|
||||
(`process.foreign-worktree-plumbing-merge`) specifically, which makes that the more damaging
|
||||
half to lose.
|
||||
"""
|
||||
repo = _scratch_repo(tmp_path)
|
||||
wt = _add_worktree(repo, "wt-a", MARKER_HOOK, SESSION_A)
|
||||
|
||||
rc, out = _run(GUARD, _commit_payload(SESSION_B, wt, "git merge --no-ff topic"), wt)
|
||||
assert rc == 0
|
||||
assert _denied(out), f"a merge into session A's worktree was not denied: {out!r}"
|
||||
|
||||
|
||||
def test_a_git_C_into_a_foreign_worktree_is_DENIED_from_the_main_tree(tmp_path):
|
||||
"""The redirection that makes the guard non-trivial.
|
||||
|
||||
The session's cwd is its OWN tree — where committing is fine — and only the `-C` argument moves
|
||||
the operation into the foreign worktree. A guard that looked at `cwd` alone would allow this,
|
||||
and `git -C` is how the sibling-worktree commit actually gets typed.
|
||||
"""
|
||||
repo = _scratch_repo(tmp_path)
|
||||
wt = _add_worktree(repo, "wt-a", MARKER_HOOK, SESSION_A)
|
||||
|
||||
rc, out = _run(GUARD, _commit_payload(SESSION_B, repo, f"git -C {wt} commit -m x"), repo)
|
||||
assert rc == 0
|
||||
assert _denied(out), f"`git -C <foreign worktree> commit` was not denied: {out!r}"
|
||||
|
||||
|
||||
# ------------------------------------------------------------------------------------------------
|
||||
# MUTATION PROOFS — four clauses, one per thing the mechanism hangs on
|
||||
#
|
||||
# Each mutant is an isolated copy with ONE clause disarmed, and each test asserts the UNMUTATED pair
|
||||
# reaches the opposite decision on the same fixture FIRST. Without that positive control a mutation
|
||||
# proof passes when the mechanism detects nothing at all, which is how the BOM guard sat fail-open
|
||||
# for months while reading as covered.
|
||||
# ------------------------------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _mutate(src: Path, tmp_path: Path, old: str, new: str, why: str) -> Path:
|
||||
assert old in src.read_text(), (
|
||||
f"the clause {old!r} has moved in {src.name}; RETARGET this mutation rather than loosening "
|
||||
f"it — a mutation that silently stops mutating is the failure this file is about ({why})"
|
||||
)
|
||||
dst = tmp_path / f"mutated-{src.name}"
|
||||
dst.write_text(src.read_text().replace(old, new, 1))
|
||||
return dst
|
||||
|
||||
|
||||
def test_MUTATION_disarming_the_guards_MARKER_READ_stops_the_deny(tmp_path):
|
||||
repo = _scratch_repo(tmp_path)
|
||||
wt = _add_worktree(repo, "wt-a", MARKER_HOOK, SESSION_A)
|
||||
payload = _commit_payload(SESSION_B, wt)
|
||||
|
||||
rc_live, out_live = _run(GUARD, payload, wt)
|
||||
assert rc_live == 0 and _denied(out_live), (
|
||||
"the UNMUTATED guard did not deny, so 'the mutant is silent' would prove nothing about the "
|
||||
f"marker read: {out_live!r}"
|
||||
)
|
||||
|
||||
mutant = _mutate(
|
||||
GUARD,
|
||||
tmp_path,
|
||||
'marker="$root/.claude-worktree-owner"',
|
||||
'marker="$root/.claude-worktree-owner-NOTHING-WRITES-THIS"',
|
||||
"the guard's marker read",
|
||||
)
|
||||
rc, out = _run(mutant, payload, wt)
|
||||
assert rc == 0
|
||||
assert out == b"", (
|
||||
"the guard still denied with its marker read pointed at a file nothing writes, so the deny "
|
||||
f"is not coming from ownership at all: {out!r}"
|
||||
)
|
||||
|
||||
|
||||
def test_MUTATION_inverting_the_OWNERSHIP_COMPARISON_blocks_the_owner(tmp_path):
|
||||
"""The allow direction, which the deny mutation above cannot reach.
|
||||
|
||||
Disarming the comparison the other way would only make the guard deny more, and every deny
|
||||
assertion in this file would stay green. Inverting it is what shows the comparison — rather than
|
||||
the mere presence of a marker — is what decides.
|
||||
"""
|
||||
repo = _scratch_repo(tmp_path)
|
||||
wt = _add_worktree(repo, "wt-a", MARKER_HOOK, SESSION_A)
|
||||
own_payload = _commit_payload(SESSION_A, wt)
|
||||
|
||||
rc_live, out_live = _run(GUARD, own_payload, wt)
|
||||
assert rc_live == 0 and out_live == b"", (
|
||||
f"the UNMUTATED guard already blocked the owner, so the inversion below proves nothing: {out_live!r}"
|
||||
)
|
||||
|
||||
mutant = _mutate(
|
||||
GUARD,
|
||||
tmp_path,
|
||||
'[ "$owner" = "$me" ] && exit 0',
|
||||
'[ "$owner" != "$me" ] && exit 0',
|
||||
"the guard's ownership comparison",
|
||||
)
|
||||
rc, out = _run(mutant, own_payload, wt)
|
||||
assert rc == 0
|
||||
assert _denied(out), (
|
||||
"inverting the ownership comparison did not change the decision for the OWNING session, so "
|
||||
f"the comparison is not what allows it through: {out!r}"
|
||||
)
|
||||
|
||||
|
||||
def test_MUTATION_a_marker_hook_that_stops_WRITING_makes_the_guard_go_quiet(tmp_path):
|
||||
"""THE CROSS-FILE PROOF — the one that could not exist while the halves were tested apart.
|
||||
|
||||
The clause disarmed here is in `posttooluse-worktree-marker.sh`; the assertion is about
|
||||
`pretooluse-worktree-guard.sh`. A regression in the write half is otherwise completely silent:
|
||||
the marker hook exits 0 either way, and the guard's fail-open turns a missing marker into an
|
||||
allowed commit that looks exactly like a correctly allowed one.
|
||||
"""
|
||||
repo = _scratch_repo(tmp_path)
|
||||
|
||||
wt_live = _add_worktree(repo, "wt-live", MARKER_HOOK, SESSION_A)
|
||||
rc_live, out_live = _run(GUARD, _commit_payload(SESSION_B, wt_live), wt_live)
|
||||
assert rc_live == 0 and _denied(out_live), (
|
||||
f"the UNMUTATED pair did not deny, so a silent mutant proves nothing about the marker write: {out_live!r}"
|
||||
)
|
||||
|
||||
mutant_marker = _mutate(
|
||||
MARKER_HOOK,
|
||||
tmp_path,
|
||||
'printf \'%s\\n\' "$me" > "$abs/.claude-worktree-owner" 2>/dev/null || true',
|
||||
"true",
|
||||
"the marker hook's write",
|
||||
)
|
||||
wt_dead = _add_worktree(repo, "wt-dead", mutant_marker, SESSION_A)
|
||||
assert not (wt_dead / MARKER_NAME).exists(), (
|
||||
"the mutated marker hook wrote a marker anyway — the mutation did not disarm the write, so "
|
||||
"the assertion below would be about nothing"
|
||||
)
|
||||
|
||||
rc, out = _run(GUARD, _commit_payload(SESSION_B, wt_dead), wt_dead)
|
||||
assert rc == 0
|
||||
assert out == b"", (
|
||||
"the guard denied a commit in a worktree that carries NO marker, which means the deny in "
|
||||
f"the live case above is not evidence that the two halves are connected: {out!r}"
|
||||
)
|
||||
|
||||
|
||||
def test_MUTATION_dropping_MERGE_from_the_detection_clause_stops_denying_a_merge(tmp_path):
|
||||
"""The alternation is two guarded operations, and losing one of them is silent.
|
||||
|
||||
This mutation is deliberately narrow: it must stop the guard denying a `merge` while leaving it
|
||||
denying a `commit`. Asserting both is what distinguishes "the alternation is load-bearing" from
|
||||
"the mutant broke the regex", which would redden everything and prove nothing about `merge`.
|
||||
"""
|
||||
repo = _scratch_repo(tmp_path)
|
||||
wt = _add_worktree(repo, "wt-a", MARKER_HOOK, SESSION_A)
|
||||
merge_payload = _commit_payload(SESSION_B, wt, "git merge --no-ff topic")
|
||||
commit_payload = _commit_payload(SESSION_B, wt)
|
||||
|
||||
rc_live, out_live = _run(GUARD, merge_payload, wt)
|
||||
assert rc_live == 0 and _denied(out_live), (
|
||||
f"the UNMUTATED guard did not deny a merge, so a silent mutant proves nothing: {out_live!r}"
|
||||
)
|
||||
|
||||
mutant = _mutate(
|
||||
GUARD,
|
||||
tmp_path,
|
||||
"(commit|merge)\\b",
|
||||
"(commit)\\b",
|
||||
"the command-detection alternation",
|
||||
)
|
||||
rc, out = _run(mutant, merge_payload, wt)
|
||||
assert rc == 0
|
||||
assert out == b"", f"dropping `merge` from the alternation did not stop the merge being denied: {out!r}"
|
||||
|
||||
rc_c, out_c = _run(mutant, commit_payload, wt)
|
||||
assert rc_c == 0 and _denied(out_c), (
|
||||
"the mutant stopped denying COMMITS too, so it broke detection wholesale rather than "
|
||||
f"removing the merge alternative — this proves nothing about `merge`: {out_c!r}"
|
||||
)
|
||||
Reference in New Issue
Block a user