Files
stack/packages/mosaic/framework/tools/git/push-guard.README.md
T
installer-7andClaude Opus 5 8310075d33
ci/woodpecker/pr/ci Pipeline was successful
fix(git): push-guard — resolve re-review blockers; mutation table now generated
Authored by installer-7; committed by mos-claude (no credential for this remote).

BLOCKER 1 — verify-clean-clone.sh had B1's own defect: it copied WORKING-TREE files into a
  scratch repo and asserted the SCRATCH index, so a stale local exec bit was laundered in and it
  reported success while the committed artifact was still 100644. v2 reads mode from 'git ls-tree'
  of the SOURCE COMMIT and runs from 'git clone --no-local --no-hardlinks' of that commit; there is
  no 'cp' in the file. Proven by A/B against one laundered repo: v1 EXIT=0, v2 EXIT=1 (both observed,
  not asserted). New test-verify-clean-clone.sh 6/6, incl. a fixture that first proves it really is
  git=100644 disk=755 before testing it.
BLOCKER 2 — empty-merge needle + non-empty-merge control, both asserting on OUTPUT; the fixture
  first proves it IS a merge with a tree identical to both parents.

The 'non-blocking' README item was not documentation: regenerating its mutation table (now generated
by mutate-push-guard.sh, not hand-numbered) found TWO genuinely surviving mutants against a 46/46
green suite — staged-but-uncommitted opt-out honoured, and unparseable committed config ignored.
Both still exit 6 under mutation because control falls to a SIBLING refusal, so an exit-code-only
assertion would have been satisfied by the wrong branch. It also found a live instance of the
substring-anchor defect already in the suite: three branches print 'OPT-OUT IS NOT REVIEWABLE', so
deleting one let the needle RE-POINT to a sibling instead of failing. Re-anchored.
Suite 44 -> 46. Verifier 6/6. 13 mutants, 0 survived.

Also: README reformatted to satisfy 'pnpm format:check' (prettier), which was failing CI at both
prior heads — a gate neither author nor reviewer had exercised.

Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01YKj59Qadrb2WBLaePvkM7H
2026-07-30 21:06:55 -05:00

16 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-staged inspects the index. Content added to the working tree after it runs is not covered — it belongs in a pre-commit hook, where the window is smallest.
  • push hardcodes HEAD:refs/heads/$branch, so it cannot verify a commit built via plumbing on a different base. A --sha option 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/46 fail killed
opt-out accepted with no written reason (L334) 1/46 fail killed
committed re-read of the opt-out skipped (L405) 5/46 fail killed
untracked config honoured as an opt-out (L409) 1/46 fail killed
staged-but-uncommitted opt-out honoured (L425) 1/46 fail killed
committed SYMLINK config honoured (L433) 1/46 fail killed
unparseable committed config ignored (L444) 1/46 fail killed
local-only opt-out (HEAD says ON) honoured (L459) 1/46 fail killed
empty MERGE exempted (L704) 1/46 fail killed
empty ROOT exempted (L715) 1/46 fail killed
--since-head ancestry check removed (L665) 1/46 fail killed
staged-file enumeration ignores git failure (L173) 1/46 fail killed
malformed config degrades to absent instead of refusing (L375) 4/46 fail killed

13 mutants, 0 survived.

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

  1. 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 MISSING is a loud failure instead.
  2. 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 inside usage() are now refused, and the heredoc's bounds are located at runtime rather than hardcoded.
  3. 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 != 1 refuses 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.

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/verify-clean-clone.sh    # proves the COMMITTED artifact runs
framework/tools/git/test-verify-clean-clone.sh  # 6 needles for the verifier itself

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 five 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.