diff --git a/Cargo.lock b/Cargo.lock index eee28b0b87..7ff1c8852a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4394,6 +4394,7 @@ dependencies = [ "pem", "pkcs1", "pkcs8", + "process-wrap", "pulldown-cmark", "rand 0.8.5", "rayon", @@ -4527,6 +4528,7 @@ dependencies = [ "indoc", "lopdf", "once_cell", + "process-wrap", "reqwest 0.13.2", "rmcp", "schemars 1.2.1", diff --git a/crates/goose-mcp/Cargo.toml b/crates/goose-mcp/Cargo.toml index 72f0cc3fe8..f028f094b2 100644 --- a/crates/goose-mcp/Cargo.toml +++ b/crates/goose-mcp/Cargo.toml @@ -39,3 +39,4 @@ docx-rs = "0.4.20" image = { version = "0.24.9", features = ["jpeg"] } umya-spreadsheet = "2.2.3" shell-words = { workspace = true } +process-wrap = { version = "9.1.0", features = ["std"] } diff --git a/crates/goose-mcp/src/subprocess.rs b/crates/goose-mcp/src/subprocess.rs index f2bd9a850f..30399f75ff 100644 --- a/crates/goose-mcp/src/subprocess.rs +++ b/crates/goose-mcp/src/subprocess.rs @@ -40,6 +40,7 @@ impl SubprocessExt for std::process::Command { /// same fix available to all MCP extensions in goose-mcp. #[cfg(not(windows))] fn resolve_login_shell_path() -> Option { + use process_wrap::std::{CommandWrap, ProcessSession}; use std::path::PathBuf; use std::process::Stdio; @@ -56,26 +57,31 @@ fn resolve_login_shell_path() -> Option { } }); - std::process::Command::new(&shell) + let mut cmd = CommandWrap::from(std::process::Command::new(&shell)); + cmd.command_mut() .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 - } - }) + .stdout(Stdio::piped()) + .stderr(Stdio::null()); + + // Spawn in a new session so that interactive shell job-control setup + // cannot steal the terminal foreground from the parent goose process. + cmd.wrap(ProcessSession); + + let child = cmd.spawn().ok()?; + let output = child.wait_with_output().ok()?; + if !output.status.success() { + return None; + } + + // 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()) } /// Returns the user's full login shell PATH, resolved once and cached. diff --git a/crates/goose/Cargo.toml b/crates/goose/Cargo.toml index a3604302be..7f5c5ace14 100644 --- a/crates/goose/Cargo.toml +++ b/crates/goose/Cargo.toml @@ -193,6 +193,7 @@ sec1 = { version = "0.7", default-features = false, features = ["der", "pkcs8"], goose-acp-macros = { path = "../goose-acp-macros" } tower-http = { workspace = true, features = ["cors"] } http-body-util = "0.1.3" +process-wrap = { version = "9.1.0", features = ["std"] } [target.'cfg(target_os = "windows")'.dependencies] diff --git a/crates/goose/src/agents/platform_extensions/developer/shell.rs b/crates/goose/src/agents/platform_extensions/developer/shell.rs index 49ac6f1112..8a229fd6a8 100644 --- a/crates/goose/src/agents/platform_extensions/developer/shell.rs +++ b/crates/goose/src/agents/platform_extensions/developer/shell.rs @@ -166,27 +166,34 @@ pub struct ShellOutput { /// source the user's profile and recover the full PATH. #[cfg(not(windows))] fn resolve_login_shell_path() -> Option { + use process_wrap::std::{CommandWrap, ProcessSession}; + let shell = unix_shell(); - let mut child = if is_flatpak() { - flatpak_spawn_process() - .args([&shell, "-l", "-i", "-c", "echo $PATH"]) - .stdin(Stdio::null()) - .stdout(Stdio::piped()) - .stderr(Stdio::null()) - .spawn() - .ok()? + // Build the command, varying only the flatpak vs direct invocation. + let mut cmd = if is_flatpak() { + let mut c = flatpak_spawn_process(); + c.args([&shell, "-l", "-i", "-c", "echo $PATH"]); + CommandWrap::from(c) } else { - std::process::Command::new(&shell) - .args(["-l", "-i", "-c", "echo $PATH"]) - .stdin(Stdio::null()) - .stdout(Stdio::piped()) - .stderr(Stdio::null()) - .spawn() - .ok()? + let mut c = std::process::Command::new(&shell); + c.args(["-l", "-i", "-c", "echo $PATH"]); + CommandWrap::from(c) }; - let mut stdout = child.stdout.take()?; + cmd.command_mut() + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::null()); + + // Spawn in a new session so that bash's interactive job-control setup + // (TIOCSPGRP) cannot steal the terminal foreground from goose, which + // would cause goose to receive SIGTTIN and be suspended on startup. + cmd.wrap(ProcessSession); + + let mut child = cmd.spawn().ok()?; + + let mut stdout = child.stdout().take()?; let (tx, rx) = std::sync::mpsc::channel(); std::thread::spawn(move || { let mut buf = Vec::new(); @@ -197,7 +204,11 @@ fn resolve_login_shell_path() -> Option { }); match rx.recv_timeout(Duration::from_secs(5)) { - Ok(buf) if child.wait().is_ok_and(|s| s.success()) => { + Ok(buf) + if child + .wait() + .is_ok_and(|s: std::process::ExitStatus| s.success()) => + { // Take the last non-empty line — interactive shells may emit // extra output from profile scripts before our echo. String::from_utf8_lossy(&buf) @@ -209,7 +220,6 @@ fn resolve_login_shell_path() -> Option { } _ => { let _ = child.kill(); - let _ = child.wait(); None } }