diff --git a/desktop/src-tauri/src/commands/agent_auth.rs b/desktop/src-tauri/src/commands/agent_auth.rs index c23263de7e..b9461ceefa 100644 --- a/desktop/src-tauri/src/commands/agent_auth.rs +++ b/desktop/src-tauri/src/commands/agent_auth.rs @@ -126,7 +126,7 @@ fn run_buzz_acp_auth_command( .or_else(|| resolve_command("buzz-acp")) .ok_or_else(|| "buzz-acp helper not found".to_string())?; - let augmented_path = auth_command_path(); + let augmented_path = crate::managed_agents::readiness::cli_probe::augmented_path(); run_buzz_acp_auth_command_with_paths( &acp_path, adapter_command.0, @@ -136,43 +136,6 @@ fn run_buzz_acp_auth_command( ) } -/// PATH for the buzz-acp auth helper child process. -/// -/// Uses the augmented agent PATH so `#!/usr/bin/env node` adapter shims -/// resolve the Buzz-managed Node runtime — the same PATH normal agent -/// launches and readiness probes use. -/// -/// On Windows, `login_shell_path()` is intentionally `None`, so the augmented -/// PATH contains only Buzz-managed directories and the exe parent. Buzz does -/// not ship a managed Node runtime on Windows, and npm `.cmd` adapters need -/// the user's normal PATH to find `node` (and often `claude`/`codex`), so the -/// inherited process PATH is appended there instead of being replaced. -fn auth_command_path() -> Option { - let augmented = crate::managed_agents::readiness::cli_probe::augmented_path(); - if !cfg!(windows) { - return augmented; - } - let inherited = std::env::var("PATH").ok().filter(|path| !path.is_empty()); - append_inherited_path(augmented, inherited) -} - -/// Append `inherited` PATH entries after `augmented` ones. Returns `None` -/// when `augmented` is `None` so the child simply inherits the process -/// environment unchanged (the pre-augmentation behavior on Windows). -fn append_inherited_path(augmented: Option, inherited: Option) -> Option { - let augmented = augmented?; - let Some(inherited) = inherited else { - return Some(augmented); - }; - let parts: Vec = std::env::split_paths(&augmented) - .chain(std::env::split_paths(&inherited)) - .collect(); - std::env::join_paths(parts) - .ok() - .map(|joined| joined.to_string_lossy().into_owned()) - .or(Some(augmented)) -} - fn run_buzz_acp_auth_command_with_paths( acp_path: &Path, adapter_name: &str, @@ -457,44 +420,10 @@ fn shell_escape(arg: &str) -> String { #[cfg(test)] mod tests { use super::{ - adapter_terminal_argv, append_inherited_path, is_claude_subscription_login, - run_buzz_acp_auth_command_with_paths, shell_escape, shell_join, uses_terminal_auth, - windows_terminal_args, AcpAuthMethod, + adapter_terminal_argv, is_claude_subscription_login, run_buzz_acp_auth_command_with_paths, + shell_escape, shell_join, uses_terminal_auth, windows_terminal_args, AcpAuthMethod, }; - /// Windows regression: the augmented PATH there holds only Buzz-managed - /// dirs and the exe parent (no login-shell PATH, no managed Node), so the - /// user's inherited PATH must be appended for npm `.cmd` adapters to find - /// `node`/`claude`/`codex`. - #[test] - fn append_inherited_path_appends_after_augmented() { - let sep = if cfg!(windows) { ';' } else { ':' }; - let augmented = format!("{0}buzz-bin{1}{0}exe-dir", std::path::MAIN_SEPARATOR, sep); - let inherited = format!( - "{0}user-bin{1}{0}system-bin", - std::path::MAIN_SEPARATOR, - sep - ); - let joined = append_inherited_path(Some(augmented.clone()), Some(inherited.clone())) - .expect("joined PATH"); - assert_eq!(joined, format!("{augmented}{sep}{inherited}")); - } - - #[test] - fn append_inherited_path_falls_back_to_inheritance_without_augmented() { - // With no augmented PATH the helper must not set PATH at all, letting - // the child inherit the process environment unchanged. - assert_eq!(append_inherited_path(None, Some("anything".into())), None); - } - - #[test] - fn append_inherited_path_keeps_augmented_without_inherited() { - assert_eq!( - append_inherited_path(Some("only-augmented".into()), None), - Some("only-augmented".into()) - ); - } - #[cfg(unix)] #[test] fn auth_command_uses_augmented_path_for_node_adapter() { diff --git a/desktop/src-tauri/src/managed_agents/readiness/cli_probe.rs b/desktop/src-tauri/src/managed_agents/readiness/cli_probe.rs index 8e2f866b7e..fd80d4e7aa 100644 --- a/desktop/src-tauri/src/managed_agents/readiness/cli_probe.rs +++ b/desktop/src-tauri/src/managed_agents/readiness/cli_probe.rs @@ -3,7 +3,8 @@ use std::path::Path; use crate::managed_agents::runtime::build_augmented_path; /// Build the augmented PATH for CLI probes, including nvm's default Node.js -/// bin directory so `#!/usr/bin/env node` shims (e.g. codex-acp) resolve. +/// bin directory and, on Windows, the inherited native PATH so Node-backed +/// shims such as `codex-acp.cmd` can find `node.exe`. pub(crate) fn augmented_path() -> Option { let home = dirs::home_dir(); let nvm_bin = home @@ -16,6 +17,7 @@ pub(crate) fn augmented_path() -> Option { .and_then(|exe| exe.parent().map(std::path::Path::to_path_buf)), crate::managed_agents::login_shell_path(), nvm_bin, + crate::managed_agents::windows_inherited_path(), ) } diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index cef669ab25..608890b5bc 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -15,7 +15,7 @@ use crate::{ }; mod path; -pub(in crate::managed_agents) use path::build_augmented_path; +pub(in crate::managed_agents) use path::{build_augmented_path, windows_inherited_path}; mod stop; pub(crate) use stop::managed_agent_runtime_keys; @@ -1692,6 +1692,7 @@ pub fn spawn_agent_child( .and_then(|exe| exe.parent().map(std::path::Path::to_path_buf)), login_shell_path(), nvm_bin, + super::windows_inherited_path(), ); let mut command = std::process::Command::new(&resolved_acp_command); diff --git a/desktop/src-tauri/src/managed_agents/runtime/path.rs b/desktop/src-tauri/src/managed_agents/runtime/path.rs index c1baadda7f..e7a6ce9253 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/path.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/path.rs @@ -11,18 +11,22 @@ use std::path::PathBuf; /// 4. `nvm_bin` — nvm's default Node.js bin dir (if the user uses nvm) /// 5. exe parent dir — DMG sidecars under `Contents/MacOS/` /// 6. user's login-shell `PATH` — runtimes like node/python from other managers +/// 7. inherited native `PATH` — Windows tools such as `node.exe` /// /// `shell_path` is the raw colon-delimited string from a login shell, so it is /// split into individual entries before joining. Pushing it as a single segment /// would make `join_paths` reject it (a segment containing the separator is an /// error), collapsing the entire augmented `PATH` to `None` — the bug this -/// guards against, which left managed agents unable to find `buzz`. Returns -/// `None` only when no entries exist. +/// guards against, which left managed agents unable to find `buzz`. +/// `inherited_path` is supplied only on Windows, where login-shell discovery is +/// intentionally disabled but native npm shims still need the GUI process's +/// PATH. Returns `None` only when no entries exist. pub(in crate::managed_agents) fn build_augmented_path( home: Option, exe_parent: Option, shell_path: Option, nvm_bin: Option, + inherited_path: Option, ) -> Option { let mut parts: Vec = Vec::new(); let home_added = home.is_some(); @@ -49,6 +53,9 @@ pub(in crate::managed_agents) fn build_augmented_path( if let Some(shell_path) = shell_path { parts.extend(std::env::split_paths(&shell_path)); } + if let Some(inherited_path) = inherited_path { + parts.extend(std::env::split_paths(&inherited_path)); + } if parts.is_empty() { return None; } @@ -75,6 +82,7 @@ mod tests { Some(PathBuf::from("/Applications/Buzz.app/Contents/MacOS")), Some("/usr/local/bin:/opt/homebrew/bin:/usr/bin:/bin".to_string()), None, + None, ); let result = result.expect("path"); assert!(result.starts_with("/home/agent/.local/bin:"), "{result}"); @@ -90,13 +98,14 @@ mod tests { #[test] fn none_when_no_inputs() { - assert_eq!(build_augmented_path(None, None, None, None), None); + assert_eq!(build_augmented_path(None, None, None, None, None), None); } #[cfg(unix)] #[test] fn shell_path_only() { - let result = build_augmented_path(None, None, Some("/usr/bin:/bin".to_string()), None); + let result = + build_augmented_path(None, None, Some("/usr/bin:/bin".to_string()), None, None); assert_eq!(result.as_deref(), Some("/usr/bin:/bin")); } @@ -108,6 +117,7 @@ mod tests { Some(PathBuf::from("/Applications/Buzz.app/Contents/MacOS")), Some("/usr/bin:/bin".to_string()), Some(PathBuf::from("/home/user/.nvm/versions/node/v20.0.0/bin")), + None, ); let result = result.expect("path"); let local = result.find("/home/user/.local/bin").unwrap(); @@ -129,9 +139,48 @@ mod tests { Some(PathBuf::from("/usr/local/bin")), None, None, + None, ); let result = result.expect("path"); assert!(result.starts_with("/home/user/.local/bin:"), "{result}"); assert!(result.ends_with(":/usr/local/bin"), "{result}"); } + + #[test] + fn inherited_path_follows_buzz_managed_paths() { + let separator = if cfg!(windows) { ';' } else { ':' }; + let inherited = format!("node-bin{separator}system-bin"); + let result = build_augmented_path( + Some(PathBuf::from("agent-home")), + Some(PathBuf::from("buzz-sidecars")), + None, + None, + Some(inherited.clone()), + ) + .expect("path"); + + assert!(result.ends_with(&inherited), "{result}"); + assert!( + result.find("buzz-sidecars").expect("sidecar path") + < result.find("node-bin").expect("inherited path"), + "{result}" + ); + } +} + +/// Return the desktop process's native PATH only on Windows. +/// +/// Windows intentionally skips login-shell discovery because Git Bash returns +/// a POSIX PATH that native children cannot use. The inherited GUI-process PATH +/// is still required by npm `.cmd` shims to find `node.exe` and other native +/// runtimes. Unix callers already receive the user's login-shell PATH. +pub(in crate::managed_agents) fn windows_inherited_path() -> Option { + #[cfg(windows)] + { + std::env::var("PATH").ok().filter(|path| !path.is_empty()) + } + #[cfg(not(windows))] + { + None + } }