Skip to content

Commit e9b9dcc

Browse files
committed
Read paths from an interactive & login shell (#5774)
1 parent e96f43b commit e9b9dcc

File tree

10 files changed

+249
-71
lines changed

10 files changed

+249
-71
lines changed

Cargo.lock

Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/goose-cli/src/cli.rs

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@ use anyhow::Result;
22
use clap::{Args, Parser, Subcommand};
33

44
use goose::config::{Config, ExtensionConfig};
5+
use goose_mcp::mcp_server_runner::{serve, McpCommand};
6+
use goose_mcp::{
7+
AutoVisualiserRouter, ComputerControllerServer, DeveloperServer, MemoryServer, TutorialServer,
8+
};
59

610
use crate::commands::acp::run_acp_agent;
711
use crate::commands::bench::agent_generator;
@@ -420,7 +424,10 @@ enum Command {
420424

421425
/// Manage system prompts and behaviors
422426
#[command(about = "Run one of the mcp servers bundled with goose")]
423-
Mcp { name: String },
427+
Mcp {
428+
#[arg(value_parser = clap::value_parser!(McpCommand))]
429+
server: McpCommand,
430+
},
424431

425432
/// Run goose as an ACP (Agent Client Protocol) agent
426433
#[command(about = "Run goose as an ACP agent server on stdio")]
@@ -877,15 +884,18 @@ pub async fn cli() -> anyhow::Result<()> {
877884
);
878885

879886
match cli.command {
880-
Some(Command::Configure {}) => {
881-
handle_configure().await?;
882-
}
883-
Some(Command::Info { verbose }) => {
884-
handle_info(verbose)?;
885-
}
886-
Some(Command::Mcp { name }) => {
887+
Some(Command::Configure {}) => handle_configure().await?,
888+
Some(Command::Info { verbose }) => handle_info(verbose)?,
889+
Some(Command::Mcp { server }) => {
890+
let name = server.name();
887891
crate::logging::setup_logging(Some(&format!("mcp-{name}")), None)?;
888-
goose_mcp::mcp_server_runner::run_mcp_server(&name).await?;
892+
match server {
893+
McpCommand::AutoVisualiser => serve(AutoVisualiserRouter::new()).await?,
894+
McpCommand::ComputerController => serve(ComputerControllerServer::new()).await?,
895+
McpCommand::Memory => serve(MemoryServer::new()).await?,
896+
McpCommand::Tutorial => serve(TutorialServer::new()).await?,
897+
McpCommand::Developer => serve(DeveloperServer::new()).await?,
898+
}
889899
}
890900
Some(Command::Acp {}) => {
891901
run_acp_agent().await?;

crates/goose-mcp/Cargo.toml

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ description.workspace = true
1111
workspace = true
1212

1313
[dependencies]
14-
goose = { path = "../goose" }
1514
rmcp = { version = "0.8.1", features = ["server", "client", "transport-io", "macros"] }
1615
anyhow = "1.0.94"
1716
tokio = { version = "1", features = ["full"] }
@@ -77,13 +76,13 @@ libc = "0.2"
7776
# ~1000 downloads). Pinned to exact version to prevent supply chain attacks.
7877
mpatch = "=0.2.0"
7978
tokio-util = "0.7.16"
79+
clap = { version = "4", features = ["derive"] }
8080

8181

8282
[dev-dependencies]
8383
serial_test = "3.0.0"
8484
sysinfo = "0.32.1"
8585
temp-env = "0.3.6"
86-
clap = { version = "4", features = ["derive"] }
8786
colored = "2"
8887

8988
[features]

crates/goose-mcp/src/developer/analyze/formatter.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,19 @@ use crate::developer::analyze::types::{
22
AnalysisMode, AnalysisResult, CallChain, EntryType, FocusedAnalysisData,
33
};
44
use crate::developer::lang;
5-
use goose::utils::safe_truncate;
65
use rmcp::model::{Content, Role};
76
use std::collections::{HashMap, HashSet};
87
use std::path::{Path, PathBuf};
98

9+
fn safe_truncate(s: &str, max_chars: usize) -> String {
10+
if s.chars().count() <= max_chars {
11+
s.to_string()
12+
} else {
13+
let truncated: String = s.chars().take(max_chars.saturating_sub(3)).collect();
14+
format!("{}...", truncated)
15+
}
16+
}
17+
1018
pub struct Formatter;
1119

1220
impl Formatter {

crates/goose-mcp/src/developer/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
pub mod analyze;
22
mod editor_models;
33
mod lang;
4+
pub mod paths;
45
mod shell;
56
mod text_editor;
67

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
use anyhow::Result;
2+
use std::env;
3+
use std::path::PathBuf;
4+
use tokio::process::Command;
5+
use tokio::sync::OnceCell;
6+
7+
static SHELL_PATH_DIRS: OnceCell<Result<Vec<PathBuf>, anyhow::Error>> = OnceCell::const_new();
8+
9+
pub async fn get_shell_path_dirs() -> Result<&'static Vec<PathBuf>> {
10+
let result = SHELL_PATH_DIRS
11+
.get_or_init(|| async {
12+
get_shell_path_async()
13+
.await
14+
.map(|path| env::split_paths(&path).collect())
15+
})
16+
.await;
17+
18+
match result {
19+
Ok(dirs) => Ok(dirs),
20+
Err(e) => Err(anyhow::anyhow!(
21+
"Failed to get shell PATH directories: {}",
22+
e
23+
)),
24+
}
25+
}
26+
27+
async fn get_shell_path_async() -> Result<String> {
28+
let shell = env::var("SHELL").unwrap_or_else(|_| {
29+
if cfg!(windows) {
30+
"cmd".to_string()
31+
} else {
32+
"/bin/bash".to_string()
33+
}
34+
});
35+
36+
if cfg!(windows) {
37+
get_windows_path_async(&shell).await
38+
} else {
39+
get_unix_path_async(&shell).await
40+
}
41+
.or_else(|e| {
42+
tracing::warn!(
43+
"Failed to get PATH from shell ({}), falling back to current PATH",
44+
e
45+
);
46+
env::var("PATH").map_err(|_| anyhow::anyhow!("No PATH variable available"))
47+
})
48+
}
49+
50+
async fn get_unix_path_async(shell: &str) -> Result<String> {
51+
let output = Command::new(shell)
52+
.args(["-l", "-i", "-c", "echo $PATH"])
53+
.output()
54+
.await
55+
.map_err(|e| anyhow::anyhow!("Failed to execute shell command: {}", e))?;
56+
57+
if !output.status.success() {
58+
let stderr = String::from_utf8_lossy(&output.stderr);
59+
return Err(anyhow::anyhow!("Shell command failed: {}", stderr));
60+
}
61+
62+
let path = String::from_utf8(output.stdout)
63+
.map_err(|e| anyhow::anyhow!("Invalid UTF-8 in shell output: {}", e))?
64+
.trim()
65+
.to_string();
66+
67+
if path.is_empty() {
68+
return Err(anyhow::anyhow!("Shell returned empty PATH"));
69+
}
70+
71+
Ok(path)
72+
}
73+
74+
async fn get_windows_path_async(shell: &str) -> Result<String> {
75+
let shell_name = std::path::Path::new(shell)
76+
.file_stem()
77+
.and_then(|s| s.to_str())
78+
.unwrap_or("cmd");
79+
80+
let output = match shell_name {
81+
"pwsh" | "powershell" => {
82+
Command::new(shell)
83+
.args(["-NoLogo", "-Command", "$env:PATH"])
84+
.output()
85+
.await
86+
}
87+
_ => {
88+
Command::new(shell)
89+
.args(["/c", "echo %PATH%"])
90+
.output()
91+
.await
92+
}
93+
};
94+
95+
let output = output.map_err(|e| anyhow::anyhow!("Failed to execute shell command: {}", e))?;
96+
97+
if !output.status.success() {
98+
let stderr = String::from_utf8_lossy(&output.stderr);
99+
return Err(anyhow::anyhow!("Shell command failed: {}", stderr));
100+
}
101+
102+
let path = String::from_utf8(output.stdout)
103+
.map_err(|e| anyhow::anyhow!("Invalid UTF-8 in shell output: {}", e))?
104+
.trim()
105+
.to_string();
106+
107+
if path.is_empty() {
108+
return Err(anyhow::anyhow!("Shell returned empty PATH"));
109+
}
110+
111+
Ok(path)
112+
}

crates/goose-mcp/src/developer/rmcp_developer.rs

Lines changed: 47 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use anyhow::anyhow;
12
use base64::Engine;
23
use ignore::gitignore::{Gitignore, GitignoreBuilder};
34
use include_dir::{include_dir, Dir};
@@ -17,6 +18,8 @@ use rmcp::{
1718
use serde::{Deserialize, Serialize};
1819
use std::{
1920
collections::HashMap,
21+
env::join_paths,
22+
ffi::OsString,
2023
future::Future,
2124
io::Cursor,
2225
path::{Path, PathBuf},
@@ -31,11 +34,11 @@ use tokio::{
3134
use tokio_stream::{wrappers::SplitStream, StreamExt as _};
3235
use tokio_util::sync::CancellationToken;
3336

37+
use crate::developer::{paths::get_shell_path_dirs, shell::ShellConfig};
38+
3439
use super::analyze::{types::AnalyzeParams, CodeAnalyzer};
3540
use super::editor_models::{create_editor_model, EditorModel};
36-
use super::shell::{
37-
configure_shell_command, expand_path, get_shell_config, is_absolute_path, kill_process_group,
38-
};
41+
use super::shell::{configure_shell_command, expand_path, is_absolute_path, kill_process_group};
3942
use super::text_editor::{
4043
text_editor_insert, text_editor_replace, text_editor_undo, text_editor_view, text_editor_write,
4144
};
@@ -179,6 +182,8 @@ pub struct DeveloperServer {
179182
pub running_processes: Arc<RwLock<HashMap<String, CancellationToken>>>,
180183
#[cfg(not(test))]
181184
running_processes: Arc<RwLock<HashMap<String, CancellationToken>>>,
185+
bash_env_file: Option<PathBuf>,
186+
extend_path_with_shell: bool,
182187
}
183188

184189
#[tool_handler(router = self.tool_router)]
@@ -549,9 +554,21 @@ impl DeveloperServer {
549554
prompts: load_prompt_files(),
550555
code_analyzer: CodeAnalyzer::new(),
551556
running_processes: Arc::new(RwLock::new(HashMap::new())),
557+
extend_path_with_shell: false,
558+
bash_env_file: None,
552559
}
553560
}
554561

562+
pub fn extend_path_with_shell(mut self, value: bool) -> Self {
563+
self.extend_path_with_shell = value;
564+
self
565+
}
566+
567+
pub fn bash_env_file(mut self, value: Option<PathBuf>) -> Self {
568+
self.bash_env_file = value;
569+
self
570+
}
571+
555572
/// List all available windows that can be used with screen_capture.
556573
/// Returns a list of window titles that can be used with the window_title parameter
557574
/// of the screen_capture tool.
@@ -942,10 +959,34 @@ impl DeveloperServer {
942959
peer: &rmcp::service::Peer<RoleServer>,
943960
cancellation_token: CancellationToken,
944961
) -> Result<String, ErrorData> {
945-
// Get platform-specific shell configuration
946-
let shell_config = get_shell_config();
962+
let mut shell_config = ShellConfig::default();
963+
let shell_name = std::path::Path::new(&shell_config.executable)
964+
.file_name()
965+
.and_then(|s| s.to_str())
966+
.unwrap_or("bash");
967+
968+
if let Some(ref env_file) = self.bash_env_file {
969+
if shell_name == "bash" {
970+
shell_config.envs.push((
971+
OsString::from("BASH_ENV"),
972+
env_file.clone().into_os_string(),
973+
))
974+
}
975+
}
976+
977+
let mut command = configure_shell_command(&shell_config, command);
978+
979+
if self.extend_path_with_shell {
980+
if let Err(e) = get_shell_path_dirs()
981+
.await
982+
.and_then(|dirs| join_paths(dirs).map_err(|e| anyhow!(e)))
983+
.map(|path| command.env("PATH", path))
984+
{
985+
tracing::error!("Failed to extend PATH with shell directories: {}", e)
986+
}
987+
}
947988

948-
let mut child = configure_shell_command(&shell_config, command)
989+
let mut child = command
949990
.spawn()
950991
.map_err(|e| ErrorData::new(ErrorCode::INTERNAL_ERROR, e.to_string(), None))?;
951992

crates/goose-mcp/src/developer/shell.rs

Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
use std::{env, ffi::OsString, process::Stdio};
22

3-
use goose::config::paths::Paths;
43
#[cfg(unix)]
54
#[allow(unused_imports)] // False positive: trait is used for process_group method
65
use std::os::unix::process::CommandExt;
@@ -22,24 +21,10 @@ impl Default for ShellConfig {
2221
#[cfg(not(windows))]
2322
{
2423
let shell = env::var("SHELL").unwrap_or_else(|_| "bash".to_string());
25-
// Get just the shell name from the path (e.g., /bin/zsh -> zsh)
26-
let shell_name = std::path::Path::new(&shell)
27-
.file_name()
28-
.and_then(|s| s.to_str())
29-
.unwrap_or("bash");
30-
31-
// Configure environment based on shell type
32-
let envs = if shell_name == "bash" {
33-
let bash_env = Paths::config_dir().join(".bash_env").into_os_string();
34-
vec![(OsString::from("BASH_ENV"), bash_env)]
35-
} else {
36-
vec![]
37-
};
38-
3924
Self {
4025
executable: shell,
4126
args: vec!["-c".to_string()], // -c is standard across bash/zsh/fish
42-
envs,
27+
envs: vec![],
4328
}
4429
}
4530
}
@@ -82,10 +67,6 @@ impl ShellConfig {
8267
}
8368
}
8469

85-
pub fn get_shell_config() -> ShellConfig {
86-
ShellConfig::default()
87-
}
88-
8970
pub fn expand_path(path_str: &str) -> String {
9071
if cfg!(windows) {
9172
// Expand Windows environment variables (%VAR%)

0 commit comments

Comments
 (0)