From be6ec4bf9dcad117ef2287a2233ecbc073fa9c3e Mon Sep 17 00:00:00 2001 From: Thomas Zarebczan Date: Tue, 21 Jul 2026 14:22:35 -0400 Subject: [PATCH 1/7] fix(desktop): Windows runtime install discovery and quiet consoles Goose install could report success while discovery still saw NotInstalled (binary in %USERPROFILE%\goose off PATH), blocking onboarding Next (#2239). Install shells also dropped the process PATH on Windows so system npm was invisible for Codex/Claude adapters (#2238). Console-subsystem probes stole focus via Git Bash. - Probe well-known Windows Node/npm/goose paths; fall back to process PATH for install shells; pin goose GOOSE_BIN_DIR to ~/.local/bin on Windows. - Post-install resolve check; SetupStep only shows Installed after discovery. - Centralize CREATE_NO_WINDOW via windows_console::hide_console (and buzz-dev-mcp hide helper) on spawn sites this fix touches. Signed-off-by: Thomas Zarebczan --- crates/buzz-dev-mcp/src/shell.rs | 16 + .../src-tauri/src/commands/agent_discovery.rs | 263 +++++--------- .../commands/agent_discovery/install_shell.rs | 336 ++++++++++++++++++ .../commands/agent_discovery/managed_node.rs | 9 +- .../src/commands/agent_model_process.rs | 5 +- .../src/commands/project_git_exec.rs | 1 + desktop/src-tauri/src/lib.rs | 1 + .../src-tauri/src/managed_agents/discovery.rs | 39 +- .../src/managed_agents/discovery/tests.rs | 56 +-- .../discovery/tests/windows_paths.rs | 79 ++++ .../managed_agents/discovery/windows_paths.rs | 102 ++++++ .../src/managed_agents/readiness/cli_probe.rs | 1 + desktop/src-tauri/src/windows_console.rs | 23 ++ 13 files changed, 689 insertions(+), 242 deletions(-) create mode 100644 desktop/src-tauri/src/commands/agent_discovery/install_shell.rs create mode 100644 desktop/src-tauri/src/managed_agents/discovery/tests/windows_paths.rs create mode 100644 desktop/src-tauri/src/managed_agents/discovery/windows_paths.rs create mode 100644 desktop/src-tauri/src/windows_console.rs diff --git a/crates/buzz-dev-mcp/src/shell.rs b/crates/buzz-dev-mcp/src/shell.rs index b7df4c6747..1ca8c0f622 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); + hide_console_window(&mut cmd); let started = Instant::now(); let mut child = match cmd.spawn() { @@ -632,6 +633,21 @@ fn set_process_group(cmd: &mut Command) { #[cfg(not(unix))] fn set_process_group(_cmd: &mut Command) {} +/// Suppress the console window on Windows GUI hosts (Buzz desktop). +/// No-op on Unix. Local helper — this crate does not share desktop's util. +fn hide_console_window(cmd: &mut Command) { + #[cfg(windows)] + { + // tokio::process::Command exposes creation_flags as an inherent Windows method. + const CREATE_NO_WINDOW: u32 = 0x0800_0000; + cmd.creation_flags(CREATE_NO_WINDOW); + } + #[cfg(not(windows))] + { + let _ = cmd; + } +} + /// Kill primitive covering the spawned bash AND every descendant it forks, /// mirroring the same guarantee across platforms. /// 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/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/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; + } +} From 58ef77ef9d223779e013b375e770482f295c355c Mon Sep 17 00:00:00 2001 From: Thomas Zarebczan Date: Tue, 21 Jul 2026 14:41:04 -0400 Subject: [PATCH 2/7] fix(desktop): drop #2241 revert; Windows-safe vite beforeDev; CHECK AGAIN MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Restore Justfile + CONTRIBUTING.md from main (accidental soft-reset squash had undone Hermit-pinned lefthook from #2241). - beforeDevCommand: use `pnpm exec vite` instead of Unix-only `exec` (tauri.conf + instance-env) so Windows cmd can start the dev server. - After install success, reuse the existing CHECK AGAIN / CHECKING… pattern so rediscovery is not a permanent spinner. Signed-off-by: Thomas Zarebczan --- desktop/src-tauri/tauri.conf.json | 2 +- .../src/features/onboarding/ui/SetupStep.tsx | 24 +++++++++++++++++++ scripts/instance-env.sh | 6 +++-- 3 files changed, 29 insertions(+), 3 deletions(-) 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..16e098bfbd 100644 --- a/desktop/src/features/onboarding/ui/SetupStep.tsx +++ b/desktop/src/features/onboarding/ui/SetupStep.tsx @@ -93,11 +93,13 @@ function RuntimeReadinessIndicator({ function RuntimeStatus({ installError, + installSuccess, isInstalling, onInstall, runtime, }: { installError: string | null; + installSuccess: boolean; isInstalling: boolean; onInstall: () => void; runtime: AcpRuntimeCatalogEntry; @@ -252,6 +254,24 @@ function RuntimeStatus({ ); } + // Install finished but rediscovery has not marked the runtime ready yet. + // Do not treat local installSuccess as READY (#2239); offer recheck instead. + if (installSuccess) { + return ( + + ); + } + const installLabel = installError ? "RETRY INSTALL" : "INSTALL"; if (runtime.canAutoInstall) { return ( @@ -464,11 +484,13 @@ function RuntimeAuthError({ runtime }: { runtime: AcpRuntimeCatalogEntry }) { function RuntimeCard({ installError, + installSuccess, isInstalling, onInstall, runtime, }: { installError: string | null; + installSuccess: boolean; isInstalling: boolean; onInstall: () => void; runtime: AcpRuntimeCatalogEntry; @@ -498,6 +520,7 @@ function RuntimeCard({ ( /dev/null; then if swift "$GENERATE_DEV_ICON" "$BASE_ICON" "$DEV_ICON" "$BUZZ_WORKTREE_LABEL"; then echo "🌳 Worktree: ${BUZZ_WORKTREE_LABEL}" export VITE_DEV_BRANCH="$BUZZ_WORKTREE_LABEL" - BUZZ_TAURI_CONFIG="{\"build\":{\"devUrl\":\"${DEV_URL}\",\"beforeDevCommand\":\"exec ./node_modules/.bin/vite --port ${BUZZ_VITE_PORT} --strictPort\"},\"identifier\":\"xyz.block.buzz.app.dev.${BUZZ_INSTANCE_SLUG}\",\"productName\":\"Buzz Dev (${BUZZ_WORKTREE_LABEL})\",\"bundle\":{\"icon\":[\"$DEV_ICON\"]}}" + BUZZ_TAURI_CONFIG="{\"build\":{\"devUrl\":\"${DEV_URL}\",\"beforeDevCommand\":\"pnpm exec vite --port ${BUZZ_VITE_PORT} --strictPort\"},\"identifier\":\"xyz.block.buzz.app.dev.${BUZZ_INSTANCE_SLUG}\",\"productName\":\"Buzz Dev (${BUZZ_WORKTREE_LABEL})\",\"bundle\":{\"icon\":[\"$DEV_ICON\"]}}" fi fi fi From b86c595fbd9463f9ba53447ef8a50c50e660aa80 Mon Sep 17 00:00:00 2001 From: Thomas Zarebczan Date: Wed, 22 Jul 2026 13:02:07 -0400 Subject: [PATCH 3/7] fix(buzz-acp): CREATE_NO_WINDOW for agent adapter spawn on Windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Desktop launches buzz-acp windowless; without the same flag the adapter child (cmd.exe/node, buzz-agent, …) allocates its own console per agent. Refs: #2292, PR #2247 review Signed-off-by: Thomas Zarebczan --- crates/buzz-acp/src/acp.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/crates/buzz-acp/src/acp.rs b/crates/buzz-acp/src/acp.rs index b553adaabb..f47ff4ca66 100644 --- a/crates/buzz-acp/src/acp.rs +++ b/crates/buzz-acp/src/acp.rs @@ -466,6 +466,15 @@ impl AcpClient { #[cfg(unix)] cmd.process_group(0); + // Desktop launches buzz-acp with CREATE_NO_WINDOW. Without the same flag + // here, each adapter (cmd.exe → node, buzz-agent, …) allocates its own + // console — one window per agent (#2292). + #[cfg(windows)] + { + const CREATE_NO_WINDOW: u32 = 0x0800_0000; + cmd.creation_flags(CREATE_NO_WINDOW); + } + let mut child = cmd.spawn()?; let stdin = child From 924778e962a73bd6c86b5467858717d3fb3e2df5 Mon Sep 17 00:00:00 2001 From: Thomas Zarebczan Date: Wed, 22 Jul 2026 13:09:37 -0400 Subject: [PATCH 4/7] fix(windows): prefer Git Bash over WindowsApps bash alias PATH often lists %LOCALAPPDATA%\Microsoft\WindowsApps\bash.exe first; that App Execution Alias launches WSL and can hang lookups (~15s+) while real Git Bash is sub-second. Prefer git-derived / install / registry paths, then PATH bash, and skip WindowsApps (and System32) on PATH. Refs: #2328, PR #2247 Signed-off-by: Thomas Zarebczan --- crates/buzz-dev-mcp/src/shell.rs | 44 ++++-- .../src-tauri/src/managed_agents/git_bash.rs | 141 +++++++++++++++++- 2 files changed, 165 insertions(+), 20 deletions(-) diff --git a/crates/buzz-dev-mcp/src/shell.rs b/crates/buzz-dev-mcp/src/shell.rs index 1ca8c0f622..e0ac8d8656 100644 --- a/crates/buzz-dev-mcp/src/shell.rs +++ b/crates/buzz-dev-mcp/src/shell.rs @@ -394,13 +394,14 @@ 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). /// /// Returns `(resolved_path, display_name)`. The display name is derived from the /// resolved path, guaranteeing the dialect hint and the spawned shell agree. @@ -428,11 +429,6 @@ 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(bash) = bash_from_git(&git) { return Ok((bash, "bash".to_string())); @@ -447,6 +443,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, \\ @@ -585,8 +586,8 @@ fn is_under_dir(dir: &Path, root: &Path) -> bool { /// 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. +/// `System32\bash.exe`, and skipping `…\Microsoft\WindowsApps` (App Execution +/// Alias bash → WSL hang, #2328). PATH is parsed with `std::env::split_paths`. #[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) @@ -594,7 +595,8 @@ fn scan_path_for_bash(path_env: &str, system_root: Option<&Path>) -> Option, ) -> Option { let needs_exe = name.extension().is_none(); + let skip_windows_apps = name + .file_name() + .is_some_and(|n| n.eq_ignore_ascii_case("bash.exe") || n.eq_ignore_ascii_case("bash")); for dir in std::env::split_paths(path_env) { if let Some(root) = system_root { if is_under_dir(&dir, root) { continue; } } + if skip_windows_apps && path_looks_like_windows_apps(&dir) { + continue; + } // Try as-is first. let candidate = dir.join(name); if candidate.is_file() { @@ -625,6 +633,18 @@ fn scan_path_for_command( None } +/// 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(unix)] fn set_process_group(cmd: &mut Command) { cmd.process_group(0); diff --git a/desktop/src-tauri/src/managed_agents/git_bash.rs b/desktop/src-tauri/src/managed_agents/git_bash.rs index 790f18cfa2..6db33fa08a 100644 --- a/desktop/src-tauri/src/managed_agents/git_bash.rs +++ b/desktop/src-tauri/src/managed_agents/git_bash.rs @@ -196,24 +196,32 @@ fn resolve_git_bash_inner( local_app_data: Option, check_registry: bool, ) -> Option { + // Prefer real Git Bash over a PATH hit on WindowsApps\bash.exe (App + // Execution Alias → WSL; hangs #2328). Order: overrides → git.exe sibling → + // well-known install dirs → registry → PATH bash (System32 + WindowsApps skipped). 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) .and_then(|git| bash_from_git(&git)) }) .or_else(|| { - git_bash_from_standard_paths([program_files, program_files_x86, local_app_data]) + git_bash_from_standard_paths([ + program_files.clone(), + program_files_x86.clone(), + local_app_data.clone(), + ]) }); if result.is_some() { 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(), local_app_data.as_deref()) } /// Like `resolve_git_bash` but skips the ambient Windows registry lookup, so @@ -260,8 +268,18 @@ 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) +fn scan_path_for_bash( + path_env: &str, + system_root: Option<&Path>, + local_app_data: Option<&Path>, +) -> Option { + let windows_apps = local_app_data.map(|base| base.join("Microsoft").join("WindowsApps")); + scan_path_for_command_skipping( + Path::new("bash.exe"), + path_env, + system_root, + windows_apps.as_deref(), + ) } #[cfg(windows)] @@ -269,12 +287,29 @@ fn scan_path_for_command( name: &Path, path_env: &str, system_root: Option<&Path>, +) -> Option { + scan_path_for_command_skipping(name, path_env, system_root, None) +} + +#[cfg(windows)] +fn scan_path_for_command_skipping( + name: &Path, + path_env: &str, + system_root: Option<&Path>, + also_skip: Option<&Path>, ) -> 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 also_skip.is_some_and(|skip| is_under_dir(&dir, skip)) { + return None; + } + // Store App Execution Alias bash launches WSL and can hang (#2328). + if path_looks_like_windows_apps(&dir) { + return None; + } let candidate = dir.join(name); if candidate.is_file() { return Some(candidate); @@ -290,6 +325,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 +590,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 +694,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" ); From 259a28afa7f77cd226fbd404d596222f6a208038 Mon Sep 17 00:00:00 2001 From: Thomas Zarebczan Date: Wed, 22 Jul 2026 13:09:37 -0400 Subject: [PATCH 5/7] fix(windows): CREATE_NO_WINDOW for MCP and system rg children Nested CreateProcess sites still flash consoles when the parent is windowless: buzz-agent MCP spawns and buzz-dev-mcp delegated rg. Refs: PR #2247 review, #2292 Signed-off-by: Thomas Zarebczan --- crates/buzz-agent/src/mcp.rs | 7 +++++++ crates/buzz-dev-mcp/src/rg.rs | 14 +++++++++----- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/crates/buzz-agent/src/mcp.rs b/crates/buzz-agent/src/mcp.rs index 71c3193a06..f3a2f2a834 100644 --- a/crates/buzz-agent/src/mcp.rs +++ b/crates/buzz-agent/src/mcp.rs @@ -732,6 +732,13 @@ async fn spawn_one( #[cfg(unix)] cmd.process_group(0); + // Windowless buzz-acp parent: MCP children must not allocate consoles. + #[cfg(windows)] + { + const CREATE_NO_WINDOW: u32 = 0x0800_0000; + cmd.creation_flags(CREATE_NO_WINDOW); + } + 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/rg.rs b/crates/buzz-dev-mcp/src/rg.rs index 199b63432e..c4d9a4a34c 100644 --- a/crates/buzz-dev-mcp/src/rg.rs +++ b/crates/buzz-dev-mcp/src/rg.rs @@ -21,11 +21,15 @@ 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); + #[cfg(windows)] + { + use std::os::windows::process::CommandExt; + const CREATE_NO_WINDOW: u32 = 0x0800_0000; + cmd.creation_flags(CREATE_NO_WINDOW); + } + let status = cmd.status().ok()?; Some(status.code().unwrap_or(2)) } From c51b9859fdd28af18730481918490f51c70ad5c6 Mon Sep 17 00:00:00 2001 From: Thomas Zarebczan Date: Wed, 22 Jul 2026 13:15:53 -0400 Subject: [PATCH 6/7] refactor(windows): consolidate no-window helpers and bash skip logic - Route desktop spawns through windows_console::hide_console (runtime, taskkill); stop re-inlining CREATE_NO_WINDOW. - Add buzz-dev-mcp windows_console for std/tokio; use from shell + rg. - Single hide_console helper in buzz-acp and buzz-agent MCP spawn. - Drop redundant WindowsApps also_skip; one path_looks_like check. - Align Doctor/MCP PATH scan API + KEEP IN SYNC notes (full shared resolver deferred: buzz-agent forbids unsafe registry code). - SetupStep: one CheckAgainButton for auth-unknown and post-install. No push. Signed-off-by: Thomas Zarebczan --- crates/buzz-acp/src/acp.rs | 23 ++++-- crates/buzz-agent/src/mcp.rs | 19 +++-- crates/buzz-dev-mcp/src/lib.rs | 1 + crates/buzz-dev-mcp/src/rg.rs | 7 +- crates/buzz-dev-mcp/src/shell.rs | 48 ++++------- crates/buzz-dev-mcp/src/windows_console.rs | 32 ++++++++ .../src-tauri/src/managed_agents/git_bash.rs | 56 +++++-------- .../src/managed_agents/process_lifecycle.rs | 13 ++- .../src-tauri/src/managed_agents/runtime.rs | 7 +- .../src/features/onboarding/ui/SetupStep.tsx | 79 +++++++++++-------- 10 files changed, 152 insertions(+), 133 deletions(-) create mode 100644 crates/buzz-dev-mcp/src/windows_console.rs diff --git a/crates/buzz-acp/src/acp.rs b/crates/buzz-acp/src/acp.rs index f47ff4ca66..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,14 +479,8 @@ impl AcpClient { #[cfg(unix)] cmd.process_group(0); - // Desktop launches buzz-acp with CREATE_NO_WINDOW. Without the same flag - // here, each adapter (cmd.exe → node, buzz-agent, …) allocates its own - // console — one window per agent (#2292). - #[cfg(windows)] - { - const CREATE_NO_WINDOW: u32 = 0x0800_0000; - cmd.creation_flags(CREATE_NO_WINDOW); - } + // Desktop launches buzz-acp windowless; adapters must not allocate consoles (#2292). + hide_console(&mut cmd); let mut child = cmd.spawn()?; diff --git a/crates/buzz-agent/src/mcp.rs b/crates/buzz-agent/src/mcp.rs index f3a2f2a834..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)] @@ -733,11 +746,7 @@ async fn spawn_one( cmd.process_group(0); // Windowless buzz-acp parent: MCP children must not allocate consoles. - #[cfg(windows)] - { - const CREATE_NO_WINDOW: u32 = 0x0800_0000; - cmd.creation_flags(CREATE_NO_WINDOW); - } + hide_console(&mut cmd); let transport = TokioChildProcess::new(cmd) .map_err(|e| AgentError::Mcp(format!("spawn {}: {e}", spec.name)))?; 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 c4d9a4a34c..63d2ca8d67 100644 --- a/crates/buzz-dev-mcp/src/rg.rs +++ b/crates/buzz-dev-mcp/src/rg.rs @@ -23,12 +23,7 @@ fn try_system_rg(args: &[String]) -> Option { let mut cmd = Command::new(&candidate); cmd.args(args).env("PATH", &cleaned_path); - #[cfg(windows)] - { - use std::os::windows::process::CommandExt; - const CREATE_NO_WINDOW: u32 = 0x0800_0000; - cmd.creation_flags(CREATE_NO_WINDOW); - } + 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 e0ac8d8656..c6342b021d 100644 --- a/crates/buzz-dev-mcp/src/shell.rs +++ b/crates/buzz-dev-mcp/src/shell.rs @@ -177,7 +177,7 @@ pub async fn run( cmd.stderr(Stdio::piped()); cmd.kill_on_drop(true); set_process_group(&mut cmd); - hide_console_window(&mut cmd); + crate::windows_console::hide_tokio(&mut cmd); let started = Instant::now(); let mut child = match cmd.spawn() { @@ -403,6 +403,9 @@ fn resolve_bash(_path_env: &str) -> Result<(PathBuf, String), String> { /// 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. /// @@ -416,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)); } @@ -429,7 +432,7 @@ fn resolve_bash(path_env: &str) -> Result<(PathBuf, String), 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())); } @@ -584,29 +587,26 @@ 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`, and skipping `…\Microsoft\WindowsApps` (App Execution -/// Alias bash → WSL hang, #2328). PATH is parsed with `std::env::split_paths`. +/// 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. For `bash.exe`, also skip WindowsApps. 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(); - let skip_windows_apps = name - .file_name() - .is_some_and(|n| n.eq_ignore_ascii_case("bash.exe") || n.eq_ignore_ascii_case("bash")); for dir in std::env::split_paths(path_env) { if let Some(root) = system_root { if is_under_dir(&dir, root) { @@ -616,12 +616,10 @@ fn scan_path_for_command( if skip_windows_apps && path_looks_like_windows_apps(&dir) { continue; } - // Try as-is first. 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"); @@ -634,6 +632,7 @@ fn scan_path_for_command( } /// 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 @@ -653,21 +652,6 @@ fn set_process_group(cmd: &mut Command) { #[cfg(not(unix))] fn set_process_group(_cmd: &mut Command) {} -/// Suppress the console window on Windows GUI hosts (Buzz desktop). -/// No-op on Unix. Local helper — this crate does not share desktop's util. -fn hide_console_window(cmd: &mut Command) { - #[cfg(windows)] - { - // tokio::process::Command exposes creation_flags as an inherent Windows method. - const CREATE_NO_WINDOW: u32 = 0x0800_0000; - cmd.creation_flags(CREATE_NO_WINDOW); - } - #[cfg(not(windows))] - { - let _ = cmd; - } -} - /// Kill primitive covering the spawned bash AND every descendant it forks, /// mirroring the same guarantee across platforms. /// 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/managed_agents/git_bash.rs b/desktop/src-tauri/src/managed_agents/git_bash.rs index 6db33fa08a..6d46ecbafa 100644 --- a/desktop/src-tauri/src/managed_agents/git_bash.rs +++ b/desktop/src-tauri/src/managed_agents/git_bash.rs @@ -196,22 +196,19 @@ fn resolve_git_bash_inner( local_app_data: Option, check_registry: bool, ) -> Option { - // Prefer real Git Bash over a PATH hit on WindowsApps\bash.exe (App - // Execution Alias → WSL; hangs #2328). Order: overrides → git.exe sibling → - // well-known install dirs → registry → PATH bash (System32 + WindowsApps skipped). + // 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_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(|| { - git_bash_from_standard_paths([ - program_files.clone(), - program_files_x86.clone(), - local_app_data.clone(), - ]) + git_bash_from_standard_paths([program_files, program_files_x86, local_app_data]) }); if result.is_some() { return result; @@ -221,7 +218,7 @@ fn resolve_git_bash_inner( return Some(bash); } } - scan_path_for_bash(path_env, system_root.as_deref(), local_app_data.as_deref()) + scan_path_for_bash(path_env, system_root.as_deref()) } /// Like `resolve_git_bash` but skips the ambient Windows registry lookup, so @@ -257,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) } } @@ -268,46 +265,29 @@ fn bash_from_git(git: &Path) -> Option { } #[cfg(windows)] -fn scan_path_for_bash( - path_env: &str, - system_root: Option<&Path>, - local_app_data: Option<&Path>, -) -> Option { - let windows_apps = local_app_data.map(|base| base.join("Microsoft").join("WindowsApps")); - scan_path_for_command_skipping( - Path::new("bash.exe"), - path_env, - system_root, - windows_apps.as_deref(), - ) +fn scan_path_for_bash(path_env: &str, system_root: Option<&Path>) -> Option { + // 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>, -) -> Option { - scan_path_for_command_skipping(name, path_env, system_root, None) -} - -#[cfg(windows)] -fn scan_path_for_command_skipping( - name: &Path, - path_env: &str, - system_root: Option<&Path>, - also_skip: 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 also_skip.is_some_and(|skip| is_under_dir(&dir, skip)) { - return None; - } - // Store App Execution Alias bash launches WSL and can hang (#2328). - if path_looks_like_windows_apps(&dir) { + if skip_windows_apps && path_looks_like_windows_apps(&dir) { return None; } let candidate = dir.join(name); 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/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/features/onboarding/ui/SetupStep.tsx b/desktop/src/features/onboarding/ui/SetupStep.tsx index 16e098bfbd..f02f4cd7df 100644 --- a/desktop/src/features/onboarding/ui/SetupStep.tsx +++ b/desktop/src/features/onboarding/ui/SetupStep.tsx @@ -91,6 +91,37 @@ 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, @@ -236,39 +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 ( - - ); - } - - // Install finished but rediscovery has not marked the runtime ready yet. - // Do not treat local installSuccess as READY (#2239); offer recheck instead. - if (installSuccess) { - return ( - + void runtimesQuery.refetch()} + runtimeId={runtime.id} + runtimeLabel={runtime.label} + testId={ + installSuccess + ? `onboarding-runtime-check-again-${runtime.id}` + : undefined + } + /> ); } @@ -277,7 +294,7 @@ function RuntimeStatus({ return (