fix(maestro): reinstall hooks when an older set is on disk

The daemon's auto-install skipped on presence, so a hook installed
before #131 never gained UserPromptSubmit and no session was ever named
after its prompt.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

crates/maestro-daemon/src/cli.rs

@@ -55,6 +55,9 @@ async fn run(config_override: Option<PathBuf>) -> Result<(), Box<dyn Error>> {
             crate::hooks::AutoInstallOutcome::Installed => {
                 tracing::info!("installed claude-code activity hook (#106)");
             }
+            crate::hooks::AutoInstallOutcome::Upgraded => {
+                tracing::info!("updated claude-code activity hook to this build (#106)");
+            }
             crate::hooks::AutoInstallOutcome::Failed => {
                 tracing::warn!("hooks auto-install failed; run `maestro hooks install` manually");
             }

crates/maestro-daemon/src/hooks.rs

@@ -1,26 +1,34 @@
 //! First-start hook auto-install (#106).
 //!
-//! The daemon runs [`auto_install`] once at startup: when the user's
-//! `~/.claude/settings.json` carries no maestro hook yet (and
-//! `[hooks].auto_install` hasn't opted out — checked by the caller), the same
-//! installer `maestro hooks install` uses writes it. Best-effort: a failure is
-//! warned about and never blocks startup. The same global hook serves every
-//! Claude Code session on the machine, so nothing here targets repositories
-//! or worktrees.
+//! The daemon runs [`auto_install`] once at startup: unless the user's
+//! `~/.claude/settings.json` already carries exactly what this build ships
+//! (and `[hooks].auto_install` hasn't opted out — checked by the caller), the
+//! same installer `maestro hooks install` uses writes it. That covers
+//! upgrades as well as first starts: an install from a maestro that had fewer
+//! events — #131 added `UserPromptSubmit` to a hook #106 had already put in
+//! everyone's settings — is rewritten rather than left half-wired. Best-effort:
+//! a failure is warned about and never blocks startup. The same global hook
+//! serves every Claude Code session on the machine, so nothing here targets
+//! repositories or worktrees.
 
 use std::path::Path;
 
-/// Install the Claude Code activity hook for `home_dir` unless one is already
-/// present or the home dir is unresolvable (`""`). Returns what happened.
+/// Install the Claude Code activity hook for `home_dir` unless the current
+/// one is already there or the home dir is unresolvable (`""`). Returns what
+/// happened; an older install being brought up to date is
+/// [`AutoInstallOutcome::Upgraded`], so the log can say so.
 pub fn auto_install(home_dir: &str) -> AutoInstallOutcome {
     if home_dir.is_empty() {
         return AutoInstallOutcome::Skipped;
     }
     let settings = maestro_hooks::settings_path(Path::new(home_dir));
-    if maestro_hooks::is_installed(&settings) {
+    let script = maestro_hooks::script_path(Path::new(home_dir));
+    if maestro_hooks::is_current(&settings, &script) {
         return AutoInstallOutcome::AlreadyPresent;
     }
+    let stale = maestro_hooks::is_installed(&settings);
     match maestro_hooks::install_for_home(Path::new(home_dir)) {
+        Ok(_) if stale => AutoInstallOutcome::Upgraded,
         Ok(_) => AutoInstallOutcome::Installed,
         Err(e) => {
             tracing::warn!(error = %e, "hooks auto-install failed (non-fatal)");
@@ -33,6 +41,8 @@ pub fn auto_install(home_dir: &str) -> AutoInstallOutcome {
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum AutoInstallOutcome {
     Installed,
+    /// An older maestro's hook was there and has been rewritten.
+    Upgraded,
     AlreadyPresent,
     Failed,
     Skipped,
@@ -58,7 +68,7 @@ mod tests {
         let dir = tempfile::tempdir().unwrap();
         let home = dir.path().to_str().unwrap();
         auto_install(home);
-        // A foreign edit between starts survives; presence alone skips.
+        // A current install is left exactly as it is.
         let settings = dir.path().join(".claude/settings.json");
         let before = std::fs::read_to_string(&settings).unwrap();
         assert_eq!(auto_install(home), AutoInstallOutcome::AlreadyPresent);
@@ -69,6 +79,27 @@ mod tests {
         );
     }
 
+    /// An install from an older maestro — present, but missing an event this
+    /// build ships — is upgraded rather than left alone (#131 shipped a new
+    /// event after #106 had already installed itself).
+    #[test]
+    fn a_stale_install_is_upgraded() {
+        let dir = tempfile::tempdir().unwrap();
+        let home = dir.path().to_str().unwrap();
+        auto_install(home);
+        let settings = dir.path().join(".claude/settings.json");
+        let script = maestro_hooks::script_path(dir.path());
+
+        let mut root: serde_json::Value =
+            serde_json::from_str(&std::fs::read_to_string(&settings).unwrap()).unwrap();
+        root["hooks"].as_object_mut().unwrap().remove("Stop");
+        std::fs::write(&settings, serde_json::to_string_pretty(&root).unwrap()).unwrap();
+        std::fs::write(&script, "#!/bin/sh\nexit 0\n").unwrap();
+
+        assert_eq!(auto_install(home), AutoInstallOutcome::Upgraded);
+        assert!(maestro_hooks::is_current(&settings, &script));
+    }
+
     #[test]
     fn unresolvable_home_is_skipped() {
         assert_eq!(auto_install(""), AutoInstallOutcome::Skipped);

crates/maestro-hooks/src/lib.rs

@@ -120,13 +120,75 @@ pub fn settings_path(home: &Path) -> std::path::PathBuf {
     home.join(".claude/settings.json")
 }
 
-/// Whether `settings_path` already carries a maestro hook (#106).
+/// Whether `settings_path` already carries a maestro hook (#106). Presence
+/// only: an install from an older release answers `true` even when it is
+/// missing events this build ships — see [`is_current`].
 pub fn is_installed(settings_path: &Path) -> bool {
     settings_json(settings_path)
         .map(|root| find_maestro_commands(&root).next().is_some())
         .unwrap_or(false)
 }
 
+/// Whether what is on disk is what this build ships: the script at
+/// `script_path` byte-for-byte, and exactly one settings entry per event in
+/// [`HOOK_EVENTS`] and [`PROMPT_EVENTS`], pointing at that script.
+///
+/// This is the upgrade question. A maestro that gains an event — #131 added
+/// `UserPromptSubmit` long after #106 had installed itself — leaves every
+/// existing user with a present but incomplete install, which
+/// [`is_installed`] cannot tell apart from a current one.
+pub fn is_current(settings_path: &Path, script_path: &Path) -> bool {
+    if std::fs::read_to_string(script_path).ok().as_deref() != Some(HOOK_SCRIPT) {
+        return false;
+    }
+    settings_json(settings_path)
+        .map(|root| maestro_entries(&root) == expected_entries(script_path))
+        .unwrap_or(false)
+}
+
+/// Every maestro entry in the file as `(event, command)`, sorted — so a
+/// duplicate, a stale event or a wrong script path all show up as a
+/// difference against [`expected_entries`].
+fn maestro_entries(root: &serde_json::Value) -> Vec<(String, String)> {
+    let mut found: Vec<(String, String)> = root
+        .get("hooks")
+        .and_then(|h| h.as_object())
+        .into_iter()
+        .flatten()
+        .flat_map(|(event, groups)| {
+            groups
+                .as_array()
+                .into_iter()
+                .flatten()
+                .filter_map(|group| group.get("hooks"))
+                .filter_map(|inner| inner.as_array())
+                .flatten()
+                .filter_map(|hook| hook.get("command"))
+                .filter_map(|cmd| cmd.as_str())
+                .filter(|cmd| cmd.contains(HOOK_MARKER))
+                .map(move |cmd| (event.clone(), cmd.to_string()))
+        })
+        .collect();
+    found.sort();
+    found
+}
+
+/// What [`install`] writes for `script_path`, in the same shape.
+fn expected_entries(script_path: &Path) -> Vec<(String, String)> {
+    let mut want: Vec<(String, String)> = HOOK_EVENTS
+        .iter()
+        .chain(PROMPT_EVENTS.iter())
+        .map(|(event, state)| {
+            (
+                (*event).to_string(),
+                format!("{} {state}", script_path.display()),
+            )
+        })
+        .collect();
+    want.sort();
+    want
+}
+
 /// Write [`HOOK_SCRIPT`] to `path`, executable. Parent directories are
 /// created as needed — this is always under the caller's chosen data dir,
 /// never inside a repository.
@@ -151,11 +213,12 @@ pub fn write_script(path: &Path) -> Result<(), HooksError> {
 pub fn install_for_home(home: &Path) -> Result<bool, HooksError> {
     let script = script_path(home);
     let settings = settings_path(home);
-    let before = std::fs::read_to_string(&settings).unwrap_or_default();
+    // A script left over from an older release counts as a change even when
+    // the settings entries pointing at it are already right.
+    let changed = !is_current(&settings, &script);
     write_script(&script)?;
     install(&settings, &script)?;
-    let after = std::fs::read_to_string(&settings).unwrap_or_default();
-    Ok(before != after)
+    Ok(changed)
 }
 
 /// Register the hook command for every event in [`HOOK_EVENTS`] in the

crates/maestro-hooks/src/tests.rs

@@ -428,3 +428,59 @@ fn the_script_posts_a_prompt_payload_unchanged() {
         "{argv}"
     );
 }
+
+/// `is_current` answers the upgrade question `is_installed` cannot: an
+/// install left over from an older maestro is present, but not what this
+/// build ships (#131 — the prompt event arrived after #106 was installed).
+#[test]
+fn is_current_rejects_an_install_missing_an_event() {
+    let dir = tempfile::tempdir().unwrap();
+    let home = dir.path();
+    install_for_home(home).unwrap();
+    assert!(is_current(&settings_path(home), &script_path(home)));
+
+    // Drop one event, as an older install would have left the file.
+    let mut root: serde_json::Value =
+        serde_json::from_str(&std::fs::read_to_string(settings_path(home)).unwrap()).unwrap();
+    root["hooks"].as_object_mut().unwrap().remove("Stop");
+    std::fs::write(
+        settings_path(home),
+        serde_json::to_string_pretty(&root).unwrap(),
+    )
+    .unwrap();
+
+    assert!(is_installed(&settings_path(home)));
+    assert!(!is_current(&settings_path(home), &script_path(home)));
+}
+
+/// A settings file that is complete but points at a script from an older
+/// release is not current either — the entries are right, the script is not.
+#[test]
+fn is_current_rejects_a_stale_script() {
+    let dir = tempfile::tempdir().unwrap();
+    let home = dir.path();
+    install_for_home(home).unwrap();
+    std::fs::write(script_path(home), "#!/bin/sh\nexit 0\n").unwrap();
+    assert!(!is_current(&settings_path(home), &script_path(home)));
+}
+
+/// Nothing installed at all is not current.
+#[test]
+fn is_current_rejects_a_missing_install() {
+    let dir = tempfile::tempdir().unwrap();
+    let home = dir.path();
+    assert!(!is_current(&settings_path(home), &script_path(home)));
+}
+
+/// `install_for_home` reports a stale script as a change, even when the
+/// settings entries pointing at it already read correctly.
+#[test]
+fn install_for_home_reports_a_stale_script_as_a_change() {
+    let dir = tempfile::tempdir().unwrap();
+    let home = dir.path();
+    assert!(install_for_home(home).unwrap());
+    assert!(!install_for_home(home).unwrap());
+
+    std::fs::write(script_path(home), "#!/bin/sh\nexit 0\n").unwrap();
+    assert!(install_for_home(home).unwrap());
+}