diff --git a/crates/buzz-acp/src/acp.rs b/crates/buzz-acp/src/acp.rs index b553adaabb..eb8fe7fa42 100644 --- a/crates/buzz-acp/src/acp.rs +++ b/crates/buzz-acp/src/acp.rs @@ -20,6 +20,19 @@ use crate::usage::{TurnUsage, UsageTracker}; /// Lines exceeding this limit are rejected to prevent OOM from rogue agents. const MAX_LINE_SIZE: usize = 10_000_000; // 10 MB +/// Suppress a console window for agent adapter children on Windows (no-op elsewhere). +fn hide_console(cmd: &mut tokio::process::Command) { + #[cfg(windows)] + { + const CREATE_NO_WINDOW: u32 = 0x0800_0000; + cmd.creation_flags(CREATE_NO_WINDOW); + } + #[cfg(not(windows))] + { + let _ = cmd; + } +} + /// An MCP server configuration passed to `session/new`. /// /// Corresponds to the `McpServerStdio` variant in the ACP schema. @@ -466,6 +479,9 @@ impl AcpClient { #[cfg(unix)] cmd.process_group(0); + // Desktop launches buzz-acp windowless; adapters must not allocate consoles (#2292). + hide_console(&mut cmd); + let mut child = cmd.spawn()?; let stdin = child diff --git a/crates/buzz-agent/src/mcp.rs b/crates/buzz-agent/src/mcp.rs index 71c3193a06..3794e33464 100644 --- a/crates/buzz-agent/src/mcp.rs +++ b/crates/buzz-agent/src/mcp.rs @@ -80,6 +80,19 @@ fn windows_child_passthrough_env() -> impl Iterator { .chain(crate::WINDOWS_SHELL_RESOLUTION_ENV.iter().copied()) } +/// Suppress a console window for MCP children on Windows (no-op elsewhere). +fn hide_console(cmd: &mut Command) { + #[cfg(windows)] + { + const CREATE_NO_WINDOW: u32 = 0x0800_0000; + cmd.creation_flags(CREATE_NO_WINDOW); + } + #[cfg(not(windows))] + { + let _ = cmd; + } +} + type Client = RunningService; #[derive(Clone)] @@ -732,6 +745,9 @@ async fn spawn_one( #[cfg(unix)] cmd.process_group(0); + // Windowless buzz-acp parent: MCP children must not allocate consoles. + hide_console(&mut cmd); + let transport = TokioChildProcess::new(cmd) .map_err(|e| AgentError::Mcp(format!("spawn {}: {e}", spec.name)))?; let pgid = transport.id(); diff --git a/crates/buzz-dev-mcp/src/lib.rs b/crates/buzz-dev-mcp/src/lib.rs index cc4725468c..e9a49a3450 100644 --- a/crates/buzz-dev-mcp/src/lib.rs +++ b/crates/buzz-dev-mcp/src/lib.rs @@ -19,6 +19,7 @@ mod str_replace; mod todo; mod tree; mod view_image; +mod windows_console; #[derive(Clone)] struct DevMcp { diff --git a/crates/buzz-dev-mcp/src/rg.rs b/crates/buzz-dev-mcp/src/rg.rs index 199b63432e..63d2ca8d67 100644 --- a/crates/buzz-dev-mcp/src/rg.rs +++ b/crates/buzz-dev-mcp/src/rg.rs @@ -21,11 +21,10 @@ fn try_system_rg(args: &[String]) -> Option { let cleaned_path = clean_path(&self_canon); let candidate = which_rg(&cleaned_path)?; - let status = Command::new(&candidate) - .args(args) - .env("PATH", &cleaned_path) - .status() - .ok()?; + let mut cmd = Command::new(&candidate); + cmd.args(args).env("PATH", &cleaned_path); + crate::windows_console::hide_std(&mut cmd); + let status = cmd.status().ok()?; Some(status.code().unwrap_or(2)) } diff --git a/crates/buzz-dev-mcp/src/shell.rs b/crates/buzz-dev-mcp/src/shell.rs index b7df4c6747..c6342b021d 100644 --- a/crates/buzz-dev-mcp/src/shell.rs +++ b/crates/buzz-dev-mcp/src/shell.rs @@ -177,6 +177,7 @@ pub async fn run( cmd.stderr(Stdio::piped()); cmd.kill_on_drop(true); set_process_group(&mut cmd); + crate::windows_console::hide_tokio(&mut cmd); let started = Instant::now(); let mut child = match cmd.spawn() { @@ -393,13 +394,17 @@ fn resolve_bash(_path_env: &str) -> Result<(PathBuf, String), String> { /// WITHOUT the System32 exclusion — the operator explicitly chose this shell, /// and cmd.exe/powershell.exe live in System32 legitimately. /// 2. `GIT_BASH` env override — legacy escape hatch (kept for back-compat). -/// 3. `bash.exe` on PATH, excluding System32 so we never resolve WSL's launcher. -/// 4. `git.exe` on PATH → its sibling `..\\bin\\bash.exe`. Git for Windows's +/// 3. `git.exe` on PATH → its sibling `..\\bin\\bash.exe`. Git for Windows's /// recommended "Git from the command line" option adds `Git\\cmd` to PATH, /// not `Git\\bin`, so this is the normal post-install route. -/// 5. Standard `ProgramFiles`, `ProgramFiles(x86)`, and `LocalAppData` paths +/// 4. Standard `ProgramFiles`, `ProgramFiles(x86)`, and `LocalAppData` paths /// when the child inherited their parent environment. -/// 6. Git for Windows's machine then user registry `InstallPath`. +/// 5. Git for Windows's machine then user registry `InstallPath`. +/// 6. `bash.exe` on PATH, excluding System32 (WSL) and WindowsApps (App +/// Execution Alias → WSL; hangs #2328). +/// +/// KEEP IN SYNC with `desktop/.../git_bash.rs` `resolve_git_bash_inner` +/// (same order after overrides; Doctor uses a pure inject-env API). /// /// Returns `(resolved_path, display_name)`. The display name is derived from the /// resolved path, guaranteeing the dialect hint and the spawned shell agree. @@ -414,7 +419,7 @@ fn resolve_bash(path_env: &str) -> Result<(PathBuf, String), String> { let name = shell_name_from_path(&p); return Ok((p, name)); } - } else if let Some(found) = scan_path_for_command(&p, path_env, None) { + } else if let Some(found) = scan_path_for_command(&p, path_env, None, false) { let name = shell_name_from_path(&found); return Ok((found, name)); } @@ -427,12 +432,7 @@ fn resolve_bash(path_env: &str) -> Result<(PathBuf, String), String> { } } - let system_root = std::env::var_os("SystemRoot").map(PathBuf::from); - if let Some(p) = scan_path_for_bash(path_env, system_root.as_deref()) { - return Ok((p, "bash".to_string())); - } - - if let Some(git) = scan_path_for_command(Path::new("git.exe"), path_env, None) { + if let Some(git) = scan_path_for_command(Path::new("git.exe"), path_env, None, false) { if let Some(bash) = bash_from_git(&git) { return Ok((bash, "bash".to_string())); } @@ -446,6 +446,11 @@ fn resolve_bash(path_env: &str) -> Result<(PathBuf, String), String> { return Ok((bash, "bash".to_string())); } + let system_root = std::env::var_os("SystemRoot").map(PathBuf::from); + if let Some(p) = scan_path_for_bash(path_env, system_root.as_deref()) { + return Ok((p, "bash".to_string())); + } + Err( "Git for Windows (Git Bash) is required but was not found. Checked \\ BUZZ_SHELL, GIT_BASH, bash.exe and git.exe on PATH, the standard Git install locations, \\ @@ -582,23 +587,24 @@ fn is_under_dir(dir: &Path, root: &Path) -> bool { true } -/// Scan the child's PATH for `bash.exe`, skipping the Windows system directory -/// (`system_root`, normally `%SystemRoot%`) so we never resolve WSL's -/// `System32\bash.exe`. PATH is parsed with `std::env::split_paths` (never a -/// hand-split on ';') so it matches exactly what the spawned child would see. +/// Scan the child's PATH for `bash.exe`, skipping System32 (WSL) and +/// WindowsApps (App Execution Alias → WSL hang, #2328). +/// +/// KEEP IN SYNC with `desktop/.../git_bash.rs` (probe order + WindowsApps skip). #[cfg(windows)] fn scan_path_for_bash(path_env: &str, system_root: Option<&Path>) -> Option { - scan_path_for_command(Path::new("bash.exe"), path_env, system_root) + scan_path_for_command(Path::new("bash.exe"), path_env, system_root, true) } -/// Scan `path_env` for `name` (or `name.exe` on Windows if `name` has no -/// extension), skipping any directory under `system_root` to avoid resolving -/// WSL helpers. Returns the first absolute path found. +/// Scan `path_env` for `name` (or `name.exe` if `name` has no extension). +/// Skips dirs under `system_root` when set. When `skip_windows_apps`, skips +/// `…\Microsoft\WindowsApps`. #[cfg(windows)] fn scan_path_for_command( name: &Path, path_env: &str, system_root: Option<&Path>, + skip_windows_apps: bool, ) -> Option { let needs_exe = name.extension().is_none(); for dir in std::env::split_paths(path_env) { @@ -607,12 +613,13 @@ fn scan_path_for_command( continue; } } - // Try as-is first. + if skip_windows_apps && path_looks_like_windows_apps(&dir) { + continue; + } let candidate = dir.join(name); if candidate.is_file() { return Some(candidate); } - // On Windows, also try with .exe suffix when the name has no extension. if needs_exe { let mut with_exe = dir.join(name); with_exe.set_extension("exe"); @@ -624,6 +631,19 @@ fn scan_path_for_command( None } +/// True for `…\Microsoft\WindowsApps` (App Execution Alias dir), any casing. +/// KEEP IN SYNC with `desktop/.../git_bash.rs`. +#[cfg(windows)] +fn path_looks_like_windows_apps(dir: &Path) -> bool { + let parts: Vec<_> = dir + .components() + .filter_map(|c| c.as_os_str().to_str()) + .collect(); + parts.windows(2).any(|w| { + w[0].eq_ignore_ascii_case("Microsoft") && w[1].eq_ignore_ascii_case("WindowsApps") + }) +} + #[cfg(unix)] fn set_process_group(cmd: &mut Command) { cmd.process_group(0); diff --git a/crates/buzz-dev-mcp/src/windows_console.rs b/crates/buzz-dev-mcp/src/windows_console.rs new file mode 100644 index 0000000000..d23f6d8aaa --- /dev/null +++ b/crates/buzz-dev-mcp/src/windows_console.rs @@ -0,0 +1,32 @@ +//! Suppress console windows for child processes on Windows GUI hosts. +//! +//! Buzz desktop launches this MCP server windowless; children that allocate a +//! console flash a conhost. Use these helpers instead of inlining +//! `CREATE_NO_WINDOW` at each spawn site. + +/// Hide the console for a `std::process::Command` (no-op on non-Windows). +pub(crate) fn hide_std(cmd: &mut std::process::Command) { + #[cfg(windows)] + { + use std::os::windows::process::CommandExt; + const CREATE_NO_WINDOW: u32 = 0x0800_0000; + cmd.creation_flags(CREATE_NO_WINDOW); + } + #[cfg(not(windows))] + { + let _ = cmd; + } +} + +/// Hide the console for a `tokio::process::Command` (no-op on non-Windows). +pub(crate) fn hide_tokio(cmd: &mut tokio::process::Command) { + #[cfg(windows)] + { + const CREATE_NO_WINDOW: u32 = 0x0800_0000; + cmd.creation_flags(CREATE_NO_WINDOW); + } + #[cfg(not(windows))] + { + let _ = cmd; + } +} diff --git a/desktop/src-tauri/src/commands/agent_discovery.rs b/desktop/src-tauri/src/commands/agent_discovery.rs index 99e83cd4a5..1ebbab4f62 100644 --- a/desktop/src-tauri/src/commands/agent_discovery.rs +++ b/desktop/src-tauri/src/commands/agent_discovery.rs @@ -232,6 +232,34 @@ fn install_acp_runtime_blocking(runtime_id: &str) -> Result Result Option<&'static str> { + if let Some(cli) = runtime.underlying_cli { + if crate::managed_agents::resolve_command(cli).is_none() { + return Some(cli); + } + } + // Goose (and similar) uses the same binary as both CLI and adapter. + // Adapter-only runtimes still need at least one command resolvable. + if runtime + .commands + .iter() + .all(|cmd| crate::managed_agents::resolve_command(cmd).is_none()) + { + return runtime.commands.first().copied(); + } + None +} + // ── Post-install auto-restart (Phase 2 of install_acp_runtime) ─────────────── // // After a successful adapter install, restart any local agents that: @@ -555,109 +605,6 @@ fn persist_last_error_on_install( save_managed_agents(app, &records) } -/// Build a login-shell `Command` for `command` with hermit env vars stripped, -/// Buzz-managed npm locations set, and the user's PATH set. This is the -/// single source of truth for -/// the shell selection and environment cleanup shared by `run_install_command` -/// and managed npm install path — keeping them in sync so the hermit-strip list -/// can't drift between command execution paths. -/// -/// On Windows, resolves Git Bash via `resolve_bash_path` (skips `BUZZ_SHELL` -/// since install commands require bash syntax). Returns `Err` when no shell -/// can be found. -fn install_shell_command(command: &str) -> Result { - let shell: std::path::PathBuf = resolve_install_shell()?; - - let mut cmd = std::process::Command::new(&shell); - cmd.args(["-l", "-c", command]); - - // Strip hermit env vars so npm/node use the user's normal registry rather - // than the project-local hermit-managed paths, then give npm defaults for - // Buzz-owned app data. Adapter install commands also pass --prefix - // explicitly; these env vars keep subprocesses/cache/corepack aligned. - cmd.env_remove("NPM_CONFIG_PREFIX"); - cmd.env_remove("NPM_CONFIG_CACHE"); - cmd.env_remove("COREPACK_HOME"); - - if let Some(prefix) = crate::managed_agents::buzz_managed_npm_prefix() { - cmd.env("NPM_CONFIG_PREFIX", &prefix); - cmd.env("npm_config_prefix", &prefix); - cmd.env("COREPACK_HOME", prefix.join("corepack")); - cmd.env("NPM_CONFIG_CACHE", prefix.join("cache")); - cmd.env("npm_config_cache", prefix.join("cache")); - } - - let mut path_parts = Vec::new(); - if let Some(managed_node_bin) = crate::managed_agents::buzz_managed_node_bin_dir() { - path_parts.push(managed_node_bin); - } - if let Some(managed_bin) = crate::managed_agents::buzz_managed_npm_bin_dir() { - path_parts.push(managed_bin); - } - if let Some(ref path) = crate::managed_agents::login_shell_path() { - path_parts.extend(std::env::split_paths(path)); - } - if !path_parts.is_empty() { - if let Ok(path) = std::env::join_paths(path_parts) { - cmd.env("PATH", path); - } - } - - // Detach from the controlling terminal so install scripts that read from - // /dev/tty (e.g. Codex's "Start Codex now? [y/N]") fall back to stdin - // (which is /dev/null) instead of blocking forever. - #[cfg(unix)] - { - use std::os::unix::process::CommandExt; - unsafe { - cmd.pre_exec(|| { - libc::setsid(); - Ok(()) - }); - } - } - - // Suppress the console window on Windows. - #[cfg(windows)] - { - use std::os::windows::process::CommandExt; - const CREATE_NO_WINDOW: u32 = 0x0800_0000; - cmd.creation_flags(CREATE_NO_WINDOW); - } - - Ok(cmd) -} - -/// Resolve the shell binary for install commands. -/// -/// Unix: `/bin/zsh` if present, else `/bin/bash`. -/// Windows: Git Bash via `resolve_bash_path` — skips `BUZZ_SHELL` because install -/// commands use bash-only `-l -c` syntax. A `BUZZ_SHELL=pwsh` user gets a green -/// Doctor prereq (their agents work) but installs use the Git Bash fallback chain. -fn resolve_install_shell() -> Result { - #[cfg(not(windows))] - { - if std::path::Path::new("/bin/zsh").exists() { - return Ok(std::path::PathBuf::from("/bin/zsh")); - } - Ok(std::path::PathBuf::from("/bin/bash")) - } - - #[cfg(windows)] - { - install_shell_from(crate::managed_agents::git_bash::resolve_bash_path()) - } -} - -/// Pure mapping from a resolved bash path to the install-shell result. -/// `None` → `Err(GIT_BASH_INSTALL_HINT)`, `Some(path)` → `Ok(path)`. -#[cfg(windows)] -pub(crate) fn install_shell_from( - resolved: Option, -) -> Result { - resolved.ok_or_else(|| crate::managed_agents::git_bash::GIT_BASH_INSTALL_HINT.to_string()) -} - /// Maximum number of attempts for a transient-looking install command. const INSTALL_MAX_ATTEMPTS: u32 = 3; @@ -905,6 +852,12 @@ use managed_node::{ npm_eacces_hint, }; +// ── install shell selection + PATH composition ──────────────────────────────── +mod install_shell; +use install_shell::install_shell_command; +#[cfg(windows)] +pub(crate) use install_shell::install_shell_from; + #[tauri::command] pub async fn discover_managed_agent_prereqs( input: DiscoverManagedAgentPrereqsRequest, @@ -1242,78 +1195,7 @@ mod tests { ); } - // ── Phase A: install shell selection ───────────────────────────────────── - - /// On Unix, resolve_install_shell always succeeds (returns zsh or bash). - #[cfg(unix)] - #[test] - fn test_resolve_install_shell_succeeds_on_unix() { - let result = super::resolve_install_shell(); - assert!(result.is_ok(), "Unix must always resolve a shell"); - let shell = result.unwrap(); - assert!( - shell == std::path::Path::new("/bin/zsh") || shell == std::path::Path::new("/bin/bash"), - "expected /bin/zsh or /bin/bash, got {shell:?}" - ); - } - - /// install_shell_command returns a valid Command on Unix. - #[cfg(unix)] - #[test] - fn test_install_shell_command_returns_ok_on_unix() { - let result = super::install_shell_command("echo test"); - assert!(result.is_ok(), "install_shell_command must succeed on Unix"); - } - - // ── Phase A: Windows install shell selection ─────────────────────────────── - - /// On Windows (CI runner has Git pre-installed), resolve_install_shell succeeds. - #[cfg(windows)] - #[test] - fn test_resolve_install_shell_succeeds_on_windows_with_git() { - let result = super::resolve_install_shell(); - assert!( - result.is_ok(), - "Windows CI runner has Git — resolve_install_shell must succeed; got: {:?}", - result.err() - ); - let shell = result.unwrap(); - // The resolved path must end with bash.exe (Git Bash). - let fname = shell.file_name().and_then(|n| n.to_str()).unwrap_or(""); - assert!( - fname.eq_ignore_ascii_case("bash.exe"), - "Windows install shell must be bash.exe, got: {shell:?}" - ); - } - - /// On Windows, when no Git Bash is found, the error carries the Doctor hint. - #[cfg(windows)] - #[test] - fn test_resolve_install_shell_error_contains_doctor_hint() { - // We can't force resolve_install_shell to fail on CI (Git is installed), - // but we can verify the error string it would use matches the hint. - let hint = crate::managed_agents::git_bash::GIT_BASH_INSTALL_HINT; - assert!( - hint.contains("Git for Windows"), - "GIT_BASH_INSTALL_HINT must mention Git for Windows; got: {hint}" - ); - assert!( - hint.contains("PATH"), - "GIT_BASH_INSTALL_HINT must mention PATH option; got: {hint}" - ); - } - - /// install_shell_command returns a valid Command on Windows. - #[cfg(windows)] - #[test] - fn test_install_shell_command_returns_ok_on_windows() { - let result = super::install_shell_command("echo test"); - assert!( - result.is_ok(), - "install_shell_command must succeed on Windows with Git; got: {:?}", - result.err() - ); - } + // Install-shell selection + PATH composition tests live in install_shell.rs. // ── Phase B: per-OS install commands ────────────────────────────────────── @@ -1329,15 +1211,32 @@ mod tests { ); } - /// Goose install commands are the same on all platforms (script is Windows-aware). + /// Goose on Windows pins GOOSE_BIN_DIR to ~/.local/bin so install lands on + /// a discovery path (#2239); Unix keeps the stock curl|bash install. #[test] - fn test_goose_install_commands_same_on_all_platforms() { + fn test_goose_install_commands_pin_bin_dir_on_windows() { let goose = crate::managed_agents::known_acp_runtime_exact("goose").unwrap(); - assert_eq!( - goose.cli_install_commands_for_os(), - goose.cli_install_commands, - "goose install commands must be identical across platforms" - ); + let cmds = goose.cli_install_commands_for_os(); + assert!(!cmds.is_empty(), "goose must have an auto-install command"); + #[cfg(windows)] + { + assert!( + cmds.iter() + .any(|c| c.contains("GOOSE_BIN_DIR") && c.contains(".local/bin")), + "Windows goose install must pin GOOSE_BIN_DIR to ~/.local/bin; got {cmds:?}" + ); + assert_eq!( + cmds, goose.cli_install_commands_windows, + "Windows must use cli_install_commands_windows for goose" + ); + } + #[cfg(not(windows))] + { + assert_eq!( + cmds, goose.cli_install_commands, + "Unix goose install stays the stock curl|bash command" + ); + } } /// buzz-agent has no install commands on any platform. diff --git a/desktop/src-tauri/src/commands/agent_discovery/install_shell.rs b/desktop/src-tauri/src/commands/agent_discovery/install_shell.rs new file mode 100644 index 0000000000..c3e02670bb --- /dev/null +++ b/desktop/src-tauri/src/commands/agent_discovery/install_shell.rs @@ -0,0 +1,336 @@ +//! Install-shell selection and PATH composition for runtime install commands. + +/// Build a login-shell `Command` for `command` with hermit env vars stripped, +/// Buzz-managed npm locations set, and the user's PATH set. This is the +/// single source of truth for the shell selection and environment cleanup +/// shared by `run_install_command` and managed npm install path — keeping them +/// in sync so the hermit-strip list can't drift between command execution paths. +/// +/// On Windows, resolves Git Bash via `resolve_bash_path` (skips `BUZZ_SHELL` +/// since install commands require bash syntax). Returns `Err` when no shell +/// can be found. +pub(super) fn install_shell_command(command: &str) -> Result { + let shell: std::path::PathBuf = resolve_install_shell()?; + + let mut cmd = std::process::Command::new(&shell); + cmd.args(["-l", "-c", command]); + + // Strip hermit env vars so npm/node use the user's normal registry rather + // than the project-local hermit-managed paths, then give npm defaults for + // Buzz-owned app data. Adapter install commands also pass --prefix + // explicitly; these env vars keep subprocesses/cache/corepack aligned. + cmd.env_remove("NPM_CONFIG_PREFIX"); + cmd.env_remove("NPM_CONFIG_CACHE"); + cmd.env_remove("COREPACK_HOME"); + + if let Some(prefix) = crate::managed_agents::buzz_managed_npm_prefix() { + cmd.env("NPM_CONFIG_PREFIX", &prefix); + cmd.env("npm_config_prefix", &prefix); + cmd.env("COREPACK_HOME", prefix.join("corepack")); + cmd.env("NPM_CONFIG_CACHE", prefix.join("cache")); + cmd.env("npm_config_cache", prefix.join("cache")); + } + + #[cfg(windows)] + let well_known = crate::managed_agents::windows_existing_well_known_path_dirs(); + #[cfg(not(windows))] + let well_known: Vec = Vec::new(); + + let path_parts = install_shell_path_parts( + crate::managed_agents::buzz_managed_node_bin_dir(), + crate::managed_agents::buzz_managed_npm_bin_dir(), + well_known, + crate::managed_agents::login_shell_path().as_deref(), + std::env::var_os("PATH"), + ); + if !path_parts.is_empty() { + if let Ok(path) = std::env::join_paths(path_parts) { + cmd.env("PATH", path); + } + } + + // Detach from the controlling terminal so install scripts that read from + // /dev/tty (e.g. Codex's "Start Codex now? [y/N]") fall back to stdin + // (which is /dev/null) instead of blocking forever. + #[cfg(unix)] + { + use std::os::unix::process::CommandExt; + unsafe { + cmd.pre_exec(|| { + libc::setsid(); + Ok(()) + }); + } + } + + crate::windows_console::hide_console(&mut cmd); + + Ok(cmd) +} + +/// Compose PATH for install shell children. +/// +/// Order: +/// 1. managed Node bin +/// 2. managed npm bin +/// 3. well-known Windows Node/npm/goose dirs that exist on disk +/// 4. login-shell PATH (Unix) **or** process PATH when login-shell PATH is +/// unavailable (Windows) +/// +/// Windows must not drop the process PATH: `login_shell_path()` is intentionally +/// `None` there (POSIX PATH poison), and without this fallback npm-backed +/// adapter installs only see the empty managed prefix (#2238). Well-known dirs +/// cover official `%ProgramFiles%\nodejs`, nvm-windows `NVM_SYMLINK` / +/// `NVM_HOME`, and similar when the GUI process PATH is thin. +pub(super) fn install_shell_path_parts( + managed_node_bin: Option, + managed_npm_bin: Option, + well_known_dirs: impl IntoIterator, + login_shell_path: Option<&str>, + process_path: Option, +) -> Vec { + let mut path_parts = Vec::new(); + if let Some(managed_node_bin) = managed_node_bin { + path_parts.push(managed_node_bin); + } + if let Some(managed_npm_bin) = managed_npm_bin { + path_parts.push(managed_npm_bin); + } + path_parts.extend(well_known_dirs); + if let Some(path) = login_shell_path { + path_parts.extend(std::env::split_paths(path)); + } else if let Some(path) = process_path { + path_parts.extend(std::env::split_paths(&path)); + } + // Dedup while preserving order — well-known dirs often already appear on PATH. + let mut seen = std::collections::HashSet::new(); + path_parts + .into_iter() + .filter(|p| seen.insert(p.clone())) + .collect() +} + +/// Resolve the shell binary for install commands. +/// +/// Unix: `/bin/zsh` if present, else `/bin/bash`. +/// Windows: Git Bash via `resolve_bash_path` — skips `BUZZ_SHELL` because install +/// commands use bash-only `-l -c` syntax. A `BUZZ_SHELL=pwsh` user gets a green +/// Doctor prereq (their agents work) but installs use the Git Bash fallback chain. +pub(super) fn resolve_install_shell() -> Result { + #[cfg(not(windows))] + { + if std::path::Path::new("/bin/zsh").exists() { + return Ok(std::path::PathBuf::from("/bin/zsh")); + } + Ok(std::path::PathBuf::from("/bin/bash")) + } + + #[cfg(windows)] + { + install_shell_from(crate::managed_agents::git_bash::resolve_bash_path()) + } +} + +/// Pure mapping from a resolved bash path to the install-shell result. +/// `None` → `Err(GIT_BASH_INSTALL_HINT)`, `Some(path)` → `Ok(path)`. +#[cfg(windows)] +pub(crate) fn install_shell_from( + resolved: Option, +) -> Result { + resolved.ok_or_else(|| crate::managed_agents::git_bash::GIT_BASH_INSTALL_HINT.to_string()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_install_shell_path_falls_back_to_process_path_without_login_shell() { + let managed = std::path::PathBuf::from("/tmp/buzz-node-tools"); + // Platform-native paths: Windows drive letters contain `:`, which + // join_paths rejects on Unix (Codex P1 / Desktop Core CI failure). + let process = std::env::join_paths([ + std::path::Path::new("/opt/nodejs/bin"), + std::path::Path::new("/usr/bin"), + ]) + .expect("join process PATH"); + + let parts = install_shell_path_parts( + None, + Some(managed.clone()), + std::iter::empty(), + None, // Windows: login_shell_path() is None + Some(process), + ); + + assert_eq!(parts.first(), Some(&managed), "managed npm bin first"); + assert!( + parts + .iter() + .any(|p| p == std::path::Path::new("/opt/nodejs/bin")), + "process PATH entries must remain visible for system npm; got {parts:?}" + ); + } + + #[test] + fn test_install_shell_path_prepends_well_known_dirs_before_process_path() { + let well_known = vec![std::path::PathBuf::from("/opt/nodejs/bin")]; + let process = std::env::join_paths([std::path::Path::new("/usr/bin")]).expect("join PATH"); + let parts = install_shell_path_parts(None, None, well_known, None, Some(process)); + assert_eq!( + parts.first().map(|p| p.as_path()), + Some(std::path::Path::new("/opt/nodejs/bin")), + "well-known Node dir should precede process PATH; got {parts:?}" + ); + assert!( + parts.iter().any(|p| p == std::path::Path::new("/usr/bin")), + "process PATH must still be included; got {parts:?}" + ); + } + + #[test] + fn test_install_shell_path_dedups_well_known_already_on_process_path() { + let well_known = vec![std::path::PathBuf::from("/opt/nodejs/bin")]; + let process = std::env::join_paths([ + std::path::Path::new("/opt/nodejs/bin"), + std::path::Path::new("/usr/bin"), + ]) + .expect("join process PATH"); + let parts = install_shell_path_parts(None, None, well_known, None, Some(process)); + let node_count = parts + .iter() + .filter(|p| p.as_path() == std::path::Path::new("/opt/nodejs/bin")) + .count(); + assert_eq!( + node_count, 1, + "duplicate PATH entries must be collapsed; got {parts:?}" + ); + } + + #[test] + fn test_install_shell_path_prefers_login_shell_over_process_path() { + // Build platform-native PATH strings so split_paths works on Windows (;) and Unix (:). + let login = std::env::join_paths([ + std::path::Path::new("/usr/local/bin"), + std::path::Path::new("/usr/bin"), + ]) + .expect("join login PATH"); + let login = login.to_string_lossy(); + let process = std::env::join_paths([std::path::Path::new("/should/not/appear")]) + .expect("join process PATH"); + let parts = install_shell_path_parts( + None, + None, + std::iter::empty(), + Some(login.as_ref()), + Some(process), + ); + assert!( + parts + .iter() + .any(|p| p == std::path::Path::new("/usr/local/bin")), + "login-shell PATH should win when present; got {parts:?}" + ); + assert!( + !parts + .iter() + .any(|p| p == std::path::Path::new("/should/not/appear")), + "process PATH must not be mixed in when login-shell PATH is present" + ); + } + + /// On Unix, resolve_install_shell always succeeds (returns zsh or bash). + #[cfg(unix)] + #[test] + fn test_resolve_install_shell_succeeds_on_unix() { + let result = resolve_install_shell(); + assert!(result.is_ok(), "Unix must always resolve a shell"); + let shell = result.unwrap(); + assert!( + shell == std::path::Path::new("/bin/zsh") || shell == std::path::Path::new("/bin/bash"), + "expected /bin/zsh or /bin/bash, got {shell:?}" + ); + } + + /// install_shell_command returns a valid Command on Unix. + #[cfg(unix)] + #[test] + fn test_install_shell_command_returns_ok_on_unix() { + let result = install_shell_command("echo test"); + assert!(result.is_ok(), "install_shell_command must succeed on Unix"); + } + + /// On Windows (CI runner has Git pre-installed), resolve_install_shell succeeds. + #[cfg(windows)] + #[test] + fn test_resolve_install_shell_succeeds_on_windows_with_git() { + let result = resolve_install_shell(); + assert!( + result.is_ok(), + "Windows CI runner has Git — resolve_install_shell must succeed; got: {:?}", + result.err() + ); + let shell = result.unwrap(); + let fname = shell.file_name().and_then(|n| n.to_str()).unwrap_or(""); + assert!( + fname.eq_ignore_ascii_case("bash.exe"), + "Windows install shell must be bash.exe, got: {shell:?}" + ); + } + + /// On Windows, when no Git Bash is found, the error carries the Doctor hint. + #[cfg(windows)] + #[test] + fn test_resolve_install_shell_error_contains_doctor_hint() { + let hint = crate::managed_agents::git_bash::GIT_BASH_INSTALL_HINT; + assert!( + hint.contains("Git for Windows"), + "GIT_BASH_INSTALL_HINT must mention Git for Windows; got: {hint}" + ); + assert!( + hint.contains("PATH"), + "GIT_BASH_INSTALL_HINT must mention PATH option; got: {hint}" + ); + } + + /// install_shell_command returns a valid Command on Windows. + #[cfg(windows)] + #[test] + fn test_install_shell_command_returns_ok_on_windows() { + let result = install_shell_command("echo test"); + assert!( + result.is_ok(), + "install_shell_command must succeed on Windows with Git; got: {:?}", + result.err() + ); + } + + /// When `resolve_bash_path` returns `None` (no Git Bash found), + /// `install_shell_from` maps it to `Err(GIT_BASH_INSTALL_HINT)`. + #[cfg(windows)] + #[test] + fn test_install_shell_from_none_returns_hint() { + use crate::managed_agents::git_bash; + + let result = install_shell_from(None); + assert_eq!( + result, + Err(git_bash::GIT_BASH_INSTALL_HINT.to_string()), + "install_shell_from(None) must return the Git Bash install hint" + ); + } + + /// When `resolve_bash_path` returns `Some(path)`, `install_shell_from` + /// passes it through as `Ok`. + #[cfg(windows)] + #[test] + fn test_install_shell_from_some_returns_path() { + let path = std::path::PathBuf::from(r"C:\Git\bin\bash.exe"); + let result = install_shell_from(Some(path.clone())); + assert_eq!( + result, + Ok(path), + "install_shell_from(Some) must return the path as Ok" + ); + } +} diff --git a/desktop/src-tauri/src/commands/agent_discovery/managed_node.rs b/desktop/src-tauri/src/commands/agent_discovery/managed_node.rs index fca5a602b0..26b5265c47 100644 --- a/desktop/src-tauri/src/commands/agent_discovery/managed_node.rs +++ b/desktop/src-tauri/src/commands/agent_discovery/managed_node.rs @@ -93,12 +93,13 @@ fn managed_node_runtime_ready() -> bool { if !node.is_file() { return false; } - let output = std::process::Command::new(&node) - .arg("--version") + let mut cmd = std::process::Command::new(&node); + cmd.arg("--version") .stdin(std::process::Stdio::null()) .stdout(std::process::Stdio::piped()) - .stderr(std::process::Stdio::null()) - .output(); + .stderr(std::process::Stdio::null()); + crate::windows_console::hide_console(&mut cmd); + let output = cmd.output(); output .ok() .filter(|output| output.status.success()) diff --git a/desktop/src-tauri/src/commands/agent_model_process.rs b/desktop/src-tauri/src/commands/agent_model_process.rs index 2093d4fe29..78b1c22b11 100644 --- a/desktop/src-tauri/src/commands/agent_model_process.rs +++ b/desktop/src-tauri/src/commands/agent_model_process.rs @@ -50,8 +50,9 @@ pub(super) async fn run_agent_models_command( } crate::managed_agents::configure_runtime_cli(&mut cmd, known_acp_runtime(&agent_command)); cmd.stdout(std::process::Stdio::piped()) - .stderr(std::process::Stdio::piped()) - .output() + .stderr(std::process::Stdio::piped()); + crate::windows_console::hide_console(&mut cmd); + cmd.output() .map_err(|e| format!("failed to spawn buzz-acp models: {e}")) }) .await diff --git a/desktop/src-tauri/src/commands/project_git_exec.rs b/desktop/src-tauri/src/commands/project_git_exec.rs index 3f16c0fba9..4ebd144458 100644 --- a/desktop/src-tauri/src/commands/project_git_exec.rs +++ b/desktop/src-tauri/src/commands/project_git_exec.rs @@ -80,6 +80,7 @@ pub(crate) fn run_git( command.stdin(Stdio::null()); command.stdout(Stdio::piped()); command.stderr(Stdio::piped()); + crate::windows_console::hide_console(&mut command); let mut child = command .spawn() diff --git a/desktop/src-tauri/src/lib.rs b/desktop/src-tauri/src/lib.rs index c22de0205f..9c32e20ca0 100644 --- a/desktop/src-tauri/src/lib.rs +++ b/desktop/src-tauri/src/lib.rs @@ -29,6 +29,7 @@ mod secret_store; mod shutdown; mod templates; mod util; +mod windows_console; use app_state::{build_app_state, resolve_persisted_identity, AppState}; use builderlab::*; use commands::*; diff --git a/desktop/src-tauri/src/managed_agents/discovery.rs b/desktop/src-tauri/src/managed_agents/discovery.rs index 50206567ad..3cb78d1c1e 100644 --- a/desktop/src-tauri/src/managed_agents/discovery.rs +++ b/desktop/src-tauri/src/managed_agents/discovery.rs @@ -42,26 +42,25 @@ fn common_binary_paths() -> &'static [PathBuf] { home.join(".asdf/shims"), ]); } - // Windows well-known dirs for npm global shims and standalone installer targets. + // Windows well-known dirs for npm global shims, Node.js, and standalone + // installer targets. GUI-launched apps often miss shell-profile PATH + // entries, so we probe standard locations + version-manager env vars. #[cfg(windows)] { - if let Some(appdata) = std::env::var_os("APPDATA") { - paths.push(PathBuf::from(appdata).join("npm")); - } - if let Some(local) = std::env::var_os("LOCALAPPDATA") { - paths.push( - PathBuf::from(local) - .join("Programs") - .join("OpenAI") - .join("Codex") - .join("bin"), - ); - } + paths.extend(windows_paths::windows_well_known_binary_dirs_from_env()); } paths }) } +#[cfg(windows)] +mod windows_paths; +#[cfg(windows)] +pub(crate) use windows_paths::{ + windows_existing_well_known_path_dirs, windows_well_known_binary_dirs, + windows_well_known_binary_dirs_from_env, WindowsWellKnownEnv, +}; + const KNOWN_ACP_RUNTIMES: &[KnownAcpRuntime] = &[ KnownAcpRuntime { id: "goose", @@ -73,7 +72,10 @@ const KNOWN_ACP_RUNTIMES: &[KnownAcpRuntime] = &[ mcp_hooks: false, underlying_cli: Some("goose"), cli_install_commands: &["curl -fsSL https://github.com/aaif-goose/goose/releases/download/stable/download_cli.sh | CONFIGURE=false bash"], - cli_install_commands_windows: &[], // goose install script is already Windows-aware + // Pin GOOSE_BIN_DIR to ~/.local/bin (already on Buzz's discovery path + // list) so install doesn't land only in %USERPROFILE%\goose off PATH + // (#2239). Back-compat: windows_paths still probes the official default. + cli_install_commands_windows: &["GOOSE_BIN_DIR=\"$HOME/.local/bin\" curl -fsSL https://github.com/aaif-goose/goose/releases/download/stable/download_cli.sh | CONFIGURE=false bash"], adapter_install_commands: &[], install_instructions_url: "https://block.github.io/goose/", cli_install_hint: "Install Goose via the official install script.", @@ -677,7 +679,12 @@ fn login_shell_candidates() -> Vec { /// Returns trimmed stdout if the command succeeds with non-empty output. fn run_in_login_shell(args: &[&str]) -> Option { for shell in login_shell_candidates() { - let Ok(output) = Command::new(&shell).args(args).output() else { + let mut cmd = Command::new(&shell); + cmd.args(args); + // Discovery probes run during onboarding / Doctor refresh — hide the + // console so Git Bash doesn't steal focus on Windows (#2239). + crate::windows_console::hide_console(&mut cmd); + let Ok(output) = cmd.output() else { continue; }; if !output.status.success() { @@ -916,6 +923,7 @@ fn probe_auth_status(binary_path: &Path, probe_args: &[&str]) -> AuthStatus { .stdin(std::process::Stdio::null()) .stdout(std::process::Stdio::piped()) .stderr(std::process::Stdio::piped()); + crate::windows_console::hide_console(&mut command); let mut child = match command.spawn() { Ok(c) => c, @@ -1076,6 +1084,7 @@ pub(crate) fn probe_codex_acp_major_version_with_path( let mut command = Command::new(binary_path); command.arg("--version"); + crate::windows_console::hide_console(&mut command); if let Some(path) = augmented_path { command.env("PATH", path); } diff --git a/desktop/src-tauri/src/managed_agents/discovery/tests.rs b/desktop/src-tauri/src/managed_agents/discovery/tests.rs index 0ed4fe0f6a..b6d300d6a0 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/tests.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/tests.rs @@ -1134,12 +1134,20 @@ fn test_cli_install_commands_for_os_selects_powershell_on_windows() { "codex Windows install must use powershell; got: {codex_cmds:?}" ); - // Goose and buzz-agent must NOT use Windows-specific commands. + // Goose pins GOOSE_BIN_DIR on Windows so the binary lands on a discovery path. let goose = super::known_acp_runtime_exact("goose").unwrap(); - assert_eq!( - goose.cli_install_commands_for_os(), - goose.cli_install_commands, - "goose must use the same commands on all platforms" + let goose_cmds = goose.cli_install_commands_for_os(); + assert!( + goose_cmds + .iter() + .any(|c| c.contains("GOOSE_BIN_DIR") && c.contains(".local/bin")), + "goose Windows install must pin GOOSE_BIN_DIR; got {goose_cmds:?}" + ); + // buzz-agent still has no OS-specific install surface. + let buzz = super::known_acp_runtime_exact("buzz-agent").unwrap(); + assert!( + buzz.cli_install_commands_for_os().is_empty(), + "buzz-agent must not grow install commands" ); } @@ -1183,6 +1191,10 @@ fn test_login_shell_path_returns_none_on_windows() { ); } +// Windows well-known binary dirs (goose + Node) live in tests/windows_paths.rs. +#[cfg(windows)] +mod windows_paths; + // ── Phase C: .cmd shim resolution on Windows ─────────────────────────────── /// `resolve_command_uncached` finds `.cmd` shims via the Windows PATH scan @@ -1236,37 +1248,3 @@ fn test_no_git_bash_resolved_returns_none() { "empty environment with registry disabled must not resolve a Git Bash" ); } - -/// When `resolve_bash_path` returns `None` (no Git Bash anywhere), -/// `install_shell_from` maps it to `Err(GIT_BASH_INSTALL_HINT)` — the exact -/// Doctor hint shown to the user. Tests the pure error-mapping seam, not the -/// resolution chain. -#[cfg(windows)] -#[test] -fn test_install_shell_from_none_returns_hint() { - use crate::commands::install_shell_from; - use crate::managed_agents::git_bash; - - let result = install_shell_from(None); - assert_eq!( - result, - Err(git_bash::GIT_BASH_INSTALL_HINT.to_string()), - "install_shell_from(None) must return the Git Bash install hint" - ); -} - -/// When `resolve_bash_path` returns `Some(path)`, `install_shell_from` -/// passes it through as `Ok`. -#[cfg(windows)] -#[test] -fn test_install_shell_from_some_returns_path() { - use crate::commands::install_shell_from; - - let path = std::path::PathBuf::from(r"C:\Git\bin\bash.exe"); - let result = install_shell_from(Some(path.clone())); - assert_eq!( - result, - Ok(path), - "install_shell_from(Some) must return the path as Ok" - ); -} diff --git a/desktop/src-tauri/src/managed_agents/discovery/tests/windows_paths.rs b/desktop/src-tauri/src/managed_agents/discovery/tests/windows_paths.rs new file mode 100644 index 0000000000..0df987a439 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/discovery/tests/windows_paths.rs @@ -0,0 +1,79 @@ +//! Windows well-known binary dir coverage (goose default install + Node layouts). + +use super::super::{windows_well_known_binary_dirs, WindowsWellKnownEnv}; + +/// Goose's official Windows CLI installer writes to %USERPROFILE%\goose. +/// Discovery must probe that directory or onboarding shows Installed then +/// blocks Next (#2239). +#[test] +fn test_windows_well_known_dirs_include_userprofile_goose() { + let home = std::path::PathBuf::from(r"C:\Users\tester"); + let dirs = windows_well_known_binary_dirs(WindowsWellKnownEnv { + home: Some(home.clone()), + appdata: Some(std::path::PathBuf::from(r"C:\Users\tester\AppData\Roaming")), + local_app_data: Some(std::path::PathBuf::from(r"C:\Users\tester\AppData\Local")), + userprofile: Some(std::path::PathBuf::from(r"C:\Users\tester")), + ..Default::default() + }); + assert!( + dirs.iter().any(|p| p == &home.join("goose")), + "expected %USERPROFILE%\\goose in well-known dirs, got {dirs:?}" + ); + assert!( + dirs.iter() + .any(|p| p.ends_with(std::path::Path::new(r"AppData\Roaming\npm"))), + "expected APPDATA\\npm in well-known dirs, got {dirs:?}" + ); +} + +/// Official Node MSI + nvm-windows env roots must be on the probe list so GUI +/// PATH gaps do not hide system npm for Codex/Claude adapter installs (#2238). +#[test] +fn test_windows_well_known_dirs_include_official_node_and_nvm_env() { + let dirs = windows_well_known_binary_dirs(WindowsWellKnownEnv { + program_files: Some(std::path::PathBuf::from(r"C:\Program Files")), + program_files_x86: Some(std::path::PathBuf::from(r"C:\Program Files (x86)")), + local_app_data: Some(std::path::PathBuf::from(r"C:\Users\tester\AppData\Local")), + nvm_symlink: Some(std::path::PathBuf::from(r"C:\Program Files\nodejs")), + nvm_home: Some(std::path::PathBuf::from( + r"C:\Users\tester\AppData\Roaming\nvm", + )), + ..Default::default() + }); + assert!( + dirs.iter() + .any(|p| p == std::path::Path::new(r"C:\Program Files\nodejs")), + "expected official Program Files\\nodejs, got {dirs:?}" + ); + assert!( + dirs.iter() + .any(|p| p == std::path::Path::new(r"C:\Program Files (x86)\nodejs")), + "expected Program Files (x86)\\nodejs, got {dirs:?}" + ); + assert!( + dirs.iter().any(|p| { + p == std::path::Path::new(r"C:\Users\tester\AppData\Local\Programs\nodejs") + }), + "expected LocalAppData\\Programs\\nodejs, got {dirs:?}" + ); + assert!( + dirs.iter() + .any(|p| { p == std::path::Path::new(r"C:\Users\tester\AppData\Roaming\nvm\nodejs") }), + "expected NVM_HOME\\nodejs, got {dirs:?}" + ); +} + +/// When home_dir is unavailable, fall back to USERPROFILE for the goose dir. +#[test] +fn test_windows_well_known_dirs_goose_falls_back_to_userprofile() { + let profile = std::path::PathBuf::from(r"C:\Users\fallback"); + let dirs = windows_well_known_binary_dirs(WindowsWellKnownEnv { + userprofile: Some(profile.clone()), + ..Default::default() + }); + assert_eq!( + dirs, + vec![profile.join("goose")], + "USERPROFILE fallback must still find goose's default install dir" + ); +} diff --git a/desktop/src-tauri/src/managed_agents/discovery/windows_paths.rs b/desktop/src-tauri/src/managed_agents/discovery/windows_paths.rs new file mode 100644 index 0000000000..258fe0c3a8 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/discovery/windows_paths.rs @@ -0,0 +1,102 @@ +//! Well-known Windows binary directories for agent CLIs and Node/npm. +//! +//! GUI-launched apps often miss shell-profile PATH entries, so discovery and +//! install shells probe standard install locations plus version-manager env +//! vars (nvm-windows / nvm4w). + +use std::path::PathBuf; + +/// Collect Windows well-known binary dirs from the live process environment. +pub(crate) fn windows_well_known_binary_dirs_from_env() -> Vec { + windows_well_known_binary_dirs(WindowsWellKnownEnv { + home: dirs::home_dir(), + appdata: std::env::var_os("APPDATA").map(PathBuf::from), + local_app_data: std::env::var_os("LOCALAPPDATA").map(PathBuf::from), + userprofile: std::env::var_os("USERPROFILE").map(PathBuf::from), + program_files: std::env::var_os("ProgramFiles").map(PathBuf::from), + program_files_x86: std::env::var_os("ProgramFiles(x86)").map(PathBuf::from), + // nvm-windows / nvm4w active symlink or install root (when set). + nvm_symlink: std::env::var_os("NVM_SYMLINK").map(PathBuf::from), + nvm_home: std::env::var_os("NVM_HOME").map(PathBuf::from), + }) +} + +/// Inputs for [`windows_well_known_binary_dirs`] — pure so unit tests inject +/// paths without mutating process-global `OnceLock` / env state. +#[derive(Debug, Default, Clone)] +pub(crate) struct WindowsWellKnownEnv { + pub home: Option, + pub appdata: Option, + pub local_app_data: Option, + pub userprofile: Option, + pub program_files: Option, + pub program_files_x86: Option, + pub nvm_symlink: Option, + pub nvm_home: Option, +} + +/// Windows-only install locations Buzz must probe for agent CLIs and Node/npm. +/// +/// Covers official Node MSI layout, per-user npm globals, nvm-windows / nvm4w +/// env roots, Codex installer, and goose's default `%USERPROFILE%\goose` dir +/// (github.com/block/buzz/issues/2239). +pub(crate) fn windows_well_known_binary_dirs(env: WindowsWellKnownEnv) -> Vec { + let mut paths = Vec::new(); + + // Per-user npm global shims (`npm install -g` without custom prefix). + if let Some(appdata) = env.appdata { + paths.push(appdata.join("npm")); + } + + // Official Node.js MSI (x64) and occasional 32-bit install. + // https://nodejs.org — default install dir is %ProgramFiles%\nodejs. + if let Some(program_files) = env.program_files { + paths.push(program_files.join("nodejs")); + } + if let Some(program_files_x86) = env.program_files_x86 { + paths.push(program_files_x86.join("nodejs")); + } + + if let Some(local) = env.local_app_data { + // Some per-user Node installers and winget layouts. + paths.push(local.join("Programs").join("nodejs")); + paths.push( + local + .join("Programs") + .join("OpenAI") + .join("Codex") + .join("bin"), + ); + } + + // nvm-windows sets NVM_SYMLINK to the active node dir (often Program Files\nodejs). + // nvm4w / similar may set NVM_HOME to the install root with a `nodejs` child. + if let Some(symlink) = env.nvm_symlink { + paths.push(symlink); + } + if let Some(nvm_home) = env.nvm_home { + paths.push(nvm_home.join("nodejs")); + paths.push(nvm_home); + } + + // Back-compat: official goose Windows installer defaults to + // %USERPROFILE%\goose when GOOSE_BIN_DIR is unset. New Buzz installs pin + // GOOSE_BIN_DIR=$HOME/.local/bin (already probed via common_binary_paths). + // Keep this probe so pre-existing installs still resolve (#2239). + if let Some(home) = env.home { + paths.push(home.join("goose")); + } else if let Some(profile) = env.userprofile { + paths.push(profile.join("goose")); + } + + paths +} + +/// Existing well-known Windows dirs that should be on the install-shell PATH so +/// `npm`/`node` resolve even when the GUI process PATH is thin. +pub(crate) fn windows_existing_well_known_path_dirs() -> Vec { + windows_well_known_binary_dirs_from_env() + .into_iter() + .filter(|dir| dir.is_dir()) + .collect() +} diff --git a/desktop/src-tauri/src/managed_agents/git_bash.rs b/desktop/src-tauri/src/managed_agents/git_bash.rs index 790f18cfa2..6d46ecbafa 100644 --- a/desktop/src-tauri/src/managed_agents/git_bash.rs +++ b/desktop/src-tauri/src/managed_agents/git_bash.rs @@ -196,12 +196,15 @@ fn resolve_git_bash_inner( local_app_data: Option, check_registry: bool, ) -> Option { + // Prefer real Git Bash over WindowsApps\bash.exe (App Execution Alias → + // WSL; #2328). Order: overrides → git.exe sibling → well-known dirs → + // registry → PATH bash (System32 + WindowsApps skipped). + // KEEP IN SYNC with `crates/buzz-dev-mcp/src/shell.rs` `resolve_bash`. let result = shell_override .and_then(|path| resolve_shell_override(&path, path_env)) .or_else(|| git_bash_override.filter(|path| path.is_file())) - .or_else(|| scan_path_for_bash(path_env, system_root.as_deref())) .or_else(|| { - scan_path_for_command(Path::new("git.exe"), path_env, None) + scan_path_for_command(Path::new("git.exe"), path_env, None, false) .and_then(|git| bash_from_git(&git)) }) .or_else(|| { @@ -211,9 +214,11 @@ fn resolve_git_bash_inner( return result; } if check_registry { - return git_bash_from_registry(); + if let Some(bash) = git_bash_from_registry() { + return Some(bash); + } } - None + scan_path_for_bash(path_env, system_root.as_deref()) } /// Like `resolve_git_bash` but skips the ambient Windows registry lookup, so @@ -249,7 +254,7 @@ fn resolve_shell_override(shell: &Path, path_env: &str) -> Option { if shell.components().count() > 1 || shell.has_root() { shell.is_file().then(|| shell.to_path_buf()) } else { - scan_path_for_command(shell, path_env, None) + scan_path_for_command(shell, path_env, None, false) } } @@ -261,20 +266,30 @@ fn bash_from_git(git: &Path) -> Option { #[cfg(windows)] fn scan_path_for_bash(path_env: &str, system_root: Option<&Path>) -> Option { - scan_path_for_command(Path::new("bash.exe"), path_env, system_root) + // Bash-only: also reject WindowsApps App Execution Alias (#2328). + scan_path_for_command(Path::new("bash.exe"), path_env, system_root, true) } +/// Scan PATH for `name`. When `skip_windows_apps`, reject +/// `…\Microsoft\WindowsApps` (App Execution Alias → WSL hang for bash). +/// +/// KEEP IN SYNC with `crates/buzz-dev-mcp/src/shell.rs` (`scan_path_for_command` +/// + `path_looks_like_windows_apps` + resolve probe order). #[cfg(windows)] fn scan_path_for_command( name: &Path, path_env: &str, system_root: Option<&Path>, + skip_windows_apps: bool, ) -> Option { let needs_exe = name.extension().is_none(); std::env::split_paths(path_env).find_map(|dir| { if system_root.is_some_and(|root| is_under_dir(&dir, root)) { return None; } + if skip_windows_apps && path_looks_like_windows_apps(&dir) { + return None; + } let candidate = dir.join(name); if candidate.is_file() { return Some(candidate); @@ -290,6 +305,18 @@ fn scan_path_for_command( }) } +/// True for `…\Microsoft\WindowsApps` (App Execution Alias dir), any casing. +#[cfg(windows)] +fn path_looks_like_windows_apps(dir: &Path) -> bool { + let parts: Vec<_> = dir + .components() + .filter_map(|c| c.as_os_str().to_str()) + .collect(); + parts.windows(2).any(|w| { + w[0].eq_ignore_ascii_case("Microsoft") && w[1].eq_ignore_ascii_case("WindowsApps") + }) +} + #[cfg(windows)] fn is_under_dir(dir: &Path, root: &Path) -> bool { let mut dir_components = dir.components(); @@ -543,13 +570,90 @@ mod tests { ); // Install path: shell_override=None skips pwsh, finds bash on PATH. + // no_registry: ambient GitForWindows would otherwise win after reorder. assert_eq!( - resolve_git_bash(path_str, None, None, None, None, None, None), + resolve_git_bash_no_registry(path_str, None, None, None, None, None, None), Some(bash), "install path must skip BUZZ_SHELL and find bash on PATH" ); } + #[test] + fn test_windows_apps_bash_skipped_in_favor_of_git() { + let temp = tempdir().expect("tempdir"); + let apps = temp + .path() + .join("Local") + .join("Microsoft") + .join("WindowsApps"); + let apps_bash = apps.join("bash.exe"); + std::fs::create_dir_all(&apps).expect("mkdir apps"); + std::fs::write(&apps_bash, []).expect("apps bash"); + + let git = temp + .path() + .join("Git") + .join("cmd") + .join("git.exe"); + let real_bash = temp + .path() + .join("Git") + .join("bin") + .join("bash.exe"); + std::fs::create_dir_all(git.parent().expect("git parent")).expect("mkdir git"); + std::fs::create_dir_all(real_bash.parent().expect("bash parent")).expect("mkdir bash"); + std::fs::write(&git, []).expect("git"); + std::fs::write(&real_bash, []).expect("bash"); + + // WindowsApps first on PATH (common machine layout). + let path = std::env::join_paths([ + apps.as_path(), + git.parent().expect("cmd dir"), + ]) + .expect("PATH"); + assert_eq!( + resolve_git_bash( + path.to_str().expect("utf8"), + None, + None, + None, + None, + None, + Some(temp.path().join("Local")), + ), + Some(real_bash), + "must prefer Git Bash over WindowsApps alias" + ); + } + + #[test] + fn test_windows_apps_bash_alone_is_not_resolved() { + let temp = tempdir().expect("tempdir"); + let apps = temp + .path() + .join("Local") + .join("Microsoft") + .join("WindowsApps"); + let apps_bash = apps.join("bash.exe"); + std::fs::create_dir_all(&apps).expect("mkdir apps"); + std::fs::write(&apps_bash, []).expect("apps bash"); + + let path = std::env::join_paths([apps.as_path()]).expect("PATH"); + assert_eq!( + resolve_git_bash_no_registry( + path.to_str().expect("utf8"), + None, + None, + None, + None, + None, + Some(temp.path().join("Local")), + ), + None, + "WindowsApps bash must not count as Git Bash" + ); + } + /// Same as above but with BUZZ_SHELL=cmd.exe. #[test] fn test_install_path_skips_buzz_shell_cmd() { @@ -570,8 +674,9 @@ mod tests { ); // Install path: shell_override=None skips cmd, finds bash on PATH. + // no_registry: ambient GitForWindows would otherwise win after reorder. assert_eq!( - resolve_git_bash(path_str, None, None, None, None, None, None), + resolve_git_bash_no_registry(path_str, None, None, None, None, None, None), Some(bash), "install path must skip BUZZ_SHELL and find bash on PATH" ); diff --git a/desktop/src-tauri/src/managed_agents/process_lifecycle.rs b/desktop/src-tauri/src/managed_agents/process_lifecycle.rs index 1ef3eae010..a0b9c04df8 100644 --- a/desktop/src-tauri/src/managed_agents/process_lifecycle.rs +++ b/desktop/src-tauri/src/managed_agents/process_lifecycle.rs @@ -107,14 +107,13 @@ fn create_job_for_child(pid: u32) -> Option { /// Kill the entire process tree rooted at `pid` via `taskkill /T`, the closest /// equivalent to the Unix process-group kill. Used on the after-restart path -/// where no job handle survived. `CREATE_NO_WINDOW` keeps taskkill's own -/// console from flashing. +/// where no job handle survived. Hides taskkill's own console via +/// [`crate::windows_console::hide_console`]. pub fn taskkill_tree(pid: u32) -> Result<(), String> { - use std::os::windows::process::CommandExt; - const CREATE_NO_WINDOW: u32 = 0x0800_0000; - let status = std::process::Command::new("taskkill") - .args(["/T", "/F", "/PID", &pid.to_string()]) - .creation_flags(CREATE_NO_WINDOW) + let mut command = std::process::Command::new("taskkill"); + command.args(["/T", "/F", "/PID", &pid.to_string()]); + crate::windows_console::hide_console(&mut command); + let status = command .status() .map_err(|error| format!("failed to run taskkill for pid {pid}: {error}"))?; if status.success() { 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..97587b920e 100644 --- a/desktop/src-tauri/src/managed_agents/readiness/cli_probe.rs +++ b/desktop/src-tauri/src/managed_agents/readiness/cli_probe.rs @@ -61,6 +61,7 @@ pub(crate) fn login_probe( if let Some(path) = augmented_path { command.env("PATH", path); } + crate::windows_console::hide_console(&mut command); match command.output() { Ok(o) if o.status.success() => ProbeOutcome::LoggedIn, diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index 77b3a241e0..30aef5afb0 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -1891,12 +1891,7 @@ pub fn spawn_agent_child( // Windows: suppress the harness console window. Without this a bare // terminal pops for buzz-acp.exe and lingers (the app itself sets // windows_subsystem="windows", but the spawned child does not inherit it). - #[cfg(windows)] - { - use std::os::windows::process::CommandExt; - const CREATE_NO_WINDOW: u32 = 0x0800_0000; - command.creation_flags(CREATE_NO_WINDOW); - } + crate::windows_console::hide_console(&mut command); let child = command.spawn().map_err(|error| { format!( diff --git a/desktop/src-tauri/src/windows_console.rs b/desktop/src-tauri/src/windows_console.rs new file mode 100644 index 0000000000..45b7915b92 --- /dev/null +++ b/desktop/src-tauri/src/windows_console.rs @@ -0,0 +1,23 @@ +//! Windows GUI helpers for child processes. +//! +//! Desktop is a GUI app: spawning console-subsystem tools without +//! `CREATE_NO_WINDOW` steals focus with a flash of conhost. Prefer this helper +//! over inlining the flag at every call site. + +/// Suppress the console window for a `std::process::Command` on Windows. +/// +/// No-op on other platforms so call sites stay uniform. +#[inline] +pub(crate) fn hide_console(cmd: &mut std::process::Command) { + #[cfg(windows)] + { + use std::os::windows::process::CommandExt; + // CREATE_NO_WINDOW — do not allocate a new console for the child. + const CREATE_NO_WINDOW: u32 = 0x0800_0000; + cmd.creation_flags(CREATE_NO_WINDOW); + } + #[cfg(not(windows))] + { + let _ = cmd; + } +} diff --git a/desktop/src-tauri/tauri.conf.json b/desktop/src-tauri/tauri.conf.json index e265735c43..35bfd8a929 100644 --- a/desktop/src-tauri/tauri.conf.json +++ b/desktop/src-tauri/tauri.conf.json @@ -5,7 +5,7 @@ "identifier": "xyz.block.buzz.app", "build": { "beforeDevCommand": { - "script": "exec ./node_modules/.bin/vite", + "script": "pnpm exec vite", "cwd": "..", "wait": false }, diff --git a/desktop/src/features/onboarding/ui/SetupStep.tsx b/desktop/src/features/onboarding/ui/SetupStep.tsx index 6111589cef..f02f4cd7df 100644 --- a/desktop/src/features/onboarding/ui/SetupStep.tsx +++ b/desktop/src/features/onboarding/ui/SetupStep.tsx @@ -91,13 +91,46 @@ function RuntimeReadinessIndicator({ ); } +const ONBOARDING_RUNTIME_ACTION_CLASS = + "buzz-onboarding-runtime-setup h-5 rounded-full bg-[var(--buzz-welcome-chartreuse)]/30 px-2.5 font-mono !text-badge font-normal uppercase text-foreground hover:bg-[var(--buzz-welcome-chartreuse)]/40"; + +function CheckAgainButton({ + checking, + onCheck, + runtimeId, + runtimeLabel, + testId, +}: { + checking: boolean; + onCheck: () => void; + runtimeId: string; + runtimeLabel: string; + testId?: string; +}) { + return ( + + ); +} + function RuntimeStatus({ installError, + installSuccess, isInstalling, onInstall, runtime, }: { installError: string | null; + installSuccess: boolean; isInstalling: boolean; onInstall: () => void; runtime: AcpRuntimeCatalogEntry; @@ -234,21 +267,25 @@ function RuntimeStatus({ ); } + // Auth unknown, or install finished but rediscovery not ready yet (#2239). + // Never treat local installSuccess as READY — offer recheck instead. if ( - runtime.availability === "available" && - runtime.authStatus.status === "unknown" + (runtime.availability === "available" && + runtime.authStatus.status === "unknown") || + installSuccess ) { return ( - + void runtimesQuery.refetch()} + runtimeId={runtime.id} + runtimeLabel={runtime.label} + testId={ + installSuccess + ? `onboarding-runtime-check-again-${runtime.id}` + : undefined + } + /> ); } @@ -257,7 +294,7 @@ function RuntimeStatus({ return (