timothy/ersatztv
1
0
Fork 0
Code Issues 52 Pull Requests 6 Actions 2 Packages Projects Releases 9 Wiki Activity

Mutation proofs for the 19 unproven guards, worst-consequence first #785

New Issue
Closed
opened 2026-08-13 21:59:10 +02:00 by timothy · 4 comments
timothy commented 2026-08-13 21:59:10 +02:00
Owner
Copy Link
Copy Source

Spawned by #775's audit. docs/guard-inventory.md measures 28 guards: 6 mutation-proved, 3 behaviour-only, 19 with no test at all. The inventory now makes the gap visible and machine-checked; it does not close it.

Ranked by what a silent failure would let through, not by test count:

  1. .claude/hooks/pretooluse-bom-guard.sh — guards a defect that has recurred three times (#311, #402, #405).
  2. .claude/hooks/pretooluse-worktree-guard.sh + .claude/hooks/posttooluse-worktree-marker.sh — a two-file mechanism guarding #289. A regression in either half is invisible, and the halves have never been tested together. (This one bit us during #774 itself: we routed a review agent around the live worktree specifically because this guard is unproven.)
  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). Reordering it after the nested git calls reintroduces the bug silently.
  4. scripts/build_decisions_catalog.py — --check is what CI runs and no test calls main(); the tests exercise render_catalog() directly.

Do these as separate commits, not one sweep. A batch of mutation proofs written to clear rows is exactly the test that passes for the wrong reason — and each proof must be demonstrated red before it is believed (testing.guard-ships-with-mutation-proof).

Done-when

  • The four above ship mutation proofs, each demonstrated red by disarming its clause
  • docs/guard-inventory.md rows updated to MUTATION with real refs
  • Remaining unproven guards either proven or given a stated reason in the inventory
  • Adversarial review passed
Spawned by #775's audit. `docs/guard-inventory.md` measures **28 guards: 6 mutation-proved, 3 behaviour-only, 19 with no test at all.** The inventory now makes the gap visible and machine-checked; it does not close it. Ranked by what a silent failure would let through, not by test count: 1. `.claude/hooks/pretooluse-bom-guard.sh` — guards a defect that has recurred three times (#311, #402, #405). 2. `.claude/hooks/pretooluse-worktree-guard.sh` + `.claude/hooks/posttooluse-worktree-marker.sh` — a two-file mechanism guarding #289. A regression in either half is invisible, and the halves have never been tested together. (This one bit us during #774 itself: we routed a review agent around the live worktree specifically because this guard is unproven.) 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). Reordering it after the nested git calls reintroduces the bug silently. 4. `scripts/build_decisions_catalog.py` — `--check` is what CI runs and no test calls `main()`; the tests exercise `render_catalog()` directly. Do these as separate commits, not one sweep. A batch of mutation proofs written to clear rows is exactly the test that passes for the wrong reason — and each proof must be demonstrated red before it is believed (`testing.guard-ships-with-mutation-proof`). ## Done-when - [x] The four above ship mutation proofs, each demonstrated red by disarming its clause - [x] `docs/guard-inventory.md` rows updated to MUTATION with real refs - [x] Remaining unproven guards either proven or given a stated reason in the inventory - [x] Adversarial review passed
timothy added the ci-cdpriority: high labels 2026-08-13 21:59:10 +02:00
timothy commented 2026-08-14 17:07:43 +02:00
Author
Owner
Copy Link
Copy Source

🔗 Related: #796 — five false greens measured in one PR's own verification code (#793/#777), each introduced by the fix for the previous one. Two points bear directly on this issue: (a) the mutation-proof rule applies to harness/wrapper code that is not in the guard population — scripts/mcp_smoke.py is a checker wired to no CI and outside docs/guard-inventory.md by design; (b) perl -e 'alarm shift; exec @ARGV' exits 0 when exec fails, so a timeout wrapper can report a clean pass having run nothing. Same family as #776's shellcheck-caught-0 finding.

