The prior commit passed a standalone 'prettier --check README.md' in the author's environment but FAILED 'pnpm format:check' in the repository, which applies the repo's own prettier configuration. Same tool, different configuration, opposite answer — and I pushed past the warning rather than stopping at it. Formatting only; no content change. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01YKj59Qadrb2WBLaePvkM7H
24 KiB
push-guard
Mechanical closure of one defect: a verification that passes when the thing it verifies never happened.
Three incidents in one evening, three agents, no coordination, same shape:
| # | Incident | Why the check passed |
|---|---|---|
| 1 | PUSH VERIFIED reported after nothing was pushed |
Commit had aborted, so local HEAD trivially equalled the remote ref |
| 2 | An empty commit carrying another commit's message was pushed | A push was chained after a failed commit and ran on stale state |
| 3 | Conflict markers + invalid JSON committed into 70 generated files | Nothing asserted the generated output still parsed |
Each is an assertion satisfied by the null case. push-guard.sh asserts the positive fact instead: a new object exists, the remote moved, the content parses.
Checks
| Sub-command | Asserts | Exit on failure |
|---|---|---|
check-staged |
index has no unmerged paths | 2 |
| no conflict markers in staged content | 2 |
|
| the conflict scan actually completed | 2 |
|
| staged JSON under the nominated paths parses | 3 |
|
| something is actually staged | 5 |
|
| an explicit JSON decision exists | 6 |
|
push |
HEAD advanced past --since-head |
5 |
| HEAD is a non-empty commit | 5 |
|
| remote moved, and now equals local HEAD | 4 |
Usage
push-guard.sh check-staged --json-path 'data/**/*.json'
BEFORE=$(git rev-parse HEAD)
git commit -m "..."
push-guard.sh push --remote origin --branch main --since-head "$BEFORE"
--since-head is what distinguishes "committed nothing" from "committed something", and capturing the remote ref before pushing is what makes remote == local mean anything. Both were missing from the guard that produced incident 1.
Design decisions that measurement forced
These are the interesting part; each one was wrong in the first draft.
A bare ======= is deliberately NOT treated as a conflict marker. It collides with reStructuredText underlines and ASCII rules. Every genuine conflict git writes also contains <<<<<<< and >>>>>>>, so requiring those loses no real detection. Measured against a real repository: zero false positives across the entire tracked tree.
The JSON check is opt-in by path, not on-by-default. The first draft checked every staged *.json, on the reasoning that broader is strictly stronger. Measured against the same repository: 17 of 119 tracked .json files fail a strict parse, all legitimately — every tsconfig*.json is JSONC (comments are legal there) and the CA templates are Go templates that merely carry a .json suffix. An on-by-default check fires on ~14% of the repo's JSON, and a guard that cries wolf gets routed around until the bypass is habitual — at which point the bypass covers the true positives too. Broader was weaker.
--json-path values are normalized to :(glob) magic. Git's default pathspec matching makes data/**/*.json require at least one intermediate directory: it matches data/sub/b.json and silently skips data/a.json. The obvious spelling would have delivered partial coverage with no warning.
Found by independent review, after the needles were already green
An adversarial review (author ≠ reviewer) found two genuine fail-opens that 17 self-written needles, five mutation runs and a repo-wide false-positive measurement had all missed. Both were reproduced before being fixed, and both now have needles.
Renamed files were invisible to every content check. Git detects renames by default (diff.renames=true since 2.9), so git mv plus a small edit is reported as a single R entry — which --diff-filter=ACM does not match. Reproduced at 97% similarity: the guard printed staged content OK and exited 0 with conflict markers in the staged blob. Fixed with --no-renames.
There is a trap inside this one worth recording. With only the renamed file staged, the guard exits 5 — the nothing-staged check fires because the file list came back empty. That looks like a catch and is pure accident; co-stage one ordinary file and the fail-open is total. The needle deliberately co-stages a clean file so it cannot pass for the wrong reason.
-I skipped files marked binary in .gitattributes, while the summary still counted them as scanned — a clean pass and a false count. Fixed with -a.
The review also correctly identified one vacuous control: the positive --since-head case asserted only exit 0, which the push produces anyway, so deleting the entire --since-head block left it passing. It now asserts on output.
Two reported findings did not reproduce and were not acted on: :(glob) does still match a bare directory pathspec, and my first rename repro failed only because the edit dropped similarity below the detection threshold — a badly built test, not an absent bug. Re-testing properly is what confirmed it.
The JSON decision is mandatory (.push-guard.json)
The first release announced JSON check NOT REQUESTED and continued. That was visible rather than silent, which is better — but it is still an absence, and an absence is not reviewable. Nobody reads a line that has printed correctly ten thousand times. A repo that needed the check and never wired it up stayed unprotected forever, and nothing ever failed.
The decision is now required, from one of exactly two places:
// nominate generated JSON for parse-checking
{"json_paths": ["data/**/*.json"], "allow_invalid_json": ["fixtures/*"]}
// or opt out — the reason is REQUIRED and is printed on every run
{"json_check": "none", "reason": "no generated JSON in this repo"}
--json-path on the command line also satisfies it. Saying nothing is refused (exit 6).
The asymmetry is deliberate. Turning the check on is safe from anywhere, so a CLI flag suffices. Turning it off is confined to a committed file, because that is the only form a human can review: you can read a reason in a diff, and you cannot review the fact that nobody typed a flag. An opt-out living in an ad-hoc command line is the old fail-open with extra steps.
A malformed config is refused outright rather than treated as absent — that fallback would mean a typo silently disables the check the file was written to enable. A config that parses but states nothing ({}) is refused too: valid JSON that says nothing is the original defect wearing a config file as a disguise.
Migration is a hard cutover, on purpose. The tempting path is "warn for one release, then enforce" — but that warning phase is the degrade-to-a-warning this tool forbids, and it leaves the fail-open open for exactly as long as the warning is ignored, which is indefinitely. Consumers add a config or the guard refuses. It breaks loudly, once. Two pre-existing cases in this repo's own harness were the first to pay that cost, which is the correct place to feel it.
Known limitations — stated, not hidden
check-stagedinspects the index. Content added to the working tree after it runs is not covered — it belongs in apre-commithook, where the window is smallest.pushhardcodesHEAD:refs/heads/$branch, so it cannot verify a commit built via plumbing on a different base. A--shaoption would close this; it is deliberately not added without a needle.
Test harness
test-push-guard.sh — 46 cases, each check in both polarities:
- NEEDLE — a deliberately broken fixture that must trip the guard.
- CONTROL — a clean fixture that must pass.
Controls are not decoration. A guard that failed unconditionally would satisfy every needle and look fully covered. Several controls additionally assert on the guard's output (--out), because exit 0 cannot distinguish "checked the files and they were fine" from "matched no files and had nothing to check" — two controls in an earlier revision were passing vacuously for exactly that reason.
Mutation results — the harness was itself tested by breaking the guard
Everything in this section is regenerated by ./mutate-push-guard.sh, not typed. The previous version of this table was hand-maintained: it was true when written, went stale as the suite grew, and ended up asserting unmodified | 32/32 pass against a 46-case suite. A README is what a reader consults when the tool misbehaves, so a confidently-wrong one is worse than none. Re-run the script and paste; do not edit the numbers.
| mutation | suite result | verdict |
|---|---|---|
| unmodified | 46 passed, 0 failed | baseline |
json decision requirement bypassed (L482) |
3/43 fail | killed |
opt-out accepted with no written reason (L334) |
1/43 fail | killed |
committed re-read of the opt-out skipped (L405) |
5/43 fail | killed |
untracked config honoured as an opt-out (L409) |
1/43 fail | killed |
staged-but-uncommitted opt-out honoured (L425) |
1/43 fail | killed |
committed SYMLINK config honoured (L433) |
1/43 fail | killed |
unparseable committed config ignored (L444) |
1/43 fail | killed |
local-only opt-out (HEAD says ON) honoured (L459) |
1/43 fail | killed |
empty MERGE exempted (L704) |
1/43 fail | killed |
empty ROOT exempted (L715) |
1/43 fail | killed |
--since-head ancestry check removed (L665) |
1/43 fail | killed |
staged-file enumeration ignores git failure (L173) |
1/43 fail | killed |
malformed config degrades to absent instead of refusing (L375) |
4/43 fail | killed |
13 mutants, 0 survived.
Why the denominator is 43 while the suite reports 46. The generator attributes kills only to cases it can parse by name, which is the PASS [KIND] <name> (exit N) form. Of the 46 passing assertions, 45 print PASS and 43 of those match that form. The three excluded lines are:
PASS [CONTROL] guard runs without emitting any interpreter warning
PASS [NEEDLE ] the warning detector fires on a known warning string
ok [e9-fixture ] fixture is a merge (3 fields) with tree identical to both parents
The two z1 warning assertions assert on a whole class of output rather than an exit code, so they carry no (exit N); e9-fixture is a fixture precondition and prints in the ok form. All three still run and still gate the suite — they are excluded from attribution, not from execution.
An earlier revision of this paragraph named the excluded set as w2-fixture, e9-fixture and g1-fixture. That was written from memory instead of from the output, and two of the three names were wrong: w2-fixture belongs to test-verify-clean-clone.sh and g1-fixture to test-mutate-push-guard.sh, so neither runs in this suite at all and neither could contribute to its tally. A reader auditing the denominator would have gone looking for them in the wrong files. It is recorded rather than quietly corrected because a confidently-wrong provenance inside the section that exists to make a generated number auditable is the same defect as everything else on this page: a claim that reads as measured and is not. The set above was produced by running the suite and applying the generator's own parser to its output.
Earlier mutants, run at the suite size of the day and kept as history rather than as a live claim: fail-open (9/16), always-fail (16/16 — controls catch it), revert :(glob) normalization (3/16, caught only by the --out assertions), drop the remote-did-not-move assertion (1/16), restore blanket || true on the scan (1/17), revert --no-renames (1/19), revert -a to -I (1/19), delete the --since-head block (2/19, incl. the control that was vacuous), stray-warning emission (1/32).
A guard nobody has watched fail is not a guard. The same applies to the harness: the :(glob) mutant would have passed silently without the output assertions, so the assertions are load-bearing rather than ornamental.
Two branches were uncovered, and writing this table is what found them
Building the generator turned up two mutants that survived a 46-case suite reporting 46/46 green: staged-but-uncommitted opt-out honoured and unparseable committed config ignored. Neither branch had any case at all. Both are now needled, which is why they read killed above.
Both survivors share a shape worth naming: the mutant still exits 6, because control falls through to a sibling refusal that rejects for a different reason. An exit-code-only assertion would have been satisfied by the wrong branch. The new cases anchor on the distinguishing clause instead.
The same audit found a live instance of that defect already in the suite. Three separate branches print the headline OPT-OUT IS NOT REVIEWABLE — untracked, staged-not-committed, and symlink — and the untracked needle was anchored on the shared headline. Delete the untracked branch and control reaches a sibling printing those same words, so the needle would not fail; it would re-point. A substring anchor does not fail when its subject is removed. It is now anchored on is not tracked in git.
The generator refuses three ways a mutation run can lie
- The anchor no longer matches. The mutant is never applied, the suite is green, and a naive report says survived — the same word a real coverage gap gets.
ANCHOR MISSINGis a loud failure instead. - The anchor matches prose. This one landed on the first run: a mutant aimed at the refuse-on-missing-config branch matched inside the
usage()heredoc, edited a help string, changed no behaviour, and duly reported survived. A documentation edit was one step from being recorded as an uncovered branch. A mutation that cannot change behaviour is not a surviving mutant, it is a non-measurement — and a non-measurement reported as a result is the same defect as the vacuous test the harness exists to hunt. Anchors resolving insideusage()are now refused, and the heredoc's bounds are located at runtime rather than hardcoded. - The anchor is ambiguous. Two unrelated branches here are both the single line
if (( status != 0 )); then; a first-match replace would silently attribute the kill to whichever came first. Multi-line anchors are supported and a match count!= 1refuses rather than guesses.
A blanket || true on the scan was a fail-open in the guard's own error handling. git grep exits 1 for "no match" but >=2 for a real failure. || true collapsed the two, so a malformed pathspec or unreadable index would have been reported as "ok, no conflict markers" — a clean pass from a scan that never ran. The status is now discriminated, and a PATH-shim fault-injection needle proves the refusal.
A <<'PY' heredoc inside $( ) made bash print a warning on every run — and 30 needles plus a clean linter all missed it. The config parser started life as an inline heredoc inside a command substitution, so every invocation emitted warning: command substitution: 1 unterminated here-document to stderr. It executed correctly, every needle stayed green, and shellcheck reported nothing. The harness missed it because expect asserts substrings it was told to look for — and nobody tells you to look for output you did not know existed. It surfaced only when the guard was run against a real repository.
The fix moves the parser to a top-level constant. The lesson is encoded as case z1, which asserts the absence of a whole output class rather than the presence of an expected string, and is paired with a needle proving the detector can fire. A tool built to refuse quiet failures was quietly polluting stderr for its entire existence.
The -E flag on git grep is load-bearing and was caught by a needle, not by review. git grep defaults to basic regex, in which (, | and {7} are literal characters. Without -E the patterns match nothing and the check reports a clean pass over a file full of conflict markers — the exact defect this tool exists to prevent, shipped inside the tool itself.
Then the review turned on the harness, and found three more
A second independent review ran everything from a fresh clone and reproduced three defects — all of them in the tools written to prevent defects. None was in push-guard.sh.
1. The clean-clone verifier could not verify the tree that ships it. verify-clean-clone.sh resolved the repository top level correctly but then passed bare basenames to git ls-tree, which is a root-relative pathspec. These files really live at packages/mosaic/framework/tools/git/, so in place every artifact came back NOT TRACKED at HEAD and the verifier exited 1 without ever running. The tool built to stop packaging false-greens was unusable against its own packaging.
Its suite could not see it, because every fixture installed the artifacts at the fixture repository root. 6/6 green proved flat-layout operation and said nothing about the deployed path. That is the third time on this tool that a control validated a model instead of the subject: v1 of the verifier measured a cp'd scratch repo, and then v2's own tests measured a layout that does not exist. A fixture is a claim about the world; an untested fixture is an unreviewed one. The committed prefix now comes from git rev-parse --show-prefix and is threaded through ls-tree, the cloned stat, and the suite's working directory; w6-nested builds the real nested layout, w6-prefix asserts the verifier reports that prefix (so a green w6 cannot mean the prefix was harmlessly ignored), and w7-nested-mode proves the mode needle still bites down there.
2. Any pre-existing suite failure satisfied every mutant. The generator called a mutant KILLED whenever failed > 0, and never required a green baseline. Inject one always-failing case that changes no guard behaviour whatsoever and the run reports 13 killed, 0 survived, emits the table above, and exits 0. A tally is not evidence; a named delta is. The generator now refuses outright unless the unmodified baseline is exit-0 with zero failures — and emits no table when it refuses — then scores each mutant by the named cases that stopped passing, printing the first one (by: <case>) beside every kill.
3. An interrupted run stranded a mutated push-guard.sh in the reviewed tree. Restoration leaned on an EXIT trap. A trap is cleanup, not isolation, and SIGKILL cannot run it. It happened to the reviewer twice and contaminated the following suite run until the clone was discarded. Isolation is now by construction: the guard and suite are installed into a temp dir and every mutation is applied to that copy, so the reviewed file is never opened for writing at all.
Note the deliberate asymmetry with verify-clean-clone.sh, which forbids cp anywhere in the file. The rule is not "never copy" — it is know whether the copy preserves the property you are about to measure. cp launders mode, so the verifier must not copy; mutation is destructive by design, so the generator must.
test-mutate-push-guard.sh (8 cases) now covers all three: g1-* proves a red baseline is refused with no table, g2-* is the positive control plus an assertion that kills are attributed by name, and g3-* kills the generator mid-mutation and asserts the subject is byte-identical afterwards.
That last one was vacuous on its first attempt. timeout -s KILL 3 looked convincing and proved nothing: at three seconds the generator is still running its baseline, so no mutation has been applied and the subject is trivially unchanged — for the unfixed in-place generator too, which I confirmed by rebuilding it and running it. The kill is now driven from inside the run (the fixture's suite counts its own invocations and kills the generator on the second, when mutant #1 is applied), and g3-needle-bites puts the reconstructed pre-fix mechanism through the identical kill to prove it does strand a mutated file. A control written to close a blocker was itself a member of the vacuous family.
Two smaller things fell out of building those cases, both worth recording because both read as the opposite of what they were:
grep -qunderpipefailturns a successful match into a failed assertion.grep -qexits at the first match, the producer dies of SIGPIPE, andpipefailreports 141. This cost a redw6-prefixagainst a verifier that was printing the right prefix all along. Capture into a variable and test the variable.$PPIDinside$( )is the subshell, not the caller. Killing it merely ends the command substitution; the parent carries on and exits 0. The fixture useskill -9 0(the process group) with the generator launched undersetsid --wait.
Linting is measured at default severity, and the earlier claim was not. "shellcheck clean on all five" was published on the strength of shellcheck -S warning, which exited 0 — while the default severity exited 1 with twelve SC2016 findings. A filtered measurement reported as an unfiltered claim is the same shape as everything else on this page. Those literals genuinely must not expand, so run_mutants() carries one scoped, documented SC2016 suppression; all six files are now clean at default severity. (A documented "this literal is intentionally unexpanded" is a different thing from a comment asserting a safety property nobody rechecks.)
Not independently reproduced here: blocker 1's original repro ran against the real PR checkout, and this session has no credential for that remote. The w6/w7 fixtures replicate the layout at the exact deployed prefix instead, which is a reconstruction, not the original observation. Stated rather than glossed.
Proposed framework path
framework/tools/git/push-guard.sh # the guard
framework/tools/git/test-push-guard.sh # 46 needles and controls
framework/tools/git/mutate-push-guard.sh # regenerates the mutation table above
framework/tools/git/test-mutate-push-guard.sh # 8 needles for the generator
framework/tools/git/verify-clean-clone.sh # proves the COMMITTED artifact runs
framework/tools/git/test-verify-clean-clone.sh # 9 needles for the verifier
Matches the existing tools/git/test-*.sh convention. Dependencies: bash 4.4+, git, python3 — python3 is already an accepted dependency of ci-queue-wait.sh.
All six must be committed mode 100755. They were once delivered 100644, so a clone exited 126 Permission denied for everyone who was not the author; verify-clean-clone.sh exists to make that unshippable and asserts the mode from git ls-tree of the source commit, never from the filesystem.
Operator-agnostic: no hostnames, credentials, remotes, or operator-specific paths. Clean under the framework-PR firewall.