test(785): mutation proofs for the three unproven ranked guards #810

Merged

timothy merged 12 commits from fix/785-mutation-proofs into main 2026-08-21 22:10:02 +02:00

Conversation 1 Commits 12 Files Changed 4

+1488 -30

12 Commits

Author	SHA1	Message	Date
timothyandClaude Opus 5	55a7df226e	test(785): pin the workflow's TRIGGERS, and sweep the retracted figure by subject ... Fifth cold review returned MERGEABLE, having reproduced all five of the previous commit's measurements exactly and verified by execution that all twelve parametrised cases decide for their labelled reason rather than incidentally. Three residual items, none of which was coverage claimed and not delivered. The `on:` trigger was an uncovered channel. `_active_runs` started at `jobs:`, so a workflow whose triggers no longer include `pull_request` left a perfectly healthy `decisions-guard` job gating nothing while the proof stayed green. Nothing claimed to cover it, which is why it was LOW rather than a false coverage claim — but it is two lines and it is now the ninth disablement case. `on` parses to the BOOLEAN True in YAML 1.1, not the string `"on"`, so the key is looked up both ways. Verified which branch is live rather than assuming: pyyaml produces `True` here, so a lookup of `"on"` alone would have yielded an empty trigger set and fired the assertion on a correct workflow — a false red that the anti-vacuity check (the unmodified workflow must still hold) is what catches. `script.replace("python3", sys.executable)` was unbounded, having been `count=1` on a single extracted line before the withdrawal. It rewrote every occurrence including inside a path, so a step spelled `/usr/bin/python3` would have become nonsense and failed with a red blaming the catalog guard. Now substituted only where `python3` is a bare command word. And the retracted "1,488 records" figure survived in a second place — the leak guard's docstring — because the previous sweep went by file rather than by subject. Replaced with the reproduction and the figure that survives it, the same wording the inventory carries. `stale-comments-sweep-by-subject-not-verb`. Two channels remain uncovered and are deliberately not guessed at: a job `if:` whose expression is false only at run time, and `continue-on-error` as a non-literal expression. `_falsey`'s docstring already declines these as undecidable outside the runner, and no row, test name or docstring claims them. Refs: #785 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-21 21:29:00 +02:00
timothyandClaude Opus 5	52ed319c8a	fix(785): withdraw the line-level workflow heuristic; land the doc fix I said I had landed ... Fourth cold review, scoped to the previous commit. Three findings, one of which is the worst kind. HIGH — a doc fix asserted as landed was not in the diff. `e4a14e282`'s message says the `test_hook_fire_log.py` "TWO clauses" correction was "verified against the files"; the file still said TWO. Cause: both doc edits were applied in one script that wrote once at the end, the second edit's `assert` fired, and `write_text` never ran — so the first edit was discarded silently while I reported it as done. Every edit here is applied, written and re-read individually. The correction itself was right and is now landed: that file proves THREE clauses — coverage, transparency, placement — and the row's Blocks column already claims two of them; the single Proof ref is a column limit, not a coverage statement. All four cited test names verified to exist. Also fixed a second instance of the same stale subject: the ranked entry still said "Three clauses were disarmed" for the worktree pair, and listed three, after the fourth was added two commits ago. MEDIUM — the line-level extractor was round three, and review found round four's defect on schedule: 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 — the exact class the previous commit claimed to close, one level deeper. Deciding which lines of a shell script execute requires parsing shell, and this repo has lost that argument repeatedly. So the line-level heuristic is WITHDRAWN rather than patched a fourth time. The step's WHOLE `run` script is handed to bash, as the runner does. The heredoc 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. Ambiguity resolves by execution. That also removed a false RED the reviewer found and two I had not: a backslash continuation, a leading `set -euo pipefail`, and the command wrapped in an `if` block are all accepted now, and are pinned as parametrised negative controls. The disablement cases are eight, and assert on the whole guarantee (fresh passes AND stale fails) rather than on whether extraction raises, because the failure surfaces in different places. MEDIUM — `if: ${{ false }}` read as wired. `_falsey` compared literals only. The unwrap is a regex over the whole value, not `.strip("${{ }}")`, which strips a character SET and would turn `"false}"` into `"false"` while reading as though it removed a wrapper. MEDIUM — the leak guard's docstring claimed it "cannot pass while anything in this file logs to the production dir, whatever the mechanism". False, and review demonstrated it: add a second launcher beside `_run()` that passes a stale environment and point the mutation tests at it — 14 passed, 18 records leaked. I reproduced that before correcting. It guards the launch path it drives; every hook in the file goes through one helper today, which is what makes it sufficient there and not a general property. Both the docstring and the inventory now say so, and #809 carries the general form. Two figures in the previous message were also wrong. "13 tests pass and 58 records leak" conflates two runs — the pre-fix 13-test file leaks 54, and 58 is the 14-test file at head. And "four sibling suites already parse this workflow" is two: the other two parse docker-build.yml. Refs: #785 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-21 21:29:00 +02:00
timothyandClaude Opus 5	b2b651779a	fix(785): close the re-review's BLOCKER — the leak guard pinned the FIX, not the property ... Third cold review, this one scoped to the previous two fix commits. Its central finding is the one worth reading, because the guard it faults is the guard I added to stop exactly this class of thing. BLOCKER — `test_the_subprocess_env_CARRIES_the_isolated_hook_fire_log_dir` asserted on `_env()`'s RETURN VALUE. `_env()` can be perfectly correct while a call site passes something else, and that is the reintroduction a future tidy-up actually produces. Demonstrated: leave `_env()` exactly as shipped, restore a module-level snapshot and point one `env=` at it — 13 tests pass and 58 records leak. I reproduced it before acting. The guard was pinned to the SHAPE OF THE FIX rather than to the property, which is `verify-against-the-REAL-predecessor`: it was witnessed red against a hand-written revert, so "pins it" and #809's "witnessed red by reintroducing the snapshot" were both overclaims. `test_driving_a_hook_LANDS_its_records_in_the_ISOLATED_dir` asserts the EFFECT instead — drive a real hook through the real `_run()`, records must land in the fixture's dir — with an anti-vacuity check that the hook reached a decision first. Measured: green at head, RED under the call-site reintroduction that the old guard passed. MEDIUM x3 — the workflow extractor was a text scanner and review broke it three ways in one pass. A `run: |` block scalar was invisible, so reformatting the step to the form its own sibling in that job already uses produced a FALSE RED whose message asserts a CI regression that has not happened. A job or step switched off with `if: false` or `continue-on-error: true` still read as wired — 7 green with the guard not gating at all. And `run:` inside block-scalar TEXT was extracted and executed, so an `echo we no longer run: PYTHONPATH=. python3 ... --check` left the subprocess proof green while the real step was deleted. Two of the three are false greens. `fixing-a-parser-bug-introduces-the-next-one` says the second round of a string predicate is not the last. The structure is available — pyyaml is installed by the `script-tests` job and four sibling suites already parse this workflow with it — so the scanner is replaced by `yaml.safe_load`, and "would actually execute" is now three explicit conditions: the job exists, the job is not disabled, the step is neither disabled nor allowed to fail. The six ways it can stop gating are a parametrised test, and the block-scalar REFORMAT is a negative control, because a detector that cannot tell a reformat from a removal trains its readers to ignore it. LOW — `_hook_calls` skipped whole-line comments but not inline ones, and broadening the marker in the previous commit is what made that reachable: a trailing `# ... .claude/hooks/removed-helper.sh` reddened a correct `pre-push`. The fix for one over-match introduced another. Verified both directions. NIT — a section header still said "three clauses" after the fourth mutation was added. Docs. Two accuracy fixes, both verified against the files rather than reasoned about: `test_hook_fire_log.py` proves THREE clauses (coverage, transparency, placement), not two, and its row's Blocks column already claims two of them — the single `Proof ref` is a column limit, not a coverage statement. And the "1,488 stray records" figure was unreproducible in a durable doc, the same commit having deleted the evidence. Replaced with the reproduction and a figure that survives it: 58 records per run, measured identically on macOS/git 2.55 and Linux/git 2.47.3. Also corrects a count in 88832047b's own message: it says the inversion reddens "8 of 12"; that file had 13 tests at that commit. Third mislabelled figure in this branch, all three in messages about mislabelled figures. Refs: #785 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-21 21:29:00 +02:00
timothyandClaude Opus 5	a01def3f10	fix(785): the new leak guard's third assertion was a tautology ... `test_the_subprocess_env_CARRIES_the_isolated_hook_fire_log_dir` shipped with assert "hook-fire" not in str(Path.home() / ".cache") or isolated != str(...) Evaluated: `str(Path.home() / ".cache")` is `/Users/timothy/.cache`, which never contains "hook-fire", so the left operand is always True and the `or` never reaches the comparison. The assertion could not fail. It was decoration inside a guard added specifically to stop vacuous checks — the recurring shape this milestone exists to close, committed by its own fix. The first two assertions in that test were doing the real work throughout, so nothing that was proved is now unproved. The third now compares the resolved paths, with the production path DERIVED the way `hook-fire-log.sh` derives it (`${HOME:-/tmp}/.cache/ersatztv/ hook-fire`) rather than restated as a literal that would drift from it. Verified reachable: with `isolated` set to the production path the comparison evaluates False, i.e. the assertion fires. Previously that same input passed. Found by re-reading the fix commit rather than by the reviews — which is the argument for re-reviewing the FIX and not only the original diff. Refs: #785 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-21 21:29:00 +02:00
timothyandClaude Opus 5	4822003004	fix(785): stop the tests writing to the PRODUCTION hook-fire log, and correct a mislabelled count ... Two independent cold reviews, both against the real subjects rather than the diff. Five findings; the first is the serious one. HIGH — `test_worktree_ownership_guard.py` was defeating conftest's isolation. `_ENV` was a module-level `{**os.environ, ...}` snapshot, taken at IMPORT time, which is before the autouse `isolate_hook_fire_log` fixture monkeypatches `ETV_HOOK_FIRE_LOG_DIR`. So every hook subprocess logged to the real `$HOME/.cache/ersatztv/hook-fire/`. Measured: 1,488 records for the two synthetic session ids, and one further run added 28 more. That is #776's defect reintroduced inside the file meant to prove #776's hooks, and it corrupts the surface `docs/guard-inventory.md` cites as the observability claim for every guard still graded NONE. Nothing surfaced it because the fire-log library is fail-open by design — a logging failure must leave the hook behaving exactly as an uninstrumented one. The env is now built per call. Measured after: a full run of the file adds 0 records. The stray records were deleted; `s1`/`unknown-session` were left alone, not being ours. `test_the_subprocess_env_CARRIES_the_isolated_hook_fire_log_dir` pins it, witnessed red by reintroducing the snapshot. It is per-file, which is the same narrowness as the guard that should have caught this: `test_hook_fire_log.py::test_the_suite_does_not_write_to_the_ PRODUCTION_log` says conftest "must isolate every test, not just this file's" but checks only the running test and its own sandbox. Recorded in the inventory and tracked in #809, including why the obvious general check (diff the log dir per test) is racy against a real session's hooks firing concurrently. MEDIUM — a9433d5cb's commit message mislabels a measurement. It reports the ownership comparison mutation as "2 failed, 8 passed (inverted)". Reproduced both: INVERTING the clause reddens 8 of 12 at today's head; 2 failed / 8 passed is what DELETING it produces (2 of 10 at that commit's head). The number is real but belongs to the mutation the parenthetical says was rejected. The shipped test is unaffected — inversion genuinely reddens `test_MUTATION_inverting_the_OWNERSHIP_COMPARISON_blocks_the_owner`. Corrected here rather than by amending, since the history is the record. LOW — `docs/guard-inventory.md` described the unproven set as "every PreToolUse hook except ..." two paragraphs above a standing list of eight hooks spanning four events. Both sentences named the same set. `design-sync-reminder.sh` is registered on BOTH PreToolUse and Stop in `.claude/settings.json`, so any "every X hook" phrasing double-counts it; the set is now named by enumeration. LOW — a truncated sentence ("`test_hook_fire_log.py` asserts TWO clauses", no verb, no closing bold) orphaned the following paragraph's "That unproven set". Completed with both clauses and their proofs. NIT — `_hook_calls` matched comment lines and only the `./` invocation form. A commented ghost hook would have reddened a correct tree; a `bash .claude/hooks/x.sh` form would have gone unstubbed and exited 127, which two tests read as "drift caught". Both verified. Refs: #785 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-21 21:29:00 +02:00
timothyandClaude Opus 5	fe01982a97	test(785): close two findings from the independent cold review ... Both are the same species the milestone is about — a coverage claim resting on source-text presence rather than on the guard actually running or the clause actually being reached. MEDIUM, `test_build_catalog_check_path.py`. `_ci_check_command()` grepped the whole workflow for the command text. So it matched a step that had been COMMENTED OUT, and it was not scoped to the `decisions-guard` job at all — a match anywhere in the file would do. Commenting out a step is the ordinary way a check gets switched off during an incident and never switched back, and the subprocess proof would have stayed green over a guard CI no longer runs. `guard-ships-with-mutation-proof`: the guard being WIRED is not the guard RUNNING. Now the extractor finds the job by key, stops at the next top-level job, strips comment lines before matching, and asserts exactly one active step. `test_the_extractor_rejects_a_ COMMENTED_OUT_step` pins it. Witnessed against the real workflow: commenting the step out reddens 2 of 7, deleting it reddens the same 2. LOW, `test_worktree_ownership_guard.py`. The docstring claimed the three mutations covered every clause the pair hangs on. They did not: the detection clause is `(commit|merge)`, and every payload in the file used `commit`, so `merge` could be dropped and the file stayed green. That is the more damaging half to lose — the mechanism exists for the plumbing-merge path (`process.foreign-worktree-plumbing-merge`). Adds a merge deny case and a fourth mutation, narrow by construction: the mutant must stop denying a merge WHILE STILL denying a commit, so a mutation that merely broke the regex cannot pass for it. Witnessed: dropping `merge` from the real hook reddens exactly those 2. The docstring now also states which clauses are NOT mutated (the `git -C`/`cd` redirection extraction, the marker hook's argument parsing) rather than implying the file is exhaustive. The review's third finding — the inventory calling both husky files thin dispatchers — was raised against ad0bed458..7bd7b4c8 and had already been corrected in fdc91056f. Refs: #785 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-21 21:29:00 +02:00
timothyandClaude Opus 5	77317e5c71	test(785): pin the pre-push premise to both platforms, measured not assumed ... The docstring said "git 2.55, macOS; the same holds on Linux". The second half was an assertion, and this repo's canonical disaster (#775, the BOM guard) is precisely a guard that behaved differently on the platform nobody measured — macOS had `xxd`, the Linux runner did not, and it was fail-open there for months. So it was measured on the runner host (Linux, git 2.47.3) rather than reasoned about: * git exports GIT_DIR=<repo>/.git/worktrees/<wt> to pre-push on a push from a worktree * `git rev-parse --show-toplevel` from web/ with that GIT_DIR answers `<wt>/web` * the nested `git diff --exit-code` reports exit 0 on a drifted file and all three new suites were run there: 25 passed. Disarming the `unset` on that host reddens 4 of the 9, the same four as on macOS. Refs: #785 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-21 21:29:00 +02:00
timothyandClaude Opus 5	9ebf8023f9	docs(785): correct three false claims in the remaining-NONE groups ... Self-review of the prose added in the previous commit found three statements that are not true of the files they describe: * `.husky/commit-msg` was called a thin dispatcher. It dispatches to nothing — it is one `grep -q '^Co-Authored-By:'` plus a `MERGE_HEAD` exemption, and nothing tests that the exemption fires only for merges. * `.husky/pre-commit` was called one too. It 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 clauses. * eight hooks were grouped as "the `PreToolUse` advisory hooks". Only four are `PreToolUse`; two are pre-push, one pre-commit, one `Stop`. The first two mattered most: "it dispatches to guards" was being used to argue those rows need no proof of their own, which is the relabelling error this file's withdrawn-guard table already exists to prevent — committed by the paragraph explaining the grades. Both are now stated as backlogs. Membership was verified by deriving the GUARD+NONE set from the table: 16 rows, 8 + 2 + 6, matching the three groups exactly. Refs: #785 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-21 21:29:00 +02:00
timothyandClaude Opus 5	3651b06c3c	docs(785): record what each mutation established, and why every remaining row is NONE ... The ranked list at the bottom of the inventory named four gaps; all four now carry clause-level proofs. It keeps the ranking — that is the reusable part — and states what each mutation actually established, because two of the four turned out worse than the ranking predicted (the BOM guard was fail-open on every non-vim host; the catalog guard's wiring can drop a failure while still printing it). Two additions the table cannot express: * what a file-level MUTATION grade does NOT claim. Three of these four rows are multi-purpose files. Per `testing.guard-ships-with-mutation-proof` the grade covers the clause its cited case mutates, not every line in the file: `.husky/pre-push` runs four other things and its row asserts only that line 11 is load-bearing. Reading it as "pre-push is tested" is the relabelling error the withdrawn-guard table above already exists to prevent. * why each remaining guard is still NONE — the third `## Done-when` box. They split three ways, and only the first is a backlog: the PreToolUse advisory hooks (unproven, now observable via #776, which is the weaker claim); the two husky dispatchers, whose mutable clauses live in the guards they call and whose dispatch is already covered by `test_every_hook_file_is_actually_WIRED`; and six that each need their own harness rather than a proof written to clear a row. That list is prose and nothing checks it. Stated as such in the file rather than left to be discovered — a guard moving between groups will not redden anything. Refs: #785 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-21 21:29:00 +02:00
timothyandClaude Opus 5	f2240b0205	test(785): drive build_decisions_catalog's --check path, and its __main__ wiring ... `--check` is what the decisions-guard job runs; the existing tests call `render_catalog()` directly and never call `main()` at all. So neither the stale-detection comparison nor the `raise SystemExit(main())` wiring had any coverage. Two mutations, both witnessed red against the real script: * disarming `want.strip() != have.strip()` alone -> 3 failed, 3 passed * replacing `raise SystemExit(main())` with a bare `main()` -> 1 failed, 5 passed, and only the subprocess case reddens. That mutant prints `build_decisions_catalog: docs/decisions/README.md is stale` on stderr and exits 0. The workflow step reads nothing but the exit code, so CI would report green over a stale catalog — the #751/#719 shape the decision record names. The subprocess case runs the command DERIVED from pr-checks.yml rather than a restated copy of it, and runs it twice: once on an untouched copy of the corpus, which must exit 0. Without that half, any failure of the harness — an import error, a missing file in the copy — would also exit non-zero and read as the guard firing. Refs: #785 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-21 21:29:00 +02:00
timothyandClaude Opus 5	d05d6623d1	test(785): pin .husky/pre-push's unset GIT_DIR ..., and measure why it matters ... The one-line fix at pre-push:11 had nothing pinning it, and #785 ranks it third because the regression is a relocation rather than a deletion — a tidy-up that reads as a no-op. What the investigation established, since the comment above it asserts the mechanism without stating when it applies (git 2.55): * git exports GIT_DIR to pre-push when the push comes FROM A WORKTREE, and exports nothing from the main tree. `process.shared-tree-readonly` makes the worktree the mandated way to work here, so the exposed path is the normal one, not an edge case. * with GIT_DIR set and GIT_WORK_TREE unset, git stops discovering the repo and treats the CURRENT DIRECTORY as the work tree: `git rev-parse --show-toplevel` from web/ answers `<wt>/web`. * so `cd web && npm run check:api`'s `git diff --exit-code` compares against index paths that do not exist there and reports NO DIFF, exit 0. Generated-API drift ships and the gate prints success. The test drives the real pre-push file unedited, in a real worktree, with that exact environment; the hook calls are stubs (DERIVED from the file, so a new hook cannot leave the harness dying at a missing script and calling that a red) and `npm` is a stand-in whose `run check:api` performs the nested diff. Both mutations witnessed red against the real file: deletion -> 4 failed, 5 passed; relocation to the end -> 5 failed, 4 passed. The relocation mutation was wrong in the first draft. Appending the `unset` at the end also makes it the script's last statement, and a bare `unset` succeeds — so a `returncode == 0` assertion held for a reason unrelated to the nested git call. The stand-in npm now records the nested diff's OWN exit code and the mutations assert on that. Verified by making the mutation a no-op: the test fails, as it must. Refs: #785 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-21 21:29:00 +02:00
timothyandClaude Opus 5	629ab0b173	test(785): prove the worktree-ownership PAIR, including the cross-file seam ... `pretooluse-worktree-guard.sh` and `posttooluse-worktree-marker.sh` are one mechanism in two files and neither had a test. #785 ranks them second because a regression in either half is invisible: both hooks are deliberately fail-open, so a marker hook that writes nothing and a guard that reads nothing both produce "the commit is allowed" — identical to a correct fail-open. Three clauses are disarmed and each was witnessed red against the REAL hooks in place, not only against the isolated copies the tests build: * the guard's marker read -> 5 failed, 5 passed * the guard's ownership comparison -> 2 failed, 8 passed (inverted, because disarming it the other way only makes the guard deny MORE and every deny assertion stays green) * the marker hook's write -> 6 failed, 4 passed The third is the one that could not exist while the halves were tested apart: the clause is in the marker hook, the assertion is about the guard's decision. The first attempt at that third demonstration used a `sed` that silently matched nothing and reported 10 passed. Re-run with an asserted replacement, hence the numbers above. Refs: #785 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-21 21:29:00 +02:00