Read paths from an interactive & login shell by jamadeo · Pull Request #5774 · block/goose

jamadeo · 2025-11-17T20:31:47Z

No description provided.

Copilot

Pull Request Overview

This PR adds functionality to read PATH environment variables from an interactive and login shell, addressing the common issue where GUI-launched applications don't inherit the user's shell PATH configuration.

Key changes:

New paths.rs module that asynchronously retrieves PATH from the user's shell using login and interactive modes
Integration in rmcp_developer.rs to override PATH before executing shell commands
Lazy initialization with caching to avoid repeatedly spawning shells

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
crates/goose-mcp/src/developer/paths.rs	New module implementing shell PATH retrieval with platform-specific logic for Unix and Windows, using OnceCell for caching
crates/goose-mcp/src/developer/rmcp_developer.rs	Integrates shell PATH retrieval by calling `get_shell_path_dirs()` and setting PATH environment variable before command execution
crates/goose-mcp/src/developer/mod.rs	Exposes the new `paths` module as public

Copilot · 2025-11-17T20:40:17Z

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

+    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);


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 · 2025-11-17T20:40:17Z

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

+        .map_err(|e| anyhow::anyhow!("Invalid UTF-8 in shell output: {}", e))?
+        .trim()
+        .to_string();
+


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.

i mean fish?

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 ...

michaelneale · 2025-11-17T21:09:55Z

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

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


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

michaelneale · 2025-11-17T21:10:09Z

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