🔗 Related: #796 — five false greens measured in one PR's own verification code (#793/#777), each introduced by the fix for the previous one. Two points bear directly on this issue: (a) the mutation-proof rule applies to harness/wrapper code that is *not* in the guard population — `scripts/mcp_smoke.py` is a checker wired to no CI and outside `docs/guard-inventory.md` by design; (b) `perl -e 'alarm shift; exec @ARGV'` **exits 0 when exec fails**, so a timeout wrapper can report a clean pass having run nothing. Same family as #776's shellcheck-caught-0 finding.
timothy added this to the Defect-shape hardening milestone 2026-08-21 19:18:16 +02:00
timothy added the in-progress label 2026-08-21 19:22:30 +02:00
timothy commented 2026-08-21 19:22:30 +02:00
Author
Owner
Copy Link
Copy Source

Claiming — Claude Code session, 2026-08-21. Checked: no open PR references #785, no remote branch names it, no prior claiming comment, fresh git fetch origin main at ad0bed458.

Claiming — Claude Code session, 2026-08-21. Checked: no open PR references #785, no remote branch names it, no prior claiming comment, fresh `git fetch origin main` at ad0bed458.
timothy commented 2026-08-21 19:55:59 +02:00
Author
Owner
Copy Link
Copy Source

Done

What was done: Independent adversarial review of ad0bed458..7bd7b4c8 against testing.guard-ships-with-mutation-proof and testing.guard-derives-population-from-source.

