feat(install): use Git 2.54 config-based hooks with --global support#853
feat(install): use Git 2.54 config-based hooks with --global support#853
Conversation
On Git 2.54+ `hk install` now writes `hook.hk-<event>.command` / `.event` config entries instead of shell shims in `.git/hooks/`, and falls back to shims on older git. A new `hk install --global` writes those entries to `~/.gitconfig` so hk applies to every repo without per-repo setup; in repos without `hk.pkl` the hook is a silent no-op via a new hidden `--from-hook` flag. Uninstall now cleans both modes regardless of the current git version. Existing behavior (script shims, `hk install`/`hk uninstall`, mise integration, worktree hooksPath) is preserved. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request introduces support for Git 2.54+ config-based hooks and global hook installation. It adds --global and --legacy flags to hk install and hk uninstall, along with a hidden --from-hook flag for graceful exits in repositories without hk configuration. Feedback was provided to improve the robustness of the installation process by validating configuration before performing cleanup and using a more specific signature when identifying existing shims for removal.
| // Clean up any prior installation so modes don't accumulate. | ||
| remove_local_shims()?; | ||
| remove_local_config_entries()?; | ||
|
|
||
| let config = Config::get()?; | ||
| let events: Vec<String> = config | ||
| .hooks | ||
| .keys() | ||
| .filter(|h| h.as_str() != "check" && h.as_str() != "fix") | ||
| .cloned() | ||
| .collect(); | ||
|
|
||
| if use_config_hooks { | ||
| install_local_config(&events, command) | ||
| } else { | ||
| "hk".to_string() | ||
| install_local_shims(&events, command) | ||
| } |
There was a problem hiding this comment.
Performing cleanup of existing hooks before validating the configuration can leave the repository in an inconsistent state (without any hooks) if Config::get() fails due to a syntax error in hk.pkl. It is safer to verify the configuration and determine the events to be installed before removing the current installation.
| // Clean up any prior installation so modes don't accumulate. | |
| remove_local_shims()?; | |
| remove_local_config_entries()?; | |
| let config = Config::get()?; | |
| let events: Vec<String> = config | |
| .hooks | |
| .keys() | |
| .filter(|h| h.as_str() != "check" && h.as_str() != "fix") | |
| .cloned() | |
| .collect(); | |
| if use_config_hooks { | |
| install_local_config(&events, command) | |
| } else { | |
| "hk".to_string() | |
| install_local_shims(&events, command) | |
| } | |
| let config = Config::get()?; | |
| let events: Vec<String> = config | |
| .hooks | |
| .keys() | |
| .filter(|h| h.as_str() != "check" && h.as_str() != "fix") | |
| .cloned() | |
| .collect(); | |
| // Clean up any prior installation so modes don't accumulate. | |
| remove_local_shims()?; | |
| remove_local_config_entries()?; | |
| if use_config_hooks { | |
| install_local_config(&events, command) | |
| } else { | |
| install_local_shims(&events, command) | |
| } |
| xx::file::write(&hook_file, git_hook_content(&command, hook))?; | ||
| xx::file::make_executable(&hook_file)?; | ||
| println!("Installed hk hook: {}", hook_file.display()); | ||
| if content.contains("hk run") { |
There was a problem hiding this comment.
The check content.contains("hk run") is quite broad and could lead to the accidental deletion of user-defined hooks that happen to contain this string (e.g., in a comment or as part of a different command). Using a more specific signature that matches the hk shim template would be safer.
| if content.contains("hk run") { | |
| if content.contains("test \"${HK:-1}\" = \"0\" || exec") { |
Greptile SummaryThis PR adds Git 2.54+ config-based hook installation ( All previously raised P1 concerns are resolved: the Confidence Score: 5/5Safe to merge — all previously raised P1 concerns are resolved, implementation is correct, and the new behavior is well-guarded. No P0 or P1 issues remain. The HK=0 escape hatch is present in both write_config_hook and the manual docs example. The double-execution semantics are accurately documented. The --from-hook two-level guard correctly short-circuits before config load and before hook dispatch. Exit code handling in remove_config_entries correctly distinguishes 'no matches' (code 1) from real errors (code ≥2). The only gap is no automated test for the Git 2.54+ config-based default path, but the PR includes thorough manual smoke tests. test/install_creates_git_hooks.bats and the other bats test files — all pinned to --legacy, so the new config-based default path has no automated coverage. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[hk install] --> B{--global?}
B -- yes --> C{git >= 2.54?}
C -- no --> D[bail! upgrade hint]
C -- yes --> E[remove_config_entries --global]
E --> F[write_config_hook --global\nfor each DEFAULT_GLOBAL_EVENT]
F --> G[print success]
B -- no --> H[use_config_hooks =\n!legacy && git>=2.54]
H --> I[Config::get - load hk.pkl]
I --> J[remove_local_shims\n+ remove_local_config_entries]
J --> K{events empty?}
K -- yes --> L[warn & return]
K -- no --> M{use_config_hooks?}
M -- yes --> N[write_config_hook --local\nfor each event]
N --> O[warn_if_global_overlap]
M -- no --> P[write script shim\nto .git/hooks/]
subgraph runtime [Runtime: hk run EVENT --from-hook]
R[Config::project_config_exists?] -- no --> S[exit 0 silently]
R -- yes --> T[Config::get]
T --> U{hook defined?}
U -- no --> V[exit 0 silently]
U -- yes --> W[run hook steps]
end
Reviews (6): Last reviewed commit: "fix(install): warn on global/local hook ..." | Re-trigger Greptile |
- Config-based hooks now include the `test "${HK:-1}" = "0" ||` guard so
`HK=0 git commit` keeps working on Git 2.54+ (regression from the
initial config-hooks change). Manual `~/.gitconfig` example in the
docs updated to match.
- `hk uninstall` again logs `"removed hook: <path>"` per file, which
the worktree bats test asserts against.
- `hk install` loads and validates the project config before removing
the prior installation, so a broken `hk.pkl` no longer leaves the
repo without any hooks.
- Shim detection now matches the HK guard in addition to `hk run`,
narrowing the signature so unrelated user-written hooks containing
the string "hk run" won't be deleted.
- Simplified the `--global` control flow so the version check isn't
buried in a dead `use_config_hooks` binding.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`install_local_shims` previously did filesystem work and silently succeeded when `hk.pkl` defined no hooks, while `install_local_config` warned. Lift the check into `Install::run` so both modes emit the same "nothing to install" warning and skip the unnecessary work. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@jdx just to make you aware: |
…ning - Distinguish `git config --get-regexp` exit 1 (no match, expected) from ≥2 (real error like unreadable config); `hk uninstall` now fails loudly instead of silently swallowing errors. - When `hk install` runs with an empty `hk.pkl`, explicitly state how many previously-installed hooks were removed rather than the generic "nothing to install" warning — makes the destructive side effect visible. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Under `hk install --global`, hk fires on every `git commit` in every repo. If the user has a broken `~/.config/hk/config.pkl` hkrc (or `pkl` is missing while an hkrc exists), `Config::get()` fails *before* the `from_hook` silent-exit guard — so every commit everywhere blows up. Move the existence check ahead of `Config::get()`: if no project `hk.pkl` exists and we're in `--from-hook` mode, skip config loading entirely. Repos with a broken project config still fail loudly, as intended. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CI upgraded from rust 1.94.1 to 1.95.0, which flags several lints that
were previously accepted. All fixes are mechanical and semantically
equivalent:
- collapsible match → match guard (migrate/mod.rs)
- sort_by → sort_by_key with Reverse (git.rs)
- write!("\n") → writeln!() (end_of_file_fixer.rs test)
- useless & on Vec arg (fix_smart_quotes.rs test)
- unwrap-after-is_ok → if let Ok (python_check_ast.rs tests)
- useless .into_iter() on IntoIterator arg (settings.rs)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
macos-latest runners now have Git 2.54+, which triggers the new config-based install path by default. The existing bats tests assert behavior specific to the legacy shim mechanism (file paths in `.git/hooks/`, per-worktree hook routing, core.hooksPath warnings, and pre-push hook firing details). Those assertions don't apply to config-based hooks, so the tests fail on macos CI. Pass `--legacy` in the install invocations that inherently depend on the shim mechanism. Config-based hooks remain the default for end users; config-mode bats coverage should land as a separate test file. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Git aggregates `hook.<name>.command` values across every config scope and runs them all — so a local `hk install` on top of an active global one fires hk twice per event. Previous doc claim that "local entries simply replace the global defaults" was inaccurate. - Update docs to describe the aggregate behavior and point users at the per-repo `hook.hk-<event>.enabled = false` escape hatch. - Emit a warning from local `hk install` when any of the events being installed already has a `hook.hk-<event>.command` at global scope, so users catch the double-fire before it bites them. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a5138db. Configure here.
| println!( | ||
| "Installed hk global hooks in ~/.gitconfig for: {}", | ||
| DEFAULT_GLOBAL_EVENTS.join(", ") | ||
| ); |
There was a problem hiding this comment.
Hardcoded ~/.gitconfig path ignores XDG config location
Low Severity
The install_global output message and uninstall --global info message hardcode ~/.gitconfig, but git config --global may write to ~/.config/git/config (XDG location) depending on user setup. The actual config write is correct (delegated to git), but the user-facing messages point to the wrong file, which is confusing when trying to verify or hand-edit the result.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit a5138db. Configure here.
| if use_config_hooks { | ||
| let result = install_local_config(&events, command); | ||
| warn_if_global_overlap(&events); | ||
| result |
There was a problem hiding this comment.
Global overlap warning fires even after failed install
Low Severity
warn_if_global_overlap is called unconditionally after install_local_config, even when the install returned an error. This means a failed install still emits a warning about global/local overlap (spawning multiple git config subprocesses), then propagates the original error — producing confusing output.
Reviewed by Cursor Bugbot for commit a5138db. Configure here.
### 🚀 Features - **(check)** implement --plan, --why, and --json by [@jdx](https://github.com/jdx) in [#848](#848) - **(cocogitto)** add cocogitto conventional commits config to hk builtin config by [@hituzi-no-sippo](https://github.com/hituzi-no-sippo) in [#838](#838) - **(git)** support GIT_DIR/GIT_WORK_TREE for bare-repo dotfile managers by [@jdx](https://github.com/jdx) in [#847](#847) - **(install)** use Git 2.54 config-based hooks with --global support by [@jdx](https://github.com/jdx) in [#853](#853) ### 🐛 Bug Fixes - use text progress in CI by [@jdx](https://github.com/jdx) in [#845](#845) ### 📚 Documentation - generalize agent guidelines by [@jdx](https://github.com/jdx) in [#846](#846) - add releases nav and aube lock by [@jdx](https://github.com/jdx) in [#849](#849) ### 🔍 Other Changes - bump communique to 1.0.1 by [@jdx](https://github.com/jdx) in [#850](#850) ### 📦️ Dependency Updates - update actions-rust-lang/setup-rust-toolchain digest to 2b1f5e9 by [@renovate[bot]](https://github.com/renovate[bot]) in [#832](#832) - update anthropics/claude-code-action digest to c3d45e8 by [@renovate[bot]](https://github.com/renovate[bot]) in [#833](#833) - update rust crate tokio to v1.52.1 by [@renovate[bot]](https://github.com/renovate[bot]) in [#834](#834) - update actions/upload-pages-artifact action to v5 by [@renovate[bot]](https://github.com/renovate[bot]) in [#835](#835) - update taiki-e/upload-rust-binary-action digest to f0d45ae by [@renovate[bot]](https://github.com/renovate[bot]) in [#839](#839) - update rust crate clx to v2 by [@renovate[bot]](https://github.com/renovate[bot]) in [#836](#836) - update anthropics/claude-code-action digest to 0d2971c by [@renovate[bot]](https://github.com/renovate[bot]) in [#841](#841) - update anthropics/claude-code-action digest to 38ec876 by [@renovate[bot]](https://github.com/renovate[bot]) in [#842](#842) - lock file maintenance by [@renovate[bot]](https://github.com/renovate[bot]) in [#851](#851) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Low risk release bookkeeping: version bumps and doc/CLI artifact updates, plus minor dependency patch updates in `Cargo.lock`. No functional Rust source changes are included in this diff. > > **Overview** > Bumps `hk` to **v1.44.0** and publishes the corresponding release notes in `CHANGELOG.md`. > > Updates generated/packaged artifacts to match the new version (CLI docs/specs and Pkl package URLs in docs/examples), and refreshes `Cargo.lock` for the release (including patch-level updates like `rustls` and `winnow`). > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit a36c7a6. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> Co-authored-by: mise-en-dev <123107610+mise-en-dev@users.noreply.github.com>
## Summary Leads the install docs with `hk install --global` as the recommended setup path. The global install is a silent no-op in repos without an `hk.pkl`, so it's safe to enable once per machine and avoids re-running `hk install` in every clone. Follow-up to #853 (the config-based hooks + `--global` feature that this documents). ## What changed - **[docs/getting_started.md](https://github.com/jdx/hk/blob/feat/config-based-hooks/docs/getting_started.md)** — restructured: binary install → **Install Hooks (recommended: global)** → Project Setup → config example. Per-repo `hk install` is now framed as an alternative for pre-Git-2.54 or selective-repo use. The overlap warning (Git aggregates `hook.<name>.command` across scopes) and the manual `~/.gitconfig` setup are retained as subsections. - **[src/cli/install.rs](https://github.com/jdx/hk/blob/feat/config-based-hooks/src/cli/install.rs)** — rewrote the `Install` struct docstring and `--global` flag help so `hk install --help` leads with the recommendation. - **docs/cli/install.md**, `hk.usage.kdl`, `docs/cli/commands.json` — regenerated via `hk usage` → `usage g markdown`. - Renamed the pre-existing "Global Configuration" section to "Global \`hkrc\` Configuration" in getting_started.md to disambiguate from global *hooks* (the sections are adjacent and share the word "global"). - **docs/package.json** — added `"version": "0.0.0"` + `"private": true` so the bun workspace resolves cleanly (the pre-push `docs:build` hook was failing on missing version). ## Why New users land on getting_started.md; leading with per-repo `hk install` made them re-install in every clone and gave no signal that the global option existed. The `--from-hook` short-circuit (also from #853) makes global genuinely safe to enable everywhere, so it deserves the headline. ## Test plan - [x] `hk check --all` passes. - [x] Pre-push hook (runs full docs build) passes. - [ ] Review rendered [getting_started](https://hk.jdx.dev/getting_started) and [cli/install](https://hk.jdx.dev/cli/install) pages after merge. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>


Summary
hk installnow writes config-based hooks (hook.hk-<event>.command/.event) instead of shell shims in.git/hooks/, keeping the hooks directory untouched and composing cleanly with other hook managers. Legacy shim mode is auto-used on older Git (and available via--legacy).hk install --globalwrites those entries to the user's~/.gitconfigso hk applies to every repository on the machine. In repos withouthk.pklthe invocation is a silent no-op — driven by a new hidden--from-hookflag onhk runthat the installed command passes.hk uninstallnow cleans both modes (script shims + config entries) regardless of current git version, so upgrading git and then uninstalling works.hk uninstall --globalis the global counterpart.docs/getting_started.mdwith a dedicated "Install Hooks Globally (Git 2.54+)" section including both the CLI flow and a hand-rolled~/.gitconfigexample.How it works
With
--globalactive, git fires every configured event against hk.hk run <event> --from-hookchecks that the current repo has anhk.pkland defines the event; if either is missing it exits 0 silently. So the user installs once and forgets, and repos without hk are unaffected.Since Git 2.54 doesn't pass the event name to the configured command, there's one config entry per event (
hook.hk-pre-commit.*,hook.hk-pre-push.*, etc.) rather than a shared entry — the name discriminates which hk invocation to run.Test plan
cargo test— 145 passed (includingtest_subcommands_are_sorted)install_creates_git_hooks,install_warn_hookspath,install_worktree_hooks,uninstall,hook_{args,env,fix_default,stdin}hk installin a repo writeshook.hk-*config entries and no script filesgit commitin that repo fires the pre-commit hook via confighk install --globalwrites 9 client-hook entries to~/.gitconfiggit commitin a repo withouthk.pkl(global install active) is a silent no-op and succeeds in ~90msgit commitin a repo with minimalhk.pklonly triggers defined events; others (commit-msg, post-commit, etc.) are silent no-ops.git/hooks/pre-commitis removed when re-runninghk installon git 2.54hk uninstallremoves both script shims and config entries;hk uninstall --globalclears~/.gitconfighk install --globalerrors cleanly with an upgrade hint; regularhk installfalls back to script shims unchangedThis PR was generated by Claude Code.
🤖 Generated with Claude Code
Note
Medium Risk
Changes how git hooks are installed/executed (switching from
.git/hooksshims togit config hook.*entries on Git 2.54+), plus adds global hook installation, which could affect developer workflows if misconfigured or if both global+local installs overlap.Overview
On Git 2.54+,
hk installnow installs hooks via config-based git hooks (hook.hk-<event>.command/.event) instead of writing scripts into.git/hooks/, with--legacyto force the prior shim behavior.Adds global hook installation/uninstallation via
hk install --global/hk uninstall --global, and introduces a hiddenhk run --from-hookmode so hook invocations silently no-op when no projecthkconfig (or no matching event) is present.hk uninstallnow removes both legacy shims and config-based entries to support mixed/upgrade scenarios; docs/CLI usage specs and tests are updated accordingly, alongside minor refactors/cleanups (git version detection helper, small test/style tweaks).Reviewed by Cursor Bugbot for commit a5138db. Bugbot is set up for automated code reviews on this repo. Configure here.