Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions crates/goose-mcp/src/developer/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
pub mod analyze;
mod editor_models;
mod lang;
pub mod paths;
mod shell;
mod text_editor;

Expand Down
124 changes: 124 additions & 0 deletions crates/goose-mcp/src/developer/paths.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
use anyhow::Result;
use std::env;
use std::path::PathBuf;
use tokio::process::Command;
use tokio::sync::OnceCell;

static SHELL_PATH_DIRS: OnceCell<Result<Vec<PathBuf>, anyhow::Error>> = OnceCell::const_new();

pub async fn get_shell_path_dirs() -> Result<&'static Vec<PathBuf>> {
let result = SHELL_PATH_DIRS
.get_or_init(|| async {
get_shell_path_async()
.await
.map(|path| env::split_paths(&path).collect())
})
.await;

match result {
Ok(dirs) => Ok(dirs),
Err(e) => Err(anyhow::anyhow!(
"Failed to get shell PATH directories: {}",
e
)),
}
}

async fn get_shell_path_async() -> Result<String> {
let shell = env::var("SHELL").unwrap_or_else(|_| {
if cfg!(windows) {
"cmd".to_string()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could use a macro, but then I think this is probably fine (and probably less lines of code)

} else {
"/bin/bash".to_string()
}
});

{
#[cfg(windows)]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if using macro here, could use it for if statement above?

{
get_windows_path_async(&shell).await
}
#[cfg(not(windows))]
{
get_unix_path_async(&shell).await
}
}
.or_else(|e| {
tracing::warn!(
"Failed to get PATH from shell ({}), falling back to current PATH",
e
);
env::var("PATH").map_err(|_| anyhow::anyhow!("No PATH variable available"))
})
}

#[cfg(not(windows))]
async fn get_unix_path_async(shell: &str) -> Result<String> {
let flags = if shell.ends_with("fish") {
vec!["-l", "-c"]
} else {
vec!["-l", "-i", "-c"]
};
let output = Command::new(shell)
.args(flags)
.output()
.await
.map_err(|e| anyhow::anyhow!("Failed to execute shell command: {}", e))?;

if !output.status.success() {
let stderr = String::from_utf8_lossy(&output.stderr);
return Err(anyhow::anyhow!("Shell command failed: {}", stderr));
}

let path = String::from_utf8(output.stdout)
.map_err(|e| anyhow::anyhow!("Invalid UTF-8 in shell output: {}", e))?
.trim()
.to_string();

Copy link

Copilot AI Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The -l -i flag combination will fail with fish shell, which doesn't support using -l with -c. Consider using only -l (login mode sources profile files) or detecting the shell type and adjusting flags accordingly.

Copilot uses AI. Check for mistakes.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i mean fish?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I blindly accepted copilot's suggestion, but if I try it myself this works just fine:

% fish -i -l -c 'echo $PATH'
/usr/local/bin ...

if path.is_empty() {
return Err(anyhow::anyhow!("Shell returned empty PATH"));
}

Ok(path)
}

#[cfg(windows)]
async fn get_windows_path_async(shell: &str) -> Result<String> {
let shell_name = std::path::Path::new(shell)
.file_stem()
.and_then(|s| s.to_str())
.unwrap_or("cmd");

let output = match shell_name {
"pwsh" | "powershell" => {
Command::new(shell)
.args(["-NoLogo", "-Command", "$env:PATH"])
.output()
.await
}
_ => {
Command::new(shell)
.args(["/c", "echo %PATH%"])
.output()
.await
}
};

let output = output.map_err(|e| anyhow::anyhow!("Failed to execute shell command: {}", e))?;

if !output.status.success() {
let stderr = String::from_utf8_lossy(&output.stderr);
Copy link

Copilot AI Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The -NoProfile flag prevents loading PowerShell profiles, which means this won't get the user's customized PATH. This is inconsistent with the Unix implementation which uses -l (login) to load profiles. Consider removing -NoProfile or using -NoLogo instead if you want to suppress the banner while still loading profiles.

Copilot uses AI. Check for mistakes.
return Err(anyhow::anyhow!("Shell command failed: {}", stderr));
}

let path = String::from_utf8(output.stdout)
.map_err(|e| anyhow::anyhow!("Invalid UTF-8 in shell output: {}", e))?
.trim()
.to_string();

if path.is_empty() {
return Err(anyhow::anyhow!("Shell returned empty PATH"));
}

Ok(path)
}
15 changes: 14 additions & 1 deletion crates/goose-mcp/src/developer/rmcp_developer.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use anyhow::anyhow;
use base64::Engine;
use ignore::gitignore::{Gitignore, GitignoreBuilder};
use include_dir::{include_dir, Dir};
Expand All @@ -17,6 +18,7 @@ use rmcp::{
use serde::{Deserialize, Serialize};
use std::{
collections::HashMap,
env::join_paths,
future::Future,
io::Cursor,
path::{Path, PathBuf},
Expand All @@ -31,6 +33,8 @@ use tokio::{
use tokio_stream::{wrappers::SplitStream, StreamExt as _};
use tokio_util::sync::CancellationToken;

use crate::developer::paths::get_shell_path_dirs;

use super::analyze::{types::AnalyzeParams, CodeAnalyzer};
use super::editor_models::{create_editor_model, EditorModel};
use super::shell::{
Expand Down Expand Up @@ -945,7 +949,16 @@ impl DeveloperServer {
// Get platform-specific shell configuration
let shell_config = get_shell_config();

let mut child = configure_shell_command(&shell_config, command)
let mut command = configure_shell_command(&shell_config, command);

if let Ok(v) = get_shell_path_dirs()
.await
.and_then(|dirs| join_paths(dirs).map_err(|e| anyhow!(e)))
{
command.env("PATH", v);
}

let mut child = command
.spawn()
.map_err(|e| ErrorData::new(ErrorCode::INTERNAL_ERROR, e.to_string(), None))?;

Expand Down
Loading