bug(hooks): repair PATH for plugin hook commands

crates/goose/src/agents/agent.rs

@@ -363,7 +363,10 @@ impl Agent {
                 permission_manager,
                 provider.clone(),
             ),
-            hook_manager: crate::hooks::HookManager::load(std::env::current_dir().ok().as_deref()),
+            hook_manager: crate::hooks::HookManager::load(
+                std::env::current_dir().ok().as_deref(),
+                use_login_shell_path,
+            ),
             #[cfg(test)]
             stop_hook_block_cap_override: None,
             container: Mutex::new(None),

crates/goose/src/agents/platform_extensions/developer/shell.rs

@@ -26,15 +26,15 @@ use crate::subprocess::SubprocessExt;
 /// When inside Flatpak, shell commands must be wrapped with `flatpak-spawn --host`
 /// to execute on the host system rather than inside the sandbox.
 #[cfg(not(windows))]
-fn is_flatpak() -> bool {
+pub(crate) fn is_flatpak() -> bool {
     std::path::Path::new("/.flatpak-info").exists()
 }
 
 #[cfg(not(windows))]
 const FLATPAK_HOST_ARGS: [&str; 2] = ["--host", "--watch-bus"];
 
 #[cfg(not(windows))]
-fn flatpak_spawn_command() -> tokio::process::Command {
+pub(crate) fn flatpak_spawn_command() -> tokio::process::Command {
     let mut command = tokio::process::Command::new("flatpak-spawn");
     command.args(FLATPAK_HOST_ARGS);
     command
@@ -201,7 +201,7 @@ pub struct ShellOutput {
 /// a minimal PATH like `/usr/bin:/bin`. This function spawns a login shell to
 /// source the user's profile and recover the full PATH.
 #[cfg(not(windows))]
-fn resolve_login_shell_path() -> Option<String> {
+pub(crate) fn resolve_login_shell_path() -> Option<String> {
     use process_wrap::std::{CommandWrap, ProcessSession};
 
     let shell = unix_shell();

crates/goose/src/hooks/mod.rs

@@ -26,6 +26,7 @@
 use std::collections::HashMap;
 use std::path::{Path, PathBuf};
 use std::process::Stdio;
+use std::sync::OnceLock;
 use std::time::Duration;
 
 use anyhow::{Context, Result};
@@ -224,21 +225,22 @@ pub enum HookDecision {
 #[derive(Debug, Default, Clone)]
 pub struct HookManager {
     rules: HashMap<HookEvent, Vec<LoadedRule>>,
+    use_login_shell_path: bool,
 }
 
 impl HookManager {
     /// Build a manager by scanning all enabled plugins for `hooks/hooks.json`.
-    pub fn load(project_root: Option<&Path>) -> Self {
+    pub fn load(project_root: Option<&Path>, use_login_shell_path: bool) -> Self {
         let plugins = discover_enabled_plugins(project_root);
-        Self::from_plugins(plugins)
+        Self::from_plugins(plugins, use_login_shell_path)
     }
 
     #[cfg(test)]
     pub(crate) fn from_plugins_for_test(plugins: Vec<DiscoveredPlugin>) -> Self {
-        Self::from_plugins(plugins)
+        Self::from_plugins(plugins, false)
     }
 
-    fn from_plugins(plugins: Vec<DiscoveredPlugin>) -> Self {
+    fn from_plugins(plugins: Vec<DiscoveredPlugin>, use_login_shell_path: bool) -> Self {
         let mut rules: HashMap<HookEvent, Vec<LoadedRule>> = HashMap::new();
         let mut total = 0usize;
 
@@ -271,7 +273,10 @@ impl HookManager {
             );
         }
 
-        Self { rules }
+        Self {
+            rules,
+            use_login_shell_path,
+        }
     }
 
     /// Returns true if any rule is registered for `event`.
@@ -314,19 +319,25 @@ impl HookManager {
                     command = %command,
                     "Running plugin hook",
                 );
-                let res = run_command_hook(command, &rule.plugin_root, &payload, *timeout)
-                    .await
-                    .and_then(|o| {
-                        if o.status.success() {
-                            Ok(())
-                        } else {
-                            anyhow::bail!(
-                                "hook `{command}` exited with {:?}: {}",
-                                o.status.code(),
-                                String::from_utf8_lossy(&o.stderr).trim()
-                            )
-                        }
-                    });
+                let res = run_command_hook(
+                    command,
+                    &rule.plugin_root,
+                    &payload,
+                    *timeout,
+                    self.use_login_shell_path,
+                )
+                .await
+                .and_then(|o| {
+                    if o.status.success() {
+                        Ok(())
+                    } else {
+                        anyhow::bail!(
+                            "hook `{command}` exited with {:?}: {}",
+                            o.status.code(),
+                            String::from_utf8_lossy(&o.stderr).trim()
+                        )
+                    }
+                });
                 if let Err(err) = res {
                     warn!(
                         plugin = %rule.plugin_name,
@@ -368,20 +379,27 @@ impl HookManager {
 
             for action in &rule.actions {
                 let LoadedAction::Command { command, timeout } = action;
-                let output =
-                    match run_command_hook(command, &rule.plugin_root, &payload, *timeout).await {
-                        Ok(o) => o,
-                        Err(err) => {
-                            warn!(
-                                plugin = %rule.plugin_name,
-                                event = %event,
-                                command = %command,
-                                error = %err,
-                                "Plugin hook failed",
-                            );
-                            continue;
-                        }
-                    };
+                let output = match run_command_hook(
+                    command,
+                    &rule.plugin_root,
+                    &payload,
+                    *timeout,
+                    self.use_login_shell_path,
+                )
+                .await
+                {
+                    Ok(o) => o,
+                    Err(err) => {
+                        warn!(
+                            plugin = %rule.plugin_name,
+                            event = %event,
+                            command = %command,
+                            error = %err,
+                            "Plugin hook failed",
+                        );
+                        continue;
+                    }
+                };
 
                 if let Some(reason) = deny_reason(&output) {
                     info!(
@@ -507,16 +525,38 @@ async fn run_command_hook(
     plugin_root: &Path,
     payload: &str,
     timeout: Duration,
+    use_login_shell_path: bool,
+) -> Result<std::process::Output> {
+    match tokio::time::timeout(
+        timeout,
+        run_command_hook_inner(raw_command, plugin_root, payload, use_login_shell_path),
+    )
+    .await
+    {
+        Ok(res) => res,
+        Err(_) => anyhow::bail!("hook `{raw_command}` timed out after {:?}", timeout),
+    }
+}
+
+async fn run_command_hook_inner(
+    raw_command: &str,
+    plugin_root: &Path,
+    payload: &str,
+    use_login_shell_path: bool,
 ) -> Result<std::process::Output> {
     let command = expand_plugin_root(raw_command, plugin_root);
-    let mut child = Command::new("sh")
-        .arg("-c")
-        .arg(&command)
-        .env("PLUGIN_ROOT", plugin_root)
+    let path = if use_login_shell_path {
+        hook_path().await
+    } else {
+        None
+    };
+    let mut process = hook_command(&command, plugin_root, path.as_deref());
+    process
         .stdin(Stdio::piped())
         .stdout(Stdio::piped())
         .stderr(Stdio::piped())
-        .kill_on_drop(true)
+        .kill_on_drop(true);
+    let mut child = process
         .spawn()
         .with_context(|| format!("spawning hook `{command}`"))?;
 
@@ -525,10 +565,87 @@ async fn run_command_hook(
         let _ = stdin.shutdown().await;
     }
 
-    match tokio::time::timeout(timeout, child.wait_with_output()).await {
-        Ok(res) => res.with_context(|| format!("waiting on hook `{command}`")),
-        Err(_) => anyhow::bail!("hook `{command}` timed out after {:?}", timeout),
+    child
+        .wait_with_output()
+        .await
+        .with_context(|| format!("waiting on hook `{command}`"))
+}
+
+fn hook_command(command: &str, plugin_root: &Path, path: Option<&str>) -> Command {
+    #[cfg(not(windows))]
+    {
+        if crate::agents::platform_extensions::developer::shell::is_flatpak() {
+            let mut process =
+                crate::agents::platform_extensions::developer::shell::flatpak_spawn_command();
+            process.arg(format!("--env=PLUGIN_ROOT={}", plugin_root.display()));
+            if let Some(path) = path {
+                process.arg(format!("--env=PATH={path}"));
+            }
+            process.arg("sh").arg("-c").arg(command);
+            return process;
+        }
+    }
+
+    let mut process = Command::new("sh");
+    process
+        .arg("-c")
+        .arg(command)
+        .env("PLUGIN_ROOT", plugin_root);
+    if let Some(path) = path {
+        process.env("PATH", path);
     }
+    process
+}
+
+async fn hook_path() -> Option<String> {
+    static HOOK_PATH: OnceLock<tokio::sync::watch::Receiver<Option<String>>> = OnceLock::new();
+    let mut rx = HOOK_PATH
+        .get_or_init(|| {
+            let (tx, rx) = tokio::sync::watch::channel(None);
+            tokio::spawn(async move {
+                let path = resolve_hook_path().await;
+                let _ = tx.send(path);
+            });
+            rx
+        })
+        .clone();
+
+    if rx.borrow().is_some() {
+        return rx.borrow().clone();
+    }
+    if rx.changed().await.is_ok() {
+        rx.borrow().clone()
+    } else {
+        None
+    }
+}
+
+async fn resolve_hook_path() -> Option<String> {
+    #[cfg(not(windows))]
+    {
+        tokio::task::spawn_blocking(|| {
+            crate::agents::platform_extensions::developer::shell::resolve_login_shell_path()
+                .map(|login| merge_paths(&login, &std::env::var("PATH").unwrap_or_default()))
+        })
+        .await
+        .ok()
+        .flatten()
+    }
+    #[cfg(windows)]
+    {
+        None
+    }
+}
+
+fn merge_paths(first: &str, second: &str) -> String {
+    let mut seen = std::collections::HashSet::new();
+    let mut merged = Vec::new();
+    for entry in first.split(':').chain(second.split(':')) {
+        if !entry.is_empty() && seen.insert(entry) {
+            merged.push(entry);
+        }
+    }
+    merged.join(":")
 }
 
 fn expand_plugin_root(command: &str, plugin_root: &Path) -> String {
@@ -548,7 +665,7 @@ mod tests {
     }
 
     fn make_manager(plugins: Vec<DiscoveredPlugin>) -> HookManager {
-        HookManager::from_plugins(plugins)
+        HookManager::from_plugins(plugins, false)
     }
 
     #[test]
@@ -652,6 +769,69 @@ mod tests {
         );
     }
 
+    #[test]
+    fn merge_paths_keeps_login_entries_first() {
+        assert_eq!(
+            merge_paths("/opt/homebrew/bin:/bin", "/bin:/usr/bin:/custom/bin"),
+            "/opt/homebrew/bin:/bin:/usr/bin:/custom/bin"
+        );
+    }
+
+    #[cfg(not(windows))]
+    #[tokio::test]
+    async fn command_hooks_repair_path_when_enabled() {
+        let tmp = tempfile::tempdir().unwrap();
+        let login_bin = tmp.path().join("login-bin");
+        std::fs::create_dir(&login_bin).unwrap();
+
+        let fake_shell = tmp.path().join("fake-login-shell");
+        std::fs::write(
+            &fake_shell,
+            "#!/bin/sh\nprintf '%s\\n' \"$FAKE_LOGIN_PATH\"\n",
+        )
+        .unwrap();
+        let helper = login_bin.join("hook-visible-tool");
+        std::fs::write(&helper, "#!/bin/sh\nprintf 'hook-visible-tool-ran'\n").unwrap();
+        #[cfg(unix)]
+        {
+            use std::os::unix::fs::PermissionsExt;
+            for path in [&fake_shell, &helper] {
+                let mut perms = std::fs::metadata(path).unwrap().permissions();
+                perms.set_mode(0o755);
+                std::fs::set_permissions(path, perms).unwrap();
+            }
+        }
+
+        let fake_shell = fake_shell.to_string_lossy().into_owned();
+        let fake_login_path = format!("{}:/usr/bin:/bin", login_bin.display());
+        let _guard = env_lock::lock_env([
+            ("GOOSE_SHELL", Some(fake_shell.as_str())),
+            ("FAKE_LOGIN_PATH", Some(fake_login_path.as_str())),
+            (
+                "PATH",
+                Some(
+                    "/Applications/Goose.app/Contents/Resources/bin:/usr/bin:/bin:/usr/sbin:/sbin",
+                ),
+            ),
+        ]);
+
+        let output = run_command_hook(
+            "hook-visible-tool",
+            tmp.path(),
+            "{}",
+            Duration::from_secs(5),
+            true,
+        )
+        .await
+        .unwrap();
+
+        assert!(output.status.success());
+        assert_eq!(
+            String::from_utf8_lossy(&output.stdout),
+            "hook-visible-tool-ran"
+        );
+    }
+
     #[tokio::test]
     async fn matcher_filters_by_tool_name() {
         let tmp = tempfile::tempdir().unwrap();