mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix(discovery): spawn PowerShell install commands natively on Windows (#2750)
## Problem On Windows, `install_shell_command` wraps every install command in Git Bash `-l -c`, including the Windows-specific `powershell.exe … irm https://chatgpt.com/codex/install.ps1 | iex` command. A Git Bash login shell prepends its POSIX dirs (`C:\Program Files\Git\usr\bin`) to PATH, so when Codex's `install.ps1` shells out to bare `tar -xzf C:\…`, it resolves Git's GNU tar (`/usr/bin/tar`) instead of Windows bundled bsdtar. GNU tar parses `C:` as a remote host: ``` tar (child): Cannot connect to C: resolve failed gzip: stdin: unexpected end of file /usr/bin/tar: Child returned status 128 /usr/bin/tar: Error is not recoverable: exiting now Downloaded Codex package archive did not contain the expected package layout. ``` Claude's installer doesn't hit the same failure because it doesn't shell out to `tar`. ## Solution On Windows, detect `powershell.exe` install commands and spawn them natively (`Command::new("powershell.exe")`) instead of routing them through Git Bash. The discriminator is a case-insensitive prefix check on the first whitespace-delimited token — minimal and precise. The native spawn preserves everything `install_shell_command` provides that applies: - `NPM_CONFIG_*` / `COREPACK` env strip + managed npm prefix env - PATH composed from managed Buzz dirs + inherited process PATH (no POSIX login-shell dirs) - `CREATE_NO_WINDOW` so no console flash - stdin null, piped-drain in `run_install_command` - The retry/backoff/annotate logic is fully shared The `-Command` body is split correctly at the boundary (case-insensitive) and passed as a single argument to preserve pipes and spaces inside the installer script call. Non-PowerShell commands (e.g. `npm install -g …` adapter steps) continue through the existing Git Bash path unchanged. ## Tests 6 unit tests: 1. `is_powershell_command` detection (positive + negative) 2. Routing: PowerShell → native spawn on Windows, non-PowerShell → Git Bash 3. Unix: non-Windows path returns the shell command unchanged (compile-time cfg) 4. `-Command` body preservation (no bash args in native spawn; body is single arg) Full `just desktop-tauri-test` suite: 1627/1627 passing. Windows CI will validate end-to-end. --------- Signed-off-by: Will Pfleger <pfleger.will@gmail.com> Co-authored-by: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
This commit is contained in:
co-authored by
npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7
parent
3bd3a014c6
commit
f3981dbfef
@@ -494,7 +494,11 @@ const overrides = new Map([
|
||||
// +53: pass 2 — three cfg(windows) install shell tests (resolve succeeds with
|
||||
// Git, error hint content, install_shell_command succeeds).
|
||||
// +8: install_shell_from pure seam extracted for deterministic testing.
|
||||
["src-tauri/src/commands/agent_discovery.rs", 1523],
|
||||
// +287: is_powershell_command + install_powershell_command + build_install_command
|
||||
// route PowerShell CLI installs natively on Windows (bypasses Git Bash PATH
|
||||
// poisoning that resolved GNU tar instead of bsdtar → Codex install failure).
|
||||
// Includes unit tests for detection, routing, and -Command body preservation.
|
||||
["src-tauri/src/commands/agent_discovery.rs", 1810],
|
||||
// draft-persistence predicate: submit-time `loadDraft` check + inline comment
|
||||
// + deps-array entry in submitMessage closes the never-persisted-boundary
|
||||
// defect (Thufir Pass-3 finding). Load-bearing correctness fix; queued to
|
||||
|
||||
@@ -552,21 +552,8 @@ fn install_shell_command(command: &str) -> Result<std::process::Command, String>
|
||||
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"));
|
||||
}
|
||||
// Strip hermit vars and set managed npm paths (see apply_npm_env).
|
||||
apply_npm_env(&mut cmd);
|
||||
|
||||
// Compose the PATH for the install shell using the same kernel as the
|
||||
// runtime/probe path so the two can never drift. managed entries first
|
||||
@@ -615,12 +602,7 @@ fn install_shell_command(command: &str) -> Result<std::process::Command, String>
|
||||
}
|
||||
|
||||
// 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);
|
||||
}
|
||||
apply_no_window(&mut cmd);
|
||||
|
||||
Ok(cmd)
|
||||
}
|
||||
@@ -721,8 +703,161 @@ fn annotate_retry_attempts(mut result: InstallStepResult, attempts: u32) -> Inst
|
||||
result
|
||||
}
|
||||
|
||||
/// Returns `true` when `command` is a Windows-native PowerShell invocation
|
||||
/// (i.e. begins with `powershell.exe`). These commands must NOT be routed
|
||||
/// through Git Bash: the Bash login shell prepends POSIX dirs to PATH, so
|
||||
/// bare `tar` inside the PowerShell script resolves to GNU tar
|
||||
/// (`/usr/bin/tar`) instead of Windows bsdtar. GNU tar parses the drive
|
||||
/// letter in `C:\…` as a remote host and fails with "Cannot connect to C:
|
||||
/// resolve failed", which is the exact failure Will observed in the Codex
|
||||
/// PowerShell installer. Non-PowerShell commands (e.g. `npm install -g …`
|
||||
/// adapter steps) are unaffected.
|
||||
///
|
||||
/// The check is case-insensitive because Windows file-system conventions do
|
||||
/// not mandate casing, and the install command constants could change.
|
||||
#[cfg(windows)]
|
||||
fn is_powershell_command(command: &str) -> bool {
|
||||
command
|
||||
.split_ascii_whitespace()
|
||||
.next()
|
||||
.is_some_and(|tok| tok.eq_ignore_ascii_case("powershell.exe"))
|
||||
}
|
||||
|
||||
/// Apply the shared npm env cleanup and managed-prefix setup to an install child.
|
||||
/// Strips hermit-managed vars and establishes the Buzz-managed npm prefix so adapters
|
||||
/// installed via either path (shell or native PowerShell) land in the same location.
|
||||
fn apply_npm_env(cmd: &mut std::process::Command) {
|
||||
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"));
|
||||
}
|
||||
}
|
||||
|
||||
/// Suppress the console window for an install child on Windows (no-op elsewhere).
|
||||
fn apply_no_window(_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);
|
||||
}
|
||||
}
|
||||
|
||||
/// Build a native `powershell.exe` [`Command`][std::process::Command] for the given command
|
||||
/// string, bypassing Git Bash so POSIX PATH entries never leak into the child.
|
||||
///
|
||||
/// Finds the first `-Command` token (case-insensitive, token-boundary match), passes preceding
|
||||
/// tokens as individual flags, and passes the rest as a single body arg. One outer double-quote
|
||||
/// pair is stripped from the body — the catalog strings (`discovery.rs:107`, `:138`) wrap it for
|
||||
/// Bash serialization; stripping here delivers the bare pipeline to PowerShell. Tokens with no
|
||||
/// `-Command` are forwarded individually. Only called from [`build_install_command`] on Windows.
|
||||
#[cfg(windows)]
|
||||
fn install_powershell_command(command: &str) -> std::process::Command {
|
||||
// Strip the leading `powershell.exe` token.
|
||||
let after_exe = command
|
||||
.split_once(|c: char| c.is_ascii_whitespace())
|
||||
.map(|(_, rest)| rest.trim())
|
||||
.unwrap_or("");
|
||||
|
||||
let mut cmd = std::process::Command::new("powershell.exe");
|
||||
|
||||
// Walk tokens to find -Command on a token boundary (not as a substring of
|
||||
// a preceding argument). The comparison is case-insensitive because
|
||||
// PowerShell itself is case-insensitive for parameters.
|
||||
let mut rest = after_exe;
|
||||
let mut found_command_flag = false;
|
||||
loop {
|
||||
let trimmed = rest.trim_start();
|
||||
if trimmed.is_empty() {
|
||||
break;
|
||||
}
|
||||
// Find the end of the current token.
|
||||
let tok_end = trimmed
|
||||
.find(|c: char| c.is_ascii_whitespace())
|
||||
.unwrap_or(trimmed.len());
|
||||
let tok = &trimmed[..tok_end];
|
||||
if tok.eq_ignore_ascii_case("-command") {
|
||||
// Everything after this token (trimmed) is the body.
|
||||
let body_raw = trimmed[tok_end..].trim();
|
||||
// Strip one matching outer double-quote pair inserted by the
|
||||
// Bash-layer catalog serialization. PowerShell does not need them.
|
||||
let body =
|
||||
if body_raw.starts_with('"') && body_raw.ends_with('"') && body_raw.len() >= 2 {
|
||||
&body_raw[1..body_raw.len() - 1]
|
||||
} else {
|
||||
body_raw
|
||||
};
|
||||
cmd.arg("-Command");
|
||||
if !body.is_empty() {
|
||||
cmd.arg(body);
|
||||
}
|
||||
found_command_flag = true;
|
||||
break;
|
||||
}
|
||||
cmd.arg(tok);
|
||||
rest = &trimmed[tok_end..];
|
||||
}
|
||||
|
||||
if !found_command_flag {
|
||||
// No -Command boundary — forward remaining tokens individually.
|
||||
for arg in rest.split_ascii_whitespace() {
|
||||
cmd.arg(arg);
|
||||
}
|
||||
}
|
||||
|
||||
apply_npm_env(&mut cmd);
|
||||
|
||||
// Compose PATH: managed Buzz dirs first, then inherited process PATH.
|
||||
// No login-shell path: login_shell_path() always returns None on Windows,
|
||||
// and we deliberately skip it here to avoid POSIX-shaped entries.
|
||||
let managed: Vec<std::path::PathBuf> = [
|
||||
crate::managed_agents::buzz_managed_node_bin_dir(),
|
||||
crate::managed_agents::buzz_managed_npm_bin_dir(),
|
||||
]
|
||||
.into_iter()
|
||||
.flatten()
|
||||
.collect();
|
||||
let inherited: Vec<std::path::PathBuf> = std::env::var_os("PATH")
|
||||
.map(|p| std::env::split_paths(&p).collect())
|
||||
.unwrap_or_default();
|
||||
// No login path → should_use_inherited returns true so inherited appended.
|
||||
let path_parts = crate::managed_agents::compose_path_entries(managed, vec![], inherited, true);
|
||||
if !path_parts.is_empty() {
|
||||
if let Ok(path) = std::env::join_paths(path_parts) {
|
||||
cmd.env("PATH", path);
|
||||
}
|
||||
}
|
||||
|
||||
apply_no_window(&mut cmd);
|
||||
cmd
|
||||
}
|
||||
|
||||
/// Select the right [`Command`][std::process::Command] builder for `command`.
|
||||
///
|
||||
/// On Windows, PowerShell-prefixed commands are spawned natively (via
|
||||
/// [`install_powershell_command`]) to avoid the Git Bash PATH poisoning that
|
||||
/// causes GNU `tar` to be resolved instead of Windows bsdtar. All other
|
||||
/// commands — including `npm install -g …` adapter steps — keep the existing
|
||||
/// Git Bash path via [`install_shell_command`].
|
||||
///
|
||||
/// On non-Windows this is always `install_shell_command`.
|
||||
fn build_install_command(command: &str) -> Result<std::process::Command, String> {
|
||||
#[cfg(windows)]
|
||||
if is_powershell_command(command) {
|
||||
return Ok(install_powershell_command(command));
|
||||
}
|
||||
install_shell_command(command)
|
||||
}
|
||||
|
||||
fn run_install_command(step: &str, command: &str) -> InstallStepResult {
|
||||
let mut cmd = match install_shell_command(command) {
|
||||
let mut cmd = match build_install_command(command) {
|
||||
Ok(cmd) => cmd,
|
||||
Err(hint) => {
|
||||
return InstallStepResult {
|
||||
@@ -1386,6 +1521,160 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
// ── PowerShell routing ────────────────────────────────────────────────────
|
||||
|
||||
/// Commands beginning with `powershell.exe` (any casing) must be identified
|
||||
/// as PowerShell commands; all others must not.
|
||||
#[cfg(windows)]
|
||||
#[test]
|
||||
fn test_is_powershell_command_detects_powershell_commands() {
|
||||
assert!(
|
||||
super::is_powershell_command(
|
||||
r#"powershell.exe -NoProfile -NonInteractive -Command "irm https://chatgpt.com/codex/install.ps1 | iex""#
|
||||
),
|
||||
"canonical codex install command must be detected as PowerShell"
|
||||
);
|
||||
assert!(
|
||||
super::is_powershell_command("POWERSHELL.EXE -Command foo"),
|
||||
"is_powershell_command must be case-insensitive"
|
||||
);
|
||||
assert!(
|
||||
!super::is_powershell_command("npm install -g @agentclientprotocol/claude-agent-acp"),
|
||||
"npm commands must NOT be detected as PowerShell"
|
||||
);
|
||||
assert!(
|
||||
!super::is_powershell_command(r"curl -fsSL https://example.com | bash"),
|
||||
"bash pipe commands must NOT be detected as PowerShell"
|
||||
);
|
||||
assert!(
|
||||
!super::is_powershell_command(""),
|
||||
"empty string must not be detected as PowerShell"
|
||||
);
|
||||
}
|
||||
|
||||
/// On Windows, `build_install_command` must return a `Command` whose
|
||||
/// program is `powershell.exe` (not `bash.exe`) for PowerShell commands.
|
||||
#[cfg(windows)]
|
||||
#[test]
|
||||
fn test_build_install_command_uses_powershell_natively_on_windows() {
|
||||
let ps_command = r#"powershell.exe -NoProfile -NonInteractive -Command "irm https://chatgpt.com/codex/install.ps1 | iex""#;
|
||||
let result = super::build_install_command(ps_command);
|
||||
assert!(
|
||||
result.is_ok(),
|
||||
"build_install_command must succeed for a PowerShell command; got: {:?}",
|
||||
result.err()
|
||||
);
|
||||
let cmd = result.unwrap();
|
||||
let program = cmd.get_program().to_string_lossy().to_lowercase();
|
||||
assert!(
|
||||
program.contains("powershell"),
|
||||
"PowerShell install command must use powershell.exe, not bash; got: {program}"
|
||||
);
|
||||
assert!(
|
||||
!program.contains("bash"),
|
||||
"PowerShell install command must NOT go through bash; got: {program}"
|
||||
);
|
||||
}
|
||||
|
||||
/// On Windows, `build_install_command` must route non-PowerShell commands
|
||||
/// through Git Bash (program must be bash.exe).
|
||||
#[cfg(windows)]
|
||||
#[test]
|
||||
fn test_build_install_command_uses_git_bash_for_non_powershell_on_windows() {
|
||||
let npm_command = "npm install -g @agentclientprotocol/claude-agent-acp";
|
||||
let result = super::build_install_command(npm_command);
|
||||
assert!(
|
||||
result.is_ok(),
|
||||
"build_install_command must succeed for an npm command on Windows with Git; got: {:?}",
|
||||
result.err()
|
||||
);
|
||||
let cmd = result.unwrap();
|
||||
let program = cmd.get_program().to_string_lossy().to_lowercase();
|
||||
assert!(
|
||||
program.contains("bash"),
|
||||
"non-PowerShell install command must still use bash.exe on Windows; got: {program}"
|
||||
);
|
||||
}
|
||||
|
||||
/// On non-Windows, `build_install_command` must always use the Unix shell
|
||||
/// (zsh or bash), never powershell.exe.
|
||||
#[cfg(not(windows))]
|
||||
#[test]
|
||||
fn test_build_install_command_uses_unix_shell_on_non_windows() {
|
||||
let command = r"curl -fsSL https://example.com/install.sh | bash";
|
||||
let result = super::build_install_command(command);
|
||||
assert!(
|
||||
result.is_ok(),
|
||||
"build_install_command must succeed on Unix; got: {:?}",
|
||||
result.err()
|
||||
);
|
||||
let cmd = result.unwrap();
|
||||
let program = cmd.get_program().to_string_lossy();
|
||||
assert!(
|
||||
program.contains("bash") || program.contains("zsh"),
|
||||
"Unix install command must use bash or zsh, got: {program}"
|
||||
);
|
||||
}
|
||||
|
||||
/// On Windows, `install_powershell_command` must build an exact argv:
|
||||
/// flags before `-Command` forwarded, body unquoted (outer catalog quotes stripped),
|
||||
/// no bash flags, and `-Command` found on token boundary not as substring.
|
||||
#[cfg(windows)]
|
||||
#[test]
|
||||
fn test_powershell_command_argv_exact() {
|
||||
// Catalog format: body wrapped in one outer double-quote pair (Bash-layer serialization).
|
||||
let body = "irm https://chatgpt.com/codex/install.ps1 | iex";
|
||||
let cmd = super::install_powershell_command(&format!(
|
||||
r#"powershell.exe -NoProfile -ExecutionPolicy Bypass -Command "{body}""#
|
||||
));
|
||||
assert_eq!(
|
||||
cmd.get_args()
|
||||
.map(|a| a.to_string_lossy().into_owned())
|
||||
.collect::<Vec<_>>(),
|
||||
vec!["-NoProfile", "-ExecutionPolicy", "Bypass", "-Command", body],
|
||||
"argv must be exact with outer quotes stripped"
|
||||
);
|
||||
}
|
||||
|
||||
/// Token that merely contains `-command` as a substring must not be treated
|
||||
/// as the `-Command` boundary; only an exact token match (case-insensitive) counts.
|
||||
#[cfg(windows)]
|
||||
#[test]
|
||||
fn test_powershell_command_token_boundary_not_substring() {
|
||||
let cmd = super::install_powershell_command(
|
||||
r#"powershell.exe -x-command-y -Command "echo hello""#,
|
||||
);
|
||||
assert_eq!(
|
||||
cmd.get_args()
|
||||
.map(|a| a.to_string_lossy().into_owned())
|
||||
.collect::<Vec<_>>(),
|
||||
vec!["-x-command-y", "-Command", "echo hello"],
|
||||
"substring must not consume -Command boundary early"
|
||||
);
|
||||
}
|
||||
|
||||
/// Claude Code catalog command (discovery.rs:107) must dequote to the bare pipeline.
|
||||
#[cfg(windows)]
|
||||
#[test]
|
||||
fn test_powershell_command_claude_catalog_dequoted() {
|
||||
let cmd = super::install_powershell_command(
|
||||
r#"powershell.exe -NoProfile -ExecutionPolicy Bypass -Command "irm https://claude.ai/install.ps1 | iex""#,
|
||||
);
|
||||
assert_eq!(
|
||||
cmd.get_args()
|
||||
.map(|a| a.to_string_lossy().into_owned())
|
||||
.collect::<Vec<_>>(),
|
||||
vec![
|
||||
"-NoProfile",
|
||||
"-ExecutionPolicy",
|
||||
"Bypass",
|
||||
"-Command",
|
||||
"irm https://claude.ai/install.ps1 | iex",
|
||||
],
|
||||
"Claude catalog command must be dequoted correctly"
|
||||
);
|
||||
}
|
||||
|
||||
// ── install retry ─────────────────────────────────────────────────────────
|
||||
|
||||
/// Build an `InstallStepResult` with just the fields the retry loop reads.
|
||||
|
||||
Reference in New Issue
Block a user