feat(install): warn about hooks that won't run with the installed shims#2205
feat(install): warn about hooks that won't run with the installed shims#2205BitWeaverDev wants to merge 4 commits into
Conversation
`prek install` only sets up shims for the requested hook types (via `--hook-type`/`default_install_hook_types`, defaulting to `pre-commit`). Hooks confined to other stages, e.g. `stages: [pre-push]`, would then silently never run, which is easy to miss. Warn at install time and list the offending hooks plus how to install the missing hook type(s). Stages are read straight from config (hook `stages`, falling back to `default_stages`), so install stays offline and fast; manual-only hooks are skipped since they have no shim.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ada96bec03
ℹ️ 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".
A remote hook can declare its stages in the repo manifest, which the install command never fetches. Falling back to default_stages for a remote hook that omits stages in the project config could therefore warn about a hook that actually runs. Only trust explicit config-level stages for remote hooks and skip the rest; local/meta/builtin hooks are fully described by the config, so they keep using default_stages.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cc3a27032d
ℹ️ 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".
A plain `prek install` at the workspace root installs a shim that runs every nested project at hook time, so the warning has to look at all of them. Scanning only the project discovered at the cwd missed a subproject whose hooks are confined to an uninstalled stage. Discover the workspace the same way `run` does and check each project's config. This stays offline since discovery only reads config files.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eec0f0e94c
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eec0f0e94c
ℹ️ 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".
A remote hook with an explicit `stages: []` overrides its manifest and resolves through `default_stages`, the same way the hook builder does, so only hooks that omit `stages` entirely are skipped as unknown. Hooks ruled out by the selectors persisted into the shim (`--include`, `--skip`) are no longer reported, since the installed scripts will never run them. Env-var skips are not written into the shim and keep the warning. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2205 +/- ##
==========================================
- Coverage 92.59% 92.57% -0.02%
==========================================
Files 125 125
Lines 26587 26680 +93
==========================================
+ Hits 24617 24699 +82
- Misses 1970 1981 +11 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
📦 Cargo Bloat ComparisonBinary size change: +0.00% (26.6 MiB → 26.6 MiB) Expand for cargo-bloat outputHead Branch ResultsBase Branch Results |
⚡️ Hyperfine BenchmarksSummary: 0 regressions, 0 improvements above the 10% threshold. Environment
CLI CommandsBenchmarking basic commands in the main repo:
|
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base --version |
1.8 ± 0.1 | 1.7 | 2.0 | 1.00 |
prek-head --version |
1.8 ± 0.1 | 1.7 | 2.2 | 1.02 ± 0.06 |
prek list
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base list |
8.5 ± 0.3 | 8.0 | 9.3 | 1.02 ± 0.05 |
prek-head list |
8.3 ± 0.3 | 7.7 | 8.9 | 1.00 |
prek validate-config .pre-commit-config.yaml
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base validate-config .pre-commit-config.yaml |
2.6 ± 0.1 | 2.4 | 2.8 | 1.04 ± 0.05 |
prek-head validate-config .pre-commit-config.yaml |
2.5 ± 0.1 | 2.4 | 2.8 | 1.00 |
prek sample-config
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base sample-config |
2.1 ± 0.1 | 2.0 | 2.4 | 1.04 ± 0.04 |
prek-head sample-config |
2.1 ± 0.1 | 2.0 | 2.2 | 1.00 |
Cold vs Warm Runs
Comparing first run (cold) vs subsequent runs (warm cache):
prek run --all-files (cold - no cache)
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run --all-files |
56.3 ± 1.8 | 54.0 | 59.1 | 1.00 ± 0.04 |
prek-head run --all-files |
56.2 ± 1.6 | 54.2 | 58.6 | 1.00 |
prek run --all-files (warm - with cache)
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run --all-files |
56.0 ± 2.0 | 52.3 | 60.4 | 1.01 ± 0.05 |
prek-head run --all-files |
55.6 ± 1.6 | 53.3 | 58.6 | 1.00 |
Full Hook Suite
Running the builtin hook suite on the benchmark workspace:
prek run --all-files (full builtin hook suite)
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run --all-files |
55.9 ± 1.6 | 52.2 | 60.0 | 1.00 |
prek-head run --all-files |
57.6 ± 3.0 | 53.4 | 73.8 | 1.03 ± 0.06 |
Individual Hook Performance
Benchmarking each hook individually on the test repo:
prek run trailing-whitespace --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run trailing-whitespace --all-files |
15.4 ± 0.4 | 14.7 | 16.6 | 1.00 |
prek-head run trailing-whitespace --all-files |
15.8 ± 0.6 | 15.0 | 16.9 | 1.03 ± 0.05 |
prek run end-of-file-fixer --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run end-of-file-fixer --all-files |
20.5 ± 1.2 | 18.8 | 23.4 | 1.02 ± 0.08 |
prek-head run end-of-file-fixer --all-files |
20.1 ± 1.0 | 18.1 | 21.7 | 1.00 |
prek run check-json --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run check-json --all-files |
6.6 ± 0.2 | 6.1 | 7.2 | 1.02 ± 0.05 |
prek-head run check-json --all-files |
6.5 ± 0.3 | 6.1 | 7.0 | 1.00 |
prek run check-yaml --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run check-yaml --all-files |
6.3 ± 0.2 | 6.0 | 6.8 | 1.00 ± 0.05 |
prek-head run check-yaml --all-files |
6.3 ± 0.2 | 5.9 | 6.8 | 1.00 |
prek run check-toml --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run check-toml --all-files |
6.3 ± 0.3 | 5.9 | 7.0 | 1.00 |
prek-head run check-toml --all-files |
6.9 ± 0.5 | 6.2 | 7.9 | 1.08 ± 0.10 |
prek run check-xml --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run check-xml --all-files |
7.1 ± 1.8 | 5.8 | 12.2 | 1.10 ± 0.28 |
prek-head run check-xml --all-files |
6.5 ± 0.3 | 6.0 | 7.0 | 1.00 |
prek run detect-private-key --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run detect-private-key --all-files |
11.6 ± 0.7 | 10.2 | 13.0 | 1.00 |
prek-head run detect-private-key --all-files |
12.0 ± 0.9 | 10.9 | 14.1 | 1.04 ± 0.10 |
prek run fix-byte-order-marker --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run fix-byte-order-marker --all-files |
15.0 ± 0.5 | 14.1 | 16.4 | 1.03 ± 0.05 |
prek-head run fix-byte-order-marker --all-files |
14.6 ± 0.5 | 13.5 | 15.7 | 1.00 |
Installation Performance
Benchmarking hook installation (fast path hooks skip Python setup):
prek install-hooks (cold - no cache)
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base install-hooks |
3.8 ± 0.1 | 3.7 | 4.0 | 1.00 |
prek-head install-hooks |
3.9 ± 0.1 | 3.8 | 4.0 | 1.02 ± 0.04 |
prek install-hooks (warm - with cache)
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base install-hooks |
3.8 ± 0.1 | 3.7 | 4.0 | 1.03 ± 0.04 |
prek-head install-hooks |
3.7 ± 0.1 | 3.6 | 3.9 | 1.00 |
File Filtering/Scoping Performance
Testing different file selection modes:
prek run (staged files only)
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run |
27.9 ± 0.8 | 26.7 | 29.2 | 1.00 |
prek-head run |
29.3 ± 1.0 | 28.4 | 33.2 | 1.05 ± 0.05 |
prek run --files '*.json' (specific file type)
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run --files '*.json' |
6.8 ± 0.2 | 6.5 | 7.3 | 1.00 |
prek-head run --files '*.json' |
7.0 ± 0.2 | 6.6 | 7.4 | 1.02 ± 0.04 |
Workspace Discovery & Initialization
Benchmarking hook discovery and initialization overhead:
prek run --dry-run --all-files (measures init overhead)
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run --dry-run --all-files |
6.1 ± 0.3 | 5.6 | 7.0 | 1.10 ± 0.06 |
prek-head run --dry-run --all-files |
5.5 ± 0.1 | 5.2 | 5.8 | 1.00 |
Meta Hooks Performance
Benchmarking meta hooks separately:
prek run check-hooks-apply --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run check-hooks-apply --all-files |
9.2 ± 0.2 | 8.9 | 9.6 | 1.00 |
prek-head run check-hooks-apply --all-files |
9.3 ± 0.3 | 8.9 | 9.8 | 1.01 ± 0.04 |
prek run check-useless-excludes --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run check-useless-excludes --all-files |
9.3 ± 0.3 | 9.0 | 9.9 | 1.00 |
prek-head run check-useless-excludes --all-files |
9.3 ± 0.4 | 9.0 | 10.2 | 1.01 ± 0.05 |
prek run identity --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run identity --all-files |
7.9 ± 0.2 | 7.5 | 8.2 | 1.00 |
prek-head run identity --all-files |
8.2 ± 0.2 | 7.9 | 8.5 | 1.04 ± 0.03 |
Closes #2056
What
prek installonly installs shims for the hook types you ask for —--hook-type, ordefault_install_hook_types, falling back topre-commit. If the config has hooks pinned to a different stage (the classic case being a slowerstages: [pre-push]hook), those hooks just silently never run after a plainprek install, and it's easy not to notice.This adds a warning at install time that names the hooks whose stages aren't covered and points at the fix:
Notes on the approach
stages→default_stages→ all), soinstallstays offline and fast. The trade-off is that a remote hook narrowed only by its manifest (not by the project config) won't be flagged — that's the conservative choice: no false positives, no new network I/O, and it covers the case from the issue.manualis skipped, since manual-only hooks have no Git shim and aren't meant to run automatically.Stages::resolvesoinstallandHookSpec::apply_project_defaultsshare one implementation instead of duplicating the fallback rules.Tests
Added integration coverage in
tests/install.rs: an unmatched hook warns (while an all-stages sibling does not), installing the matching--hook-typesilences it, manual-only never warns, anddefault_stagesinheritance is taken into account.