+    });
+
+    {
+        #[cfg(windows)]


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

michaelneale · 2025-11-17T21:11:14Z

I think this is good - the copilot feedback if right is quite helpful (ie powershell woudln't pick things up, and fish shell would break with -c - not sure if true though!)

DOsinga · 2025-11-18T00:16:25Z

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

+        .map_err(|e| anyhow::anyhow!("Invalid UTF-8 in shell output: {}", e))?
+        .trim()
+        .to_string();
+


i mean fish?

michaelneale · 2025-11-18T00:26:21Z

@DOsinga fish shell has a lot of fans!

Copilot

Pull Request Overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.

Copilot · 2025-11-18T18:09:59Z

crates/goose-cli/src/cli.rs

+        Some(Command::Mcp { server }) => match server {
+            McpCommand::AutoVisualiser => serve(AutoVisualiserRouter::new()).await?,
+            McpCommand::ComputerController => serve(ComputerControllerServer::new()).await?,
+            McpCommand::Memory => serve(MemoryServer::new()).await?,
+            McpCommand::Tutorial => serve(TutorialServer::new()).await?,
+            McpCommand::Developer => serve(DeveloperServer::new()).await?,


Logging setup was removed for MCP commands. This means MCP servers started via goose-cli won't have logging configured. Consider adding crate::logging::setup_logging(Some(&format!("mcp-{}", server.name())), None)?; before the match statement.

Suggested change

Some(Command::Mcp { server }) => match server {

McpCommand::AutoVisualiser => serve(AutoVisualiserRouter::new()).await?,

McpCommand::ComputerController => serve(ComputerControllerServer::new()).await?,

McpCommand::Memory => serve(MemoryServer::new()).await?,

McpCommand::Tutorial => serve(TutorialServer::new()).await?,

McpCommand::Developer => serve(DeveloperServer::new()).await?,

Some(Command::Mcp { server }) => {

crate::logging::setup_logging(Some(&format!("mcp-{}", server.name())), None)?;

match server {

McpCommand::AutoVisualiser => serve(AutoVisualiserRouter::new()).await?,

McpCommand::ComputerController => serve(ComputerControllerServer::new()).await?,

McpCommand::Memory => serve(MemoryServer::new()).await?,

McpCommand::Tutorial => serve(TutorialServer::new()).await?,

McpCommand::Developer => serve(DeveloperServer::new()).await?,

}

Copilot · 2025-11-18T18:10:00Z

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

+    let output = Command::new(shell)
+        .args(["-l", "-i", "-c", "echo $PATH"])


The -i flag is not supported by fish shell, which will cause this command to fail for fish users. Consider conditionally using ["-l", "-c", "echo $PATH"] for fish and ["-l", "-i", "-c", "echo $PATH"] for bash/zsh.

Suggested change

let output = Command::new(shell)

.args(["-l", "-i", "-c", "echo $PATH"])

// Use different args for fish shell, which does not support -i

let shell_name = std::path::Path::new(shell)

.file_stem()

.and_then(|s| s.to_str())

.unwrap_or("");

let args: [&str; 4] = if shell_name == "fish" {

["-l", "", "-c", "echo $PATH"]

} else {

["-l", "-i", "-c", "echo $PATH"]

};

// Remove empty string from args for fish

let args: Vec<&str> = args.iter().filter(|&&a| !a.is_empty()).copied().collect();

let output = Command::new(shell)

.args(&args)

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot

Pull Request Overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.

Copilot · 2025-11-18T23:02:57Z

crates/goose-cli/src/cli.rs

+                McpCommand::ComputerController => serve(ComputerControllerServer::new()).await?,
+                McpCommand::Memory => serve(MemoryServer::new()).await?,
+                McpCommand::Tutorial => serve(TutorialServer::new()).await?,
+                McpCommand::Developer => serve(DeveloperServer::new()).await?,


The goose-cli Developer server doesn't enable shell PATH extension like goose-server does. This creates inconsistent behavior - users will get different PATH environments depending on which binary they use. Either add .extend_path_with_shell(true).bash_env_file(Some(Paths::config_dir().join(\".bash_env\"))) here to match goose-server, or document why the CLI version intentionally has different behavior.

yes that is the idea!

Copilot · 2025-11-18T23:02:58Z

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

+fn safe_truncate(s: &str, max_chars: usize) -> String {
+    if s.chars().count() <= max_chars {
+        s.to_string()
+    } else {
+        let truncated: String = s.chars().take(max_chars.saturating_sub(3)).collect();
+        format!("{}...", truncated)
+    }
+}


Duplicating safe_truncate creates a maintenance burden - changes to the original won't propagate here. Consider creating a shared utility crate that both goose and goose-mcp can depend on, or re-evaluate whether breaking the goose dependency is worth this duplication.

Suggested change

fn safe_truncate(s: &str, max_chars: usize) -> String {

if s.chars().count() <= max_chars {

s.to_string()

} else {

let truncated: String = s.chars().take(max_chars.saturating_sub(3)).collect();

format!("{}...", truncated)

}

}

use goose_util::safe_truncate;

…xt-test * 'main' of github.com:block/goose: chore: Add Adrian Cole to Maintainers (#5815) [MCP-UI] Proxy and Better Message Handling (#5487) Release 1.15.0 Document New Window menu in macOS dock (#5811) Catch cron errors (#5707) feat/fix Re-enabled WAL with commit transaction management (Linux Verification Requested) (#5793) chore: remove autopilot experimental feature (#5781) Read paths from an interactive & login shell (#5774) docs: acp clients (#5800)

* main: feat/fix Re-enabled WAL with commit transaction management (Linux Verification Requested) (#5793) chore: remove autopilot experimental feature (#5781) Read paths from an interactive & login shell (#5774) docs: acp clients (#5800) Provider error proxy for simulating various types of errors (#5091) chore: Add links to maintainer profiles (#5788) Quick fix for community all stars script (#5798) Document Mistral AI provider (#5799) docs: Add Community Stars recipe script and txt file (#5776)

* main: (33 commits) fix: support Gemini 3's thought signatures (#5806) chore: Add Adrian Cole to Maintainers (#5815) [MCP-UI] Proxy and Better Message Handling (#5487) Release 1.15.0 Document New Window menu in macOS dock (#5811) Catch cron errors (#5707) feat/fix Re-enabled WAL with commit transaction management (Linux Verification Requested) (#5793) chore: remove autopilot experimental feature (#5781) Read paths from an interactive & login shell (#5774) docs: acp clients (#5800) Provider error proxy for simulating various types of errors (#5091) chore: Add links to maintainer profiles (#5788) Quick fix for community all stars script (#5798) Document Mistral AI provider (#5799) docs: Add Community Stars recipe script and txt file (#5776) chore: incorporate LF feedback (#5787) docs: quick launcher (#5779) Bump auto scroll threshold (#5738) fix: add one-time cleanup for linux hermit locking issues (#5742) Don't show update tray icon if GOOSE_VERSION is set (#5750) ...

Signed-off-by: Blair Allan <Blairallan@icloud.com>

sw-square · 2026-02-02T19:15:18Z

It feels weird to have goose use -i here—in my interactive shells, I do a lot of setup like keybinds, syntax highlighting, etc—all the $PATH setup is done in the .profiles.

DOsinga · 2026-02-02T20:44:18Z

It feels weird to have goose use -i here—in my interactive shells, I do a lot of setup like keybinds, syntax highlighting, etc—all the $PATH setup is done in the .profiles.

I don't think there is an ideal answer here. users expect certain things to carry over from their shell settings

Copilot AI review requested due to automatic review settings November 17, 2025 20:31

Read paths from an interacive & login shell

07981df

jamadeo force-pushed the jackamadeo/all-the-paths branch from 6136df0 to 07981df Compare November 17, 2025 20:32

Copilot started reviewing on behalf of jamadeo November 17, 2025 20:37 View session

Copilot finished reviewing on behalf of jamadeo November 17, 2025 20:39

Copilot AI reviewed Nov 17, 2025

View reviewed changes

jamadeo requested a review from DOsinga November 17, 2025 20:42

copilot had good feedback

3a0938b

michaelneale reviewed Nov 17, 2025

View reviewed changes

DOsinga approved these changes Nov 18, 2025

View reviewed changes

Do it for the server, not for the CLI

6d670a3

Copilot AI review requested due to automatic review settings November 18, 2025 18:02

Copilot started reviewing on behalf of jamadeo November 18, 2025 18:03 View session

Copilot finished reviewing on behalf of jamadeo November 18, 2025 18:09

Copilot AI reviewed Nov 18, 2025

View reviewed changes

jamadeo added 3 commits November 18, 2025 13:12

Merge remote-tracking branch 'origin/main' into jackamadeo/all-the-paths

67e7796

apparently we need this

9393073

default because clippy

bcaffbc

Copilot AI review requested due to automatic review settings November 18, 2025 18:44

Copilot AI reviewed Nov 18, 2025

View reviewed changes

jamadeo added 2 commits November 18, 2025 17:47

Put logging back

645af80

Prefer runtime

1e64341

Copilot AI review requested due to automatic review settings November 18, 2025 23:00

Copilot started reviewing on behalf of jamadeo November 18, 2025 23:01 View session

Copilot finished reviewing on behalf of jamadeo November 18, 2025 23:02

Copilot AI reviewed Nov 18, 2025

View reviewed changes

jamadeo merged commit 5ba636f into main Nov 19, 2025
23 checks passed

jamadeo added a commit that referenced this pull request Nov 19, 2025

Read paths from an interactive & login shell (#5774)

e9b9dcc

github-actions bot mentioned this pull request Nov 19, 2025

chore(release): release version 1.15.0 (minor) #5780

Merged

jamadeo changed the title ~~Read paths from an interacive & login shell~~ Read paths from an interactive & login shell Nov 19, 2025

This was referenced Nov 19, 2025

Run the developer mcp in a login shell #5617

Closed

Goose Desktop unable to find Homebrew, pip, Node, and other non-system commands #5680

Closed

github-actions bot mentioned this pull request Nov 25, 2025

chore(release): release version 1.16.0 (minor) #5867

Closed

BlairAllan pushed a commit to BlairAllan/goose that referenced this pull request Nov 29, 2025

Read paths from an interactive & login shell (block#5774)

cc9e339

Signed-off-by: Blair Allan <Blairallan@icloud.com>

github-actions bot mentioned this pull request Dec 4, 2025

chore(release): release version 1.16.0 (minor) #5985

Merged

		let output = Command::new(shell)
		.args(["-l", "-i", "-c", "echo $PATH"])

-    let output = Command::new(shell)
-        .args(["-l", "-i", "-c", "echo $PATH"])
+    // Use different args for fish shell, which does not support -i
+    let shell_name = std::path::Path::new(shell)
+        .file_stem()
+        .and_then(|s| s.to_str())
+        .unwrap_or("");
+    let args: [&str; 4] = if shell_name == "fish" {
+        ["-l", "", "-c", "echo $PATH"]
+    } else {
+        ["-l", "-i", "-c", "echo $PATH"]
+    };
+    // Remove empty string from args for fish
+    let args: Vec<&str> = args.iter().filter(|&&a| !a.is_empty()).copied().collect();
+    let output = Command::new(shell)
+        .args(&args)

Conversation

jamadeo commented Nov 17, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

DOsinga Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

jamadeo Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

michaelneale Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

michaelneale Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

michaelneale commented Nov 17, 2025

Uh oh!

DOsinga Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

michaelneale commented Nov 18, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

jamadeo Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sw-square commented Feb 2, 2026

Uh oh!

DOsinga commented Feb 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants