From 71834387e69a1f96e579b9ced0ef31efe874b74b Mon Sep 17 00:00:00 2001 From: Paritosh Mishra Date: Thu, 23 Jul 2026 14:09:42 +0530 Subject: [PATCH] fix(dev-mcp): hide background shell windows on Windows Signed-off-by: Paritosh Mishra --- crates/buzz-dev-mcp/src/shell.rs | 56 +++++++++++++++++++++++--------- 1 file changed, 40 insertions(+), 16 deletions(-) diff --git a/crates/buzz-dev-mcp/src/shell.rs b/crates/buzz-dev-mcp/src/shell.rs index b7df4c6747..76241b73a5 100644 --- a/crates/buzz-dev-mcp/src/shell.rs +++ b/crates/buzz-dev-mcp/src/shell.rs @@ -23,6 +23,11 @@ const TAIL_BYTES: usize = 8 * 1024; const ARTIFACT_RING_SIZE: usize = 8; const READ_CHUNK: usize = 16 * 1024; +// Windows tests below temporarily change process-wide shell environment +// variables. Keep those tests separate from tests that create a real shell. +#[cfg(all(test, windows))] +static TEST_ENV_MUTEX: tokio::sync::Mutex<()> = tokio::sync::Mutex::const_new(()); + pub struct SharedState { pub cwd: PathBuf, pub shim: Shim, @@ -177,6 +182,7 @@ pub async fn run( cmd.stderr(Stdio::piped()); cmd.kill_on_drop(true); set_process_group(&mut cmd); + suppress_console_window(&mut cmd); let started = Instant::now(); let mut child = match cmd.spawn() { @@ -322,6 +328,19 @@ pub async fn run( Ok(CallToolResult::success(vec![Content::text(text)])) } +/// Shell tool calls are background work for an agent. On Windows, a console +/// application such as Git Bash otherwise creates a visible terminal window +/// for every command, even though stdout and stderr are captured by this MCP +/// server. +#[cfg(windows)] +fn suppress_console_window(cmd: &mut Command) { + const CREATE_NO_WINDOW: u32 = 0x0800_0000; + cmd.creation_flags(CREATE_NO_WINDOW); +} + +#[cfg(not(windows))] +fn suppress_console_window(_cmd: &mut Command) {} + /// The flag used to pass a command string to the shell. /// /// bash/zsh/sh: `-c` @@ -952,11 +971,14 @@ mod tests { Some(t) => t.text.clone(), None => panic!("no text content"), }; - serde_json::from_str(&text).expect("json") + serde_json::from_str(&text) + .unwrap_or_else(|error| panic!("expected JSON result ({error}); received: {text:?}")) } #[tokio::test(flavor = "current_thread")] async fn basic_echo() { + #[cfg(windows)] + let _env_guard = TEST_ENV_MUTEX.lock().await; let dir = tempdir().expect("tempdir"); let state = make_state(dir.path()); let r = run( @@ -964,7 +986,11 @@ mod tests { ShellParams { command: "echo hello".into(), workdir: None, - timeout_ms: Some(5_000), + // Git Bash can take several seconds to initialise on a busy + // Windows CI worker. This test verifies output, not startup + // performance, so avoid treating that startup variance as a + // shell-tool failure. + timeout_ms: Some(15_000), }, CancellationToken::new(), ) @@ -978,6 +1004,8 @@ mod tests { #[tokio::test(flavor = "current_thread")] async fn timeout_fires() { + #[cfg(windows)] + let _env_guard = TEST_ENV_MUTEX.lock().await; let dir = tempdir().expect("tempdir"); let state = make_state(dir.path()); let r = run( @@ -1003,6 +1031,8 @@ mod tests { #[tokio::test(flavor = "current_thread")] async fn workdir_is_honored() { + #[cfg(windows)] + let _env_guard = TEST_ENV_MUTEX.lock().await; let dir = tempdir().expect("tempdir"); let sub = dir.path().join("sub"); std::fs::create_dir(&sub).expect("mkdir sub"); @@ -1036,14 +1066,8 @@ mod tests { mod windows_resolver_tests { use super::*; use std::env; - use std::sync::Mutex; use tempfile::tempdir; - // Process-global env mutation guard: tests that mutate BUZZ_SHELL, - // SystemRoot, or GIT_BASH must hold this lock for the duration of the - // test so parallel test threads cannot race on these env vars. - static ENV_MUTEX: Mutex<()> = Mutex::new(()); - fn touch(path: &Path) { if let Some(parent) = path.parent() { std::fs::create_dir_all(parent).expect("mkdir"); @@ -1053,7 +1077,7 @@ mod windows_resolver_tests { #[test] fn buzz_shell_override_wins_over_everything() { - let _guard = ENV_MUTEX.lock().unwrap_or_else(|p| p.into_inner()); + let _guard = TEST_ENV_MUTEX.blocking_lock(); // BUZZ_SHELL pointing at a real file must be returned without probing // the standard Git-for-Windows locations or PATH. let dir = tempdir().expect("tempdir"); @@ -1069,7 +1093,7 @@ mod windows_resolver_tests { #[test] fn buzz_shell_override_skipped_when_path_absent() { - let _guard = ENV_MUTEX.lock().unwrap_or_else(|p| p.into_inner()); + let _guard = TEST_ENV_MUTEX.blocking_lock(); // If BUZZ_SHELL points at a non-existent path the resolver must fall // through rather than returning a dead path. env::set_var("BUZZ_SHELL", r"C:\does\not\exist\bash.exe"); @@ -1091,7 +1115,7 @@ mod windows_resolver_tests { /// exclusion — cmd/pwsh live in System32 legitimately. #[test] fn buzz_shell_explicit_bare_name_resolves_from_system32() { - let _guard = ENV_MUTEX.lock().unwrap_or_else(|p| p.into_inner()); + let _guard = TEST_ENV_MUTEX.blocking_lock(); // Simulate cmd.exe living in a dir that would be excluded by the WSL guard. // The explicit BUZZ_SHELL branch must NOT skip System32. let sys32 = tempdir().expect("sys32"); @@ -1123,7 +1147,7 @@ mod windows_resolver_tests { /// Implicit bash.exe scan still skips System32 (WSL guard intact). #[test] fn implicit_bash_scan_still_skips_system32() { - let _guard = ENV_MUTEX.lock().unwrap_or_else(|p| p.into_inner()); + let _guard = TEST_ENV_MUTEX.blocking_lock(); // Same setup: bash.exe is only in a dir that is under SystemRoot. // Without an explicit BUZZ_SHELL, the fallback scan must skip it. let sys32 = tempdir().expect("sys32"); @@ -1163,7 +1187,7 @@ mod windows_resolver_tests { /// resolved_shell whose display name appears in bootstrap_instructions. #[test] fn shared_state_bootstrap_hint_matches_resolved_shell() { - let _guard = ENV_MUTEX.lock().unwrap_or_else(|p| p.into_inner()); + let _guard = TEST_ENV_MUTEX.blocking_lock(); let dir = tempdir().expect("tempdir"); let fake_pwsh = dir.path().join("pwsh.exe"); touch(&fake_pwsh); @@ -1186,7 +1210,7 @@ mod windows_resolver_tests { /// When pwsh.exe is on PATH, resolve_bash must return it and report "pwsh". #[test] fn buzz_shell_bare_name_resolved_through_path_when_present() { - let _guard = ENV_MUTEX.lock().unwrap_or_else(|p| p.into_inner()); + let _guard = TEST_ENV_MUTEX.blocking_lock(); let dir = tempdir().expect("tempdir"); let fake_pwsh = dir.path().join("pwsh.exe"); touch(&fake_pwsh); @@ -1205,7 +1229,7 @@ mod windows_resolver_tests { /// report pwsh as the active shell. #[test] fn buzz_shell_bare_name_absent_from_path_falls_through() { - let _guard = ENV_MUTEX.lock().unwrap_or_else(|p| p.into_inner()); + let _guard = TEST_ENV_MUTEX.blocking_lock(); // Set BUZZ_SHELL to a command that won't be on any real PATH. env::set_var("BUZZ_SHELL", "buzz-shell-does-not-exist-xyz"); let result = resolve_bash(""); @@ -1226,7 +1250,7 @@ mod windows_resolver_tests { #[test] fn git_cmd_on_path_resolves_sibling_git_bash() { - let _guard = ENV_MUTEX.lock().unwrap_or_else(|p| p.into_inner()); + let _guard = TEST_ENV_MUTEX.blocking_lock(); let dir = tempdir().expect("tempdir"); let git = dir.path().join("Git").join("cmd").join("git.exe"); let bash = dir.path().join("Git").join("bin").join("bash.exe");