From aaaece1881c4ddd41b1fe877a7785746cba4fed3 Mon Sep 17 00:00:00 2001 From: jeffa-block <233853744+jeffa-block@users.noreply.github.com> Date: Fri, 27 Mar 2026 03:57:40 +1100 Subject: [PATCH] fix: resolve user login PATH for MCP extension subprocesses (#7896) --- .../goose-mcp/src/computercontroller/mod.rs | 47 ++++++----- crates/goose-mcp/src/subprocess.rs | 84 +++++++++++++++++++ 2 files changed, 111 insertions(+), 20 deletions(-) diff --git a/crates/goose-mcp/src/computercontroller/mod.rs b/crates/goose-mcp/src/computercontroller/mod.rs index 7ea8a613..ea4803ba 100644 --- a/crates/goose-mcp/src/computercontroller/mod.rs +++ b/crates/goose-mcp/src/computercontroller/mod.rs @@ -1,3 +1,5 @@ +#[cfg(not(windows))] +use crate::subprocess::merged_path; use crate::subprocess::SubprocessExt; #[cfg(target_os = "macos")] use base64::Engine; @@ -863,21 +865,24 @@ impl ComputerControllerServer { ) })? } - _ => Command::new(shell) - .arg(shell_arg) - .arg(&command) - .env("GOOSE_TERMINAL", "1") - .env("AGENT", "goose") - .set_no_window() - .output() - .await - .map_err(|e| { + _ => { + let mut cmd = Command::new(shell); + cmd.arg(shell_arg) + .arg(&command) + .env("GOOSE_TERMINAL", "1") + .env("AGENT", "goose"); + #[cfg(not(windows))] + if let Some(path) = merged_path() { + cmd.env("PATH", path); + } + cmd.set_no_window().output().await.map_err(|e| { ErrorData::new( ErrorCode::INTERNAL_ERROR, format!("Failed to run script: {}", e), None, ) - })?, + })? + } }; let output_str = String::from_utf8_lossy(&output.stdout).into_owned(); @@ -1065,16 +1070,18 @@ impl ComputerControllerServer { #[cfg(target_os = "macos")] fn run_peekaboo_cmd(&self, args: &[&str]) -> Result { - let output = std::process::Command::new("peekaboo") - .args(args) - .output() - .map_err(|e| { - ErrorData::new( - ErrorCode::INTERNAL_ERROR, - format!("Failed to run peekaboo: {}", e), - None, - ) - })?; + let mut cmd = std::process::Command::new("peekaboo"); + cmd.args(args); + if let Some(path) = merged_path() { + cmd.env("PATH", path); + } + let output = cmd.output().map_err(|e| { + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to run peekaboo: {}", e), + None, + ) + })?; let stdout = String::from_utf8_lossy(&output.stdout).into_owned(); let stderr = String::from_utf8_lossy(&output.stderr).into_owned(); if !output.status.success() { diff --git a/crates/goose-mcp/src/subprocess.rs b/crates/goose-mcp/src/subprocess.rs index c2b63d15..f2bd9a85 100644 --- a/crates/goose-mcp/src/subprocess.rs +++ b/crates/goose-mcp/src/subprocess.rs @@ -1,3 +1,4 @@ +use std::sync::OnceLock; use tokio::process::Command; #[cfg(windows)] @@ -27,3 +28,86 @@ impl SubprocessExt for std::process::Command { self } } + +/// Resolve the user's full PATH by running a login shell. +/// +/// When goosed is launched from a desktop app (e.g. Electron), it may inherit +/// a minimal PATH like `/usr/bin:/bin`. This function spawns a login shell to +/// source the user's profile and recover the full PATH. +/// +/// Ported from `crates/goose/src/agents/platform_extensions/developer/shell.rs` +/// where it was introduced in #5774 for the developer extension. This makes the +/// same fix available to all MCP extensions in goose-mcp. +#[cfg(not(windows))] +fn resolve_login_shell_path() -> Option { + use std::path::PathBuf; + use std::process::Stdio; + + // Prefer the user's configured shell so we source the right profile files. + // Fall back to /bin/bash (common default) then sh as last resort. + let shell = std::env::var("SHELL") + .ok() + .filter(|s| PathBuf::from(s).is_file()) + .unwrap_or_else(|| { + if PathBuf::from("/bin/bash").is_file() { + "/bin/bash".to_string() + } else { + "sh".to_string() + } + }); + + std::process::Command::new(&shell) + .args(["-l", "-i", "-c", "echo $PATH"]) + .stdin(Stdio::null()) + .stderr(Stdio::null()) + .output() + .ok() + .and_then(|output| { + if output.status.success() { + // Take the last non-empty line — interactive shells may emit + // extra output from profile scripts before our echo. + String::from_utf8_lossy(&output.stdout) + .lines() + .rev() + .find(|line| !line.trim().is_empty()) + .map(|line| line.trim().to_string()) + .filter(|path| !path.is_empty()) + } else { + None + } + }) +} + +/// Returns the user's full login shell PATH, resolved once and cached. +/// +/// Call this before spawning subprocesses to ensure they inherit the user's +/// full PATH rather than the restricted one from the desktop app launcher. +#[cfg(not(windows))] +pub fn user_login_path() -> Option<&'static str> { + static CACHED: OnceLock> = OnceLock::new(); + CACHED.get_or_init(resolve_login_shell_path).as_deref() +} + +/// Merge the login shell PATH with the current process PATH. +/// +/// Prepends login shell entries so user tools are found first, while +/// preserving any runtime PATH additions (e.g. from direnv, nix, or +/// auto-install helpers like ensure_peekaboo). +#[cfg(not(windows))] +pub fn merged_path() -> Option { + let login = user_login_path()?; + let current = std::env::var("PATH").unwrap_or_default(); + if current.is_empty() { + return Some(login.to_string()); + } + // Deduplicate: login shell entries first, then any current entries not already present. + let login_entries: Vec<&str> = login.split(':').collect(); + let mut seen: std::collections::HashSet<&str> = login_entries.iter().copied().collect(); + let mut merged = login_entries; + for entry in current.split(':') { + if seen.insert(entry) { + merged.push(entry); + } + } + Some(merged.join(":")) +}