ci(hooks): run pre-commit hooks inside git worktrees - #19945
ci(hooks): run pre-commit hooks inside git worktrees#19945vlad-scherbich wants to merge 3 commits into
Conversation
vlad-scherbich
added
the
changelog/no-changelog
🎉 All green!🧪 All tests passed 🔗 Commit SHA: 10a7991 | Docs | View more details | Give us feedback! |
vlad-scherbich
changed the title
Codeowners resolved asResolved from the full PR diff against |
There was a problem hiding this comment.
Pull request overview
Updates the contributor Git hook installer to work correctly from git worktree checkouts by installing hooks into the shared/common Git directory and removing repo-scoped relative core.hooksPath overrides that break hook resolution in worktrees.
Changes:
- Install hook symlinks into
$(git rev-parse --git-common-dir)/hooksso one install covers the main checkout and all worktrees. - Remove repo-scoped relative
core.hooksPathentries (common + worktree config) to prevent hooks being silently skipped in worktrees. - Add a worktree-focused install test script and expand documentation with troubleshooting guidance.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| hooks/autohook.sh | Install hooks into the shared Git hooks directory and drop relative core.hooksPath overrides. |
| hooks/tests/test-autohook-worktree.sh | Adds a regression test covering install behavior when executed from a worktree. |
| hooks/README.md | Updates install description and adds troubleshooting for worktree hook skipping. |
Suppressed comments (1)
hooks/README.md:202
- This section explains that
.git/hookswon’t exist in a worktree, but other troubleshooting steps in this README still instruct users to inspect.git/hooks/. Adding a pointer togit rev-parse --git-path hookshere helps users find the effective hooks directory from either a main checkout or a worktree.
Make them executable if needed:
```bash
chmod +x hooks/post-merge/*
chmod +x hooks/post-checkout/*
chmod +x hooks/scripts/*
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Circular import analysis
|
Dependency direction analysis
|
A relative core.hooksPath of .git/hooks resolves against the worktree root, where .git is a file, so 02-run-mypy never ran.
4e27334 to
10a7991
Compare
| if [[ $configured == /* ]] | ||
| then | ||
| echo "core.hooksPath in $scope_file is '$configured'; leaving it alone." | ||
| continue | ||
| fi | ||
| git config --file "$scope_file" --unset-all core.hooksPath | ||
| echo "Removed relative core.hooksPath ('$configured') from $scope_file; it never resolves inside a worktree." | ||
| done |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 10a79914e2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| git_common_dir="$1" | ||
| git_dir=$(cd "$(git rev-parse --git-dir)" && pwd) | ||
| # Repo-scoped files only; leave absolute / --global hooksPath alone. | ||
| for scope_file in "$git_dir/config" "$git_common_dir/config" |
There was a problem hiding this comment.
When extensions.worktreeConfig is enabled and core.hooksPath=.git/hooks was set with git config --worktree, Git stores it in $GIT_DIR/config.worktree, as described by the --worktree documentation. This loop instead checks $git_dir/config, so the override survives installation and commits in that worktree continue silently skipping the newly installed common hooks; include config.worktree when removing the broken override.
Useful? React with 👍 / 👎.
| [[ -f $scope_file ]] || continue | ||
| configured=$(git config --file "$scope_file" --get core.hooksPath 2>/dev/null) || continue | ||
| [[ -n $configured ]] || continue | ||
| if [[ $configured == /* ]] |
There was a problem hiding this comment.
When a repository intentionally sets a relative path such as core.hooksPath=.githooks, this condition treats it as broken and deletes it, even though Git documents that core.hooksPath may be relative and resolves it relative to the directory where hooks run (githooks documentation). Such a tracked path works across worktrees, so installation now permanently discards a valid custom hook configuration; restrict removal to the known-broken .git/hooks value rather than every non-absolute path.
Useful? React with 👍 / 👎.
| hooks_dir="$git_common_dir/hooks" | ||
| mkdir -p "$hooks_dir" | ||
| autohook_linktarget="../../hooks/autohook.sh" |
There was a problem hiding this comment.
When the checkout is attached to a bare repository or was created with git init --separate-git-dir—an option documented as placing the Git directory separately from the working tree (git-init documentation)—$git_common_dir is not <worktree>/.git. The symlink installed there still targets ../../hooks/autohook.sh, so it resolves relative to the bare/external Git directory instead of the current worktree and is left dangling; Git then silently skips the hook. Derive the target from the actual worktree root or use an absolute target.
Useful? React with 👍 / 👎.
| printf '%s\n' '#!/bin/sh' 'exit 0' > "$REPO/hooks/pre-commit/00-noop" | ||
| chmod +x "$REPO/hooks/pre-commit/00-noop" | ||
| git -C "$REPO" add hooks | ||
| git -C "$REPO" commit -q -m init |
There was a problem hiding this comment.
When a developer has a global setting such as commit.gpgSign=true without an available test key, or a global core.hooksPath containing a blocking hook, this initial commit reads that configuration and the test exits before reaching any assertions. The later probe commits explicitly set GIT_CONFIG_GLOBAL=/dev/null and GIT_CONFIG_SYSTEM=/dev/null, but setup does not; apply the same isolation to the initial commit (or export it for the whole script) so scripts/lint hook-tests is independent of each developer's Git configuration.
Useful? React with 👍 / 👎.
| This will create symlinks in the repository's shared hooks directory for all configured | ||
| hook types. Running it once covers the main checkout and every `git worktree` created | ||
| from it, so it does not need to be repeated per worktree. |
There was a problem hiding this comment.
After documenting that installation covers linked worktrees, the later “Disabling Hooks Temporarily” and “Hook Not Running” sections still tell users to run chmod or ls on .git/hooks; in a linked worktree .git is a file, so every one of those commands fails with “Not a directory.” Update those examples to resolve the shared directory through git rev-parse --git-common-dir so the newly documented workflow is usable from a worktree.
AGENTS.md reference: AGENTS.md:L17-L17
Useful? React with 👍 / 👎.
Description
This PR does not sit on #19928. It is on main.
hooks/autohook.sh installwrites into$(git rev-parse --git-common-dir)/hooksinstead of$repo_root/.git/hooks(a file in a worktree), and drops a relativecore.hooksPathfrom repo-scoped git config.Git resolves a relative hooksPath against the worktree root.
.git/hookstherefore does not exist in a worktree, so02-run-mypynever ran and typing failures reached CI.Testing
hooks/tests/test-autohook-worktree.shbuilds a throwaway repo + worktree, setscore.hooksPath=.git/hookson the common config, runs install from the worktree, and checks the common hooks dir plus the unset.scripts/lint hook-testsis green.Risks
installunsets only a relativecore.hooksPathin repo-scoped config files. Absolute and--globalvalues are left alone.Additional Notes
No release note: contributor hook install,
changelog/no-changelog.If this PR merges and nothing else lands, worktree commits start running the existing pre-commit hooks after
hooks/autohook.sh install.