From cd63ccca9f2e1d270d3cc8d32804d94fe049beff Mon Sep 17 00:00:00 2001
From: "@permadeath.com" <did:plc:nlzmjyfv6loqtxyzvdcznwgf>
Date: Sat, 5 Sep 2026 14:30:19 -0400
Subject: [PATCH] fix(repo): stop telling repo configure a declared hook is
 missing

install_change_id_hook told a framework that already declares the
change-id script "was not installed" and to go add it -- the same
latent copy of 3mum4e67kbd2n flagged in the stack create fix. It now
reads clients/git::hooks::declared too, via a new Installed::describe_given,
and reports the hook as already in place instead.

Change-Id: Ic8363c26e3b1735b6f10182f275e9326026267ff
---
 src/clients/git/hooks.rs | 67 ++++++++++++++++++++++++++++++++++++++++
 src/cmd/repo/checkout.rs | 10 +++++-
 2 files changed, 76 insertions(+), 1 deletion(-)

diff --git a/src/clients/git/hooks.rs b/src/clients/git/hooks.rs
index 87a6d5f..7f9a084 100644
--- a/src/clients/git/hooks.rs
+++ b/src/clients/git/hooks.rs
@@ -99,6 +99,28 @@ impl Installed {
             )),
         }
     }
+
+    /// The one-line report, given what [`declared`] already knows about this
+    /// repo's framework config.
+    ///
+    /// `describe` alone cannot tell a framework that already runs the script
+    /// apart from one that ignores it — it only ever sees the literal
+    /// `.git/hooks/commit-msg` file, which looks identical either way. Left
+    /// uncorrected, a repo whose config already declares the hook was told
+    /// the hook "was not installed" and to go add it, which is the
+    /// contradiction `doctor local` disagreed with (3mum4e67kbd2n): its own
+    /// report reads this same repo as configured, from this same `declared`.
+    /// Every other `Installed` variant reports exactly as `describe` does —
+    /// only a framework's silence is worth correcting.
+    pub fn describe_given(&self, declared: Option<&(&'static str, PathBuf)>) -> Option<String> {
+        match (self, declared) {
+            (Installed::Framework { .. }, Some((name, config))) => Some(format!(
+                "{name} already runs the change-id hook (see {}); nothing left to install",
+                config.display()
+            )),
+            _ => self.describe(),
+        }
+    }
 }
 
 /// Whether a hook file is one of the dispatchers a hook framework installs.
@@ -519,6 +541,51 @@ mod tests {
         assert!(said.contains("scripts/change-id-hook.sh"), "{said}");
     }
 
+    /// `describe_given` is what `repo configure` reads instead of `describe`,
+    /// specifically so it does not repeat `install_change_id_hook`'s old
+    /// mistake: telling a repo whose config already declares the script to
+    /// go add it. Given `declared`'s answer, a framework's silence reads
+    /// "already runs it" instead of "was not installed" — every other
+    /// variant, and a framework with no declaration, read exactly as
+    /// `describe` already did. Filed as 3mum4e67kbd2n.
+    #[test]
+    fn a_declared_frameworks_report_says_the_hook_is_already_in_place() {
+        let installed = Installed::Framework {
+            path: PathBuf::from(".git/hooks/commit-msg"),
+            name: "prek",
+        };
+        let config = PathBuf::from("prek.toml");
+
+        let declared = installed
+            .describe_given(Some(&("prek", config.clone())))
+            .expect("a declared framework still gets a line");
+        assert!(
+            declared.contains("already runs the change-id hook"),
+            "{declared}"
+        );
+        assert!(
+            !declared.contains("was not installed"),
+            "must not say the hook is missing when the config already declares it\n{declared}"
+        );
+
+        let undeclared = installed
+            .describe_given(None)
+            .expect("an undeclared framework still gets a line");
+        assert_eq!(
+            undeclared,
+            installed.describe().unwrap(),
+            "with nothing declared, describe_given must fall back to describe verbatim"
+        );
+
+        // Every other variant ignores `declared` entirely: only a
+        // framework's silence needed correcting.
+        let wrote = Installed::Wrote(PathBuf::from(".git/hooks/commit-msg"));
+        assert_eq!(
+            wrote.describe_given(Some(&("prek", config))),
+            wrote.describe()
+        );
+    }
+
     /// A hand-written hook is left exactly as it is, and said so about.
     #[test]
     fn a_hand_written_hook_is_left_alone() {
diff --git a/src/cmd/repo/checkout.rs b/src/cmd/repo/checkout.rs
index 89abdc4..1445627 100644
--- a/src/cmd/repo/checkout.rs
+++ b/src/cmd/repo/checkout.rs
@@ -345,6 +345,14 @@ pub(super) fn install_change_id_hook(
     if dry_run {
         return "skipped";
     }
+    // Whether a framework's own config already declares the script —
+    // `doctor local`'s "ok" reading of this same repo, from the same
+    // check. Without it, a framework that already runs the hook was told
+    // it "was not installed" and to go add it, which is the contradiction
+    // filed as 3mum4e67kbd2n.
+    let declared = gitconfig::toplevel(dir)
+        .as_deref()
+        .and_then(crate::clients::git::hooks::declared);
     match crate::clients::git::hooks::install_commit_msg(dir) {
         Ok(installed) => {
             // "add it to your framework's config" is not worth saying to
@@ -354,7 +362,7 @@ pub(super) fn install_change_id_hook(
                 installed,
                 crate::clients::git::hooks::Installed::Framework { .. }
             );
-            if let Some(line) = installed.describe()
+            if let Some(line) = installed.describe_given(declared.as_ref())
                 && !(declaring && framework)
             {
                 crate::term::say::step!(Git, "{line}");
-- 
2.51.2