fix(git): push-guard harness — resolve three re-review blockers + two corrections
ci/woodpecker/pr/ci Pipeline was successful
ci/woodpecker/pr/ci Pipeline was successful
Authored by installer-7; committed by mos-claude (no credential for this remote). push-guard.sh and test-push-guard.sh are BYTE-IDENTICAL to the previously cleared versions — every change this round is in the harness, confirming the reviewer's framing that none of the three blockers was in the guard itself. B1 verify-clean-clone.sh could not verify the artifact in its real monorepo location: it resolved ROOT but kept artifacts as bare basenames, so running it in place reported all artifacts NOT TRACKED. Its own suite missed this because every fixture installed artifacts at fixture ROOT — a fixture encoding a layout the real subject does not have. PREFIX now comes from 'git rev-parse --show-prefix' and is threaded through the ls-tree pathspec, the cloned stat, and the suite cwd; the verifier PRINTS the prefix. Three needles: nested-layout pass, prefix-reported (else a green only means the prefix was ignored harmlessly), and mode-needle-still- bites-nested (a prefix threaded into the clone but not ls-tree would silently stop checking modes). B2 the generator reported full coverage and exited 0 on a RED baseline — any pre-existing failure marked every mutant killed. Now refuses unless baseline is exit 0 with zero failures, prints the actual tally on refusal, emits no table, and scores kills by NAMED DELTA rather than a raw red count. B3 the generator mutated the reviewed source in place; SIGKILL stranded a mutant and contaminated a following run. Guard and suite are now copied into a temp dir and mutations apply to that copy — no restore step to fail. The author's first control for this was itself vacuous (a 3s kill lands during baseline, before any mutation, so it passed against the unfixed mechanism too); the real control drives the kill from inside the run on the second suite invocation, plus a needle proving the reconstructed pre-fix mechanism DOES strand. Corrections: 'shellcheck clean' had been measured at -S warning and published unqualified — a filtered measurement stated as an unfiltered claim. Now clean at DEFAULT severity across six files with one scoped, documented SC2016 disable. ARTIFACTS extended five -> six so the test files no longer omit themselves. Verified before commit: six files sha256-matched to the author's hashes; shellcheck exit 0 at default severity; push-guard 46/46 and verifier 9/9 run directly; the in-place verifier resolved the real nested prefix against this tree. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01YKj59Qadrb2WBLaePvkM7H
This commit is contained in:
installer-7
co-authored by
Claude Opus 5
Claude Opus 5
parent
8310075d33
commit
8fdc8738ed
@@ -105,21 +105,21 @@ Controls are not decoration. A guard that failed unconditionally would satisfy e
|
||||
| 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 |
|
||||
| `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.
|
||||
13 mutants, 0 survived. The denominator is the 43 cases the suite reports BY NAME; the three fixture-assertion lines (`w2-fixture`, `e9-fixture`, `g1-fixture`) print in a different format and are excluded from attribution, which is why 46 pass but 43 are attributable.
|
||||
|
||||
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).
|
||||
|
||||
@@ -147,18 +147,46 @@ The fix moves the parser to a top-level constant. The lesson is encoded as case
|
||||
|
||||
**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 `install`ed 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 -q` under `pipefail` turns a successful match into a failed assertion.** `grep -q` exits at the first match, the producer dies of SIGPIPE, and `pipefail` reports 141. This cost a red `w6-prefix` against a verifier that was printing the right prefix all along. Capture into a variable and test the variable.
|
||||
- **`$PPID` inside `$( )` is the subshell, not the caller.** Killing it merely ends the command substitution; the parent carries on and exits 0. The fixture uses `kill -9 0` (the process group) with the generator launched under `setsid --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/verify-clean-clone.sh # proves the COMMITTED artifact runs
|
||||
framework/tools/git/test-verify-clean-clone.sh # 6 needles for the verifier itself
|
||||
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 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.
|
||||
**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.
|
||||
|
||||
Reference in New Issue
Block a user