fix(shell): prevent login-shell PATH probe from suspending goose on startup (#8804)

Signed-off-by: Adam Miller <admiller@redhat.com>
Co-authored-by: Jack Amadeo <jackamadeo@squareup.com>
This commit is contained in:
Adam Miller
2026-04-27 09:56:50 -05:00
committed by GitHub
parent d8e4b55d16
commit 739f4e88b8
5 changed files with 56 additions and 36 deletions
Generated
+2
View File
@@ -4394,6 +4394,7 @@ dependencies = [
"pem", "pem",
"pkcs1", "pkcs1",
"pkcs8", "pkcs8",
"process-wrap",
"pulldown-cmark", "pulldown-cmark",
"rand 0.8.5", "rand 0.8.5",
"rayon", "rayon",
@@ -4527,6 +4528,7 @@ dependencies = [
"indoc", "indoc",
"lopdf", "lopdf",
"once_cell", "once_cell",
"process-wrap",
"reqwest 0.13.2", "reqwest 0.13.2",
"rmcp", "rmcp",
"schemars 1.2.1", "schemars 1.2.1",
+1
View File
@@ -39,3 +39,4 @@ docx-rs = "0.4.20"
image = { version = "0.24.9", features = ["jpeg"] } image = { version = "0.24.9", features = ["jpeg"] }
umya-spreadsheet = "2.2.3" umya-spreadsheet = "2.2.3"
shell-words = { workspace = true } shell-words = { workspace = true }
process-wrap = { version = "9.1.0", features = ["std"] }
+24 -18
View File
@@ -40,6 +40,7 @@ impl SubprocessExt for std::process::Command {
/// same fix available to all MCP extensions in goose-mcp. /// same fix available to all MCP extensions in goose-mcp.
#[cfg(not(windows))] #[cfg(not(windows))]
fn resolve_login_shell_path() -> Option<String> { fn resolve_login_shell_path() -> Option<String> {
use process_wrap::std::{CommandWrap, ProcessSession};
use std::path::PathBuf; use std::path::PathBuf;
use std::process::Stdio; use std::process::Stdio;
@@ -56,26 +57,31 @@ fn resolve_login_shell_path() -> Option<String> {
} }
}); });
std::process::Command::new(&shell) let mut cmd = CommandWrap::from(std::process::Command::new(&shell));
cmd.command_mut()
.args(["-l", "-i", "-c", "echo $PATH"]) .args(["-l", "-i", "-c", "echo $PATH"])
.stdin(Stdio::null()) .stdin(Stdio::null())
.stderr(Stdio::null()) .stdout(Stdio::piped())
.output() .stderr(Stdio::null());
.ok()
.and_then(|output| { // Spawn in a new session so that interactive shell job-control setup
if output.status.success() { // cannot steal the terminal foreground from the parent goose process.
// Take the last non-empty line — interactive shells may emit cmd.wrap(ProcessSession);
// extra output from profile scripts before our echo.
String::from_utf8_lossy(&output.stdout) let child = cmd.spawn().ok()?;
.lines() let output = child.wait_with_output().ok()?;
.rev() if !output.status.success() {
.find(|line| !line.trim().is_empty()) return None;
.map(|line| line.trim().to_string()) }
.filter(|path| !path.is_empty())
} else { // Take the last non-empty line — interactive shells may emit
None // 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. /// Returns the user's full login shell PATH, resolved once and cached.
+1
View File
@@ -193,6 +193,7 @@ sec1 = { version = "0.7", default-features = false, features = ["der", "pkcs8"],
goose-acp-macros = { path = "../goose-acp-macros" } goose-acp-macros = { path = "../goose-acp-macros" }
tower-http = { workspace = true, features = ["cors"] } tower-http = { workspace = true, features = ["cors"] }
http-body-util = "0.1.3" http-body-util = "0.1.3"
process-wrap = { version = "9.1.0", features = ["std"] }
[target.'cfg(target_os = "windows")'.dependencies] [target.'cfg(target_os = "windows")'.dependencies]
@@ -166,27 +166,34 @@ pub struct ShellOutput {
/// source the user's profile and recover the full PATH. /// source the user's profile and recover the full PATH.
#[cfg(not(windows))] #[cfg(not(windows))]
fn resolve_login_shell_path() -> Option<String> { fn resolve_login_shell_path() -> Option<String> {
use process_wrap::std::{CommandWrap, ProcessSession};
let shell = unix_shell(); let shell = unix_shell();
let mut child = if is_flatpak() { // Build the command, varying only the flatpak vs direct invocation.
flatpak_spawn_process() let mut cmd = if is_flatpak() {
.args([&shell, "-l", "-i", "-c", "echo $PATH"]) let mut c = flatpak_spawn_process();
.stdin(Stdio::null()) c.args([&shell, "-l", "-i", "-c", "echo $PATH"]);
.stdout(Stdio::piped()) CommandWrap::from(c)
.stderr(Stdio::null())
.spawn()
.ok()?
} else { } else {
std::process::Command::new(&shell) let mut c = std::process::Command::new(&shell);
.args(["-l", "-i", "-c", "echo $PATH"]) c.args(["-l", "-i", "-c", "echo $PATH"]);
.stdin(Stdio::null()) CommandWrap::from(c)
.stdout(Stdio::piped())
.stderr(Stdio::null())
.spawn()
.ok()?
}; };
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(); let (tx, rx) = std::sync::mpsc::channel();
std::thread::spawn(move || { std::thread::spawn(move || {
let mut buf = Vec::new(); let mut buf = Vec::new();
@@ -197,7 +204,11 @@ fn resolve_login_shell_path() -> Option<String> {
}); });
match rx.recv_timeout(Duration::from_secs(5)) { 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 // Take the last non-empty line — interactive shells may emit
// extra output from profile scripts before our echo. // extra output from profile scripts before our echo.
String::from_utf8_lossy(&buf) String::from_utf8_lossy(&buf)
@@ -209,7 +220,6 @@ fn resolve_login_shell_path() -> Option<String> {
} }
_ => { _ => {
let _ = child.kill(); let _ = child.kill();
let _ = child.wait();
None None
} }
} }