Uh oh!
There was an error while loading. Please reload this page.
fix(2441): the mutation harnesses verify their baseline instead of assuming it - #340
Conversation
…suming it All seven `scripts/tests/*-mutations.py` runners overwrite a tracked file, run the suite, and restore it in a `finally`. That covers a crash. It does not cover SIGKILL, a runner timeout, or a second harness racing the first in the same worktree -- and the mutation left on disk becomes the NEXT run's `pristine`. Every mutation is then measured against a premise nobody typed, the restore writes the corruption back, and the run reports `0 uncaught`, which is byte-identical to real coverage. Fail-open in the one direction that matters, in the tier whose whole job is proving guards catch bugs. It has already destroyed a tracked file here (`scripts/pipefail-early-close.awk`). `scripts/tests/mutation_baseline.py` refuses to start unless every file the runner is about to mutate matches its committed content at HEAD, and every way of not being able to tell -- unreadable file, git that will not run, no HEAD, untracked path, any git status that is neither "matches" nor "differs" -- refuses too. It does not restore: those bytes might be somebody's work. Only the writing path is guarded. `--dry` writes nothing and is what `make check` runs on every push, where refusing on an uncommitted edit would block the pre-push tier for whoever is editing the target. Closestracebloc/backend#2441 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
saqlainsyed007
left a comment
There was a problem hiding this comment.
The harness itself is excellent and I'm not blocking on the code — I verified it holds the house test bar: the "every runner is guarded" check globs *-mutations.py and fails closed on an empty glob (derived, not restated), the guard's own mutation runner distinguishes "suite caught it" from "harness crashed" so a broken harness can't be miscounted as coverage, the refusal assertions require named needles (which file, why, backend#2441, git checkout --), and every cannot-tell arm fail-closes with rc 2. That's the right shape.
One thing to fix before I approve — scripts/tests/mutation_baseline.py docstring (and the selftest comment) says "seven*-mutations.py runners" in three places, but this PR adds the eighth (I count 8 on head; the Makefile's MUTATION_TARGETS has 8). It's wrong the moment it lands, and it reintroduces the exact stale-hardcoded-count trap your own Makefile:277 warns "went stale twice over" — a little off-message for a change whose thesis is "verify the baseline instead of assuming it." Either bump the three "seven"s to eight, or (better, matching the module's own ethos) phrase it without a literal count / derive it.
Two other inline notes (read_bytes micro-nit, duplicated rationale prose) I've marked non-blocking and resolved.
…insyed007) The module docstring and the selftest comment said "seven" *-mutations.py runners in three places -- and this PR adds the eighth, so all three were wrong the moment it landed. Off-message for a change whose thesis is verify the baseline rather than assume it, and it is the same stale-hardcoded-count trap Makefile:277 already records as having "gone stale twice over". Bumping seven to eight would have been wrong for the same reason it was wrong at seven. The counts are gone instead: the guard already derives the roster by globbing *-mutations.py and fails closed on an empty glob, so there was never a reason for prose to hold a second copy of the answer. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
saqlainsyed007
left a comment
There was a problem hiding this comment.
Re-review of f580d83 — my requested change is done. You reworded all three spots to "Every *-mutations.py runner" / "two of the runners" / "Not a hand-written list", dropping the literal count entirely rather than bumping it to eight — which is the better fix and matches the module's own "derive, don't assume" ethos. Verified no "seven" remains. Thank you.
Not re-approving yet for one reason, and it's not yours: the selftests check is red on its reason-citations (the live inventory) step — client-runtime#192 is cited by the blocked-gate.yml exempt anchor across ~21 repos in repo-inventory.yml and that PR was closed without merging, so the citation guard fails the whole run. That's a shared-inventory data problem on develop (the sibling inventory PRs are already in this area), unrelated to your mutation-harness change, but the green gate blocks on any red required check. Once the dead citation is resolved and selftests goes green, this is an approve — the harness itself already cleared review.
LukasWodka
commented
Aug 26, 2026
The fix is already written and green on #342 (past-tense restatement of the anchor + |
Uh oh!
There was an error while loading. Please reload this page.
saqlainsyed007
left a comment
There was a problem hiding this comment.
Re-review of e96f68c. My blocker is resolved — the stale "seven" count is gone (reworded to "Every *-mutations.py runner" / "two of the runners"), and the fleet audit that was reddening CI has cleared, so CI is green now.
Not flipping to approve yet because of the new Bugbot Medium at mutation-baseline-mutations.py:120 ("self-guard trusts a mutated module"), and it's a real one worth answering, not noise: the runner imports mutation_baseline from disk and then asks that same in-memory copy to certify mutation_baseline.py, so a leftover mutation that makes guard() fail-open would let the pristine check pass on the mutation and finally write it back — disabling the guard every other harness then imports. The usual fix is to certify the file with a pristine copy (re-read from a known-good ref / subprocess) rather than the possibly-mutated in-memory module, or to assert the module's own hash before trusting it. Address that (or reply why it can't happen) and I'll approve — everything else is already there.
…tated Bugbot Medium on #340, and it is this change failing in its own terms. The runner imported `mutation_baseline` from disk and then handed that same in-memory copy `mutation_baseline.py` to certify. A run killed mid-mutation leaves a fail-open `guard()` on disk, so: the import binds the broken one, it certifies its own corruption as clean, `pristine` captures the MUTATION, and the `finally` writes it back for good. Every later harness then imports a guard that no longer guards -- backend#2441 fail-open, one level up, inside the fix for it. It now loads the guard from HEAD (`git show HEAD:<rel>`). Not a second copy of the rule -- that would be the thing that drifts -- but the SAME function read from the one version a mid-run kill cannot have touched. Fails closed on no git, no HEAD, no such path, or a file that will not compile, because the fallback IS the defect. Measured: with a fail-open guard planted in the tree, the runner refuses with exit 2 and leaves the corrupted bytes on disk rather than adopting them. Before this it would have written them back as the baseline. The selftest`s per-runner check matched `mutation_baseline.guard(`, so it would have failed the fix; it now matches `.guard(` -- the question is whether a guard runs before the first write, not how it was bound. Two assertions added for the regression itself, both mutation-proven. Verified: make lint clean, selftests 28 passed 0 failed, 12 mutations 0 stale 0 uncaught. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
LukasWodka
commented
Aug 26, 2026
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 248a4a8. Configure here.
saqlainsyed007
left a comment
There was a problem hiding this comment.
Approving 248a4a8. Both blockers are resolved and verified:
- My original finding (the docstring's stale "seven" runner count) — reworded to drop the literal count.
- The Bugbot Medium (self-guard trusts a mutated module) — the runner now loads the guard via
git show HEAD:<rel>into a fresh module and certifies with THAT, so a mid-run kill leaving a fail-openguard()on disk can no longer self-certify and get written back; it fails closed (sys.exit, rc 2) on no-git / no-HEAD / missing-path / won't-compile, and you measured that a planted fail-open guard is now refused rather than adopted. That's the right shape — the pristine copy from the one version a kill can't have touched, not a drifting second rule.
The harness held the house bar throughout (derived coverage glob, mutation-proof, specific refusal needles, deliberate fail-closed). CI green, no open threads. LGTM.
Uh oh!
There was an error while loading. Please reload this page.
The defect
All seven
scripts/tests/*-mutations.pyrunners do the same thing on the tracked file in place:The
finallycovers a crash. It does not cover SIGKILL, a runner timeout, or a second harness racing the first in the same worktree — and nothing else does either. When one of those lands, the mutated text stays on disk, the next run reads it as itspristinebaseline, every mutation is measured against a premise nobody typed, and the restore writes the corruption back.The run then reports
0 uncaught, which is byte-identical to real coverage. Fail-open in the one direction that matters, in the tier whose entire job is proving guards catch bugs. It has already destroyed a tracked file here (scripts/pipefail-early-close.awk, the symptom behind the ticket).Measured, not asserted.
develop's ownbugbot-gate-mutations.py, run against ascripts/bugbot-gate.pywith a line appended to it:Green, and it wrote the corruption back.
Which shape, and why
The ticket offers three; this is shape 1 — assert the baseline before starting, refuse if it cannot be shown clean.
Shape 2 (never touch the tracked file; run in a temp copy) is cleaner in principle and is what
release-train's runner does. It does not transplant here, and that is measured rather than assumed: two of the eight targets are whole-tree gates that callgit ls-filesin the directory they are pointed at, so a plain copy is not merely different — it refuses:Making that work needs a real clone or worktree per run — a different and much larger change. Detecting a corrupted baseline closes the fail-open direction now and does not stand in the way of relocating later.
It does not restore, deliberately: a modified file may be somebody's work in progress, and quietly overwriting that is a worse bug than the one being fixed. The operator is told which file and what to run.
What landed
scripts/tests/mutation_baseline.py— extracted once, called by all eight runners, rather than pasted eight times.guard(ROOT, targets)returns 0 or prints an::error::and returns 2.Cannot-tell is a refusal, never a pass (this repo's rule 3). A missing file, an unreadable one, a
gitthat will not run, a repository with no HEAD, a path git does not track, a target outside the repo, any git exit status that is neither "matches" (0) nor "differs" (1), and any unexpected exception inside the check itself — all refuse.Only the writing path is guarded.
--drywrites nothing, so it has no restore to lose, and it is whatmake checkruns on every push — refusing there on an uncommitted edit would block the pre-push tier for exactly the person editing the target. A corrupted file still cannot hide from--dry: its anchors go stale, which is loud.Against HEAD, not the index — so a committed edit to a mutation target is the sanctioned way to keep working on one, and a staged corruption is still caught.
Two new files complete the tier, because a guard nobody tests is the thing this repo keeps writing up:
mutation-baseline-selftest.py— 26 assertions, real throwaway git repos, every refusal paired with the neighbouring situation that must not refuse. It also derives the coverage claim: it globsscripts/tests/*-mutations.pyand fails if any runner does not callmutation_baseline.guard()before its first write, so a new runner added without the guard reddens. Fails closed if the glob finds nothing.mutation-baseline-mutations.py— 12 mutations, 0 stale, 0 uncaught. Guards its own baseline with the function it mutates.Both wired into
SELFTEST_TARGETS/MUTATION_TARGETS(the lists, not one member of them — .github#300).selftests-covergains two exact-name cases: the shared module, and__pycache__(a gitignored build artifact can never be a suite; this import is the first sibling import underscripts/tests/, and every runner setssys.dont_write_bytecodebefore it so the directory stays clean anyway).Verification
The refusal fires on a dirty tree — one line appended to each runner's target, then the runner:
::error::scripts/house-rules.sh differs from its committed content at HEAD.::error::scripts/pipefail-early-close.awk differs …::error::scripts/bugbot-gate.py differs …::error::scripts/closing-ref-gate.py differs …::error::.github/workflows/customer-priority-bump.yml differs …::error::scripts/branch_owner.py differs …::error::scripts/reason-citations.py differs …::error::scripts/tests/mutation_baseline.py differs …Same situation, same file, before this PR:
rc = 0,0 uncaught.It does not fire spuriously, and the existing coverage is unchanged — every runner end to end on a clean tree, before (
origin/develop) and after:All eight exit 0.
Also green:
make check(ruffall checks passed,shellcheck: clean,actionlint: 0 findings, house-rules clean,selftests-cover: all 19 selftests and 8 mutation runner(s) are wired to a target, and CI runs both tiers,26 passed, 0 failedfor the new suite), andpython3 -m py_compileon every file touched.repo-inventory.ymlis untouched.Closes tracebloc/backend#2441
Note
Medium Risk
Changes how all mutation harnesses gate runs before mutating tracked sources; a bug could block CI/local mutation runs or weaken the baseline check, but production workflow gates are untouched.
Overview
Closes backend#2441: mutation runners that overwrite tracked files could leave corruption on disk after SIGKILL, timeout, or concurrent runs. The next run then treated that text as
pristineand reported0 uncaught— the same output as real coverage.Adds
mutation_baseline.guard()so every*-mutations.pyrunner (except--dry) refuses to start unless mutation targets match committed HEAD viagit diff --quiet HEAD. Ambiguous cases (missing git, no HEAD, untracked targets, unexpected diff exit codes) refuse rather than pass. The guard does not auto-restore dirty files; it names the path and suggestsgit checkout.Wires the guard into all seven existing mutation harnesses before their first
write_text(mutated, withsys.dont_write_bytecodesoselftests-coverstays clean.Adds a full tier for the guard itself:
mutation-baseline-selftest.py(throwaway git repos, paired accept/refuse cases, glob check that every runner calls.guard(before writing) andmutation-baseline-mutations.py(loadsguardfrom HEAD viagit show, not the working tree copy it mutates). Makefile registersselftest-mutation-baseline/mutation-mutation-baselineand extendsselftests-coverallowlists formutation_baseline.pyand__pycache__.Reviewed by Cursor Bugbot for commit 248a4a8. Bugbot is set up for automated code reviews on this repo. Configure here.