diff --git a/BUILD-LOG.md b/BUILD-LOG.md index b912071b1..00a6a6682 100644 --- a/BUILD-LOG.md +++ b/BUILD-LOG.md @@ -5816,3 +5816,22 @@ Before: agents can read the library; supporting-path and push-credential depende 2026-10-26 | Sage | mos1884/#662 scope after330/331landing | ReadQ222/Q261/Q263 and659/660/662; posted bounded human-only factory settings/triage/defaulttwo-roundmandate/trackedno-exemptionflake scope with10acceptance criteria, task662comment1338 exactwholebody/author13GETverified. No implementation/review/gate launched.660c1335notready(review/merge, noVM); gatefacts1329/1334 retained; target integration/fullrunner confirmation follows readiness, no localfallback. Landedproduct/evidence unchanged;657/658backlog. 2026-10-26 | Sage | Mos1909/1915/1917 real postlanding reviews | Boyd/Dewey/Filbert/Gladstone real authored candidate52ba verdictfiles independently byte/hash-verified, allAPPROVE scoped; Dewey656broaderPARTIAL retained. Formal330R1/331R4 requests failedpre-send/no comments or approvals; no borrowed/copied/mintedkeys. LegacyhelperGETname jason.woltje is NOT ownJarvis, ownslotabsent; rows tracking-open/#436, files cited via normalqueue. NewFilbertNEW-330-01 filedtask665 exactreadback, notlandingreopen/gate. Repair646-648/652-656 remainOPEN untilactualcomments;1917 conditionalbacklog routing retained. #662scope1338posted;660PR405REVISE/notready. Product1968inputs unchanged; no source/helper/test/gate edits/runs; peerjournal prefixes preserved. + +## 2026-11-02T19:39:18Z: row 211 (#1709, #426) R3, push guard and busy check fixes from the GPT review + +Before: row 211 as picked onto 09a847eda for the lead's landing batch (5734c1a80, c8f2cb70c on +`t3/row211-push-guard-r3-fix`). The GPT review (batch1-a-gpt.md) found five gaps: slot-busy.sh read a +failed cmdline read as an exited process and didn't check its own status read; its shell-snapshot +exclusion matched a substring; the pre-push hook let an existing tag be re-made or moved to a descendant; +the installer wrote through a symlinked hooks directory; the snapshot fixture didn't kill its mutant. The +hook also took hex object names of any length. + +After: one R3 commit by the lead's implementation sub-agent. An unreadable live process makes slot-busy.sh +exit 2 and never print CLEAR, and an exited one is still skipped. The exclusion matches only the wrapper's +shape. Any change to an existing `refs/tags/*` ref is refused. The installer refuses a symlinked hooks +directory, or one that resolves elsewhere, before any write. Object names must be 40 or 64 hex +characters. test-git-guard.sh: RED 67 passed, 18 failed (19:27:01Z to 19:27:06Z), GREEN 85 passed, 0 +failed (19:28:58Z to 19:29:04Z). The run before GREEN gave 80/5 because the new error text said "not +CLEAR"; it was reworded. Mutants 21 of 21 killed (19:30:11Z to 19:33:22Z, base green). No 64-hex test +pin names a changed path. Not pushed. Gaps: the installer's resolved-path refusal has no test of its own +(it needs a bind mount); details in agents/darkwing/work/queue-211/BUILD.md, section "R3". diff --git a/agents/darkwing/work/queue-211/BUILD.md b/agents/darkwing/work/queue-211/BUILD.md index da0b15f70..735c43cde 100644 --- a/agents/darkwing/work/queue-211/BUILD.md +++ b/agents/darkwing/work/queue-211/BUILD.md @@ -149,3 +149,176 @@ happened. The suite, the mutants and every other `scripts/test-*.sh` run in a CL - The hook doesn't check tags differently from branches: a moved tag is refused like a rewritten branch. - `slot-busy.sh` can call a process BUSY that isn't a run, for example an editor with a `test-*.sh` open. It never reads files under a seat's directory, so it can't tell. + +## R3, the GPT review (2026-11-02) + +The lead's implementation sub-agent (Claude Opus 5.5, a child of Sage's T3 c5c1e8ba) wrote this round from +the brief `/mnt/storage/scratch/mosaic-lead/briefs/row211-r3-fix.md` and the review +`/mnt/storage/scratch/mosaic-lead/reviews/batch1-a-gpt.md` (F1 to F5 and the edge-case table). Base +09a847eda, branch `t3/row211-push-guard-r3-fix`: dca5f10cc and 3e39689c1 picked clean as 5734c1a80 and +c8f2cb70c, then one R3 commit. The suite goes from 61 cases to 85 and the mutants from 13 to 21. The +sections above stay as written; where R3 changes them, it says so here. + +### F1: a failed read counted as a vanished process + +What: `slot-busy.sh` skipped any process whose cmdline read failed, so a live process it couldn't read (a +mode-000 cmdline of a `node --test`, say) could end in CLEAR. Its own status read was unchecked too: it put +`$$` in the skip set first, and a failed read only ended the ancestor walk. + +Fix: after a failed read the scan checks the pid directory. Gone means the process exited, and it's +skipped. Still there is an error: the scan names that cmdline, counts it and goes on, so BUSY lines still +print, then exits 2 and never prints CLEAR. Its own status must read and give a numeric PPid, or it exits 2 +before the scan. A failed read further up the walk still only ends the walk, which can give a false BUSY but +never a false CLEAR. + +Tests, section "slot-busy, read errors" (fake `/proc`): "a pid directory with no cmdline: exit 2", "it +names that cmdline and never says CLEAR", "a cmdline that can't be read (a directory): exit 2, no CLEAR", "a +read error beside a BUSY process: exit 2, the BUSY line kept, no CLEAR", "a node --test process whose +cmdline is unreadable (mode 000) is never CLEAR", "a failing read (tr exits 1) with every pid directory +still there: exit 2, no CLEAR", "a process that vanishes during its read isn't an error: exit 0 and CLEAR" +(a `tr` shim on PATH removes the pid directory, then fails) and "its own process unreadable (awk exits 1): +exit 2, no CLEAR" (an `awk` shim). + +Mutants: `unreadable-read-clear` (every failed read skipped), `vanished-is-error` (no directory check, so +an exited process is an error), `read-errors-ignored` (no final error exit) and `self-read-unchecked` (its +own failed read ends the walk, as before). + +### F2: the snapshot exclusion matched a substring + +What: any command line holding `/shell-snapshots/snapshot-` was skipped, so a real `node --test` of a file +under a `shell-snapshots` directory read as CLEAR. + +Fix: `RE_SNAPSHOT` matches only the wrapper's shape: a shell (`sh`, `bash` or `zsh`, with or without a +path), options that include `-c`, then `source` or `.` of a `shell-snapshots/snapshot-*.sh` file as the +first command. Any other command line that names such a path goes through the rules. + +Tests: "busy on: node --test /seat/tests/shell-snapshots/snapshot-guard.test.mjs (node --test)" and "busy +on: bash /seat/shell-snapshots/snapshot-gate.sh (gate)". + +Mutant: `snapshot-substring` (the old substring test put back). + +### F3: an existing tag could be replaced or moved + +What: tags went through the ancestor check, and `git merge-base --is-ancestor` peels a tag. A new annotated +tag object on the same commit, or a lightweight tag moved to a descendant, passed as a fast-forward. + +Fix: after the delete and new-ref checks and before the object and ancestor checks, a `refs/tags/*` ref +whose remote object name isn't zero and differs from the local one is refused ("an existing tag can't be +moved or replaced"). A new tag still passes, and a tag delete is still refused as a delete. This corrects +the tag line in "Known limits" above: a moved tag was refused only when it left its line, not when +it moved to a descendant or was re-made on the same commit. + +Tests, section "tags": "a new annotated tag passes", "push --force of a new tag object on the same commit +is refused", "the remote tag is still the first tag object", "a new lightweight tag passes", "push --force +of a lightweight tag moved to a descendant is refused", "the remote lightweight tag hasn't moved" and "a +tag delete is refused, and the tag stays". + +Mutant: `tag-replace-passes`. + +### F4: a symlinked hooks directory + +What: the installer wrote through `.git/hooks` even when it was a symlink, so an install could set the hook +of every repository sharing that directory, or write outside the repository. + +Fix: `own_hooks` refuses (exit 2) when `/hooks` is a symlink, or when it exists and its physical +path isn't the common directory's physical path plus `/hooks`. It runs before the pre-push symlink check +and again after `mkdir -p`, so both checks come before the temporary file and any install write. The +existing refusals (`core.hooksPath`, a different hook without `--force-install`, a symlinked `pre-push`) +are unchanged. + +Tests: "the install refuses a symlinked hooks directory (exit 2)", "it refuses it with --force-install too" +and "nothing was written in the directory the symlink names". + +Mutant: `hooks-dir-symlink-followed` (`own_hooks` returns at once). + +### F5: the snapshot fixture proved nothing + +What: the fixture wrapper ran `eval 'node --test x.test.mjs'`. There `node` follows a quote, and the node +rule needs a space or `/` before it, so the fixture wasn't BUSY even without the exclusion and the +`snapshot-matched` mutant survived. + +Fix: two wrappers whose text matches a rule without the exclusion. Pid 4194403 runs `node --test +x.test.mjs` straight after the `source ... &&`. Pid 4194405 has the harness's real shape: `source ... +2>/dev/null || true && eval 'bash scripts/test-x.sh > x.log' && pwd -P >| ...`. The paired BUSY case is the +same node command under `/bin/bash -c` with no snapshot source. The F2 cases are above. + +Tests: "an agent's shell-snapshot wrapper isn't a match" (now neither 4194403 nor 4194405) and "busy on: +/bin/bash -c node --test x.test.mjs (node --test)". + +Mutant: `snapshot-matched`, whose site is now the `RE_SNAPSHOT` test. It is killed (3 FAIL). The suite +alone can't fail before this fix, because the old substring exclusion skips these wrappers too; the killed +mutant is the before-and-after evidence. + +### Object names of the wrong length (the review's table) + +What: the hook took any hex string as an object name, so a remote field of `0` read as a new ref. + +Fix: after the non-hex check, each field must be exactly 40 or 64 characters, or the ref is refused. + +Tests: "a remote object name of 0 is refused, not read as a new ref", "a 39-character local object name is +refused" and the control "a 64-character object name passes the length check (64 zeros refuse as a +delete)". + +Mutant: `short-sha-accepted`. + +### What didn't change + +The earlier cases all still pass: an empty push, a new branch and a fast-forward pass; a delete, `--force`, +`+refspec` and `--force-with-lease` rewrites are refused; a remote tip missing locally is refused with +"fetch first", and nothing is fetched. + +### R3 runs + +`TMPDIR` was a fresh `mktemp -d /mnt/storage/scratch/tmp/XXXXXX`. Every run was in the foreground, with +output to files there. + +- RED, all 24 new cases in place and no fix: 67 passed, 18 failed, 2026-11-02 19:27:01Z to 19:27:06Z. The + six new cases that pass there are controls that must hold before and after: both new tags pass, the tag + delete is refused, the 64-character control, the bare `bash -c` node case and the vanished process. +- GREEN: 85 passed, 0 failed, 19:28:58Z to 19:29:04Z. The run before it gave 80 passed, 5 failed: the new + closing error said "not CLEAR", which the "no CLEAR" assertions caught. It now says "the host isn't + known to be idle". +- Mutants: 19:30:11Z to 19:33:22Z, base green (85/0), runner exit 0, 21 of 21 killed, none survived, no + NO MATCH. Every kill exited 1. + +| Mutant | FAIL | First failing case | +|---|---|---| +| ancestor-dropped | 8 | push --force of a rewritten branch is refused | +| unknown-object-passes | 2 | a push over a remote tip missing locally is refused, saying fetch first | +| delete-passes | 12 | the install into a clean clone exits 0 | +| install-over-hookspath | 2 | it says core.hooksPath is set | +| busy-exits-0 | 14 | a node --test process: exit 1 | +| differing-hook-replaced | 3 | the install refuses a different installed pre-push (exit 2) | +| replace-in-place | 1 | the replacement was a rename: the old file keeps its bytes | +| symlink-followed | 1 | the install refuses a symlinked pre-push, even with --force-install | +| ancestor-error-passes | 1 | a failed ancestor check (an unknown local object) is refused | +| snapshot-matched | 3 | no matching process: exit 0 and CLEAR | +| ancestors-matched | 1 | this suite, an ancestor, has no BUSY line | +| no-node-rule | 9 | a node --test process: exit 1 | +| no-compose-rule | 2 | busy on: docker compose -p x up -d (docker compose) | +| unreadable-read-clear | 6 | a pid directory with no cmdline: exit 2 | +| vanished-is-error | 1 | a process that vanishes during its read isn't an error: exit 0 and CLEAR | +| read-errors-ignored | 6 | a pid directory with no cmdline: exit 2 | +| self-read-unchecked | 1 | its own process unreadable (awk exits 1): exit 2, no CLEAR | +| snapshot-substring | 2 | busy on: node --test /seat/tests/shell-snapshots/snapshot-guard.test.mjs (node --test) | +| tag-replace-passes | 4 | push --force of a new tag object on the same commit is refused | +| hooks-dir-symlink-followed | 3 | the install refuses a symlinked hooks directory (exit 2) | +| short-sha-accepted | 2 | a remote object name of 0 is refused, not read as a new ref | + +- Real host: `bash scripts/slot-busy.sh` on the real `/proc` exited 1 on other seats' gate and `node + --test` runs, with no error line. Both live shell-snapshot wrappers on the host matched `RE_SNAPSHOT`. +- Pins: `git grep -nE '[0-9a-f]{64}' -- 'packages/*/tests/*' 'scripts/test-*.sh'` gives 33 lines (the + positive control). None names a path this round changes. The only test that names one is + `scripts/test-git-guard.sh` itself, and no test reads `docs/TOOLS.md`. + +### Known limits after R3 + +- The tag line in "Known limits" above is superseded by F3. The other two lines still hold. +- The resolved-path refusal in `own_hooks` has no test of its own. A hooks directory that isn't a symlink + but resolves elsewhere needs a bind mount, so root, and the symlink check catches the symlink case first. + `hooks-dir-symlink-followed` turns off both checks together. +- A wrapper that sources a test script into its own shell, with no child process, is still skipped, as + before R3. That script's children are still matched. +- The installer's temporary file is named by pid (`.pre-push.install-git-hooks.$$`). R3 leaves it alone. +- The "Runs" section above was written before any run. The R3 runs here are the ones on record for this + round. diff --git a/agents/darkwing/work/queue-211/mutants.sh b/agents/darkwing/work/queue-211/mutants.sh index e40e0d9cc..6df21b36f 100755 --- a/agents/darkwing/work/queue-211/mutants.sh +++ b/agents/darkwing/work/queue-211/mutants.sh @@ -68,7 +68,16 @@ run differing-hook-replaced $I '[ "$FORCE" = 1 ] || die 2 "refused: a different' run replace-in-place $I 'mv -f -- "$tmp" "$HOOK" ||' 'cat -- "$tmp" > "$HOOK" && rm -f -- "$tmp" ||' run symlink-followed $I 'if [ -L "$HOOK" ]; then die 2' 'if false; then die 2' run ancestor-error-passes $H ' *) refuse "$rref: the ancestor check failed' ' *) : "$rref: the ancestor check failed' -run snapshot-matched $S $' if [[ "$c" == *"/shell-snapshots/snapshot-"* ]]; then return 1; fi\n' '' +run snapshot-matched $S $' if [[ "$c" =~ $RE_SNAPSHOT ]]; then return 1; fi\n' '' run ancestors-matched $S ' [ -z "${SKIP[$pid]:-}" ] || continue' ' :' run no-node-rule $S 'if [[ "$c" =~ $RE_NODE ]]; then echo "node --test"; return 0; fi' ':' run no-compose-rule $S 'if [[ "$c" =~ $RE_COMPOSE ]]; then echo "docker compose"; return 0; fi' ':' +# R3, the GPT review (F1 to F5 and the object-name length check). +run unreadable-read-clear $S ' [ -e "$d" ] || continue' ' continue' +run vanished-is-error $S $' [ -e "$d" ] || continue\n' '' +run read-errors-ignored $S '[ "$errors" = 0 ] || {' 'true || {' +run self-read-unchecked $S ' || { echo "slot-busy: error: cannot read its own process" >&2; exit 2; }' ' || p=""' +run snapshot-substring $S '[[ "$c" =~ $RE_SNAPSHOT ]]' '[[ "$c" == *"/shell-snapshots/snapshot-"* ]]' +run tag-replace-passes $H 'if [ "${rref#refs/tags/}" != "$rref" ] && [ "$lsha" != "$rsha" ]; then' 'if false; then' +run hooks-dir-symlink-followed $I 'own_hooks() {' 'own_hooks() { return 0;' +run short-sha-accepted $H ' if ! idlen "$lsha" || ! idlen "$rsha"; then' ' if false; then' diff --git a/docs/SESSIONS.md b/docs/SESSIONS.md index 70d32e024..1edf76dc5 100644 --- a/docs/SESSIONS.md +++ b/docs/SESSIONS.md @@ -1879,3 +1879,5 @@ are never rewritten or removed; corrections are new entries. 2026-10-26 | Sage | mos1884/#662 scope after330/331landing | ReadQ222/Q261/Q263 and659/660/662; posted bounded human-only factory settings/triage/defaulttwo-roundmandate/trackedno-exemptionflake scope with10acceptance criteria, task662comment1338 exactwholebody/author13GETverified. No implementation/review/gate launched.660c1335notready(review/merge, noVM); gatefacts1329/1334 retained; target integration/fullrunner confirmation follows readiness, no localfallback. Landedproduct/evidence unchanged;657/658backlog. 2026-10-26 | Sage | Mos1909/1915/1917 real postlanding reviews | Boyd/Dewey/Filbert/Gladstone real authored candidate52ba verdictfiles independently byte/hash-verified, allAPPROVE scoped; Dewey656broaderPARTIAL retained. Formal330R1/331R4 requests failedpre-send/no comments or approvals; no borrowed/copied/mintedkeys. LegacyhelperGETname jason.woltje is NOT ownJarvis, ownslotabsent; rows tracking-open/#436, files cited via normalqueue. NewFilbertNEW-330-01 filedtask665 exactreadback, notlandingreopen/gate. Repair646-648/652-656 remainOPEN untilactualcomments;1917 conditionalbacklog routing retained. #662scope1338posted;660PR405REVISE/notready. Product1968inputs unchanged; no source/helper/test/gate edits/runs; peerjournal prefixes preserved. + +2026-11-02 | Sage implementation sub-agent (Claude Opus 5.5, child of T3 c5c1e8ba) | row 211 (#1709/#426) R3 from the GPT review, on t3/row211-push-guard-r3-fix over 09a847eda (picks 5734c1a80, c8f2cb70c) | F1-F5 and the object-name length check fixed with tests; suite RED 67/18, GREEN 85/0; 21 of 21 mutants killed; one local commit, not pushed diff --git a/docs/TOOLS.md b/docs/TOOLS.md index e87287484..bb4b4d51c 100644 --- a/docs/TOOLS.md +++ b/docs/TOOLS.md @@ -166,9 +166,9 @@ resolved through the CLI. | `scripts/test-conductor.sh` | Auto-apply selftests (sandboxed) | 17 cases | | `scripts/test-auth.sh` | Auth checkpoint selftests (no Docker) | 13 cases | | `scripts/test-land-work.sh` | Land-work selftests (sandboxed, fake Docker) | 84 cases | -| `scripts/test-git-guard.sh` | Push guard, its installer and `slot-busy.sh` (scratch bare remote, fake `/proc`) | 61 cases | -| `scripts/install-git-hooks.sh [--repo DIR] [--force-install]` | Install the push guard as `pre-push` in the common `.git/hooks` | Refuses a set `core.hooksPath` or a different installed hook; exit 0 ok · 1 failed · 2 refused · 4 usage | -| `scripts/slot-busy.sh` | Busy check: one `BUSY` line per gate or test process, or `CLEAR` | Exit 0 CLEAR · 1 BUSY · 2 error; chain on the exit code | +| `scripts/test-git-guard.sh` | Push guard, its installer and `slot-busy.sh` (scratch bare remote, fake `/proc`) | 85 cases | +| `scripts/install-git-hooks.sh [--repo DIR] [--force-install]` | Install the push guard as `pre-push` in the common `.git/hooks` | Refuses a set `core.hooksPath`, a symlinked hooks directory or a different installed hook; exit 0 ok · 1 failed · 2 refused · 4 usage | +| `scripts/slot-busy.sh` | Busy check: one `BUSY` line per gate or test process, or `CLEAR` | Exit 0 CLEAR · 1 BUSY · 2 error, including a process it can't read; chain on the exit code | | `scripts/gitea-api.sh [body]` | Gitea API helper; reads the JSON credential file or a raw per-seat token file | Token never on argv/stdout | ### Push guard and busy check (row 211, #426) @@ -176,11 +176,14 @@ resolved through the CLI. `scripts/git-hooks/pre-push` refuses a push that deletes a branch, or that moves a branch to a commit that doesn't descend from the remote tip (a `--force`, `--force-with-lease` or `+refspec` push of a rewritten branch). A -new branch and a fast-forward pass. The hook fetches nothing: when the remote -tip isn't in the local repository it refuses and says to fetch first. +new branch and a fast-forward pass. It refuses any change to an existing tag, +including a new tag object on the same commit; a new tag passes. The hook +fetches nothing: when the remote tip isn't in the local repository it refuses +and says to fetch first. `git push --no-verify` skips it, so Gitea's branch protection (ops-01) is the guard that holds. `scripts/install-git-hooks.sh` copies it into the common -`.git/hooks`, which every worktree of the checkout shares, then runs a canary +`.git/hooks`, which every worktree of the checkout shares, refusing a hooks +directory that is a symlink, then runs a canary through `git hook run`. Installing it in the canonical checkout is a step for the lead after the row lands. @@ -194,8 +197,10 @@ bash scripts/slot-busy.sh > "$LOG" && gate Never `bash scripts/slot-busy.sh | tail -1 && gate`: `tail` exits 0, so the gate runs while BUSY. It matches `node ... --test`, a `*gate*.sh`, `mutant`, a `test-*.sh` and `docker compose` command lines, skips itself, its ancestors -and agent shell-snapshot wrappers, and reads only `/proc//cmdline` and -the cwd link. +and agent shell-snapshot wrappers (a shell whose `-c` text starts by sourcing +`.../shell-snapshots/snapshot-*.sh`), and reads only `/proc//cmdline`, the +cwd link and its own and its ancestors' `status`. A process it can't read that +hasn't exited makes it exit 2, never CLEAR. ## Seat launch with registration (`scripts/mosaic`) diff --git a/scripts/git-hooks/pre-push b/scripts/git-hooks/pre-push index 9d67a7e5a..4fa106043 100755 --- a/scripts/git-hooks/pre-push +++ b/scripts/git-hooks/pre-push @@ -3,8 +3,9 @@ # .git/hooks by `scripts/install-git-hooks.sh`; never edit the installed copy. # For each ref git is about to push, it refuses a delete and any update whose # remote tip isn't an ancestor of the local tip: a force-push or a +refspec. -# A new ref passes. It fetches nothing, so a remote tip that isn't in this -# repository refuses. Any failure of the check refuses too. +# It refuses any change to an existing tag, and an object name that isn't 40 +# or 64 hex characters. A new ref passes. It fetches nothing, so a remote tip +# that isn't in this repository refuses. Any failure of the check refuses too. # `git push --no-verify` skips this hook; Gitea's branch protection is the # guard that holds (ops-01). Canon: never force-push. refused=0 @@ -16,6 +17,9 @@ zero() { case "$1" in *[!0]*) return 1 ;; esac return 0 } +idlen() { # $1: an object name. True at 40 (SHA-1) or 64 (SHA-256) characters. + [ "${#1}" -eq 40 ] || [ "${#1}" -eq 64 ] +} while read -r lref lsha rref rsha extra || [ -n "${lref:-}" ]; do if [ -z "${rsha:-}" ] || [ -n "${extra:-}" ]; then refuse "unreadable ref line from git: $lref ${lsha:-} ${rref:-} ${rsha:-} ${extra:-}" @@ -24,6 +28,10 @@ while read -r lref lsha rref rsha extra || [ -n "${lref:-}" ]; do case "$lsha$rsha" in *[!0-9a-f]*) refuse "$rref: a ref line with a non-hex object name (local $lsha, remote $rsha)"; continue ;; esac + if ! idlen "$lsha" || ! idlen "$rsha"; then + refuse "$rref: a ref line with an object name that isn't 40 or 64 hex characters (local $lsha, remote $rsha)" + continue + fi if zero "$lsha"; then refuse "$rref: a delete (remote $rsha); seat branches are never deleted by push" continue @@ -31,6 +39,12 @@ while read -r lref lsha rref rsha extra || [ -n "${lref:-}" ]; do if zero "$rsha"; then continue fi + # An existing tag never changes. A new tag object on the same commit, or + # a tag moved to a descendant, would pass the ancestor check below. + if [ "${rref#refs/tags/}" != "$rref" ] && [ "$lsha" != "$rsha" ]; then + refuse "$rref: an existing tag can't be moved or replaced (remote $rsha, local $lsha); push a new tag instead" + continue + fi if ! git cat-file -e "$rsha^{commit}" 2>/dev/null; then refuse "$rref: the remote tip $rsha isn't in this repository; fetch first, then push again" continue diff --git a/scripts/install-git-hooks.sh b/scripts/install-git-hooks.sh index 03be9d3c6..a110bb1e0 100755 --- a/scripts/install-git-hooks.sh +++ b/scripts/install-git-hooks.sh @@ -7,8 +7,10 @@ # # DIR defaults to this script's own checkout. The hook's bytes are this # checkout's scripts/git-hooks/pre-push. It refuses when core.hooksPath is set -# (git would not run the hook), and when a different pre-push is installed, -# unless --force-install is given. It never edits an installed hook in place: +# (git would not run the hook), when the hooks directory is a symlink or +# resolves anywhere but the common directory's hooks, and when a different +# pre-push is installed, unless --force-install is given. It never edits an +# installed hook in place: # a new hook is linked in, a replacement is renamed over the old one. A canary # then runs the installed hook through git and needs a delete refused. # @@ -55,8 +57,20 @@ HOOK="$HOOKS/pre-push" out=$(g config --show-scope --get-all core.hooksPath 2>/dev/null) [ -z "$out" ] || die 2 "refused: core.hooksPath is set ($(printf '%s' "$out" | tr '\n\t' '; ')), so git would not run the push guard; unset it first" +# The hooks directory must be the common directory's own. Through a symlink +# to a shared directory, the install would set the hook of every repository +# that shares it. Checked before and after mkdir, before any file is written. +own_hooks() { + local real want + [ ! -L "$HOOKS" ] || die 2 "refused: $HOOKS is a symlink; install only into the repository's own hooks directory" + [ -e "$HOOKS" ] || return 0 + real=$(cd -P -- "$HOOKS" && pwd -P) && want="$(cd -P -- "$common" && pwd -P)/hooks" || die 1 "cannot resolve $HOOKS" + [ "$real" = "$want" ] || die 2 "refused: $HOOKS resolves to $real, not $want" +} +own_hooks if [ -L "$HOOK" ]; then die 2 "refused: $HOOK is a symlink; remove it by hand if it is not wanted"; fi mkdir -p "$HOOKS" || die 1 "cannot create $HOOKS" +own_hooks tmp="$HOOKS/.pre-push.install-git-hooks.$$" if [ -e "$HOOK" ]; then [ -f "$HOOK" ] || die 2 "refused: $HOOK is not a regular file" diff --git a/scripts/slot-busy.sh b/scripts/slot-busy.sh index 27f186da5..27edc9791 100755 --- a/scripts/slot-busy.sh +++ b/scripts/slot-busy.sh @@ -6,17 +6,21 @@ # scripts/slot-busy.sh [--proc-root DIR] # # It prints one BUSY line per matching process, or one CLEAR line. Exit codes: -# 0 CLEAR, 1 BUSY, 2 its own error. Chain it on the exit code, never on a -# filter of its output: +# 0 CLEAR, 1 BUSY, 2 its own error, which includes a process it can't read +# that hasn't exited (it still prints any BUSY lines, never CLEAR). Chain it +# on the exit code, never on a filter of its output: # bash scripts/slot-busy.sh > LOG && gate (set -o pipefail if piped) # never `bash scripts/slot-busy.sh | tail -1 && gate`, which runs while BUSY. # # A process matches when its command line holds `node ... --test`, a # `*gate*.sh`, `mutant`, a `test-*.sh` or `docker compose`/`docker-compose`. # It skips itself and its ancestors (a gate script may run the check), and an -# agent harness's shell-snapshot wrapper, whose command line is the agent's -# command text; that shell's real children are still matched. It reads only -# /proc//cmdline and the cwd link, never a file under a seat's directory. +# agent harness's shell-snapshot wrapper (`bash -c 'source .../shell-snapshots/ +# snapshot-*.sh ...'`), whose command line is the agent's command text; that +# shell's real children are still matched, and so is any other command line +# that only names a shell-snapshots path. It reads only /proc//cmdline, +# the cwd link and its own and its ancestors' /proc//status, never a +# file under a seat's directory. # --proc-root names a stand-in for /proc in the selftests. set -uo pipefail @@ -30,22 +34,28 @@ while [ $# -gt 0 ]; do done [ -d "$PROC" ] || { echo "slot-busy: error: $PROC is not a directory" >&2; exit 2; } -# This process and its ancestors, from the real /proc. +# This process and its ancestors, from the real /proc. Its own status must +# read and name a parent; a failed read further up only ends the walk, which +# can give a false BUSY but never a false CLEAR. declare -A SKIP=() -p=$$ +p=$(awk '/^PPid:/{print $2; exit}' "/proc/$$/status" 2>/dev/null) && [[ "$p" =~ ^[0-9]+$ ]] \ + || { echo "slot-busy: error: cannot read its own process" >&2; exit 2; } +SKIP[$$]=1 while [ -n "$p" ] && [ "$p" != 0 ]; do SKIP[$p]=1 p=$(awk '/^PPid:/{print $2; exit}' "/proc/$p/status" 2>/dev/null) || p="" done -[ "${#SKIP[@]}" -ge 1 ] && [ -n "${SKIP[$$]:-}" ] || { echo "slot-busy: error: cannot read its own process" >&2; exit 2; } RE_NODE='[ /]node[^ ]* (.* )?--test ' RE_GATE='[ /_-]gate[^ /]*\.sh ' RE_TEST='[ /]test-[^ /]*\.sh ' RE_COMPOSE='[ /]docker( compose|-compose) ' +# The wrapper's shape: a shell, its options with -c, then `source` (or `.`) +# of a shell-snapshots/snapshot-*.sh file as the first command. +RE_SNAPSHOT='^ (/[^ ]*/)?(ba|z)?sh( -[a-zA-Z]+)* -[a-zA-Z]*c[a-zA-Z]*( -[a-zA-Z]+)* (source|\.) [^ ]*/shell-snapshots/snapshot-[^ /]*\.sh ' match() { # $1: the command line, NULs as spaces. Prints the rule that matched. local c=" $1 " - if [[ "$c" == *"/shell-snapshots/snapshot-"* ]]; then return 1; fi + if [[ "$c" =~ $RE_SNAPSHOT ]]; then return 1; fi if [[ "$c" =~ $RE_NODE ]]; then echo "node --test"; return 0; fi if [[ "$c" =~ $RE_GATE ]]; then echo "gate"; return 0; fi if [[ "$c" == *mutant* ]]; then echo "mutant"; return 0; fi @@ -56,6 +66,7 @@ match() { # $1: the command line, NULs as spaces. Prints the rule that matched. seen=0 busy=0 +errors=0 for d in "$PROC"/[0-9]*; do [ -d "$d" ] || continue pid=${d##*/} @@ -63,7 +74,14 @@ for d in "$PROC"/[0-9]*; do seen=$((seen + 1)) [ -z "${SKIP[$pid]:-}" ] || continue # A process can exit between the listing and the read; that's not an error. - cmd=$(tr '\0' ' ' < "$d/cmdline" 2>/dev/null) || continue + # A failed read while its directory is still there is: the scan can't say + # what that process runs. + if ! cmd=$(tr '\0' ' ' < "$d/cmdline" 2>/dev/null); then + [ -e "$d" ] || continue + echo "slot-busy: error: cannot read $d/cmdline" >&2 + errors=$((errors + 1)) + continue + fi cmd=${cmd% } [ -n "$cmd" ] || continue rule=$(match "$cmd") || continue @@ -72,6 +90,7 @@ for d in "$PROC"/[0-9]*; do busy=1 done [ "$seen" -ge 1 ] || { echo "slot-busy: error: no processes under $PROC" >&2; exit 2; } +[ "$errors" = 0 ] || { echo "slot-busy: error: $errors process(es) could not be read, so the host isn't known to be idle" >&2; exit 2; } if [ "$busy" = 1 ]; then exit 1; fi echo "CLEAR" exit 0 diff --git a/scripts/test-git-guard.sh b/scripts/test-git-guard.sh index c6539f0c0..7ff7041e1 100755 --- a/scripts/test-git-guard.sh +++ b/scripts/test-git-guard.sh @@ -117,6 +117,32 @@ check "a push over a remote tip missing locally is refused, saying fetch first" check "the remote ref is unchanged after that refusal" "$([ "$(rtip seat/w)" = "$moved" ]; echo $?)" check "the guard fetched nothing" "$(! git -C "$C" cat-file -e "$moved^{commit}" 2>/dev/null; echo $?)" +echo "== tags ==" +# An existing tag never changes. Both cases below pass the ancestor check: a +# new tag object on the same commit peels to that commit, and a moved +# lightweight tag's old commit is an ancestor of its new one. +tagtip() { git -C "$B" rev-parse -q --verify "refs/tags/$1"; } +git -C "$W" tag -a -m first t-ann HEAD +out=$(git -C "$W" push origin t-ann 2>&1); rc=$? +check "a new annotated tag passes" "$([ "$rc" = 0 ] && [ "$(tagtip t-ann)" = "$(git -C "$W" rev-parse t-ann)" ]; echo $?)" +first=$(tagtip t-ann) +git -C "$W" tag -f -a -m second t-ann HEAD >/dev/null +out=$(git -C "$W" push --force origin t-ann 2>&1); rc=$? +check "push --force of a new tag object on the same commit is refused" \ + "$([ "$rc" -ne 0 ] && has "$out" "mosaic push guard: refused: refs/tags/t-ann: an existing tag" && has "$out" "$first"; echo $?)" +check "the remote tag is still the first tag object" "$([ "$(tagtip t-ann)" = "$first" ]; echo $?)" +git -C "$W" tag t-light HEAD~1 +out=$(git -C "$W" push origin t-light 2>&1); rc=$? +check "a new lightweight tag passes" "$rc" +git -C "$W" tag -f t-light HEAD >/dev/null +out=$(git -C "$W" push --force origin t-light 2>&1); rc=$? +check "push --force of a lightweight tag moved to a descendant is refused" \ + "$([ "$rc" -ne 0 ] && has "$out" "mosaic push guard: refused: refs/tags/t-light: an existing tag"; echo $?)" +check "the remote lightweight tag hasn't moved" "$([ "$(tagtip t-light)" = "$(git -C "$W" rev-parse HEAD~1)" ]; echo $?)" +out=$(git -C "$W" push origin :refs/tags/t-light 2>&1); rc=$? +check "a tag delete is refused, and the tag stays" \ + "$([ "$rc" -ne 0 ] && has "$out" "refs/tags/t-light: a delete" && [ -n "$(tagtip t-light)" ]; echo $?)" + echo "== the hook, fed by hand ==" z=0000000000000000000000000000000000000000 hookrun() { (cd "$C" && printf '%s\n' "$1" | sh "$C/.git/hooks/pre-push" origin "$B" 2>&1); } @@ -124,6 +150,14 @@ out=$(hookrun "refs/heads/x $(git -C "$C" rev-parse HEAD) refs/heads/x"); rc=$? check "a short ref line is refused" "$([ "$rc" -ne 0 ] && has "$out" "unreadable ref line"; echo $?)" out=$(hookrun "refs/heads/x HEAD refs/heads/x $z"); rc=$? check "a non-hex object name is refused" "$([ "$rc" -ne 0 ] && has "$out" "non-hex"; echo $?)" +out=$(hookrun "refs/heads/x $(git -C "$C" rev-parse HEAD) refs/heads/x 0"); rc=$? +check "a remote object name of 0 is refused, not read as a new ref" \ + "$([ "$rc" -ne 0 ] && has "$out" "isn't 40 or 64 hex characters"; echo $?)" +out=$(hookrun "refs/heads/x $(git -C "$C" rev-parse HEAD | cut -c1-39) refs/heads/x $z"); rc=$? +check "a 39-character local object name is refused" "$([ "$rc" -ne 0 ] && has "$out" "isn't 40 or 64 hex characters"; echo $?)" +out=$(hookrun "refs/heads/x ${z}${z:0:24} refs/heads/x $(git -C "$C" rev-parse HEAD)"); rc=$? +check "a 64-character object name passes the length check (64 zeros refuse as a delete)" \ + "$([ "$rc" -ne 0 ] && has "$out" "refs/heads/x: a delete" && ! has "$out" "isn't 40 or 64"; echo $?)" out=$(hookrun "refs/heads/x 1234567890123456789012345678901234567890 refs/heads/x $(git -C "$C" rev-parse HEAD)"); rc=$? check "a failed ancestor check (an unknown local object) is refused" \ "$([ "$rc" -ne 0 ] && has "$out" "the ancestor check failed"; echo $?)" @@ -162,6 +196,18 @@ ln -s /dev/null "$SANDBOX/ln/.git/hooks/pre-push" out=$(bash "$INSTALL" --repo "$SANDBOX/ln" --force-install 2>&1); rc=$? check "the install refuses a symlinked pre-push, even with --force-install" "$([ "$rc" = 2 ] && has "$out" "is a symlink"; echo $?)" +# A hooks directory that links to a shared one would put the hook outside +# the repository, in every repository that shares it. +fresh "$SANDBOX/sl" +mkdir -p "$SANDBOX/shared-hooks" +rm -rf "$SANDBOX/sl/.git/hooks" +ln -s "$SANDBOX/shared-hooks" "$SANDBOX/sl/.git/hooks" +out=$(bash "$INSTALL" --repo "$SANDBOX/sl" 2>&1); rc=$? +check "the install refuses a symlinked hooks directory (exit 2)" "$([ "$rc" = 2 ] && has "$out" "/.git/hooks is a symlink"; echo $?)" +out=$(bash "$INSTALL" --repo "$SANDBOX/sl" --force-install 2>&1); rc=$? +check "it refuses it with --force-install too" "$([ "$rc" = 2 ] && has "$out" "/.git/hooks is a symlink"; echo $?)" +check "nothing was written in the directory the symlink names" "$([ -z "$(ls -A "$SANDBOX/shared-hooks")" ]; echo $?)" + fresh "$SANDBOX/wd" git -C "$SANDBOX/wd" worktree add -q -b seat/x "$SANDBOX/wd-wt" origin/main out=$(bash "$INSTALL" --repo "$SANDBOX/wd-wt" 2>&1); rc=$? @@ -187,11 +233,13 @@ fakeproc() { # $1: pid, $2: cwd, rest: argv busyrun() { bash "$BUSY" --proc-root "$P" 2>&1; } fakeproc 4194401 /srv node server.mjs fakeproc 4194402 /srv grep -r gate . -fakeproc 4194403 /srv /bin/bash -c "source /home/u/.claude/shell-snapshots/snapshot-bash-1.sh && eval 'node --test x.test.mjs'" +# Both wrappers' text would match a rule without the snapshot exclusion. +fakeproc 4194403 /srv /bin/bash -c "source /home/u/.claude/shell-snapshots/snapshot-bash-1.sh && node --test x.test.mjs" +fakeproc 4194405 /srv /usr/bin/bash -c "source /home/u/.claude/shell-snapshots/snapshot-bash-2-ab.sh 2>/dev/null || true && eval 'bash scripts/test-x.sh > x.log' && pwd -P >| /home/u/cwd" mkdir -p "$P/4194404"; : > "$P/4194404/cmdline" out=$(busyrun); rc=$? check "no matching process: exit 0 and CLEAR" "$([ "$rc" = 0 ] && [ "$out" = CLEAR ]; echo $?)" -check "an agent's shell-snapshot wrapper isn't a match" "$(! has "$out" "4194403"; echo $?)" +check "an agent's shell-snapshot wrapper isn't a match" "$(! has "$out" "4194403" && ! has "$out" "4194405"; echo $?)" fakeproc 4194410 /seat/a node --test packages/x/tests/a.test.mjs out=$(busyrun); rc=$? @@ -215,8 +263,58 @@ rule "mutant" bash agents/x/work/queue-1/mutants.sh M3 rule "test script" bash scripts/test-land-work.sh rule "docker compose" docker compose -p x up -d rule "docker compose" /usr/bin/docker-compose up +# The snapshot exclusion is for the wrapper's shape only: the same command +# without the snapshot source, and real runs whose arguments hold the path. +rule "node --test" /bin/bash -c "node --test x.test.mjs" +rule "node --test" node --test /seat/tests/shell-snapshots/snapshot-guard.test.mjs +rule "gate" bash /seat/shell-snapshots/snapshot-gate.sh rm -rf "${P:?}/4194420" +echo "== slot-busy, read errors ==" +# A pid directory that stays while its cmdline can't be read is an error, +# never a vanished process: exit 2 and no CLEAR. +mkdir -p "$P/4194430" +out=$(busyrun); rc=$? +check "a pid directory with no cmdline: exit 2" "$([ "$rc" = 2 ]; echo $?)" +check "it names that cmdline and never says CLEAR" "$(has "$out" "cannot read $P/4194430/cmdline" && ! has "$out" CLEAR; echo $?)" +rm -rf "${P:?}/4194430" +mkdir -p "$P/4194431/cmdline" +out=$(busyrun); rc=$? +check "a cmdline that can't be read (a directory): exit 2, no CLEAR" "$([ "$rc" = 2 ] && ! has "$out" CLEAR; echo $?)" +fakeproc 4194410 /seat/a node --test a.test.mjs +out=$(busyrun); rc=$? +check "a read error beside a BUSY process: exit 2, the BUSY line kept, no CLEAR" \ + "$([ "$rc" = 2 ] && has "$out" "BUSY 4194410 node --test" && ! has "$out" CLEAR; echo $?)" +rm -rf "${P:?}/4194431" +chmod 000 "$P/4194410/cmdline" +out=$(busyrun); rc=$? +check "a node --test process whose cmdline is unreadable (mode 000) is never CLEAR" "$([ "$rc" != 0 ] && ! has "$out" CLEAR; echo $?)" +rm -rf "${P:?}/4194410" + +mkdir -p "$SANDBOX/failtr" "$SANDBOX/vanishtr" "$SANDBOX/failawk" +printf '#!/bin/sh\nexit 1\n' > "$SANDBOX/failtr/tr" +printf '#!/bin/sh\nexit 1\n' > "$SANDBOX/failawk/awk" +# This tr stands in for a process that exits during the read: for pid +# 4194440 it removes the pid directory and fails; otherwise it is tr. +cat > "$SANDBOX/vanishtr/tr" <&1); rc=$? +check "a failing read (tr exits 1) with every pid directory still there: exit 2, no CLEAR" \ + "$([ "$rc" = 2 ] && has "$out" "cannot read" && ! has "$out" CLEAR; echo $?)" +fakeproc 4194440 /seat/v node --test v.test.mjs +out=$(PATH="$SANDBOX/vanishtr:$PATH" bash "$BUSY" --proc-root "$P" 2>&1); rc=$? +check "a process that vanishes during its read isn't an error: exit 0 and CLEAR" \ + "$([ "$rc" = 0 ] && [ "$out" = CLEAR ] && [ ! -e "$P/4194440" ]; echo $?)" +rm -rf "${P:?}/4194440" +out=$(PATH="$SANDBOX/failawk:$PATH" bash "$BUSY" --proc-root "$P" 2>&1); rc=$? +check "its own process unreadable (awk exits 1): exit 2, no CLEAR" \ + "$([ "$rc" = 2 ] && has "$out" "cannot read its own process" && ! has "$out" CLEAR; echo $?)" + out=$(bash "$BUSY" --proc-root "$SANDBOX/no-such-proc" 2>&1); rc=$? check "a missing proc root: exit 2" "$([ "$rc" = 2 ]; echo $?)" mkdir -p "$SANDBOX/empty-proc"