ci: pre-push guard against pushing an uncommitted working-tree change (H13) #424

Merged
timothy merged 1 commits from ci/dirty-worktree-guard into main 2026-07-17 23:25:31 +02:00
Owner

Refs #416 (structural follow-up from that session; not fixes).

Why

The #416 session shipped a PR whose committed tree lacked a fix that lived only in the working tree: a git reset --soft + git add committed a stale index, so the push, CI, and a cold reviewer each saw a different tree — the reviewer "confirmed" a --no-renames fix that hadn't been committed. Local build/test/review operate on the working tree; what ships is the committed tree, and nothing enforced that they match. This is the mechanized half of that lesson (the review-process half — point reviewers at git show <sha>:<file> — stays guidance).

What

New fail-open pre-push hook .claude/hooks/prepush-clean-worktree-check.sh, wired into .husky/pre-push after the H11 rebase check (and before the slow web CI-parity build, so it fails fast). It blocks a push when a file in the branch's diff vs origin/main also has uncommitted working-tree or index changes — i.e. the pushed commit wouldn't match what you built/reviewed.

  • Precise scope (low false-positive): only files in the pushed diff; unrelated uncommitted scratch and untracked files never block.
  • Fail-open on anything undecidable (not a repo, offline, no origin/main); deliberate escape ETV_ALLOW_DIRTY_PUSH=1.
  • Same hook family / rationale as H11 (rebase-not-merge) and H12 (issue-qualification): "make the process rule a hook, not prose to remember" (#303).

Verification

  • Local: shellcheck clean; 4-case matrix — dirty PR-file → block; clean → allow; dirty non-PR file → allow; escape hatch → allow. Dogfooded: this very push ran through the new hook (clean tree → allowed).
  • Cold review (review-only agent): MERGEABLE, no blocker, no false-allow. One MEDIUM noted — pushing already-committed work while deliberately keeping uncommitted WIP in the same file is blocked; that's by-design (the escape hatch covers it), and for our commit-then-push worktree flow it's rare. A warning instead of a block would just be ignored, so the block stays.

Docs

docs/decisions.md entry (folds into docs/decisions/release-ci-governance.md alongside H11/H12 at the next release consolidation).

🤖 Generated with Claude Code

Refs #416 (structural follow-up from that session; not `fixes`). ## Why The #416 session shipped a PR whose committed tree lacked a fix that lived only in the **working tree**: a `git reset --soft` + `git add` committed a stale index, so the push, CI, and a cold reviewer each saw a *different* tree — the reviewer "confirmed" a `--no-renames` fix that hadn't been committed. Local build/test/review operate on the working tree; what ships is the *committed* tree, and nothing enforced that they match. This is the mechanized half of that lesson (the review-process half — point reviewers at `git show <sha>:<file>` — stays guidance). ## What New fail-open pre-push hook `.claude/hooks/prepush-clean-worktree-check.sh`, wired into `.husky/pre-push` after the H11 rebase check (and before the slow web CI-parity build, so it fails fast). It **blocks a push when a file in the branch's diff vs `origin/main` also has uncommitted working-tree or index changes** — i.e. the pushed commit wouldn't match what you built/reviewed. - **Precise scope** (low false-positive): only files in the pushed diff; unrelated uncommitted scratch and untracked files never block. - **Fail-open** on anything undecidable (not a repo, offline, no `origin/main`); deliberate escape `ETV_ALLOW_DIRTY_PUSH=1`. - Same hook family / rationale as H11 (rebase-not-merge) and H12 (issue-qualification): "make the process rule a hook, not prose to remember" (#303). ## Verification - Local: shellcheck clean; 4-case matrix — dirty PR-file → **block**; clean → allow; dirty **non**-PR file → allow; escape hatch → allow. Dogfooded: this very push ran through the new hook (clean tree → allowed). - Cold review (review-only agent): **MERGEABLE**, no blocker, no false-allow. One MEDIUM noted — pushing already-committed work while deliberately keeping uncommitted WIP in the *same* file is blocked; that's by-design (the escape hatch covers it), and for our commit-then-push worktree flow it's rare. A warning instead of a block would just be ignored, so the block stays. ## Docs `docs/decisions.md` entry (folds into `docs/decisions/release-ci-governance.md` alongside H11/H12 at the next release consolidation). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Author
Owner

Cold-context review-only agent, read the committed hook + .husky/pre-push wiring + the H11 sibling for house style.

  • No false-allow of the target bug: dirty = git diffgit diff --cached is a superset of "worktree ≠ committed(HEAD)", so the #416 trap (fix in worktree, stale index committed) is caught. origin/main...HEAD correctly scopes the pushed diff; comm -12 inputs identically sort -u'd (space-paths verified on macOS).
  • Fail-open confirmed: set -uo pipefail without -e; every undecidable branch is an explicit || exit 0 / -z guard; no pipeline $? tested except via ||, so pipefail can't spuriously block. Runs under its own bash shebang (mode 100755), so <(...)/comm are fine regardless of husky's launcher.
  • All adversarial cases pass (non-pushed dirty file → allow; untracked → allow; staged-uncommitted → block; clean/amend → allow; detached HEAD; first push; offline → allow).
  • MEDIUM (by-design): pushing committed work while keeping uncommitted WIP in the same pushed-diff file is blocked; escape ETV_ALLOW_DIRTY_PUSH=1. Kept as a block (a warning would be ignored; rare in our commit-then-push flow). NITs: redundant git fetch after H11 (harmless, keeps the hook self-contained); docs updated in-PR (good).

Review-verdict: MERGEABLE @ f59cd187

Cold-context review-only agent, read the committed hook + `.husky/pre-push` wiring + the H11 sibling for house style. - **No false-allow of the target bug**: `dirty` = `git diff` ∪ `git diff --cached` is a superset of "worktree ≠ committed(HEAD)", so the #416 trap (fix in worktree, stale index committed) is caught. `origin/main...HEAD` correctly scopes the pushed diff; `comm -12` inputs identically `sort -u`'d (space-paths verified on macOS). - **Fail-open confirmed**: `set -uo pipefail` without `-e`; every undecidable branch is an explicit `|| exit 0` / `-z` guard; no pipeline `$?` tested except via `||`, so `pipefail` can't spuriously block. Runs under its own `bash` shebang (mode 100755), so `<(...)`/`comm` are fine regardless of husky's launcher. - All adversarial cases pass (non-pushed dirty file → allow; untracked → allow; staged-uncommitted → block; clean/amend → allow; detached HEAD; first push; offline → allow). - **MEDIUM (by-design)**: pushing committed work while keeping uncommitted WIP in the *same* pushed-diff file is blocked; escape `ETV_ALLOW_DIRTY_PUSH=1`. Kept as a block (a warning would be ignored; rare in our commit-then-push flow). NITs: redundant `git fetch` after H11 (harmless, keeps the hook self-contained); docs updated in-PR (good). Review-verdict: MERGEABLE @ f59cd187
timothy force-pushed ci/dirty-worktree-guard from f59cd18781 to 0f29da8f00 2026-07-17 21:00:50 +02:00 Compare
Author
Owner

Rebased onto current main (1b355660, past #422/#423) to resolve a docs/decisions.md EOF-append conflict with #423 — kept both entries in order (#416 docs-only entry from main, then the H13 entry). Verified: git diff f59cd187 0f29da8f -- .claude/hooks/prepush-clean-worktree-check.sh .husky/pre-push is empty (the reviewed hook + wiring are byte-identical); the only delta is the decisions.md conflict resolution (insertions only, no deletions; append-only guard passes).

Review-verdict: MERGEABLE @ 0f29da8f

Rebased onto current `main` (`1b355660`, past #422/#423) to resolve a `docs/decisions.md` EOF-append conflict with #423 — kept **both** entries in order (#416 docs-only entry from main, then the H13 entry). Verified: `git diff f59cd187 0f29da8f -- .claude/hooks/prepush-clean-worktree-check.sh .husky/pre-push` is **empty** (the reviewed hook + wiring are byte-identical); the only delta is the decisions.md conflict resolution (insertions only, no deletions; append-only guard passes). Review-verdict: MERGEABLE @ 0f29da8f
timothy force-pushed ci/dirty-worktree-guard from 0f29da8f00 to 1df08e86bb 2026-07-17 23:05:46 +02:00 Compare
Author
Owner

Rebased onto current main (5e0c53c3, past #429/#426) to re-resolve the docs/decisions.md EOF-append conflict — kept all entries in order (incl. #429's shallow-fix entry), H13 last. git diff 0f29da8f 1df08e86 -- .claude/hooks/prepush-clean-worktree-check.sh .husky/pre-push is empty (reviewed hook + wiring byte-identical); only delta is the decisions.md re-resolution (insertions only, append-only guard passes).

Review-verdict: MERGEABLE @ 1df08e86

Rebased onto current `main` (`5e0c53c3`, past #429/#426) to re-resolve the `docs/decisions.md` EOF-append conflict — kept all entries in order (incl. #429's shallow-fix entry), H13 last. `git diff 0f29da8f 1df08e86 -- .claude/hooks/prepush-clean-worktree-check.sh .husky/pre-push` is **empty** (reviewed hook + wiring byte-identical); only delta is the decisions.md re-resolution (insertions only, append-only guard passes). Review-verdict: MERGEABLE @ 1df08e86
timothy merged commit ba707f2e23 into main 2026-07-17 23:25:31 +02:00
timothy deleted branch ci/dirty-worktree-guard 2026-07-17 23:25:31 +02:00
Sign in to join this conversation.