diff --git a/docs/guard-inventory.md b/docs/guard-inventory.md index 02cdba063..908270137 100644 --- a/docs/guard-inventory.md +++ b/docs/guard-inventory.md @@ -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. diff --git a/scripts/tests/test_build_catalog_check_path.py b/scripts/tests/test_build_catalog_check_path.py new file mode 100644 index 000000000..8a0ae5fde --- /dev/null +++ b/scripts/tests/test_build_catalog_check_path.py @@ -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//bin/python3` and fail with ENOENT, a red blaming the catalog guard for + # something this line did. + return ["bash", "-c", re.sub(r"(? 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" + ) diff --git a/scripts/tests/test_prepush_unsets_git_env.py b/scripts/tests/test_prepush_unsets_git_env.py new file mode 100644 index 000000000..c7173a87a --- /dev/null +++ b/scripts/tests/test_prepush_unsets_git_env.py @@ -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 `/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" + ) diff --git a/scripts/tests/test_worktree_ownership_guard.py b/scripts/tests/test_worktree_ownership_guard.py new file mode 100644 index 000000000..9a461c352 --- /dev/null +++ b/scripts/tests/test_worktree_ownership_guard.py @@ -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 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}" + )