Root cause: Two coverage claims still substitute source-text presence for active wiring / actual protected clauses:

  • MEDIUM — scripts/tests/test_build_catalog_check_path.py:147-153: _ci_check_command() accepts a commented-out # run: PYTHONPATH=. python3 scripts/build_decisions_catalog.py --check line (and does not bind it to the decisions-guard job). The subprocess test therefore stays green after CI stops running the guard.
  • MEDIUM — docs/guard-inventory.md:258-262: the new NONE rationale calls both Husky files thin dispatchers and says test_every_hook_file_is_actually_WIRED covers their dispatch. In fact .husky/commit-msg implements its grep guard inline, .husky/pre-commit implements root-PNG and format guards inline, and the named inventory test only checks .claude/hooks/*.sh basenames.
  • LOW — scripts/tests/test_worktree_ownership_guard.py:23-24 says the three mutations cover every clause the pair hangs on, but removing the merge alternative from the command-detection clause leaves this file green because every mutating payload uses commit. The cited marker-read/write grades remain valid; the prose is broader than the proof.

Files changed: None; review was read-only.

Verification: git diff --check ad0bed458..HEAD passed; all 35 focused tests collect; inventory functions independently report 57 derived/listed rows and the documented 34 GUARD / 5 TOOLING / 18 PROOF, with 12 MUTATION / 6 BEHAVIOUR-ONLY / 16 NONE; current catalog --check passes; an in-memory stale/live comparison returned 1 and the comparison mutant returned 0. Full pytest execution was not possible in this read-only session because pytest's autouse fixture requires a writable temporary directory; both attempts stopped before test bodies with No usable temporary directory.

Deferred: Correct the two MEDIUM findings and rerun the focused pytest files on a writable Linux runner.

Follow-up issues: None created.

Docs updated: None; the false inventory rationale is itself a blocking finding.

Reviewer verdict: BLOCKED. Issue left open; no commit, push, or closure performed.

## Done **What was done**: Independent adversarial review of `ad0bed458..7bd7b4c8` against `testing.guard-ships-with-mutation-proof` and `testing.guard-derives-population-from-source`. **Root cause**: Two coverage claims still substitute source-text presence for active wiring / actual protected clauses: - MEDIUM — `scripts/tests/test_build_catalog_check_path.py:147-153`: `_ci_check_command()` accepts a commented-out `# run: PYTHONPATH=. python3 scripts/build_decisions_catalog.py --check` line (and does not bind it to the `decisions-guard` job). The subprocess test therefore stays green after CI stops running the guard. - MEDIUM — `docs/guard-inventory.md:258-262`: the new NONE rationale calls both Husky files thin dispatchers and says `test_every_hook_file_is_actually_WIRED` covers their dispatch. In fact `.husky/commit-msg` implements its `grep` guard inline, `.husky/pre-commit` implements root-PNG and format guards inline, and the named inventory test only checks `.claude/hooks/*.sh` basenames. - LOW — `scripts/tests/test_worktree_ownership_guard.py:23-24` says the three mutations cover every clause the pair hangs on, but removing the `merge` alternative from the command-detection clause leaves this file green because every mutating payload uses `commit`. The cited marker-read/write grades remain valid; the prose is broader than the proof. **Files changed**: None; review was read-only. **Verification**: `git diff --check ad0bed458..HEAD` passed; all 35 focused tests collect; inventory functions independently report 57 derived/listed rows and the documented 34 GUARD / 5 TOOLING / 18 PROOF, with 12 MUTATION / 6 BEHAVIOUR-ONLY / 16 NONE; current catalog `--check` passes; an in-memory stale/live comparison returned 1 and the comparison mutant returned 0. Full pytest execution was not possible in this read-only session because pytest's autouse fixture requires a writable temporary directory; both attempts stopped before test bodies with `No usable temporary directory`. **Deferred**: Correct the two MEDIUM findings and rerun the focused pytest files on a writable Linux runner. **Follow-up issues**: None created. **Docs updated**: None; the false inventory rationale is itself a blocking finding. **Reviewer verdict**: BLOCKED. Issue left open; no commit, push, or closure performed.
timothy commented 2026-08-21 22:10:36 +02:00
Author
Owner
Copy Link
Copy Source

Closing record

Outcome: Shipped in PR #810, merged as 706674272. The three still-unproven guards from #785's ranked list now carry
clause-level mutation proofs, each witnessed red against the real subject in place (not only against
the isolated copies the tests build): the pretooluse-worktree-guard.sh + posttooluse-worktree-marker.sh
PAIR (4 clauses, including the cross-file seam), .husky/pre-push:11's unset GIT_DIR ... (deletion
and relocation), and scripts/build_decisions_catalog.py --check (the stale comparison and the
__main__ wiring). Item 1 of the list, the BOM guard, was already proven before this session.
docs/guard-inventory.md regraded: 12 MUTATION / 6 BEHAVIOUR-ONLY / 16 NONE across 34 guards, with
three new PROOF rows — all machine-checked against the table.

Root cause: Not a bug — a measured coverage gap #775's audit created and #785 tracked. Worth
recording is why two of the four ranked entries were worse than the ranking predicted. The
.husky/pre-push unset guards the NORMAL case, not an edge case: git exports GIT_DIR to
pre-push only when the push comes from a worktree, and process.shared-tree-readonly makes the
worktree the mandated way to work here. And build_decisions_catalog.py's __main__ wiring can drop
a failure while still printing it — replacing raise SystemExit(main()) with a bare main() leaves
the script printing "is stale" on stderr and exiting 0, which the workflow step cannot see.

Decisions/conventions changed: none. testing.guard-ships-with-mutation-proof and
testing.guard-derives-population-from-source were applied, not amended. #790 (clause-level grading)
and #809 (new) carry the follow-ups.

Reusable knowledge:

  • A guard can be pinned to the SHAPE OF ITS FIX rather than to the property. The leak guard
    asserted on _env()'s return value; leaving the helper correct and reverting one env= at the
    call site gave 13 passing tests and 58 leaked records. Assert the EFFECT. Witnessing a mutation
    against a hand-written revert is not witnessing it against the code a future tidy-up produces.
  • Deciding which lines of a shell script execute needs a shell parser. A heredoc body and an
    echo argument are indistinguishable from a command by any line-matching rule. Executing the
    step's WHOLE run script resolves every such case by execution and costs nothing — it also fixed
    three false REDS (backslash continuation, set -euo pipefail, an if wrapper) that the
    line-matcher would have needed teaching one at a time.
  • pyyaml parses on: as the boolean True, not the string "on" (YAML 1.1). A lookup of "on"
    alone yields an empty trigger set and fires on a correct workflow.
  • isolation: "worktree" bounds the FILES, not shared git state. Sibling worktrees share .git;
    my index came back holding a pre-fix blob while HEAD and the working tree were both correct. A
    git add -A && git commit in that window would have committed a silent partial revert with every
    test still passing.
  • A batched edit script that writes once at the end loses ALL edits if any assert fires — and I
    then reported the discarded edit as landed. Apply, write and re-read each edit individually.

Verification: scripts/tests 771 passed / 2 skipped locally; CI Script tests (pytest) green in
4m12s with 773 collected — identical to local, so nothing silently skipped. The three new files run
on the Linux runner host (git 2.47.3): 41 passed, and the pre-push deletion mutation reddens there
too. Every commit green in isolation. No flakiness across 5 sequential + 3 concurrent runs. All 18
proof refs resolve to a real def, checked independently of the inventory guard. Production
hook-fire log: head adds 0 records, verified at full-suite scale on both hosts.

Deferred: #809 (generalise the production-log isolation guard beyond one file — includes why the
obvious per-test log diff is racy against a live session's hooks). #790 already tracks clause-level
grading. Two workflow channels deliberately not covered because they are undecidable outside the
runner, and named as such in _falsey's docstring: a job if: false only at run time, and
continue-on-error as a non-literal expression.

Docs updated: docs/guard-inventory.md (rows, counts, the ranked list with what each mutation
established, what a file-level MUTATION grade does NOT claim, and the three groups every remaining
NONE row falls into — verified member-for-member against the derived set).

Process note: five cold review rounds, each finding a defect introduced by the previous round's
fix, every one of them a green test over a dead check — the exact class this issue exists to close,
reproduced inside the work meant to close it. The pattern broke at round 4 by WITHDRAWING the
line-level predicate rather than patching it a fourth time. Limitation stated rather than hidden:
Codex failed twice to produce a cross-family review, so all five rounds came from one model family.

## Closing record **Outcome:** Shipped in PR #810, merged as 706674272. The three still-unproven guards from #785's ranked list now carry clause-level mutation proofs, each witnessed red against the real subject in place (not only against the isolated copies the tests build): the `pretooluse-worktree-guard.sh` + `posttooluse-worktree-marker.sh` PAIR (4 clauses, including the cross-file seam), `.husky/pre-push:11`'s `unset GIT_DIR ...` (deletion and relocation), and `scripts/build_decisions_catalog.py --check` (the stale comparison and the `__main__` wiring). Item 1 of the list, the BOM guard, was already proven before this session. `docs/guard-inventory.md` regraded: 12 MUTATION / 6 BEHAVIOUR-ONLY / 16 NONE across 34 guards, with three new PROOF rows — all machine-checked against the table. **Root cause:** Not a bug — a measured coverage gap #775's audit created and #785 tracked. Worth recording is *why* two of the four ranked entries were worse than the ranking predicted. The `.husky/pre-push` `unset` guards the NORMAL case, not an edge case: git exports `GIT_DIR` to `pre-push` only when the push comes from a worktree, and `process.shared-tree-readonly` makes the worktree the mandated way to work here. And `build_decisions_catalog.py`'s `__main__` wiring can drop a failure while still printing it — replacing `raise SystemExit(main())` with a bare `main()` leaves the script printing "is stale" on stderr and exiting 0, which the workflow step cannot see. **Decisions/conventions changed:** none. `testing.guard-ships-with-mutation-proof` and `testing.guard-derives-population-from-source` were applied, not amended. #790 (clause-level grading) and #809 (new) carry the follow-ups. **Reusable knowledge:** - **A guard can be pinned to the SHAPE OF ITS FIX rather than to the property.** The leak guard asserted on `_env()`'s return value; leaving the helper correct and reverting one `env=` at the call site gave 13 passing tests and 58 leaked records. Assert the EFFECT. Witnessing a mutation against a hand-written revert is not witnessing it against the code a future tidy-up produces. - **Deciding which lines of a shell script execute needs a shell parser.** A heredoc body and an `echo` argument are indistinguishable from a command by any line-matching rule. Executing the step's WHOLE `run` script resolves every such case by execution and costs nothing — it also fixed three false REDS (backslash continuation, `set -euo pipefail`, an `if` wrapper) that the line-matcher would have needed teaching one at a time. - **pyyaml parses `on:` as the boolean `True`**, not the string `"on"` (YAML 1.1). A lookup of `"on"` alone yields an empty trigger set and fires on a correct workflow. - **`isolation: "worktree"` bounds the FILES, not shared git state.** Sibling worktrees share `.git`; my index came back holding a pre-fix blob while HEAD and the working tree were both correct. A `git add -A && git commit` in that window would have committed a silent partial revert with every test still passing. - **A batched edit script that writes once at the end loses ALL edits if any assert fires** — and I then reported the discarded edit as landed. Apply, write and re-read each edit individually. **Verification:** `scripts/tests` 771 passed / 2 skipped locally; CI `Script tests (pytest)` green in 4m12s with 773 collected — identical to local, so nothing silently skipped. The three new files run on the Linux runner host (git 2.47.3): 41 passed, and the pre-push deletion mutation reddens there too. Every commit green in isolation. No flakiness across 5 sequential + 3 concurrent runs. All 18 proof refs resolve to a real `def`, checked independently of the inventory guard. Production hook-fire log: head adds 0 records, verified at full-suite scale on both hosts. **Deferred:** #809 (generalise the production-log isolation guard beyond one file — includes why the obvious per-test log diff is racy against a live session's hooks). #790 already tracks clause-level grading. Two workflow channels deliberately not covered because they are undecidable outside the runner, and named as such in `_falsey`'s docstring: a job `if:` false only at run time, and `continue-on-error` as a non-literal expression. **Docs updated:** `docs/guard-inventory.md` (rows, counts, the ranked list with what each mutation established, what a file-level MUTATION grade does NOT claim, and the three groups every remaining `NONE` row falls into — verified member-for-member against the derived set). **Process note:** five cold review rounds, each finding a defect introduced by the previous round's fix, every one of them a green test over a dead check — the exact class this issue exists to close, reproduced inside the work meant to close it. The pattern broke at round 4 by WITHDRAWING the line-level predicate rather than patching it a fourth time. Limitation stated rather than hidden: Codex failed twice to produce a cross-family review, so all five rounds came from one model family.
timothy removed the in-progress label 2026-08-21 22:10:43 +02:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
ad-hoc
One-off / ad-hoc work not tracked by a dedicated issue
 api
REST API / HTTP endpoints
 bug
Something isn't working
 ci-cd
Build, test, deploy pipeline
 content
Channel content / schedules / playlists
 dependencies
Dependency updates (Renovate)
 enhancement
New feature or improvement
 frontend
ChicoryTV React SPA frontend
 in-progress
Claimed by an active session — do not pick up
 jellyfin
Jellyfin tuner / IPTV integration
 parked
Excluded from automatic queue pickup; work only when explicitly selected
 priority: high
priority: low
priority: medium
review
Adversarial review finding
 security
Security / vulnerability fix
No labels ci-cd priority: high
Milestone
No items
No Milestone Defect-shape hardening
Projects
Clear projects
No projects
Assignees
Clear assignees
renovate timothy (Timothy)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: timothy/ersatztv#785