From 926aefcc8f4179cd7cefa721f31485ec5831660a Mon Sep 17 00:00:00 2001 From: Gadget Lab Date: Mon, 24 Aug 2026 03:02:56 +0000 Subject: [PATCH 1/4] fix(ci): stop pre-commit auto-fix failing when nothing is staged `pre-commit run --all-files` scans the whole repo, so it can modify files a PR never touched. The auto-fix step staged only PR-diff files (correct, per #213) and then ran `git commit` unconditionally. When the hooks had touched ONLY files outside the diff, nothing was staged, `git commit` exited 1, and because steps run under `bash -e` the whole step died - reddening PRs that did not cause the violation. Reproduced before fixing: old logic exits 1 with 'no changes added to commit'. New logic exits 0 and skips the commit. Changes: - split hook-modified files into in-PR and outside-PR sets - guard the commit with `git diff --cached --quiet` (the actual defect: a file can be in the PR diff yet untouched by hooks, so the add stages nothing) - surface outside-PR files as a warning naming them, so a failure here is diagnosable as pre-existing repo debt rather than the author's change #213's protection is preserved and verified: with hooks touching both an in-PR and an out-of-PR file, only the in-PR file is committed. Follow-up promised in #262. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_013JdYCSp4nmVgsNWuwNkUaZ --- .github/workflows/pre-commit-ci.yml | 61 ++++++++++++++++++++--------- 1 file changed, 43 insertions(+), 18 deletions(-) diff --git a/.github/workflows/pre-commit-ci.yml b/.github/workflows/pre-commit-ci.yml index c8b81fa..8c511dd 100644 --- a/.github/workflows/pre-commit-ci.yml +++ b/.github/workflows/pre-commit-ci.yml @@ -63,25 +63,50 @@ jobs: git config --local user.email "github-actions[bot]@users.noreply.github.com" git config --local user.name "github-actions[bot]" - # Check if there are changes to commit - if ! git diff --quiet; then - echo "Auto-fixing issues found by pre-commit hooks..." - # Only stage files that are part of THIS PR's diff, so unrelated - # repo-wide reformatting is never dragged into the PR. - mapfile -t changed < <(git diff --name-only "origin/${{ github.event.pull_request.base.ref }}...HEAD") - if [ ${#changed[@]} -eq 0 ]; then - echo "No PR-diff files to auto-fix; skipping commit." - else - git add "${changed[@]}" - git commit -m "style: auto-fix pre-commit issues [skip ci]" - if for i in 1 2 3; do git pull --rebase origin "${{ github.head_ref }}" && git push origin "HEAD:${{ github.head_ref }}" && break || sleep $((i * 5)); done; then - echo "AUTO_FIXED=true" >> $GITHUB_ENV - else - echo "::warning::Auto-fix push failed (fork/restricted branch) — manual pull required" - fi - fi - else + if git diff --quiet; then echo "No auto-fixable changes found" + exit 0 + fi + + echo "Auto-fixing issues found by pre-commit hooks..." + + # `pre-commit run --all-files` scans the whole repo, so it can modify + # files this PR never touched. Split what it changed into two sets. + git diff --name-only | sort -u > "$RUNNER_TEMP/touched.txt" + git diff --name-only "origin/${{ github.event.pull_request.base.ref }}...HEAD" \ + | sort -u > "$RUNNER_TEMP/in_pr.txt" + comm -12 "$RUNNER_TEMP/touched.txt" "$RUNNER_TEMP/in_pr.txt" > "$RUNNER_TEMP/fixable.txt" + comm -23 "$RUNNER_TEMP/touched.txt" "$RUNNER_TEMP/in_pr.txt" > "$RUNNER_TEMP/outside.txt" + + # Files outside this PR's diff are pre-existing repo debt. Never stage + # them (that was the #213 footgun), but DO report them - they are the + # usual reason this job reddens a PR that did not cause the violation. + if [ -s "$RUNNER_TEMP/outside.txt" ]; then + echo "::warning::pre-commit modified $(wc -l < "$RUNNER_TEMP/outside.txt") file(s) OUTSIDE this PR's diff - pre-existing repo debt, deliberately not staged:" + sed 's/^/ /' "$RUNNER_TEMP/outside.txt" + fi + + if [ ! -s "$RUNNER_TEMP/fixable.txt" ]; then + echo "No PR-diff files to auto-fix; skipping commit." + exit 0 + fi + + xargs -a "$RUNNER_TEMP/fixable.txt" -d '\n' -r git add -- + + # A file can be in the PR diff yet left untouched by the hooks, so the + # add above may stage nothing at all. `git commit` then exits 1 and, + # because GitHub runs steps under `bash -e`, kills this step for a + # non-reason. That is the defect this guard closes. + if git diff --cached --quiet; then + echo "Nothing staged after add; skipping commit." + exit 0 + fi + + git commit -m "style: auto-fix pre-commit issues [skip ci]" + if for i in 1 2 3; do git pull --rebase origin "${{ github.head_ref }}" && git push origin "HEAD:${{ github.head_ref }}" && break || sleep $((i * 5)); done; then + echo "AUTO_FIXED=true" >> $GITHUB_ENV + else + echo "::warning::Auto-fix push failed (fork/restricted branch) — manual pull required" fi - name: Comment on PR From 97a77bc97d80eda1977933d9cfaf3875337e9113 Mon Sep 17 00:00:00 2001 From: Gadget Lab Date: Mon, 24 Aug 2026 03:06:16 +0000 Subject: [PATCH 2/4] test(ci): temporary probe to exercise the auto-fix path Deliberate trailing whitespace + missing EOF newline, so trailing-whitespace and end-of-file-fixer both fire and the rewritten auto-fix step actually RUNS in CI instead of being skipped. Removed again before merge. Co-Authored-By: Claude Opus 5 --- probe-autofix.txt | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 probe-autofix.txt diff --git a/probe-autofix.txt b/probe-autofix.txt new file mode 100644 index 0000000..6dece11 --- /dev/null +++ b/probe-autofix.txt @@ -0,0 +1,2 @@ +temporary probe to exercise the auto-fix path +no trailing newline here \ No newline at end of file From 70336b7bcc2bd4e2653cd72c19f1f5e8b52b61d7 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Mon, 24 Aug 2026 03:06:43 +0000 Subject: [PATCH 3/4] style: auto-fix pre-commit issues [skip ci] --- probe-autofix.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/probe-autofix.txt b/probe-autofix.txt index 6dece11..d774a19 100644 --- a/probe-autofix.txt +++ b/probe-autofix.txt @@ -1,2 +1,2 @@ -temporary probe to exercise the auto-fix path +temporary probe to exercise the auto-fix path no trailing newline here \ No newline at end of file From 92b6f16d04b55afa4c84963c2f93793a5f925dcd Mon Sep 17 00:00:00 2001 From: Gadget Lab Date: Mon, 24 Aug 2026 03:07:25 +0000 Subject: [PATCH 4/4] test(ci): remove the temporary auto-fix probe Its job is done: it forced the rewritten auto-fix step to actually execute in CI (run 32685272621, step 7 -> success, commit 70336b7), proving $RUNNER_TEMP, comm and xargs -d behave on the real runner rather than only in local tests. Co-Authored-By: Claude Opus 5 --- probe-autofix.txt | 2 -- 1 file changed, 2 deletions(-) delete mode 100644 probe-autofix.txt diff --git a/probe-autofix.txt b/probe-autofix.txt deleted file mode 100644 index d774a19..0000000 --- a/probe-autofix.txt +++ /dev/null @@ -1,2 +0,0 @@ -temporary probe to exercise the auto-fix path -no trailing newline here \ No newline at end of file