Repository navigation
bug(hooks): post-checkout chmods +x onto 13 pytest files and a sourced library, dirtying every working tree on every checkout #15253
Description
Problem
scripts/hooks/post-checkout (installed as .git/hooks/post-checkout) makes hook scripts
executable after every checkout and worktree creation. Its file selection is too broad, so it
also chmods files that must not be executable — and because the executable bit is not what
git has tracked, every working tree in the repository is left permanently dirty.
Observed today: two git worktree add operations each printed
✅ Fixed permissions on 40 hook script(s), and the main checkout was left with 24 mode-only
modifications that regenerate as fast as they are reverted.
Root cause
scripts/hooks/post-checkout:47-57:
find "$HOOK_SCRIPTS_DIR/hooks" "$HOOK_SCRIPTS_DIR/utilities" \
-type f \( -name "pre-commit-*" -o -name "post-commit-*" \
-o -name "inject-*" -o -name "slm-*" \
-o -name "*.sh" \) \
-not -perm -u+x -print0Two patterns over-match:
-name "pre-commit-*"also matchespre-commit-*_test.py. Those are pytest files —
collected by the runner, never invoked directly, and none carries a shebang.-name "*.sh"also matcheshooks/lib/_common.sh, which is a sourced library. Its own
header documents the contract:source "$SCRIPT_DIR/lib/_common.sh"(line 10).
The split, measured
Of the 24 files the hook currently modifies:
| Count | Verdict | |
|---|---|---|
utilities/*.sh with a #! shebang — invoked directly |
10 | executable bit is correct; git's tracked 644 is stale |
pre-commit-*_test.py — no shebang, pytest-collected |
13 | executable bit is wrong |
hooks/lib/_common.sh — no shebang, sourced |
1 | executable bit is wrong |
So the hook is right about 10 files and wrong about 14, and neither set is committed.
Why it matters
- Every session starts with a dirty tree. A session cannot tell its own changes from the
hook's without inspecting modes, andgit statusnoise is what makes real leftovers easy to
miss — this session found a genuine uncommitted documentation fix buried among 24 mode entries. - It defeats "leave nothing uncommitted", which the session-lifecycle protocol requires before
ending a session. The condition is unreachable while the hook re-dirties the tree. - It re-runs on every worktree creation, and the mandated workflow is one worktree per task.
Direction
- Narrow the selection to files that declare themselves executable — filter on a
#!first
line rather than on filename patterns. That is the property that actually justifies the bit,
and it fixes both over-matches at once without an exclusion list to maintain.
As a narrower alternative, add-not -name "*_test.py"and skiplib/, but shebang detection
is the rule that will not need revisiting when the next file type is added. - Commit the 10 correct modes so the hook stops having to fix them on every checkout. That is
a separate, reviewable change — it should not be bundled with the hook fix, and it must not
include the 14. - Consider whether the hook needs to run at all once tracked modes are correct; its purpose is
repairing drift that would no longer exist.
Acceptance criteria
-
post-checkoutdoes not set the executable bit on*_test.pyor onlib/_common.sh. - Selection is based on a shebang, or the exclusions are explicit and documented.
- The 10 genuinely-executable
utilities/*.shfiles are tracked as100755. - A fresh
git worktree addleavesgit statusclean. -
.git/hooks/post-checkoutis re-installed from the tracked source, since existing checkouts
carry the old copy.
References
scripts/hooks/post-checkout:47-57 (tracked source), .git/hooks/post-checkout (installed copy),
autobot-infrastructure/shared/scripts/hooks/lib/_common.sh:10 (documents that it is sourced),
autobot-infrastructure/shared/scripts/hooks/pre-commit-*_test.py (the 13 pytest files).
Activity
fix(hooks): keep the shebang of a first line with no trailing newline (…
fix(hooks): gate post-checkout's chmod on a shebang so worktrees stop…
mrveiss commentedon Aug 29, 2026
Verified in merged
Dev_new_gui— closingPR #15262 merged as
606850dac.Closesdoes not fire on this base, so closing by hand against evidence gathered after the merge.post-checkoutdoes not set the bit on*_test.pyorlib/_common.sh*_test.pyand 0 are underlib/. All 17pre-commit-*_test.pyand_common.shwere confirmed to lack a shebang, so they are excluded on the property itself, not by a listscripts/hooks/post-checkoutreads each candidate's first line and skips anything without#!utilities/*.share tracked100755git worktree addleavesgit statuscleanFixed permissions on N hook script(s)line at all. The same command printed40 hook script(s)before.git/hooks/post-checkoutre-installed from tracked sourcepost-checkout:132-146) copies canonical over the installed copy on any difference;diffof installed vs tracked now reports identical, with no manual stepThe tree this issue was filed about is now clean. The shared checkout went
24 dirty entries -> 0and stays there. Reverting used to be futile — the hook re-dirtied instantly; it now holds.Two corrections to this issue's own numbers
The split of 10 / 13 / 1 was measured from the 24 files that happened to be dirty at the time, not from the full set the rule selects. Measured across all 60 candidates:
100755utilities/*.sh100644*_test.py+lib/_common.sh100644So the pytest files are 17, not 13, and the stale-mode set is 22, not 10.
Why the name patterns were kept as the candidate set
Direction item 1 suggested replacing them with shebang detection alone. Measured, that would have been worse: 51 further
.pyfiles under those two directories carry a shebang and are tracked644, so scanning on shebang alone would newly capture all of them — trading 18 files of recurring dirt for 51. The patterns now nominate candidates and the shebang decides, which fixes both over-matches without enlarging the blast radius.One defect found in the fix itself
The first version read the line as
IFS= read -r firstline < "$f" || firstline="".readreturns non-zero at EOF but still assigns, so a file whose first line has no trailing newline was read correctly and then wiped by the||clause — such a script would have silently stopped being made executable, the same class of bug this issue is about. Fixed in72f9eb5e8by resetting before the read, which also preserves what the||was guarding: an unreadable file must not inherit the previous iteration's value. An independent review reached the identical finding by the same reproduction.On direction item 3
Whether the permission section is needed at all once tracked modes are correct: it is now a no-op in a healthy tree but still repairs one whose modes have drifted, which costs nothing. Left in place. Follow-up #15264 proposes enforcing the invariant instead of repairing it.