diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f4d6ddf0085..0f75e21c1cc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -837,89 +837,38 @@ jobs: - name: Test (buzz-dev-mcp) # The Windows-only bash resolver lives in buzz-dev-mcp; its unit tests # only gate if this crate is tested ON Windows. - run: cargo test -p buzz-dev-mcp --target $env:TARGET - # The bundled-bash staging (PortableGit download + SFX extract of the WHOLE - # tree, including mingw64/) runs only in release.yml on tag — so without this - # step the agent's only Windows transport would ship UNEXERCISED until a - # tagged release hits users. Stage the tree and spawn the LAUNCHER bash - # (bin/bash.exe, the entry the resolver now uses) to prove all of: the SFX - # `-o` POSIX-path extraction works; the launcher spawns and sets up - # MSYSTEM/PATH; the lazily-loaded MSYS DLL closure survives; and — the whole - # point of bundling the full toolchain — git/jq/curl resolve from the bundled - # mingw64/bin and git is functional (a real commit round-trips). The launcher - # prepending mingw64/bin to PATH under `-c` is the load-bearing behavior that - # only a real Windows host can confirm; this gate is where it gets proven. - # (End-to-end nostr-SIGNED commits compose build_git_env's GIT_CONFIG_* with - # the git-sign-nostr helper — covered by buzz-dev-mcp unit tests + a bare-host - # check, not here, since this job stages only empty sidecar placeholders.) - - name: Smoke-test bundled bash + git toolchain staging + # Serial: windows_resolver_tests mutate process-global env + # (BUZZ_SHELL/GIT_BASH/SystemRoot) that SharedState::new reads. + run: cargo test -p buzz-dev-mcp --target $env:TARGET -- --test-threads=1 + # Smoke-test the new host-prereq contract: Git for Windows (which provides + # bash) is available on the runner, a shell command round-trips, and bash + # does NOT resolve from System32 (so WSL's launcher is never picked up). + # windows-latest runners have Git for Windows pre-installed; the unit tests + # above exercise the MCP resolver itself. This step verifies the host env. + - name: Smoke-test host Git Bash prereq (host env check) shell: bash run: | set -euo pipefail - stage_dir="$RUNNER_TEMP/git-bash" - scripts/stage-windows-bash.sh "$stage_dir" - launcher="$stage_dir/bin/bash.exe" - [[ -f "$launcher" ]] || { echo "launcher bash missing: $launcher" >&2; exit 1; } + # Git for Windows ships bash.exe under its bin/ directory; confirm it + # resolves from the standard location the runtime resolver probes first. + bash_path=$(command -v bash 2>/dev/null || true) + [[ -n "$bash_path" ]] || { echo "ERROR: bash not found on PATH — host Git for Windows missing" >&2; exit 1; } + echo "Resolved bash: $bash_path" + [[ "$bash_path" != *System32* ]] || { echo "ERROR: resolved bash is WSL's System32 launcher" >&2; exit 1; } - # Faithfulness to the agent: shell.rs spawns the launcher with PATH set - # wholesale to the shim PATH (shim tempdir + the MCP process's inherited - # PATH). We strip PATH down to just the Windows system dir before - # spawning — stricter than a bare host (ambient PATH stripped to force - # resolution through the launcher). This stops the runner's ambient - # git/jq from masking the launcher's own mingw64/bin prepend, so anything - # that resolves had to come from the launcher itself. The fidelity gap is - # benign: the real shim tempdir only ever holds rg/tree/buzz and the two - # nostr helpers, never git/jq/curl, so a richer real PATH cannot shadow - # mingw64/bin for these three tools. - win_root="${SYSTEMROOT:-${SystemRoot:-C:\\Windows}}" - bare_path="$win_root\\System32;$win_root" + # Run a basic pipeline through the resolved bash (same invocation the + # agent uses: bash -c '...'). + out=$(bash -c 'echo hello | tr a-z A-Z') + [[ "$out" == "HELLO" ]] || { echo "bash pipeline failed: got '$out'" >&2; exit 1; } - # Coreutils pipeline through the launcher (the resolver's entry point). - out=$(PATH="$bare_path" "$launcher" -c 'echo hello | tr a-z A-Z') - [[ "$out" == "HELLO" ]] || { echo "staged bash pipeline failed: got '$out'" >&2; exit 1; } - - # The full-toolchain payoff: git/jq/curl must resolve from the bundle's - # mingw64/bin or usr/bin. Use NON-login `-c` to match the agent's actual - # invocation (shell.rs spawns `bash -c`): if the launcher only fixes PATH - # under a login shell, `-lc` would pass here while the real agent stays - # broken — the exact green-but-broken trap this gate guards. - # - # Anchor to the ACTUAL staged tree, not a mount-shape: command -v reports - # the MSYS mount path (/mingw64/bin/git), so we cygpath -m it back to the - # real Windows location and assert it sits under $stage_dir. A bare - # `/mingw64/*` shape check would pass for any /mingw64-rooted mount; the - # cygpath round-trip proves the tool is literally inside the bundle we - # just staged. cygpath ships in the bundle's usr/bin (MSYS2 core, same - # tier as the `tr` proven above), so it resolves under the launcher. - # - # curl is the one tool of the three with a System32 twin - # (C:\Windows\System32\curl.exe, on $bare_path) — if a future PortableGit - # ever drops curl from mingw64/bin, the launcher would resolve the - # System32 twin and this REDs on a non-bug; git/jq have no such twin. - stage_m=$("$launcher" -c "cygpath -m '$stage_dir'") - for tool in git jq curl; do - located=$(PATH="$bare_path" "$launcher" -c "command -v $tool >/dev/null 2>&1 && cygpath -m \"\$(command -v $tool)\"" || true) - [[ -n "$located" ]] || { echo "$tool did not resolve in bundled bash" >&2; exit 1; } - case "$located" in - "$stage_m"/*) ;; - *) echo "$tool resolved outside the staged bundle: $located (stage: $stage_m)" >&2; exit 1 ;; - esac - echo "$tool -> $located" - done - - # git is not just present but functional: a real commit round-trips, - # again through non-login `-c` with the bare PATH. Single-quoted on - # purpose — this script body runs in the launcher's shell, not ours. - # shellcheck disable=SC2016 - PATH="$bare_path" "$launcher" -c ' - set -euo pipefail - repo=$(mktemp -d) - cd "$repo" - git init -q - git -c user.name=ci -c user.email=ci@example.com commit -q --allow-empty -m smoke - git log -1 --format=%s | grep -qx smoke - ' || { echo "bundled git failed a commit round-trip" >&2; exit 1; } - echo "staged launcher bash spawned; git/jq/curl resolve from the bundle; git commit works" + # Confirm git itself works — agents run git commands frequently. + git --version + repo=$(mktemp -d) + cd "$repo" + git init -q + git -c user.name=ci -c user.email=ci@example.com commit -q --allow-empty -m smoke + git log -1 --format=%s | grep -qx smoke + echo "Host bash resolved and functional; git commit round-trip passed" - name: Check (Tauri crate) run: cargo check --manifest-path desktop/src-tauri/Cargo.toml --target $env:TARGET env: diff --git a/README.md b/README.md index 07523f1a9bd..9d6a75b4d6d 100644 --- a/README.md +++ b/README.md @@ -135,6 +135,16 @@ For agents, set `BUZZ_PRIVATE_KEY` and use [`buzz-cli`](crates/buzz-cli) — JSO --- +## Windows prerequisites + +The agent shell tool runs commands under bash. On macOS and Linux that's already there; on Windows you need to bring it. + +Install [Git for Windows](https://git-scm.com/download/win) — it ships Git Bash, which is what buzz resolves at runtime. Once it's installed, everything works the same as on other platforms. + +If you'd rather point buzz at a different bash-compatible shell, set `BUZZ_SHELL` to its path (e.g. `BUZZ_SHELL=C:\path\to\bash.exe`). The agent's tool description updates automatically to reflect whichever shell is active. + +--- + ## Architecture ``` diff --git a/crates/buzz-acp/src/base_prompt.md b/crates/buzz-acp/src/base_prompt.md index 2a1a98dcf26..fe2ea7107bc 100644 --- a/crates/buzz-acp/src/base_prompt.md +++ b/crates/buzz-acp/src/base_prompt.md @@ -2,7 +2,7 @@ You are operating inside the Buzz platform — a Nostr-based messaging platform ## Buzz CLI -The `buzz` CLI is your primary interface. Auth env vars: `BUZZ_RELAY_URL`, `BUZZ_PRIVATE_KEY`, `BUZZ_AUTH_TAG`. Exit codes: 0 ok, 1 user error, 2 network, 3 auth, 4 other. Output is structured JSON — pipe through `jq` as needed. +The `buzz` CLI is your primary interface. Auth env vars: `BUZZ_RELAY_URL`, `BUZZ_PRIVATE_KEY`, `BUZZ_AUTH_TAG`. Exit codes: 0 ok, 1 user error, 2 network, 3 auth, 4 other. Output is structured JSON. | Group | Key commands | |-------|-------------| diff --git a/crates/buzz-dev-mcp/src/lib.rs b/crates/buzz-dev-mcp/src/lib.rs index 102a1c65510..f9fced8e81d 100644 --- a/crates/buzz-dev-mcp/src/lib.rs +++ b/crates/buzz-dev-mcp/src/lib.rs @@ -39,7 +39,7 @@ impl DevMcp { #[tool( name = "shell", - description = "Run a bash command. Ephemeral process per call. Output tail-truncated to ~8KB for the LLM; full output (first 10MB) saved to artifact file. timeout_ms defaults to 120000 (2 min) if omitted; capped at 600000 (10 min). For long-running commands (git push with hooks, cargo build, test suites), use 300000+. On PATH: rg (prefer over grep; flags: -n -i -l -g -C --files), tree (flags: -d ; shows line counts), and buzz (Buzz relay CLI — run buzz --help for commands)." + description = "Run a shell command (bash by default; set `BUZZ_SHELL` to use cmd, PowerShell, or another shell). Ephemeral process per call. Output tail-truncated to ~8KB for the LLM; full output (first 10MB) saved to artifact file. timeout_ms defaults to 120000 (2 min) if omitted; capped at 600000 (10 min). For long-running commands (git push with hooks, cargo build, test suites), use 300000+. On PATH: rg (prefer over grep; flags: -n -i -l -g -C --files), tree (flags: -d ; shows line counts), and buzz (Buzz relay CLI — run buzz --help for commands)." )] async fn shell( &self, diff --git a/crates/buzz-dev-mcp/src/paths.rs b/crates/buzz-dev-mcp/src/paths.rs index 8fa28cd640f..1770d562fa2 100644 --- a/crates/buzz-dev-mcp/src/paths.rs +++ b/crates/buzz-dev-mcp/src/paths.rs @@ -52,12 +52,12 @@ pub(crate) fn resolve_path(root: &Path, path: &str) -> Result { /// - UNC: `//server/share/x` -> `\\server\share\x`. /// /// A third form — root-anchored `/tmp`, `/usr/...`, `/bin` — maps under the -/// MSYS install root (the bundled `git-bash` dir), which this process does not -/// reliably know. We deliberately do NOT guess it: such a path falls through -/// untranslated and fails with the clear `path not accessible` error rather than -/// being silently mis-mapped to the wrong location. Resolving it correctly would -/// require shelling out to the bundled `cygpath`; that is out of scope here and -/// these paths are not a normal target for agent file I/O. +/// MSYS install root (from the host's Git for Windows install), which this +/// process does not reliably know. We deliberately do NOT guess it: such a path +/// falls through untranslated and fails with the clear `path not accessible` +/// error rather than being silently mis-mapped to the wrong location. Resolving +/// it correctly would require shelling out to `cygpath`; that is out of scope +/// here and these paths are not a normal target for agent file I/O. #[cfg(windows)] fn msys_to_windows(path: &str) -> String { // UNC: exactly two leading slashes then a non-empty host segment. diff --git a/crates/buzz-dev-mcp/src/shell.rs b/crates/buzz-dev-mcp/src/shell.rs index 76aec2c77a7..2dde39f3d95 100644 --- a/crates/buzz-dev-mcp/src/shell.rs +++ b/crates/buzz-dev-mcp/src/shell.rs @@ -28,6 +28,10 @@ pub struct SharedState { pub shim: Shim, pub session_dir: TempDir, pub bootstrap_instructions: String, + /// The shell resolved at construction: `Ok((path, display_name))` when a shell + /// is available, `Err(msg)` when none was found. Stored once so both the + /// bootstrap hint and every `run()` call read the SAME resolution — no drift. + pub resolved_shell: Result<(PathBuf, String), String>, pub artifacts: Mutex>, next_call_id: Mutex, } @@ -37,12 +41,22 @@ impl SharedState { let session_dir = tempfile::Builder::new() .prefix("buzz-dev-mcp-session-") .tempdir()?; - let bootstrap_instructions = build_bootstrap(&cwd); + // Resolve the shell ONCE using the same PATH the spawn will use. + // Both the bootstrap dialect hint and every run() call read this result, + // so they can never disagree. A failed resolution is stored as Err and + // surfaces as an actionable error on the first tool call. + let resolved_shell = resolve_bash(&shim.path_env); + let shell_hint = match &resolved_shell { + Ok((_, name)) => name.as_str(), + Err(_) => "bash", + }; + let bootstrap_instructions = build_bootstrap(&cwd, shell_hint); Ok(Self { cwd, shim, session_dir, bootstrap_instructions, + resolved_shell, artifacts: Mutex::new(VecDeque::with_capacity(ARTIFACT_RING_SIZE)), next_call_id: Mutex::new(0), }) @@ -58,7 +72,7 @@ impl SharedState { } } -fn build_bootstrap(cwd: &Path) -> String { +fn build_bootstrap(cwd: &Path, shell_hint: &str) -> String { let stack = detect_stack(cwd); let buzz_hint = if std::env::var("BUZZ_RELAY_URL").is_ok() && std::env::var("BUZZ_PRIVATE_KEY").is_ok() { @@ -69,6 +83,7 @@ fn build_bootstrap(cwd: &Path) -> String { format!( "Working directory: {}\n\ Detected stack: {}\n\ + Shell: {shell_hint} (set BUZZ_SHELL to override) — write command strings in that shell's syntax.\n\ Pass `workdir` per call rather than `cd`.\n\ {buzz_hint}", cwd.display(), @@ -143,12 +158,13 @@ pub async fn run( )); } - let bash = match resolve_bash(&state.shim.path_env) { - Ok(path) => path, - Err(msg) => return Ok(CallToolResult::error(vec![Content::text(msg)])), + let bash = match &state.resolved_shell { + Ok((path, _)) => path.clone(), + Err(msg) => return Ok(CallToolResult::error(vec![Content::text(msg.clone())])), }; + let shell_arg = shell_flag(&bash); let mut cmd = Command::new(&bash); - cmd.arg("-c").arg(&p.command); + cmd.arg(shell_arg).arg(&p.command); cmd.current_dir(&workdir); cmd.env("PATH", &state.shim.path_env); // NOSTR_PRIVATE_KEY is already removed from this process's env (shim.rs). @@ -167,7 +183,7 @@ pub async fn run( Ok(c) => c, Err(e) => { return Ok(CallToolResult::error(vec![Content::text(format!( - "failed to spawn bash: {e}" + "failed to spawn shell: {e}" ))])); } }; @@ -306,55 +322,119 @@ pub async fn run( Ok(CallToolResult::success(vec![Content::text(text)])) } -/// The bundled bash subtree's directory name under the install root, and the -/// relative path to its `bash.exe`. This is the THREE-FILE PATH CONTRACT — it must -/// stay byte-identical with: -/// 1. `scripts/bundle-sidecars.sh` — stages the bash tree to -/// `desktop/src-tauri/binaries/git-bash/` (the bundle-source dir). -/// 2. `desktop/scripts/build-release-config.mjs` — emits the Windows-only -/// `bundle.resources` Map `{ "binaries/git-bash": "git-bash" }`, whose TARGET -/// (`git-bash`) is what Tauri's NSIS/MSI installer stages next to the exe. -/// 3. this resolver — joins `current_exe().parent()` + `git-bash\bin\bash.exe`. +/// The flag used to pass a command string to the shell. /// -/// Drift between (2)'s target and this string ships a working bundle but a broken -/// runtime path. Keep all three in lockstep. +/// bash/zsh/sh: `-c` +/// cmd.exe: `/C` +/// powershell/pwsh: `-Command` /// -/// The bundled constant points at `bin\bash.exe` — the git-for-windows launcher, -/// NOT `usr\bin\bash.exe`. The bundle now ships the WHOLE PortableGit tree -/// (including `mingw64/`), so it has the sibling `mingw64\bin` the launcher needs: -/// the launcher is the correct entry because it sets `MSYSTEM=MINGW64` and prepends -/// `mingw64\bin` to the in-shell PATH, which is what makes `git`/`jq`/`curl` resolve -/// inside the agent's shell with no Rust-side PATH injection. The installed-Git -/// branch below uses the SAME `bin\bash.exe` entry for the same reason — same tree -/// shape, same correct entry point. -#[cfg(windows)] -const BUNDLED_BASH_REL: &str = r"git-bash\bin\bash.exe"; +/// The resolver supports `BUZZ_SHELL=cmd`/`pwsh` (explicit operator overrides +/// resolve without the System32 exclusion, so these shells work). The dispatch +/// here ensures each shell receives the correct flag regardless of which one +/// was resolved. +fn shell_flag(shell: &Path) -> &'static str { + match shell + .file_stem() + .and_then(|s| s.to_str()) + .map(|s| s.to_ascii_lowercase()) + .as_deref() + { + Some("cmd") => "/C", + Some("powershell" | "pwsh") => "-Command", + _ => "-c", + } +} + +/// Extract a short display name from a resolved shell path (e.g. `pwsh.exe` → `"pwsh"`). +fn shell_name_from_path(p: &Path) -> String { + p.file_stem() + .and_then(|s| s.to_str()) + .map(|s| s.to_ascii_lowercase()) + .unwrap_or_else(|| "bash".to_string()) +} -/// Resolve a genuine, non-WSL bash to an absolute path so we spawn it directly -/// instead of letting `Command::new("bash")` re-enter PATH search — on Windows -/// that search finds `System32\bash.exe` (the WSL launcher), which fails at spawn -/// with `0x8007072c` and can never run the agent's POSIX commands. +/// Resolve the shell to spawn. On Unix, bash on PATH is correct and was never +/// broken, so the resolver only needs to honor BUZZ_SHELL. The probe logic and +/// System32 exclusion are Windows-only. /// -/// On Unix, bare `bash` resolved via PATH is correct and was never broken, so the -/// resolver is a no-op there. The probe logic is Windows-only. +/// Returns `(resolved_path, display_name)`. The display name is derived from the +/// resolved path so the caller can use it for diagnostics without a second lookup. #[cfg(not(windows))] -fn resolve_bash(_path_env: &str) -> Result { - Ok(PathBuf::from("bash")) +fn resolve_bash(_path_env: &str) -> Result<(PathBuf, String), String> { + // Honor BUZZ_SHELL on Unix so power users can opt into zsh or another shell. + if let Some(raw) = std::env::var_os("BUZZ_SHELL") { + let p = PathBuf::from(&raw); + // Absolute / rooted path: must exist as a file. + if p.components().count() > 1 || p.has_root() { + if p.is_file() { + let name = shell_name_from_path(&p); + return Ok((p, name)); + } + // Non-existent path: fall through to bash. + } else { + // Bare command name: scan the process PATH directly. + let path_var = std::env::var_os("PATH").unwrap_or_default(); + for dir in std::env::split_paths(&path_var) { + let candidate = dir.join(&p); + if candidate.is_file() { + let name = shell_name_from_path(&candidate); + return Ok((candidate, name)); + } + } + // Not found: fall through to bash. + } + } + Ok((PathBuf::from("bash"), "bash".to_string())) } /// Windows bash resolution. Probe order (first hit wins): -/// 1. `GIT_BASH` env override (escape hatch / explicit operator choice). -/// 2. Installed Git for Windows (fast path when the user has Git). -/// 3. The bundled bash staged next to our exe (guaranteed target — this -/// is what makes a bare, Git-less host work since the app is self-contained). -/// 4. PATH scan, EXCLUDING System32 (so we never resolve WSL's `bash.exe`). +/// 1. `BUZZ_SHELL` env override — explicit operator choice, any shell (cmd, +/// PowerShell, bash, etc.). Bare command names are resolved through PATH +/// 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. Installed Git for Windows (fast path when the user has Git). +/// 4. PATH scan for `bash.exe`, EXCLUDING System32 (so we never resolve WSL's +/// `bash.exe` — the `0x8007072c` hazard). +/// +/// Returns `(resolved_path, display_name)`. The display name is derived from the +/// resolved path, guaranteeing the dialect hint and the spawned shell agree. /// -/// No bash found -> actionable error returned BEFORE spawn. +/// The previously-bundled PortableGit fallback (probe 3 in the old order) has +/// been removed: Git for Windows is a documented host prerequisite, and shipping +/// a multi-hundred-MB runtime contradicts the VISION_AGENT.md "minimal" principle. +/// +/// No bash found -> actionable error pointing at the prerequisite. #[cfg(windows)] -fn resolve_bash(path_env: &str) -> Result { +fn resolve_bash(path_env: &str) -> Result<(PathBuf, String), String> { + // BUZZ_SHELL: explicit operator override — any shell, including cmd or PowerShell. + // Bare command names are resolved WITHOUT System32 exclusion: the operator + // chose this shell on purpose, and cmd/pwsh legitimately live in System32. + if let Some(raw) = std::env::var_os("BUZZ_SHELL") { + let p = PathBuf::from(&raw); + // Absolute / rooted path: must exist as a file. + if p.components().count() > 1 || p.has_root() { + if p.is_file() { + let name = shell_name_from_path(&p); + return Ok((p, name)); + } + // Non-existent absolute path: fall through, do NOT report this shell. + } else { + // Bare command name (e.g. "pwsh", "cmd"): scan PATH, NO System32 + // exclusion — the operator explicitly wants this shell. + if let Some(found) = scan_path_for_command(&p, path_env, None) { + let name = shell_name_from_path(&found); + return Ok((found, name)); + } + // Not found on PATH: fall through. + } + } + + // GIT_BASH: legacy override kept for back-compat. if let Some(p) = std::env::var_os("GIT_BASH").map(PathBuf::from) { if p.is_file() { - return Ok(p); + let name = shell_name_from_path(&p); + return Ok((p, name)); } } @@ -367,42 +447,24 @@ fn resolve_bash(path_env: &str) -> Result { .join("bin") .join("bash.exe"); if candidate.is_file() { - return Ok(candidate); - } - } - } - - // Bundled bash, located relative to OUR OWN executable. On Windows, Tauri - // stages `bundle.resources` flat in the directory that contains the exe - // (tauri 2.11.2 `resource_dir()` == exe parent on Windows), and every sidecar - // — including this one — lives in that same dir. This relative-to-self resolution - // is Windows-ONLY: macOS stages resources to `../Resources` and Linux to - // `usr/lib/`, so a cross-platform "resource relative to exe" helper would - // be wrong on those platforms. - if let Ok(exe) = std::env::current_exe() { - if let Some(dir) = exe.parent() { - if let Some(p) = bundled_bash(dir) { - return Ok(p); + return Ok((candidate, "bash".to_string())); } } } + // PATH scan for bash.exe, skipping System32 to avoid WSL's bash.exe launcher. if let Some(p) = scan_path_for_bash(path_env, std::env::var_os("SystemRoot").map(PathBuf::from)) { - return Ok(p); + return Ok((p, "bash".to_string())); } - Err("no bash found: install Git for Windows, or set GIT_BASH to a bash.exe path".into()) -} - -/// Compute the bundled bash path relative to the install dir (the exe's parent), -/// `is_file`-gated so dev/CI builds without a staged resource return None and let -/// the caller fall through cleanly — never returning a non-existent path that -/// would fail later at spawn with a worse message. -#[cfg(windows)] -fn bundled_bash(install_dir: &Path) -> Option { - let bundled = install_dir.join(BUNDLED_BASH_REL); - bundled.is_file().then_some(bundled) + Err( + "Git for Windows (git bash) is required but was not found.\n\ + Install it from https://git-scm.com/download/win and re-launch Buzz,\n\ + or set BUZZ_SHELL to the path of any bash-compatible executable (or a bare\n\ + command name like cmd or pwsh if it is on PATH)." + .into(), + ) } /// True if `dir` is `root` or lives under it, comparing path components @@ -432,18 +494,38 @@ fn is_under_dir(dir: &Path, root: &Path) -> bool { /// 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) -> Option { + scan_path_for_command(Path::new("bash.exe"), path_env, system_root.as_deref()) +} + +/// Scan `path_env` for `name` (or `name.exe` on Windows if `name` has no +/// extension), skipping any directory under `system_root` to avoid resolving +/// WSL helpers. Returns the first absolute path found. +#[cfg(windows)] +fn scan_path_for_command( + name: &Path, + path_env: &str, + system_root: Option<&Path>, +) -> Option { + let needs_exe = name.extension().is_none(); for dir in std::env::split_paths(path_env) { - if let Some(ref root) = system_root { - // Skip System32 (and any other dir under %SystemRoot%) — that's where - // WSL's bash.exe lives. + if let Some(root) = system_root { if is_under_dir(&dir, root) { continue; } } - let candidate = dir.join("bash.exe"); + // 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"); + if with_exe.is_file() { + return Some(with_exe); + } + } } None } @@ -860,8 +942,14 @@ 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"); @@ -870,23 +958,176 @@ mod windows_resolver_tests { } #[test] - fn bundled_branch_returns_none_when_path_absent() { - // Dev/CI: no staged resource next to the exe -> the bundled branch must - // yield None so the resolver falls through instead of returning a - // non-existent path that would fail at spawn. + fn buzz_shell_override_wins_over_everything() { + let _guard = ENV_MUTEX.lock().unwrap_or_else(|p| p.into_inner()); + // 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"); - assert!(bundled_bash(dir.path()).is_none()); + let fake_bash = dir.path().join("my-bash.exe"); + touch(&fake_bash); + // Temporarily set BUZZ_SHELL; clean up after the test. + env::set_var("BUZZ_SHELL", &fake_bash); + let result = resolve_bash(""); + env::remove_var("BUZZ_SHELL"); + let (resolved, _name) = result.expect("BUZZ_SHELL override should resolve"); + assert_eq!(resolved, fake_bash); + } + + #[test] + fn buzz_shell_override_skipped_when_path_absent() { + let _guard = ENV_MUTEX.lock().unwrap_or_else(|p| p.into_inner()); + // 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"); + // We cannot easily assert the fallback here without a full Git install, + // but we can assert the override itself is not returned. + let result = resolve_bash(""); + env::remove_var("BUZZ_SHELL"); + if let Ok((resolved, _)) = result { + assert_ne!( + resolved.to_str().unwrap_or(""), + r"C:\does\not\exist\bash.exe", + "non-existent BUZZ_SHELL must not be returned" + ); + } + // An Err is also acceptable (no Git installed on test host). } + /// Explicit BUZZ_SHELL bare name resolves through PATH and uses NO System32 + /// exclusion — cmd/pwsh live in System32 legitimately. #[test] - fn bundled_branch_returns_absolute_path_when_staged() { - // A staged PortableGit bash runtime next to the exe resolves to the absolute bash path. + fn buzz_shell_explicit_bare_name_resolves_from_system32() { + let _guard = ENV_MUTEX.lock().unwrap_or_else(|p| p.into_inner()); + // 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"); + let fake_cmd = sys32.path().join("cmd.exe"); + touch(&fake_cmd); + + // Build a path_env with only sys32 (the WSL exclusion would skip this dir + // for bash.exe, but must NOT skip it for an explicit BUZZ_SHELL). + let path_env = env::join_paths([sys32.path().to_path_buf()]).expect("join"); + env::set_var("BUZZ_SHELL", "cmd"); + // Override SystemRoot so the exclusion would trigger on sys32 if applied. + let old_sysroot = env::var_os("SystemRoot"); + env::set_var("SystemRoot", sys32.path()); + + let result = resolve_bash(path_env.to_str().expect("utf8")); + + env::remove_var("BUZZ_SHELL"); + match old_sysroot { + Some(v) => env::set_var("SystemRoot", v), + None => env::remove_var("SystemRoot"), + } + + let (resolved, name) = + result.expect("explicit BUZZ_SHELL=cmd should resolve even from System32-like dir"); + assert_eq!(resolved, fake_cmd); + assert_eq!(name, "cmd"); + } + + /// 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()); + // 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"); + touch(&sys32.path().join("bash.exe")); + + let path_env = env::join_paths([sys32.path().to_path_buf()]).expect("join"); + // No BUZZ_SHELL — trigger the implicit bash fallback scan. + env::remove_var("BUZZ_SHELL"); + env::remove_var("GIT_BASH"); + // Point SystemRoot at sys32's parent so sys32 is "under SystemRoot". + let parent = sys32.path().parent().unwrap().to_path_buf(); + let old_sysroot = env::var_os("SystemRoot"); + env::set_var("SystemRoot", &parent); + + let result = resolve_bash(path_env.to_str().expect("utf8")); + + match old_sysroot { + Some(v) => env::set_var("SystemRoot", v), + None => env::remove_var("SystemRoot"), + } + + // Should be Err (no Git installed on test host, and the only bash.exe was + // under SystemRoot so it was skipped). Ok is also acceptable if git bash + // happens to be installed at the fixed Program Files path — we just assert + // the System32 bash was NOT returned. + if let Ok((resolved, _)) = result { + assert!( + !resolved.starts_with(sys32.path()), + "implicit bash scan must not return the System32 bash: {resolved:?}" + ); + } + } + + /// The bootstrap hint (resolved_shell field) and the spawn path are the + /// same object — both come from SharedState.resolved_shell. + /// Verify that constructing SharedState with BUZZ_SHELL set produces a + /// 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 dir = tempdir().expect("tempdir"); - let bash = dir.path().join(BUNDLED_BASH_REL); - touch(&bash); - let resolved = bundled_bash(dir.path()).expect("bundled bash"); - assert!(resolved.is_absolute()); - assert_eq!(resolved, bash); + let fake_pwsh = dir.path().join("pwsh.exe"); + touch(&fake_pwsh); + env::set_var("BUZZ_SHELL", &fake_pwsh); + + let shim = crate::shim::Shim::install().expect("shim"); + let state = SharedState::new(dir.path().to_path_buf(), shim).expect("state"); + + env::remove_var("BUZZ_SHELL"); + + let (_, name) = state.resolved_shell.as_ref().expect("resolved ok"); + assert_eq!(name, "pwsh"); + assert!( + state.bootstrap_instructions.contains("pwsh"), + "bootstrap must mention the resolved shell name" + ); + } + + /// F3: BUZZ_SHELL bare command name (e.g. "pwsh") resolved through PATH. + /// 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 dir = tempdir().expect("tempdir"); + let fake_pwsh = dir.path().join("pwsh.exe"); + touch(&fake_pwsh); + + let path_env = env::join_paths([dir.path().to_path_buf()]).expect("join"); + env::set_var("BUZZ_SHELL", "pwsh"); + let result = resolve_bash(path_env.to_str().expect("utf8")); + env::remove_var("BUZZ_SHELL"); + + let (resolved, name) = result.expect("bare BUZZ_SHELL=pwsh should resolve from PATH"); + assert_eq!(resolved, fake_pwsh, "should resolve to pwsh.exe on PATH"); + assert_eq!(name, "pwsh", "display name must match resolved shell"); + } + + /// F3: BUZZ_SHELL bare command name absent from PATH → fall through, do not + /// 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()); + // 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(""); + env::remove_var("BUZZ_SHELL"); + if let Ok((resolved, name)) = result { + assert_ne!( + resolved.file_name().and_then(|n| n.to_str()).unwrap_or(""), + "buzz-shell-does-not-exist-xyz.exe", + "absent BUZZ_SHELL must not be returned as the resolved path" + ); + assert_ne!( + name, "buzz-shell-does-not-exist-xyz", + "absent BUZZ_SHELL must not be reported as the shell name" + ); + } + // Err is also acceptable (no Git on test host). } #[test] @@ -947,4 +1188,27 @@ mod windows_resolver_tests { "case-divergent System32 must still be excluded" ); } + + /// PATH-only discovery — a bash.exe custom-installed on PATH (not under + /// the standard Program Files locations) must be found by the runtime + /// resolver. This verifies the PATH fallback in resolve_bash: a custom + /// install that lives outside Program Files is still usable as a shell. + #[test] + fn path_only_bash_is_found_by_scan() { + // scan_path_for_bash is the runtime resolver's PATH fallback helper. + // Verify it returns the bash. + let real = tempdir().expect("real"); + let real_bash = real.path().join("bash.exe"); + touch(&real_bash); + + let path_env = env::join_paths([real.path().to_path_buf()]).expect("join"); + let sys_root = tempdir().expect("sysroot"); // empty — no System32 here + + let found = scan_path_for_bash( + path_env.to_str().expect("utf8"), + Some(sys_root.path().to_path_buf()), + ) + .expect("bash on PATH must be found"); + assert_eq!(found, real_bash); + } } diff --git a/desktop/scripts/build-release-config.mjs b/desktop/scripts/build-release-config.mjs index 71ea2c7946a..389d18aec5d 100644 --- a/desktop/scripts/build-release-config.mjs +++ b/desktop/scripts/build-release-config.mjs @@ -52,29 +52,6 @@ const releaseConfig = { }, }; -// Windows-only: bundle the full PortableGit toolchain (bash runtime + git + -// curl/coreutils in mingw64/, plus a vendored standalone jq.exe) as a resource so -// the MCP shell tool always has a genuine, non-WSL bash AND a real dev toolchain -// to spawn on a bare host (the app must be self-contained — we cannot assume Git -// for Windows is installed). -// -// This is emitted ONLY on the Windows runner because the static tauri.conf.json -// uses `targets: "all"` with a shared bundle block — a bare `resources` entry -// there would ship the (now ~350MB+) tree into the macOS .dmg and Linux packages -// too. The release build runs THIS generator on each platform's own runner and -// merges the output via --config, so guarding on process.platform keeps the tree -// off mac/Linux. -// -// PATH CONTRACT (keep byte-identical across three files): -// - source `binaries/git-bash` (relative to src-tauri/) is staged by -// scripts/bundle-sidecars.sh. -// - target `git-bash` is the install-root subdir; Tauri's Windows installer -// stages it next to the exe, and crates/buzz-dev-mcp/src/shell.rs resolves -// `git-bash\bin\bash.exe` relative to its own executable at runtime. -if (process.platform === "win32") { - releaseConfig.bundle.resources = { "binaries/git-bash": "git-bash" }; -} - console.log(`Updater enabled -> ${updaterEndpoint}`); writeFileSync(outputConfigPath, `${JSON.stringify(releaseConfig, null, 2)}\n`); diff --git a/scripts/bundle-sidecars.sh b/scripts/bundle-sidecars.sh index 12405917041..be37cbce0dd 100755 --- a/scripts/bundle-sidecars.sh +++ b/scripts/bundle-sidecars.sh @@ -38,12 +38,3 @@ for bin in "${SIDECARS[@]}"; do cp "$SRC_DIR/${bin}${EXE}" "$BINARIES_DIR/${bin}-${TARGET}${EXE}" done echo "Sidecars bundled for $TARGET" - -# Windows-only: stage a genuine, non-WSL bash plus the full git toolchain next to -# the sidecars so the MCP shell tool works on a bare host. The download/extract -# logic lives in a self-contained script (no release-binary precondition) so CI can -# call it directly to exercise this path on a real Windows runner — see -# scripts/stage-windows-bash.sh for the full rationale and the PATH CONTRACT. -if [[ "$TARGET" == *windows* ]]; then - "$(dirname "$0")/stage-windows-bash.sh" "$BINARIES_DIR/git-bash" -fi diff --git a/scripts/stage-windows-bash.sh b/scripts/stage-windows-bash.sh deleted file mode 100755 index 1bdc172fc63..00000000000 --- a/scripts/stage-windows-bash.sh +++ /dev/null @@ -1,106 +0,0 @@ -#!/usr/bin/env bash -set -euo pipefail - -# Stage a genuine, non-WSL bash for the Windows MCP shell tool. The app is -# self-contained — we cannot assume Git for Windows is installed — so we bundle -# the full toolchain rather than probing for an install. -# -# There is no standalone "bash for Windows" upstream: working Windows bash ships -# only inside git-for-windows. We download PortableGit and keep the WHOLE tree — -# the MSYS2 bash runtime (`usr/` + `bin/`) AND the `mingw64/` subtree that carries -# `git.exe` plus `curl`/full `sed`/`awk`/`grep`/`find` — so a bare host gets a -# real dev env, not just bash + coreutils. `jq` is NOT in PortableGit, so we -# additionally vendor a pinned standalone `jq.exe` into `mingw64/bin` (see below). -# We do NOT trim INSIDE the tree: bash and git load `msys-2.0.dll` and other -# libraries lazily, and load-bearing pieces (terminfo, gawk libs in `usr/`; git -# templates, certs, DLLs in `mingw64/`) live alongside the docs, so a hand-trimmed -# copy can pass an existence check yet fail mid-command with a cryptic error — -# exactly the bug class this avoids. The retained runtime is the untouched, -# complete closure git-for-windows maintains. -# -# Self-contained (no release-binary precondition) so CI can call it directly to -# exercise the download/extract path on a real Windows runner — the only -# automated gate on this logic before it ships to users. -# -# Single arg: the destination dir for the staged tree (the launcher bash lands at -# /bin/bash.exe; git lands at /mingw64/bin/git.exe). Idempotent: a -# versioned `.stage-complete-v2` marker, written last, proves a whole prior stage -# and skips the re-download; a partial stage (or a stale v1 marker from the old -# mingw64-dropped layout) lacks it and re-extracts cleanly. -# -# PATH CONTRACT (keep byte-identical across three files): -# - dest `git-bash` (== desktop/src-tauri/binaries/git-bash) is the -# `bundle.resources` SOURCE in desktop/scripts/build-release-config.mjs. -# - that resource's TARGET `git-bash` is staged next to the exe by Tauri's -# Windows installer, and crates/buzz-dev-mcp/src/shell.rs resolves -# `git-bash\bin\bash.exe` relative to its own executable at runtime. - -GIT_BASH_DIR=${1:?usage: stage-windows-bash.sh } -PORTABLEGIT_VERSION="2.54.0" -PORTABLEGIT_TAG="v${PORTABLEGIT_VERSION}.windows.1" -PORTABLEGIT_EXE="PortableGit-${PORTABLEGIT_VERSION}-64-bit.7z.exe" -PORTABLEGIT_URL="https://github.com/git-for-windows/git/releases/download/${PORTABLEGIT_TAG}/${PORTABLEGIT_EXE}" - -# jq is NOT shipped in PortableGit (it is an independent MSYS2 package, not a git -# component), but agents need it for JSON piping, so we vendor the standalone -# static jq.exe (single binary, no DLL closure) into the bundle's mingw64/bin so -# it resolves alongside git/curl through the launcher. Pinned by SHA-256 — never -# an unpinned fetch. -JQ_VERSION="1.8.1" -JQ_URL="https://github.com/jqlang/jq/releases/download/jq-${JQ_VERSION}/jq-windows-amd64.exe" -JQ_SHA256="23cb60a1354eed6bcc8d9b9735e8c7b388cd1fdcb75726b93bc299ef22dd9334" - -STAGE_MARKER="$GIT_BASH_DIR/.stage-complete-v2" -if [[ -f "$STAGE_MARKER" ]]; then - echo "PortableGit bash already staged at $GIT_BASH_DIR" - exit 0 -fi - -echo "Downloading PortableGit ${PORTABLEGIT_VERSION}..." -tmp_dir=$(mktemp -d -t portablegit.XXXXXX) -trap 'rm -rf "$tmp_dir"' EXIT -tmp_sfx="$tmp_dir/portablegit.7z.exe" -extract_dir="$tmp_dir/extract" -curl -fsSL "$PORTABLEGIT_URL" -o "$tmp_sfx" -# PortableGit is a 7-Zip self-extracting archive; -o/-y are its SFX flags, -# so we don't need a separate 7z on PATH. -chmod +x "$tmp_sfx" -"$tmp_sfx" -y "-o$extract_dir" - -# Keep the whole extracted tree — bash runtime AND the mingw64/ git+toolchain -# subtree — so the bundle is a real self-contained dev env. -rm -rf "$GIT_BASH_DIR" -mkdir -p "$GIT_BASH_DIR" -cp -a "$extract_dir/." "$GIT_BASH_DIR/" - -# Vendor the pinned standalone jq.exe into mingw64/bin (alongside git/curl) so it -# resolves through the launcher. Verify the SHA-256 before it lands — a checksum -# mismatch fails the stage so a tampered/wrong binary never reaches the bundle. -echo "Downloading jq ${JQ_VERSION}..." -jq_tmp="$tmp_dir/jq.exe" -curl -fsSL "$JQ_URL" -o "$jq_tmp" -actual_sha=$(sha256sum "$jq_tmp" | cut -d' ' -f1) -[[ "$actual_sha" == "$JQ_SHA256" ]] || { - echo "Error: jq.exe SHA-256 mismatch: got $actual_sha, expected $JQ_SHA256" >&2 - exit 1 -} -cp "$jq_tmp" "$GIT_BASH_DIR/mingw64/bin/jq.exe" - -rm -rf "$tmp_dir" -trap - EXIT -# Assert the load-bearing entry points landed: the launcher bash we resolve at -# runtime, git.exe in mingw64/ (the binary the whole restore exists to ship), and -# the vendored jq.exe. A stale or partial extract that lacks any must fail the -# gate, not write the marker — otherwise the idempotency skip would lock in a -# broken tree. -for required in bin/bash.exe mingw64/bin/git.exe mingw64/bin/jq.exe; do - [[ -f "$GIT_BASH_DIR/$required" ]] || { - echo "Error: PortableGit extracted but $GIT_BASH_DIR/$required is missing" >&2 - exit 1 - } -done -# Written last, only after cp -a and the integrity checks all succeed, so it is -# positive proof the whole tree landed. An interrupted stage never writes it, so -# the idempotency skip falls through to a clean re-extract. -touch "$STAGE_MARKER" -echo "PortableGit full toolchain staged at $GIT_BASH_DIR (bash + git + jq + coreutils)"