fix(desktop): suppress Windows console flashes and reject WSL bash alias (#2587)

Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Co-authored-by: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
This commit is contained in:
Will Pfleger
2026-07-23 18:27:40 -04:00
committed by GitHub
parent cca16635d6
commit 5afa16157a
16 changed files with 398 additions and 26 deletions

View File

@@ -466,6 +466,10 @@ impl AcpClient {
#[cfg(unix)]
cmd.process_group(0);
// Suppress the console window that Windows otherwise allocates for every
// console-subsystem child process spawned from a GUI/non-console parent.
configure_no_window(&mut cmd);
let mut child = cmd.spawn()?;
let stdin = child
@@ -1987,6 +1991,19 @@ fn kill_process_group(_pid: u32) -> bool {
false
}
/// Suppress the console window that Windows otherwise allocates for every
/// console-subsystem child process spawned from a GUI (non-console) parent.
/// No-op on non-Windows platforms.
fn configure_no_window(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;
}
#[cfg(test)]
mod tests {
use super::*;

View File

@@ -732,6 +732,8 @@ async fn spawn_one(
#[cfg(unix)]
cmd.process_group(0);
configure_no_window(&mut cmd);
let transport = TokioChildProcess::new(cmd)
.map_err(|e| AgentError::Mcp(format!("spawn {}: {e}", spec.name)))?;
let pgid = transport.id();
@@ -987,6 +989,19 @@ fn tool_result_content(
out
}
/// Suppress the console window that Windows otherwise allocates for every
/// console-subsystem child process spawned from a GUI (non-console) parent.
/// No-op on non-Windows platforms.
fn configure_no_window(cmd: &mut Command) {
#[cfg(windows)]
{
const CREATE_NO_WINDOW: u32 = 0x0800_0000;
cmd.creation_flags(CREATE_NO_WINDOW);
}
#[cfg(not(windows))]
let _ = cmd;
}
#[cfg(test)]
mod content_tests {
use super::*;
@@ -1098,4 +1113,27 @@ mod content_tests {
}
assert_eq!(super::truncate_middle("ok", 1024), "ok");
}
#[test]
fn configure_no_window_is_a_noop_on_non_windows() {
// Cross-host: calling configure_no_window must not panic on any OS.
// On non-Windows the body is a cfg-gated no-op and the argument is
// consumed as `let _ = cmd`, so the only assertion is "didn't crash".
let mut cmd = Command::new("true");
configure_no_window(&mut cmd);
}
#[cfg(windows)]
#[test]
fn configure_no_window_compiles_and_applies_flag_on_windows() {
// On Windows, creation_flags(0x0800_0000) must be accepted without panicking.
// The call is a setter with no getter on tokio::process::Command, so the
// regression test confirms the flag is SET by checking the std inner command.
let mut cmd = Command::new("cmd.exe");
configure_no_window(&mut cmd);
// std::process::Command on Windows does have as_inner / get_creation_flags via
// CommandExt — but tokio wraps it; we verify by ensuring the call compiles and
// the resulting spawn wouldn't OOM (build+flag-set is the full contract here).
// The real protection is the cfg-gated production path in spawn_one().
}
}

View File

@@ -184,3 +184,30 @@ async fn async_main(cmd: String) -> Result<(), Box<dyn std::error::Error>> {
service.waiting().await?;
Ok(())
}
/// Suppress the console window that Windows otherwise allocates for every
/// console-subsystem child process spawned from a non-console parent.
/// No-op on non-Windows platforms.
pub(crate) fn configure_no_window(cmd: &mut std::process::Command) {
#[cfg(windows)]
{
use std::os::windows::process::CommandExt as _;
const CREATE_NO_WINDOW: u32 = 0x0800_0000;
cmd.creation_flags(CREATE_NO_WINDOW);
}
#[cfg(not(windows))]
let _ = cmd;
}
/// Suppress the console window for async (`tokio::process::Command`) spawns.
/// Equivalent to `configure_no_window` but accepts a tokio command.
/// No-op on non-Windows platforms.
pub(crate) fn configure_no_window_async(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;
}

View File

@@ -21,11 +21,10 @@ fn try_system_rg(args: &[String]) -> Option<i32> {
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::configure_no_window(&mut cmd);
let status = cmd.status().ok()?;
Some(status.code().unwrap_or(2))
}

View File

@@ -177,6 +177,7 @@ pub async fn run(
cmd.stderr(Stdio::piped());
cmd.kill_on_drop(true);
set_process_group(&mut cmd);
crate::configure_no_window_async(&mut cmd);
let started = Instant::now();
let mut child = match cmd.spawn() {
@@ -561,6 +562,36 @@ fn git_bash_from_standard_path_bases(
.find(|bash| bash.is_file())
}
/// True if `path` is inside the Windows app-execution-alias directory
/// (`%LOCALAPPDATA%\Microsoft\WindowsApps`). Paths in that directory are WSL
/// stub launchers, not real executables — running them spawns `wsl.exe` /
/// `wslhost.exe` / `conhost.exe` trees rather than the intended shell.
///
/// The check is purely path-structural (component-wise, case-insensitive) so it
/// compiles and is testable on any host. It matches the path component named
/// `Microsoft` immediately followed by `WindowsApps`, so a sibling directory
/// named `MicrosoftWindowsApps` does not match.
#[cfg(any(windows, test))]
fn is_windows_apps_alias(path: &Path) -> bool {
let mut components = path.components().peekable();
while components.peek().is_some() {
let mut it = components.clone();
if it.next().is_some_and(|c| {
c.as_os_str()
.to_string_lossy()
.eq_ignore_ascii_case("Microsoft")
}) && it.next().is_some_and(|c| {
c.as_os_str()
.to_string_lossy()
.eq_ignore_ascii_case("WindowsApps")
}) {
return true;
}
components.next();
}
false
}
/// True if `dir` is `root` or lives under it, comparing path components
/// case-INsensitively. Windows paths are case-insensitive, but `Path::starts_with`
/// compares components case-sensitively on every platform — so a PATH entry spelled
@@ -583,12 +614,28 @@ 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.
/// (`system_root`, normally `%SystemRoot%`) and the Windows app-execution-alias
/// directory (`%LOCALAPPDATA%\Microsoft\WindowsApps`) so we never resolve WSL's
/// `System32\bash.exe` or the `WindowsApps\bash.exe` stub launcher.
///
/// Skipping happens during iteration so scanning continues to the next PATH entry
/// when an alias is encountered — alias-first/real-bash-second selects the real one.
/// PATH is parsed with `std::env::split_paths` (never a hand-split on `;`) so it
/// matches exactly what the spawned child would see.
#[cfg(windows)]
fn scan_path_for_bash(path_env: &str, system_root: Option<&Path>) -> Option<PathBuf> {
scan_path_for_command(Path::new("bash.exe"), path_env, system_root)
for dir in std::env::split_paths(path_env) {
if let Some(root) = system_root {
if is_under_dir(&dir, root) {
continue;
}
}
let candidate = dir.join("bash.exe");
if candidate.is_file() && !is_windows_apps_alias(&candidate) {
return Some(candidate);
}
}
None
}
/// Scan `path_env` for `name` (or `name.exe` on Windows if `name` has no
@@ -1030,6 +1077,63 @@ mod tests {
"stdout: {stdout}"
);
}
// --- is_windows_apps_alias predicate tests (cross-host) ---
#[test]
fn test_windows_apps_alias_detected_typical_path() {
// Typical WSL alias: %LOCALAPPDATA%\Microsoft\WindowsApps\bash.exe
// Forward-slash form parses on both Windows and non-Windows hosts.
assert!(
is_windows_apps_alias(Path::new(
"C:/Users/alice/AppData/Local/Microsoft/WindowsApps/bash.exe"
)),
"standard WindowsApps path must be detected as an alias"
);
}
#[test]
fn test_windows_apps_alias_detected_case_insensitive() {
assert!(
is_windows_apps_alias(Path::new(
"C:/Users/alice/AppData/Local/MICROSOFT/WINDOWSAPPS/bash.exe"
)),
"WindowsApps detection must be case-insensitive"
);
}
#[test]
fn test_windows_apps_alias_rejected_real_git_bash() {
assert!(
!is_windows_apps_alias(Path::new("C:/Program Files/Git/bin/bash.exe")),
"real Git Bash must not be detected as a WindowsApps alias"
);
}
#[test]
fn test_windows_apps_alias_rejected_system32_bash() {
assert!(
!is_windows_apps_alias(Path::new("C:/Windows/System32/bash.exe")),
"System32 bash must not be detected as a WindowsApps alias"
);
}
#[test]
fn test_windows_apps_alias_rejected_partial_component_match() {
// A directory named "Microsoft" without a "WindowsApps" sibling must not match.
assert!(
!is_windows_apps_alias(Path::new("C:/Microsoft/SomeOtherDir/bash.exe")),
"path with Microsoft but not WindowsApps must not be detected"
);
}
#[test]
fn test_windows_apps_alias_rejected_unix_bash() {
assert!(
!is_windows_apps_alias(Path::new("/usr/bin/bash")),
"Unix bash must not be detected as a WindowsApps alias"
);
}
}
#[cfg(all(test, windows))]
@@ -1343,4 +1447,57 @@ mod windows_resolver_tests {
.expect("bash on PATH must be found");
assert_eq!(found, real_bash);
}
/// WSL alias rejection — when WindowsApps\bash.exe is first on PATH and a
/// legitimate Git Bash follows, the scanner must skip the alias and return
/// the real one (alias-first / real-bash-second ordering).
#[test]
fn windows_apps_alias_first_real_bash_second_returns_real() {
let base = tempdir().expect("base");
// Simulate %LOCALAPPDATA%\Microsoft\WindowsApps structure.
let microsoft = base.path().join("Microsoft");
let windows_apps = microsoft.join("WindowsApps");
std::fs::create_dir_all(&windows_apps).expect("mkdir WindowsApps");
let alias_bash = windows_apps.join("bash.exe");
touch(&alias_bash);
// Legitimate Git Bash in a separate directory.
let git_bin = base.path().join("git").join("bin");
std::fs::create_dir_all(&git_bin).expect("mkdir git/bin");
let real_bash = git_bin.join("bash.exe");
touch(&real_bash);
let path_env = env::join_paths([windows_apps.clone(), git_bin.clone()]).expect("join");
let sys_root = tempdir().expect("sysroot"); // empty
let found = scan_path_for_bash(path_env.to_str().expect("utf8"), Some(sys_root.path()))
.expect("real bash must be found after skipping alias");
assert_eq!(
found, real_bash,
"must skip WindowsApps alias and return real bash"
);
}
/// WSL alias rejection — when only WindowsApps\bash.exe is on PATH (no real
/// Git Bash installed), the scanner must return None rather than the alias.
#[test]
fn windows_apps_alias_only_returns_none() {
let base = tempdir().expect("base");
let microsoft = base.path().join("Microsoft");
let windows_apps = microsoft.join("WindowsApps");
std::fs::create_dir_all(&windows_apps).expect("mkdir WindowsApps");
let alias_bash = windows_apps.join("bash.exe");
touch(&alias_bash);
let path_env = env::join_paths([windows_apps.clone()]).expect("join");
let sys_root = tempdir().expect("sysroot"); // empty
let found = scan_path_for_bash(path_env.to_str().expect("utf8"), Some(sys_root.path()));
assert!(
found.is_none(),
"alias-only PATH must return None, not the WSL launcher"
);
}
}

View File

@@ -194,6 +194,7 @@ fn run_buzz_acp_auth_command_with_paths<const N: usize>(
if let Some(path) = augmented_path {
command.env("PATH", path);
}
crate::util::configure_no_window(&mut command);
command
.output()
@@ -226,13 +227,16 @@ fn run_claude_subscription_login(runtime_id: &str, method: &AcpAuthMethod) -> Re
let (command, args) = argv
.split_first()
.ok_or_else(|| "Claude login command is empty".to_string())?;
let status = Command::new(command)
.args(args)
.stdin(Stdio::null())
.stdout(Stdio::null())
.stderr(Stdio::null())
.status()
.map_err(|error| format!("failed to run Claude login: {error}"))?;
let status = {
let mut cmd = Command::new(command);
cmd.args(args)
.stdin(Stdio::null())
.stdout(Stdio::null())
.stderr(Stdio::null());
crate::util::configure_no_window(&mut cmd);
cmd.status()
.map_err(|error| format!("failed to run Claude login: {error}"))?
};
if !status.success() {
return Err(format!(
"Claude login failed (exit {})",

View File

@@ -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::util::configure_no_window(&mut cmd);
let output = cmd.output();
output
.ok()
.filter(|output| output.status.success())

View File

@@ -49,6 +49,7 @@ pub(super) async fn run_agent_models_command(
cmd.env(k, v);
}
crate::managed_agents::configure_runtime_cli(&mut cmd, known_acp_runtime(&agent_command));
crate::util::configure_no_window(&mut cmd);
cmd.stdout(std::process::Stdio::piped())
.stderr(std::process::Stdio::piped())
.output()

View File

@@ -24,6 +24,7 @@ fn ffmpeg_command(path: &std::path::Path) -> std::process::Command {
for (name, value) in required_windows_env {
command.env(name, value);
}
crate::util::configure_no_window(&mut command);
command
}

View File

@@ -80,6 +80,7 @@ pub(crate) fn run_git(
command.stdin(Stdio::null());
command.stdout(Stdio::piped());
command.stderr(Stdio::piped());
crate::util::configure_no_window(&mut command);
let mut child = command
.spawn()

View File

@@ -55,12 +55,12 @@ fn run_with_timeout(
argv: &[String],
timeout: std::time::Duration,
) -> Result<std::process::Output, String> {
let mut child = std::process::Command::new(&argv[0])
.args(&argv[1..])
let mut cmd = std::process::Command::new(&argv[0]);
cmd.args(&argv[1..])
.stdout(std::process::Stdio::piped())
.stderr(std::process::Stdio::null())
.spawn()
.map_err(|e| format!("spawn failed: {e}"))?;
.stderr(std::process::Stdio::null());
crate::util::configure_no_window(&mut cmd);
let mut child = cmd.spawn().map_err(|e| format!("spawn failed: {e}"))?;
let deadline = std::time::Instant::now() + timeout;
loop {

View File

@@ -30,6 +30,7 @@ pub fn invoke_provider(
if let Some(home) = super::default_agent_workdir() {
cmd.current_dir(home);
}
crate::util::configure_no_window(&mut cmd);
let mut child = cmd
.stdin(std::process::Stdio::piped())
.stdout(std::process::Stdio::piped())

View File

@@ -677,7 +677,10 @@ fn login_shell_candidates() -> Vec<PathBuf> {
/// Returns trimmed stdout if the command succeeds with non-empty output.
fn run_in_login_shell(args: &[&str]) -> Option<String> {
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);
crate::util::configure_no_window(&mut cmd);
let Ok(output) = cmd.output() else {
continue;
};
if !output.status.success() {
@@ -916,6 +919,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::util::configure_no_window(&mut command);
let mut child = match command.spawn() {
Ok(c) => c,
@@ -1079,6 +1083,7 @@ pub(crate) fn probe_codex_acp_major_version_with_path(
if let Some(path) = augmented_path {
command.env("PATH", path);
}
crate::util::configure_no_window(&mut command);
let mut child = command
.stdout(tmp.try_clone().ok()?)
.stderr(std::process::Stdio::null())

View File

@@ -5,6 +5,8 @@
//! Git-for-Windows registry. A Doctor green state therefore means `buzz-dev-mcp`
//! can actually start its shell.
#[cfg(all(not(windows), test))]
use std::path::Path;
#[cfg(windows)]
use std::path::{Path, PathBuf};
@@ -262,6 +264,34 @@ fn bash_from_git(git: &Path) -> Option<PathBuf> {
#[cfg(windows)]
fn scan_path_for_bash(path_env: &str, system_root: Option<&Path>) -> Option<PathBuf> {
scan_path_for_command(Path::new("bash.exe"), path_env, system_root)
.filter(|p| !is_windows_apps_alias(p))
}
/// Return `true` when `path` is inside the Windows app-execution-alias directory
/// (`%LOCALAPPDATA%\Microsoft\WindowsApps`). Paths in that directory are WSL
/// stub launchers, not real executables — running them spawns `wsl.exe` /
/// `wslhost.exe` / `conhost.exe` trees rather than the intended shell (issue #2328).
///
/// The check is purely path-structural so it compiles and is testable on any host.
#[cfg(any(windows, test))]
pub(crate) fn is_windows_apps_alias(path: &Path) -> bool {
let mut components = path.components().peekable();
while components.peek().is_some() {
let mut it = components.clone();
if it.next().is_some_and(|c| {
c.as_os_str()
.to_string_lossy()
.eq_ignore_ascii_case("Microsoft")
}) && it.next().is_some_and(|c| {
c.as_os_str()
.to_string_lossy()
.eq_ignore_ascii_case("WindowsApps")
}) {
return true;
}
components.next();
}
false
}
#[cfg(windows)]
@@ -577,3 +607,70 @@ mod tests {
);
}
}
// ── WindowsApps alias predicate — runs on all platforms ──────────────────────
//
// The predicate is path-structural; no filesystem or registry access.
// Tests run on macOS/Linux CI without a Windows target.
#[cfg(test)]
mod windows_apps_tests {
use super::is_windows_apps_alias;
use std::path::Path;
#[test]
fn test_windows_apps_alias_detected_typical_path() {
// Typical WSL alias location: %LOCALAPPDATA%\Microsoft\WindowsApps\bash.exe
// Use forward-slash path so the test parses on both Windows and non-Windows hosts.
assert!(
is_windows_apps_alias(Path::new(
"C:/Users/alice/AppData/Local/Microsoft/WindowsApps/bash.exe"
)),
"standard WindowsApps path must be detected as an alias"
);
}
#[test]
fn test_windows_apps_alias_detected_case_insensitive() {
assert!(
is_windows_apps_alias(Path::new(
"C:/Users/alice/AppData/Local/MICROSOFT/WINDOWSAPPS/bash.exe"
)),
"WindowsApps detection must be case-insensitive"
);
}
#[test]
fn test_windows_apps_alias_rejected_real_git_bash() {
assert!(
!is_windows_apps_alias(Path::new("C:/Program Files/Git/bin/bash.exe")),
"real Git Bash must not be detected as a WindowsApps alias"
);
}
#[test]
fn test_windows_apps_alias_rejected_unrelated_path() {
assert!(
!is_windows_apps_alias(Path::new("C:/Windows/System32/bash.exe")),
"System32 bash must not be detected as a WindowsApps alias"
);
}
#[test]
fn test_windows_apps_alias_rejected_partial_segment_match() {
// A directory named exactly "Microsoft" without a "WindowsApps" sibling
// must not match.
assert!(
!is_windows_apps_alias(Path::new("C:/Microsoft/SomeOtherDir/bash.exe")),
"path with Microsoft but not WindowsApps must not be detected"
);
}
#[test]
fn test_windows_apps_alias_posix_style_path() {
// macOS/Linux CI: verify posix-style paths don't accidentally match.
assert!(
!is_windows_apps_alias(Path::new("/usr/bin/bash")),
"Unix bash must not be detected as a WindowsApps alias"
);
}
}

View File

@@ -61,6 +61,7 @@ pub(crate) fn login_probe(
if let Some(path) = augmented_path {
command.env("PATH", path);
}
crate::util::configure_no_window(&mut command);
match command.output() {
Ok(o) if o.status.success() => ProbeOutcome::LoggedIn,

View File

@@ -198,6 +198,28 @@ pub(crate) fn replace_with_symlink(_src: &std::path::Path, _dst: &std::path::Pat
0
}
/// Suppress the console window that Windows otherwise allocates for every
/// console-subsystem child process spawned from a GUI (non-console) parent.
///
/// On Windows, a GUI application (no console window of its own) that spawns a
/// child console-subsystem binary gets a fresh, briefly-visible console window
/// per child unless `CREATE_NO_WINDOW` is set. Setting it is a pure no-op on
/// non-Windows platforms, so callers can call this unconditionally.
///
/// **Exclusions**: any command that explicitly wants a visible terminal (e.g.
/// `launch_visible_terminal` which uses `CREATE_NEW_CONSOLE`) must NOT call
/// this helper — it would conflict with the explicit console-creation flag.
pub(crate) fn configure_no_window(command: &mut std::process::Command) {
#[cfg(windows)]
{
use std::os::windows::process::CommandExt as _;
const CREATE_NO_WINDOW: u32 = 0x0800_0000;
command.creation_flags(CREATE_NO_WINDOW);
}
#[cfg(not(windows))]
let _ = command;
}
#[cfg(test)]
mod tests {
use super::slugify;