From 1a60ed820d7b80604fd34111c160ff8dcf4a39c4 Mon Sep 17 00:00:00 2001 From: BitWeaver Date: Thu, 11 Jun 2026 22:34:22 +0200 Subject: [PATCH 1/5] feat(install): warn about hooks that won't run with the installed shims `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. --- crates/prek/src/cli/install.rs | 50 +++++++++++++++- crates/prek/src/config.rs | 24 ++++++++ crates/prek/src/hook.rs | 9 +-- crates/prek/tests/install.rs | 102 +++++++++++++++++++++++++++++++++ 4 files changed, 176 insertions(+), 9 deletions(-) diff --git a/crates/prek/src/cli/install.rs b/crates/prek/src/cli/install.rs index 25eddd2b9..fafb446c1 100644 --- a/crates/prek/src/cli/install.rs +++ b/crates/prek/src/cli/install.rs @@ -15,7 +15,7 @@ use crate::cli::run; use crate::cli::run::InstallCache; use crate::cli::run::{SelectorSource, Selectors}; use crate::cli::{ExitStatus, HookType}; -use crate::config::load_config; +use crate::config::{Repo, Stage, Stages, load_config}; use crate::fs::{CWD, Simplified}; use crate::git::{GIT_ROOT, git_cmd}; use crate::printer::Printer; @@ -105,6 +105,10 @@ pub(crate) async fn install( None }; + if let Some(project) = &project { + warn_unmatched_hook_stages(project, &hook_types); + } + for hook_type in hook_types { install_hook_script( project.as_ref(), @@ -202,6 +206,50 @@ fn get_hook_types( hook_types } +/// Warn about configured hooks whose stages aren't covered by the hook types being installed, +/// since their shims won't be installed and they won't run automatically. +fn warn_unmatched_hook_stages(project: &Project, hook_types: &[HookType]) { + let installed = Stages::from( + hook_types + .iter() + .map(|&hook_type| Stage::from(hook_type)) + .collect::>(), + ); + + let config = project.config(); + let unmatched: Vec<(&str, Stages)> = config + .repos + .iter() + .flat_map(Repo::iter_hooks) + .filter_map(|(id, options)| { + // `Stage::Manual` has no git shim, so it can never be "installed" and is ignored here. + let required = Stages::from( + Stages::resolve(options.stages, config.default_stages) + .iter() + .filter(|&stage| stage != Stage::Manual) + .collect::>(), + ); + (!required.is_empty() && !required.intersects(installed)).then_some((id, required)) + }) + .collect(); + + if unmatched.is_empty() { + return; + } + + let list = unmatched + .iter() + .map(|(id, stages)| format!(" - `{}` ({})", id.cyan(), stages.yellow())) + .collect::>() + .join("\n"); + warn_user!( + "The following hooks won't run automatically because their stages aren't being installed:\n{list}\n{} install the missing hook type(s) with `{}`, or set `{}` in your config.", + "hint:".bold().yellow(), + "prek install --hook-type ".cyan(), + "default_install_hook_types".cyan(), + ); +} + #[allow(clippy::fn_params_excessive_bools)] fn install_hook_script( project: Option<&Project>, diff --git a/crates/prek/src/config.rs b/crates/prek/src/config.rs index 3faebb491..e56cc32b7 100644 --- a/crates/prek/src/config.rs +++ b/crates/prek/src/config.rs @@ -424,6 +424,19 @@ impl Stages { pub(crate) fn contains(self, stage: Stage) -> bool { (self.0 & stage.bit()) != 0 } + + pub(crate) fn intersects(self, other: Self) -> bool { + (self.0 & other.0) != 0 + } + + /// A hook with no `stages` (or an empty list) inherits `default_stages`, + /// which itself falls back to [`Stages::ALL`] when unset. + pub(crate) fn resolve(hook: Option, default: Option) -> Self { + match hook { + Some(stages) if !stages.is_empty() => stages, + _ => default.unwrap_or(Self::ALL), + } + } } impl Display for Stages { @@ -1122,6 +1135,17 @@ impl<'de> Deserialize<'de> for Repo { } } +impl Repo { + pub(crate) fn iter_hooks(&self) -> Box + '_> { + match self { + Repo::Remote(repo) => Box::new(repo.hooks.iter().map(|h| (h.id.as_str(), &h.options))), + Repo::Local(repo) => Box::new(repo.hooks.iter().map(|h| (h.id.as_str(), &h.options))), + Repo::Meta(repo) => Box::new(repo.hooks.iter().map(|h| (h.id.as_str(), &h.options))), + Repo::Builtin(repo) => Box::new(repo.hooks.iter().map(|h| (h.id.as_str(), &h.options))), + } + } +} + fn remote_hook_to_local(hook: RemoteHook) -> Result where E: DeError, diff --git a/crates/prek/src/hook.rs b/crates/prek/src/hook.rs index 6c569ec01..eb0337834 100644 --- a/crates/prek/src/hook.rs +++ b/crates/prek/src/hook.rs @@ -89,14 +89,7 @@ impl HookSpec { .and_then(|v| v.get(&language).cloned()); } - if self - .options - .stages - .as_ref() - .is_none_or(|stages| stages.is_empty()) - { - self.options.stages = Some(config.default_stages.unwrap_or(Stages::ALL)); - } + self.options.stages = Some(Stages::resolve(self.options.stages, config.default_stages)); if let Some(default_env) = &config.default_env { let mut env = default_env.clone(); diff --git a/crates/prek/tests/install.rs b/crates/prek/tests/install.rs index 52637b18f..c1943100b 100644 --- a/crates/prek/tests/install.rs +++ b/crates/prek/tests/install.rs @@ -1810,3 +1810,105 @@ fn install_invalid_config_warning() { | "); } + +#[test] +fn install_warns_on_unmatched_hook_stages() { + let context = TestContext::new(); + context.init_project(); + + context.write_pre_commit_config(indoc! {r" + repos: + - repo: local + hooks: + - id: push-only + name: Push only + language: system + entry: echo push + stages: [pre-push] + - id: every-stage + name: Every stage + language: system + entry: echo all + "}); + + // Default install only sets up the `pre-commit` shim, so `push-only` won't run, + // while `every-stage` (no stages, runs everywhere) is not reported. + cmd_snapshot!(context.filters(), context.install(), @r" + success: true + exit_code: 0 + ----- stdout ----- + prek installed at `.git/hooks/pre-commit` + + ----- stderr ----- + warning: The following hooks won't run automatically because their stages aren't being installed: + - `push-only` (pre-push) + hint: install the missing hook type(s) with `prek install --hook-type `, or set `default_install_hook_types` in your config. + "); + + // Installing the `pre-push` shim covers `push-only`, so no warning is emitted. + cmd_snapshot!(context.filters(), context.install().arg("--hook-type").arg("pre-push"), @r" + success: true + exit_code: 0 + ----- stdout ----- + prek installed at `.git/hooks/pre-push` + + ----- stderr ----- + "); +} + +#[test] +fn install_no_warn_for_manual_only_hook() { + let context = TestContext::new(); + context.init_project(); + + context.write_pre_commit_config(indoc! {r" + repos: + - repo: local + hooks: + - id: manual-only + name: Manual only + language: system + entry: echo manual + stages: [manual] + "}); + + // `manual` has no git shim, so a manual-only hook is never reported. + cmd_snapshot!(context.filters(), context.install(), @r" + success: true + exit_code: 0 + ----- stdout ----- + prek installed at `.git/hooks/pre-commit` + + ----- stderr ----- + "); +} + +#[test] +fn install_warns_on_unmatched_default_stages() { + let context = TestContext::new(); + context.init_project(); + + context.write_pre_commit_config(indoc! {r" + default_stages: [pre-push] + repos: + - repo: local + hooks: + - id: inherits-default + name: Inherits default + language: system + entry: echo default + "}); + + // The hook inherits `default_stages: [pre-push]`, which the default install doesn't cover. + cmd_snapshot!(context.filters(), context.install(), @r" + success: true + exit_code: 0 + ----- stdout ----- + prek installed at `.git/hooks/pre-commit` + + ----- stderr ----- + warning: The following hooks won't run automatically because their stages aren't being installed: + - `inherits-default` (pre-push) + hint: install the missing hook type(s) with `prek install --hook-type `, or set `default_install_hook_types` in your config. + "); +} From 00509e0a685bf8c13c26861e5180b6ae48ce890c Mon Sep 17 00:00:00 2001 From: BitWeaver Date: Thu, 11 Jun 2026 22:44:16 +0200 Subject: [PATCH 2/5] fix(install): don't assume default_stages for remote hooks 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. --- crates/prek/src/cli/install.rs | 32 +++++++++++++------- crates/prek/tests/install.rs | 54 ++++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 10 deletions(-) diff --git a/crates/prek/src/cli/install.rs b/crates/prek/src/cli/install.rs index fafb446c1..36797b609 100644 --- a/crates/prek/src/cli/install.rs +++ b/crates/prek/src/cli/install.rs @@ -217,21 +217,33 @@ fn warn_unmatched_hook_stages(project: &Project, hook_types: &[HookType]) { ); let config = project.config(); - let unmatched: Vec<(&str, Stages)> = config - .repos - .iter() - .flat_map(Repo::iter_hooks) - .filter_map(|(id, options)| { - // `Stage::Manual` has no git shim, so it can never be "installed" and is ignored here. - let required = Stages::from( + let mut unmatched: Vec<(&str, Stages)> = Vec::new(); + for repo in &config.repos { + // A remote hook may inherit `stages` from its manifest, which we can't see without + // fetching the repo, so only trust explicit config-level stages and skip the rest. + let is_remote = matches!(repo, Repo::Remote(_)); + for (id, options) in repo.iter_hooks() { + let stages = if is_remote { + match options.stages { + Some(stages) if !stages.is_empty() => stages, + _ => continue, + } + } else { Stages::resolve(options.stages, config.default_stages) + }; + + // `Stage::Manual` has no git shim, so it is ignored here. + let required = Stages::from( + stages .iter() .filter(|&stage| stage != Stage::Manual) .collect::>(), ); - (!required.is_empty() && !required.intersects(installed)).then_some((id, required)) - }) - .collect(); + if !required.is_empty() && !required.intersects(installed) { + unmatched.push((id, required)); + } + } + } if unmatched.is_empty() { return; diff --git a/crates/prek/tests/install.rs b/crates/prek/tests/install.rs index c1943100b..4368a5c45 100644 --- a/crates/prek/tests/install.rs +++ b/crates/prek/tests/install.rs @@ -1912,3 +1912,57 @@ fn install_warns_on_unmatched_default_stages() { hint: install the missing hook type(s) with `prek install --hook-type `, or set `default_install_hook_types` in your config. "); } + +#[test] +fn install_skips_remote_hook_without_config_stages() { + let context = TestContext::new(); + context.init_project(); + + // A remote hook may set its stages in the manifest, so `default_stages` must not be + // assumed to apply here; the hook should not be reported. + context.write_pre_commit_config(indoc! {r" + default_stages: [pre-push] + repos: + - repo: https://github.com/pre-commit/pre-commit-hooks + rev: v5.0.0 + hooks: + - id: trailing-whitespace + "}); + + cmd_snapshot!(context.filters(), context.install(), @r" + success: true + exit_code: 0 + ----- stdout ----- + prek installed at `.git/hooks/pre-commit` + + ----- stderr ----- + "); +} + +#[test] +fn install_warns_on_unmatched_remote_hook_with_config_stages() { + let context = TestContext::new(); + context.init_project(); + + // Explicit config-level stages override the manifest, so they can be trusted. + context.write_pre_commit_config(indoc! {r" + repos: + - repo: https://github.com/pre-commit/pre-commit-hooks + rev: v5.0.0 + hooks: + - id: trailing-whitespace + stages: [pre-push] + "}); + + cmd_snapshot!(context.filters(), context.install(), @r" + success: true + exit_code: 0 + ----- stdout ----- + prek installed at `.git/hooks/pre-commit` + + ----- stderr ----- + warning: The following hooks won't run automatically because their stages aren't being installed: + - `trailing-whitespace` (pre-push) + hint: install the missing hook type(s) with `prek install --hook-type `, or set `default_install_hook_types` in your config. + "); +} From 392f62ed663ad40c7486a2a6250fc19e6668c4d9 Mon Sep 17 00:00:00 2001 From: BitWeaver Date: Fri, 12 Jun 2026 10:06:33 +0200 Subject: [PATCH 3/5] fix(install): scan every workspace project for unmatched stages 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. --- crates/prek/src/cli/install.rs | 69 +++++++++++++++++++++------------- crates/prek/tests/install.rs | 47 +++++++++++++++++++++++ 2 files changed, 90 insertions(+), 26 deletions(-) diff --git a/crates/prek/src/cli/install.rs b/crates/prek/src/cli/install.rs index 36797b609..2ba10ced7 100644 --- a/crates/prek/src/cli/install.rs +++ b/crates/prek/src/cli/install.rs @@ -105,8 +105,8 @@ pub(crate) async fn install( None }; - if let Some(project) = &project { - warn_unmatched_hook_stages(project, &hook_types); + if project.is_some() { + warn_unmatched_hook_stages(store, config.as_deref(), refresh, &hook_types); } for hook_type in hook_types { @@ -208,7 +208,12 @@ fn get_hook_types( /// Warn about configured hooks whose stages aren't covered by the hook types being installed, /// since their shims won't be installed and they won't run automatically. -fn warn_unmatched_hook_stages(project: &Project, hook_types: &[HookType]) { +fn warn_unmatched_hook_stages( + store: &Store, + config: Option<&Path>, + refresh: bool, + hook_types: &[HookType], +) { let installed = Stages::from( hook_types .iter() @@ -216,31 +221,43 @@ fn warn_unmatched_hook_stages(project: &Project, hook_types: &[HookType]) { .collect::>(), ); - let config = project.config(); + // A plain root install runs every workspace project, so scan them all, the same way `run` does. + let Ok(root) = Workspace::find_root(config, &CWD) else { + return; + }; + let Ok(workspace) = + Workspace::discover(store, root, config.map(Path::to_path_buf), None, refresh) + else { + return; + }; + let mut unmatched: Vec<(&str, Stages)> = Vec::new(); - for repo in &config.repos { - // A remote hook may inherit `stages` from its manifest, which we can't see without - // fetching the repo, so only trust explicit config-level stages and skip the rest. - let is_remote = matches!(repo, Repo::Remote(_)); - for (id, options) in repo.iter_hooks() { - let stages = if is_remote { - match options.stages { - Some(stages) if !stages.is_empty() => stages, - _ => continue, + for project in workspace.projects() { + let config = project.config(); + for repo in &config.repos { + // A remote hook may inherit `stages` from its manifest, which we can't see without + // fetching the repo, so only trust explicit config-level stages and skip the rest. + let is_remote = matches!(repo, Repo::Remote(_)); + for (id, options) in repo.iter_hooks() { + let stages = if is_remote { + match options.stages { + Some(stages) if !stages.is_empty() => stages, + _ => continue, + } + } else { + Stages::resolve(options.stages, config.default_stages) + }; + + // `Stage::Manual` has no git shim, so it is ignored here. + let required = Stages::from( + stages + .iter() + .filter(|&stage| stage != Stage::Manual) + .collect::>(), + ); + if !required.is_empty() && !required.intersects(installed) { + unmatched.push((id, required)); } - } else { - Stages::resolve(options.stages, config.default_stages) - }; - - // `Stage::Manual` has no git shim, so it is ignored here. - let required = Stages::from( - stages - .iter() - .filter(|&stage| stage != Stage::Manual) - .collect::>(), - ); - if !required.is_empty() && !required.intersects(installed) { - unmatched.push((id, required)); } } } diff --git a/crates/prek/tests/install.rs b/crates/prek/tests/install.rs index 4368a5c45..bbe34ea77 100644 --- a/crates/prek/tests/install.rs +++ b/crates/prek/tests/install.rs @@ -1966,3 +1966,50 @@ fn install_warns_on_unmatched_remote_hook_with_config_stages() { hint: install the missing hook type(s) with `prek install --hook-type `, or set `default_install_hook_types` in your config. "); } + +#[test] +fn install_warns_for_workspace_subproject_hooks() -> anyhow::Result<()> { + let context = TestContext::new(); + context.init_project(); + + context.write_pre_commit_config(indoc! {r" + repos: + - repo: local + hooks: + - id: root-hook + name: Root hook + language: system + entry: echo root + "}); + + let nested = context.work_dir().child("nested"); + nested.create_dir_all()?; + nested + .child(".pre-commit-config.yaml") + .write_str(indoc! {r" + repos: + - repo: local + hooks: + - id: nested-push-hook + name: Nested push hook + language: system + entry: echo nested + stages: [pre-push] + "})?; + context.git_add("."); + + // A plain root install runs the nested project too, so its pre-push-only hook is reported. + cmd_snapshot!(context.filters(), context.install(), @r" + success: true + exit_code: 0 + ----- stdout ----- + prek installed at `.git/hooks/pre-commit` + + ----- stderr ----- + warning: The following hooks won't run automatically because their stages aren't being installed: + - `nested-push-hook` (pre-push) + hint: install the missing hook type(s) with `prek install --hook-type `, or set `default_install_hook_types` in your config. + "); + + Ok(()) +} From 9141a48a6e39bada4c7deef7614f38acf87a4046 Mon Sep 17 00:00:00 2001 From: BitWeaver Date: Fri, 12 Jun 2026 12:10:22 +0200 Subject: [PATCH 4/5] fix(install): respect selectors and empty remote stages in the warning 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 --- crates/prek/src/cli/install.rs | 70 ++++++++++++---- crates/prek/src/cli/run/selector.rs | 2 +- crates/prek/src/config.rs | 27 ++++-- crates/prek/tests/install.rs | 126 ++++++++++++++++++++++++++++ 4 files changed, 205 insertions(+), 20 deletions(-) diff --git a/crates/prek/src/cli/install.rs b/crates/prek/src/cli/install.rs index 2ba10ced7..20bd9e978 100644 --- a/crates/prek/src/cli/install.rs +++ b/crates/prek/src/cli/install.rs @@ -13,7 +13,7 @@ use same_file::is_same_file; use crate::cli::reporter::{HookInitReporter, HookInstallReporter}; use crate::cli::run; use crate::cli::run::InstallCache; -use crate::cli::run::{SelectorSource, Selectors}; +use crate::cli::run::{ConfiguredHook, SelectorSource, Selectors}; use crate::cli::{ExitStatus, HookType}; use crate::config::{Repo, Stage, Stages, load_config}; use crate::fs::{CWD, Simplified}; @@ -106,7 +106,13 @@ pub(crate) async fn install( }; if project.is_some() { - warn_unmatched_hook_stages(store, config.as_deref(), refresh, &hook_types); + warn_unmatched_hook_stages( + store, + config.as_deref(), + selectors.as_ref(), + refresh, + &hook_types, + ); } for hook_type in hook_types { @@ -206,11 +212,39 @@ fn get_hook_types( hook_types } +/// Return whether the selectors persisted into the installed hook scripts will run this hook. +/// +/// Mirrors `install_hook_script`: every include is persisted, but only CLI-flag skips are +/// (env-var skips are ignored at install time). An include that only matches a remote hook +/// through its manifest alias can't be recognized from the config alone, so such a hook is +/// treated as not selected. +fn shim_selects_hook(selectors: Option<&Selectors>, hook: &ConfiguredHook<'_>) -> bool { + let Some(selectors) = selectors else { + return true; + }; + + if selectors + .skips() + .iter() + .filter(|skip| matches!(skip.source(), SelectorSource::CliFlag(_))) + .any(|skip| skip.matches_configured_hook(hook)) + { + return false; + } + + let includes = selectors.includes(); + includes.is_empty() + || includes + .iter() + .any(|include| include.matches_configured_hook(hook)) +} + /// Warn about configured hooks whose stages aren't covered by the hook types being installed, /// since their shims won't be installed and they won't run automatically. fn warn_unmatched_hook_stages( store: &Store, config: Option<&Path>, + selectors: Option<&Selectors>, refresh: bool, hook_types: &[HookType], ) { @@ -235,18 +269,26 @@ fn warn_unmatched_hook_stages( for project in workspace.projects() { let config = project.config(); for repo in &config.repos { - // A remote hook may inherit `stages` from its manifest, which we can't see without - // fetching the repo, so only trust explicit config-level stages and skip the rest. let is_remote = matches!(repo, Repo::Remote(_)); - for (id, options) in repo.iter_hooks() { - let stages = if is_remote { - match options.stages { - Some(stages) if !stages.is_empty() => stages, - _ => continue, - } - } else { - Stages::resolve(options.stages, config.default_stages) - }; + for hook in repo.iter_hooks() { + let configured = ConfiguredHook::new( + project.relative_path(), + hook.id, + hook.options.alias.as_deref(), + hook.groups, + ); + if !shim_selects_hook(selectors, &configured) { + continue; + } + + // A remote hook that omits `stages` may inherit them from its manifest, which + // install never fetches, so skip it rather than risk a false positive. An explicit + // value (even an empty list) overrides the manifest and resolves through + // `default_stages`, the same way the hook builder does. + if is_remote && hook.options.stages.is_none() { + continue; + } + let stages = Stages::resolve(hook.options.stages, config.default_stages); // `Stage::Manual` has no git shim, so it is ignored here. let required = Stages::from( @@ -256,7 +298,7 @@ fn warn_unmatched_hook_stages( .collect::>(), ); if !required.is_empty() && !required.intersects(installed) { - unmatched.push((id, required)); + unmatched.push((hook.id, required)); } } } diff --git a/crates/prek/src/cli/run/selector.rs b/crates/prek/src/cli/run/selector.rs index 6831f3ea0..855097968 100644 --- a/crates/prek/src/cli/run/selector.rs +++ b/crates/prek/src/cli/run/selector.rs @@ -164,7 +164,7 @@ impl Selector { } } - fn matches_configured_hook(&self, hook: &ConfiguredHook<'_>) -> bool { + pub(crate) fn matches_configured_hook(&self, hook: &ConfiguredHook<'_>) -> bool { let matches_hook_id = |selector: &str| { hook.id == selector || hook.alias.is_some_and(|alias| alias == selector) }; diff --git a/crates/prek/src/config.rs b/crates/prek/src/config.rs index e56cc32b7..33f0193d1 100644 --- a/crates/prek/src/config.rs +++ b/crates/prek/src/config.rs @@ -1135,13 +1135,30 @@ impl<'de> Deserialize<'de> for Repo { } } +/// The parts of a config hook entry shared by every repo type. +pub(crate) struct ConfigHook<'a> { + pub id: &'a str, + pub groups: Option<&'a [String]>, + pub options: &'a HookOptions, +} + impl Repo { - pub(crate) fn iter_hooks(&self) -> Box + '_> { + pub(crate) fn iter_hooks(&self) -> Box> + '_> { + macro_rules! iter { + ($repo:expr) => { + Box::new($repo.hooks.iter().map(|h| ConfigHook { + id: h.id.as_str(), + groups: h.groups.as_deref(), + options: &h.options, + })) + }; + } + match self { - Repo::Remote(repo) => Box::new(repo.hooks.iter().map(|h| (h.id.as_str(), &h.options))), - Repo::Local(repo) => Box::new(repo.hooks.iter().map(|h| (h.id.as_str(), &h.options))), - Repo::Meta(repo) => Box::new(repo.hooks.iter().map(|h| (h.id.as_str(), &h.options))), - Repo::Builtin(repo) => Box::new(repo.hooks.iter().map(|h| (h.id.as_str(), &h.options))), + Repo::Remote(repo) => iter!(repo), + Repo::Local(repo) => iter!(repo), + Repo::Meta(repo) => iter!(repo), + Repo::Builtin(repo) => iter!(repo), } } } diff --git a/crates/prek/tests/install.rs b/crates/prek/tests/install.rs index bbe34ea77..20d3ed282 100644 --- a/crates/prek/tests/install.rs +++ b/crates/prek/tests/install.rs @@ -2011,5 +2011,131 @@ fn install_warns_for_workspace_subproject_hooks() -> anyhow::Result<()> { hint: install the missing hook type(s) with `prek install --hook-type `, or set `default_install_hook_types` in your config. "); + // Skipping the nested project excludes its hooks from the installed shims, so no warning. + cmd_snapshot!(context.filters(), context.install().arg("--skip").arg("nested/"), @r" + success: true + exit_code: 0 + ----- stdout ----- + prek installed at `.git/hooks/pre-commit` + + ----- stderr ----- + "); + Ok(()) } + +#[test] +fn install_warns_on_remote_hook_with_empty_config_stages() { + let context = TestContext::new(); + context.init_project(); + + // An explicit `stages: []` overrides the manifest and resolves through `default_stages`, + // so the hook is effectively pre-push-only and must be reported. + context.write_pre_commit_config(indoc! {r" + default_stages: [pre-push] + repos: + - repo: https://github.com/pre-commit/pre-commit-hooks + rev: v5.0.0 + hooks: + - id: trailing-whitespace + stages: [] + "}); + + cmd_snapshot!(context.filters(), context.install(), @r" + success: true + exit_code: 0 + ----- stdout ----- + prek installed at `.git/hooks/pre-commit` + + ----- stderr ----- + warning: The following hooks won't run automatically because their stages aren't being installed: + - `trailing-whitespace` (pre-push) + hint: install the missing hook type(s) with `prek install --hook-type `, or set `default_install_hook_types` in your config. + "); +} + +#[test] +fn install_no_warn_for_unselected_hooks() { + let context = TestContext::new(); + context.init_project(); + + context.write_pre_commit_config(indoc! {r" + repos: + - repo: local + hooks: + - id: push-only + name: Push only + language: system + entry: echo push + stages: [pre-push] + - id: other-hook + name: Other hook + language: system + entry: echo other + "}); + + // The installed shim persists `--skip=push-only`, so the hook never runs and isn't reported. + cmd_snapshot!(context.filters(), context.install().arg("--skip").arg("push-only"), @r" + success: true + exit_code: 0 + ----- stdout ----- + prek installed at `.git/hooks/pre-commit` + + ----- stderr ----- + "); + + // `--include other-hook` rules out `push-only` from the shim entirely. + cmd_snapshot!(context.filters(), context.install().arg("other-hook"), @r" + success: true + exit_code: 0 + ----- stdout ----- + prek installed at `.git/hooks/pre-commit` + + ----- stderr ----- + "); + + // Including the unmatched hook itself keeps the warning. + cmd_snapshot!(context.filters(), context.install().arg("push-only"), @r" + success: true + exit_code: 0 + ----- stdout ----- + prek installed at `.git/hooks/pre-commit` + + ----- stderr ----- + warning: The following hooks won't run automatically because their stages aren't being installed: + - `push-only` (pre-push) + hint: install the missing hook type(s) with `prek install --hook-type `, or set `default_install_hook_types` in your config. + "); +} + +#[test] +fn install_warns_despite_env_var_skip() { + let context = TestContext::new(); + context.init_project(); + + context.write_pre_commit_config(indoc! {r" + repos: + - repo: local + hooks: + - id: push-only + name: Push only + language: system + entry: echo push + stages: [pre-push] + "}); + + // Env-var skips are not persisted into the shim, so the hook would still run on + // pre-push and the warning must not be suppressed. + cmd_snapshot!(context.filters(), context.install().env(EnvVars::PREK_SKIP, "push-only"), @r" + success: true + exit_code: 0 + ----- stdout ----- + prek installed at `.git/hooks/pre-commit` + + ----- stderr ----- + warning: The following hooks won't run automatically because their stages aren't being installed: + - `push-only` (pre-push) + hint: install the missing hook type(s) with `prek install --hook-type `, or set `default_install_hook_types` in your config. + warning: Skip selectors from environment variables `PREK_SKIP` are ignored during installing hooks. + "); +} From 8a8de51abba5641fb3fc194332ecede7317bfabd Mon Sep 17 00:00:00 2001 From: BitWeaver Date: Sun, 5 Jul 2026 14:48:06 +0200 Subject: [PATCH 5/5] fix(install): reframe unmatched hook stages as a tip, not a warning Skipping a stage on purpose (e.g. running a hook manually by id, or only from CI) is a valid setup, not a problem to fix, so this drops the `warning:` prefix and prints to stdout instead of stderr as an informational note. --- crates/prek/src/cli/install.rs | 34 +++++++++++------- crates/prek/tests/install.rs | 66 +++++++++++++++++----------------- 2 files changed, 54 insertions(+), 46 deletions(-) diff --git a/crates/prek/src/cli/install.rs b/crates/prek/src/cli/install.rs index 20bd9e978..f10f52284 100644 --- a/crates/prek/src/cli/install.rs +++ b/crates/prek/src/cli/install.rs @@ -106,13 +106,14 @@ pub(crate) async fn install( }; if project.is_some() { - warn_unmatched_hook_stages( + tip_unmatched_hook_stages( store, config.as_deref(), selectors.as_ref(), refresh, &hook_types, - ); + printer, + )?; } for hook_type in hook_types { @@ -239,15 +240,19 @@ fn shim_selects_hook(selectors: Option<&Selectors>, hook: &ConfiguredHook<'_>) - .any(|include| include.matches_configured_hook(hook)) } -/// Warn about configured hooks whose stages aren't covered by the hook types being installed, -/// since their shims won't be installed and they won't run automatically. -fn warn_unmatched_hook_stages( +/// Print a tip about configured hooks whose stages aren't covered by the hook types being +/// installed, since their shims won't be installed and they won't run automatically. +/// +/// This is intentionally not a warning: skipping a stage (e.g. to only run a hook manually by +/// id, or from CI) is a valid setup, not something that needs fixing. +fn tip_unmatched_hook_stages( store: &Store, config: Option<&Path>, selectors: Option<&Selectors>, refresh: bool, hook_types: &[HookType], -) { + printer: Printer, +) -> Result<()> { let installed = Stages::from( hook_types .iter() @@ -257,12 +262,12 @@ fn warn_unmatched_hook_stages( // A plain root install runs every workspace project, so scan them all, the same way `run` does. let Ok(root) = Workspace::find_root(config, &CWD) else { - return; + return Ok(()); }; let Ok(workspace) = Workspace::discover(store, root, config.map(Path::to_path_buf), None, refresh) else { - return; + return Ok(()); }; let mut unmatched: Vec<(&str, Stages)> = Vec::new(); @@ -305,7 +310,7 @@ fn warn_unmatched_hook_stages( } if unmatched.is_empty() { - return; + return Ok(()); } let list = unmatched @@ -313,12 +318,15 @@ fn warn_unmatched_hook_stages( .map(|(id, stages)| format!(" - `{}` ({})", id.cyan(), stages.yellow())) .collect::>() .join("\n"); - warn_user!( - "The following hooks won't run automatically because their stages aren't being installed:\n{list}\n{} install the missing hook type(s) with `{}`, or set `{}` in your config.", - "hint:".bold().yellow(), + writeln!( + printer.stdout(), + "{} the following hooks are configured for stages that aren't installed, so they'll only run when invoked manually or from CI, not automatically:\n{list}\nIf that's intentional, no action is needed. Otherwise, install the missing hook type(s) with `{}`, or set `{}` in your config.", + "tip:".cyan().bold(), "prek install --hook-type ".cyan(), "default_install_hook_types".cyan(), - ); + )?; + + Ok(()) } #[allow(clippy::fn_params_excessive_bools)] diff --git a/crates/prek/tests/install.rs b/crates/prek/tests/install.rs index 20d3ed282..72405f157 100644 --- a/crates/prek/tests/install.rs +++ b/crates/prek/tests/install.rs @@ -1812,7 +1812,7 @@ fn install_invalid_config_warning() { } #[test] -fn install_warns_on_unmatched_hook_stages() { +fn install_tips_on_unmatched_hook_stages() { let context = TestContext::new(); context.init_project(); @@ -1837,15 +1837,15 @@ fn install_warns_on_unmatched_hook_stages() { success: true exit_code: 0 ----- stdout ----- + tip: the following hooks are configured for stages that aren't installed, so they'll only run when invoked manually or from CI, not automatically: + - `push-only` (pre-push) + If that's intentional, no action is needed. Otherwise, install the missing hook type(s) with `prek install --hook-type `, or set `default_install_hook_types` in your config. prek installed at `.git/hooks/pre-commit` ----- stderr ----- - warning: The following hooks won't run automatically because their stages aren't being installed: - - `push-only` (pre-push) - hint: install the missing hook type(s) with `prek install --hook-type `, or set `default_install_hook_types` in your config. "); - // Installing the `pre-push` shim covers `push-only`, so no warning is emitted. + // Installing the `pre-push` shim covers `push-only`, so no tip is emitted. cmd_snapshot!(context.filters(), context.install().arg("--hook-type").arg("pre-push"), @r" success: true exit_code: 0 @@ -1857,7 +1857,7 @@ fn install_warns_on_unmatched_hook_stages() { } #[test] -fn install_no_warn_for_manual_only_hook() { +fn install_no_tip_for_manual_only_hook() { let context = TestContext::new(); context.init_project(); @@ -1884,7 +1884,7 @@ fn install_no_warn_for_manual_only_hook() { } #[test] -fn install_warns_on_unmatched_default_stages() { +fn install_tips_on_unmatched_default_stages() { let context = TestContext::new(); context.init_project(); @@ -1904,12 +1904,12 @@ fn install_warns_on_unmatched_default_stages() { success: true exit_code: 0 ----- stdout ----- + tip: the following hooks are configured for stages that aren't installed, so they'll only run when invoked manually or from CI, not automatically: + - `inherits-default` (pre-push) + If that's intentional, no action is needed. Otherwise, install the missing hook type(s) with `prek install --hook-type `, or set `default_install_hook_types` in your config. prek installed at `.git/hooks/pre-commit` ----- stderr ----- - warning: The following hooks won't run automatically because their stages aren't being installed: - - `inherits-default` (pre-push) - hint: install the missing hook type(s) with `prek install --hook-type `, or set `default_install_hook_types` in your config. "); } @@ -1940,7 +1940,7 @@ fn install_skips_remote_hook_without_config_stages() { } #[test] -fn install_warns_on_unmatched_remote_hook_with_config_stages() { +fn install_tips_on_unmatched_remote_hook_with_config_stages() { let context = TestContext::new(); context.init_project(); @@ -1958,17 +1958,17 @@ fn install_warns_on_unmatched_remote_hook_with_config_stages() { success: true exit_code: 0 ----- stdout ----- + tip: the following hooks are configured for stages that aren't installed, so they'll only run when invoked manually or from CI, not automatically: + - `trailing-whitespace` (pre-push) + If that's intentional, no action is needed. Otherwise, install the missing hook type(s) with `prek install --hook-type `, or set `default_install_hook_types` in your config. prek installed at `.git/hooks/pre-commit` ----- stderr ----- - warning: The following hooks won't run automatically because their stages aren't being installed: - - `trailing-whitespace` (pre-push) - hint: install the missing hook type(s) with `prek install --hook-type `, or set `default_install_hook_types` in your config. "); } #[test] -fn install_warns_for_workspace_subproject_hooks() -> anyhow::Result<()> { +fn install_tips_for_workspace_subproject_hooks() -> anyhow::Result<()> { let context = TestContext::new(); context.init_project(); @@ -2003,15 +2003,15 @@ fn install_warns_for_workspace_subproject_hooks() -> anyhow::Result<()> { success: true exit_code: 0 ----- stdout ----- + tip: the following hooks are configured for stages that aren't installed, so they'll only run when invoked manually or from CI, not automatically: + - `nested-push-hook` (pre-push) + If that's intentional, no action is needed. Otherwise, install the missing hook type(s) with `prek install --hook-type `, or set `default_install_hook_types` in your config. prek installed at `.git/hooks/pre-commit` ----- stderr ----- - warning: The following hooks won't run automatically because their stages aren't being installed: - - `nested-push-hook` (pre-push) - hint: install the missing hook type(s) with `prek install --hook-type `, or set `default_install_hook_types` in your config. "); - // Skipping the nested project excludes its hooks from the installed shims, so no warning. + // Skipping the nested project excludes its hooks from the installed shims, so no tip. cmd_snapshot!(context.filters(), context.install().arg("--skip").arg("nested/"), @r" success: true exit_code: 0 @@ -2025,7 +2025,7 @@ fn install_warns_for_workspace_subproject_hooks() -> anyhow::Result<()> { } #[test] -fn install_warns_on_remote_hook_with_empty_config_stages() { +fn install_tips_on_remote_hook_with_empty_config_stages() { let context = TestContext::new(); context.init_project(); @@ -2045,17 +2045,17 @@ fn install_warns_on_remote_hook_with_empty_config_stages() { success: true exit_code: 0 ----- stdout ----- + tip: the following hooks are configured for stages that aren't installed, so they'll only run when invoked manually or from CI, not automatically: + - `trailing-whitespace` (pre-push) + If that's intentional, no action is needed. Otherwise, install the missing hook type(s) with `prek install --hook-type `, or set `default_install_hook_types` in your config. prek installed at `.git/hooks/pre-commit` ----- stderr ----- - warning: The following hooks won't run automatically because their stages aren't being installed: - - `trailing-whitespace` (pre-push) - hint: install the missing hook type(s) with `prek install --hook-type `, or set `default_install_hook_types` in your config. "); } #[test] -fn install_no_warn_for_unselected_hooks() { +fn install_no_tip_for_unselected_hooks() { let context = TestContext::new(); context.init_project(); @@ -2094,22 +2094,22 @@ fn install_no_warn_for_unselected_hooks() { ----- stderr ----- "); - // Including the unmatched hook itself keeps the warning. + // Including the unmatched hook itself keeps the tip. cmd_snapshot!(context.filters(), context.install().arg("push-only"), @r" success: true exit_code: 0 ----- stdout ----- + tip: the following hooks are configured for stages that aren't installed, so they'll only run when invoked manually or from CI, not automatically: + - `push-only` (pre-push) + If that's intentional, no action is needed. Otherwise, install the missing hook type(s) with `prek install --hook-type `, or set `default_install_hook_types` in your config. prek installed at `.git/hooks/pre-commit` ----- stderr ----- - warning: The following hooks won't run automatically because their stages aren't being installed: - - `push-only` (pre-push) - hint: install the missing hook type(s) with `prek install --hook-type `, or set `default_install_hook_types` in your config. "); } #[test] -fn install_warns_despite_env_var_skip() { +fn install_tips_despite_env_var_skip() { let context = TestContext::new(); context.init_project(); @@ -2125,17 +2125,17 @@ fn install_warns_despite_env_var_skip() { "}); // Env-var skips are not persisted into the shim, so the hook would still run on - // pre-push and the warning must not be suppressed. + // pre-push and the tip must not be suppressed. cmd_snapshot!(context.filters(), context.install().env(EnvVars::PREK_SKIP, "push-only"), @r" success: true exit_code: 0 ----- stdout ----- + tip: the following hooks are configured for stages that aren't installed, so they'll only run when invoked manually or from CI, not automatically: + - `push-only` (pre-push) + If that's intentional, no action is needed. Otherwise, install the missing hook type(s) with `prek install --hook-type `, or set `default_install_hook_types` in your config. prek installed at `.git/hooks/pre-commit` ----- stderr ----- - warning: The following hooks won't run automatically because their stages aren't being installed: - - `push-only` (pre-push) - hint: install the missing hook type(s) with `prek install --hook-type `, or set `default_install_hook_types` in your config. warning: Skip selectors from environment variables `PREK_SKIP` are ignored during installing hooks. "); }