mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix(desktop): fix Windows PATH clobber and .cmd shim EINVAL (#2563)
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
1e6e743a35
commit
cca16635d6
@@ -178,6 +178,10 @@ const overrides = new Map([
|
||||
// team-instructions-first-class: ManagedAgentRecord fixture gains the new
|
||||
// team_id field (+1 line).
|
||||
["src-tauri/src/managed_agents/readiness.rs", 1765],
|
||||
// Windows PATH-correctness fix: 3 #[cfg(windows)] test functions covering
|
||||
// .cmd shim rejection, .bat shim rejection, and .exe acceptance for
|
||||
// configure_runtime_cli (fix #2397). Test-only growth; queued to split.
|
||||
["src-tauri/src/managed_agents/runtime/tests.rs", 1041],
|
||||
// applyWorkspace reposDir parameter plus the validateReposDir binding,
|
||||
// threaded through Tauri invokes for configurable repos_dir, plus the
|
||||
// harness-persona-sync `harnessOverride` create-input bit — load-bearing
|
||||
|
||||
@@ -568,16 +568,32 @@ fn install_shell_command(command: &str) -> Result<std::process::Command, String>
|
||||
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));
|
||||
}
|
||||
// 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
|
||||
// (Node/npm bins keep precedence); login-shell entries next; inherited
|
||||
// process PATH appended last on Windows when no login-shell PATH exists
|
||||
// (login_shell_path() always returns None on Windows — Git Bash paths are
|
||||
// POSIX-shaped and poison native children; cmd.env("PATH", …) replaces
|
||||
// rather than extends, so without inherited the install shell loses npm).
|
||||
let login_path = crate::managed_agents::login_shell_path();
|
||||
let had_login = login_path.is_some();
|
||||
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 login: Vec<std::path::PathBuf> = login_path
|
||||
.as_deref()
|
||||
.map(|p| std::env::split_paths(p).collect())
|
||||
.unwrap_or_default();
|
||||
let inherited: Vec<std::path::PathBuf> = std::env::var_os("PATH")
|
||||
.map(|p| std::env::split_paths(&p).collect())
|
||||
.unwrap_or_default();
|
||||
let use_inherited = crate::managed_agents::should_use_inherited(had_login, true, cfg!(windows));
|
||||
let path_parts =
|
||||
crate::managed_agents::compose_path_entries(managed, login, inherited, use_inherited);
|
||||
if !path_parts.is_empty() {
|
||||
if let Ok(path) = std::env::join_paths(path_parts) {
|
||||
cmd.env("PATH", path);
|
||||
@@ -1296,6 +1312,45 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
/// On Windows, `install_shell_command` must set PATH to a value that
|
||||
/// includes the inherited process PATH, so node/npm are visible inside
|
||||
/// the install shell even when no managed Node runtime is present.
|
||||
#[cfg(windows)]
|
||||
#[test]
|
||||
fn test_install_shell_command_includes_process_path_on_windows() {
|
||||
let _guard = crate::managed_agents::lock_path_mutex();
|
||||
let previous = std::env::var_os("PATH");
|
||||
// Plant a sentinel in the process PATH that the test can detect.
|
||||
let sentinel = r"C:\TestSentinel\bin";
|
||||
std::env::set_var("PATH", sentinel);
|
||||
|
||||
let result = super::install_shell_command("echo test");
|
||||
|
||||
match previous {
|
||||
Some(p) => std::env::set_var("PATH", p),
|
||||
None => std::env::remove_var("PATH"),
|
||||
}
|
||||
|
||||
let cmd = result.expect("install_shell_command must succeed on Windows with Git");
|
||||
let path_value = cmd
|
||||
.get_envs()
|
||||
.find(|(key, _)| *key == "PATH")
|
||||
.and_then(|(_, val)| val)
|
||||
.map(|v| v.to_string_lossy().into_owned())
|
||||
.expect("install_shell_command must always set a PATH env var on Windows");
|
||||
|
||||
// The sentinel (inherited process PATH) must appear in the composed PATH.
|
||||
assert!(
|
||||
path_value.contains(sentinel),
|
||||
"install_shell_command PATH must include the inherited process PATH; got: {path_value}"
|
||||
);
|
||||
// The sentinel must appear LAST — managed Buzz dirs must have precedence.
|
||||
assert!(
|
||||
path_value.ends_with(sentinel),
|
||||
"inherited process PATH must be appended LAST so managed dirs keep precedence; got: {path_value}"
|
||||
);
|
||||
}
|
||||
|
||||
// ── Phase B: per-OS install commands ──────────────────────────────────────
|
||||
|
||||
/// On non-Windows, cli_install_commands_for_os returns the default commands.
|
||||
|
||||
@@ -16,6 +16,9 @@ use crate::{
|
||||
|
||||
mod path;
|
||||
pub(in crate::managed_agents) use path::build_augmented_path;
|
||||
pub(crate) use path::compose_path_entries;
|
||||
pub(crate) use path::should_skip_claude_executable;
|
||||
pub(crate) use path::should_use_inherited;
|
||||
|
||||
mod stop;
|
||||
pub(crate) use stop::managed_agent_runtime_keys;
|
||||
@@ -1602,6 +1605,15 @@ pub(crate) fn configure_runtime_cli(
|
||||
return;
|
||||
}
|
||||
if let Some(cli_path) = runtime.underlying_cli.and_then(resolve_command) {
|
||||
// On Windows, `.cmd` and `.bat` files are batch shims — they cannot be
|
||||
// passed directly to `CreateProcess` and cause EINVAL when the Claude
|
||||
// adapter tries to spawn them (issue #2397). Skip setting
|
||||
// `CLAUDE_CODE_EXECUTABLE` for shim paths so the adapter falls back to
|
||||
// its own PATH lookup and finds the real binary instead.
|
||||
// Non-Windows: `.cmd`/`.bat` are valid executables and must be assigned.
|
||||
if should_skip_claude_executable(&cli_path, cfg!(windows)) {
|
||||
return;
|
||||
}
|
||||
command.env("CLAUDE_CODE_EXECUTABLE", cli_path);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2,6 +2,86 @@
|
||||
|
||||
use std::path::PathBuf;
|
||||
|
||||
/// Return `true` when `path` is a Windows batch shim (`.cmd` or `.bat`,
|
||||
/// case-insensitive) that cannot be passed directly to `CreateProcess`.
|
||||
///
|
||||
/// Extracted as a pure function so it can be unit-tested on any host without
|
||||
/// touching the global PATH or `resolve_command` cache (issue #2397).
|
||||
pub(crate) fn is_batch_shim(path: &std::path::Path) -> bool {
|
||||
path.extension()
|
||||
.map(|ext| {
|
||||
let lower = ext.to_string_lossy().to_lowercase();
|
||||
lower == "cmd" || lower == "bat"
|
||||
})
|
||||
.unwrap_or(false)
|
||||
}
|
||||
|
||||
/// Return `true` when the resolved CLI path should be skipped for
|
||||
/// `CLAUDE_CODE_EXECUTABLE` assignment.
|
||||
///
|
||||
/// On Windows, `.cmd`/`.bat` batch shims cannot be passed directly to
|
||||
/// `CreateProcess` (EINVAL, issue #2397). On non-Windows those extensions are
|
||||
/// valid executables and must not be suppressed — the `is_windows` flag keeps
|
||||
/// this decision testable cross-host on macOS CI.
|
||||
pub(crate) fn should_skip_claude_executable(path: &std::path::Path, is_windows: bool) -> bool {
|
||||
is_windows && is_batch_shim(path)
|
||||
}
|
||||
|
||||
/// Decide whether the inherited process PATH should be appended to the
|
||||
/// composed PATH.
|
||||
///
|
||||
/// On Windows, `login_shell_path()` always returns `None` because Git Bash
|
||||
/// returns POSIX colon-delimited paths that poison native children.
|
||||
/// `Command::env("PATH", …)` replaces rather than extends, so without the
|
||||
/// inherited PATH every child loses node/npm/git.
|
||||
///
|
||||
/// This pure function takes an explicit `is_windows` flag so it can be
|
||||
/// unit-tested cross-host (macOS CI can pass `true` to exercise the Windows
|
||||
/// policy without needing the `cfg!(windows)` target).
|
||||
///
|
||||
/// Rules:
|
||||
/// - Only append when `is_windows` — on Unix the login-shell PATH always covers
|
||||
/// the needed runtimes.
|
||||
/// - Suppress when `had_shell_path` is `true` — if a login-shell PATH was
|
||||
/// supplied it already carries the user's native entries; appending the
|
||||
/// process PATH would double them.
|
||||
/// - Suppress when `has_local_context` is `false` — callers that pass no home
|
||||
/// or exe-parent context must not receive a PATH manufactured from ambient
|
||||
/// process state alone.
|
||||
pub(crate) fn should_use_inherited(
|
||||
had_shell_path: bool,
|
||||
has_local_context: bool,
|
||||
is_windows: bool,
|
||||
) -> bool {
|
||||
is_windows && !had_shell_path && has_local_context
|
||||
}
|
||||
|
||||
/// Pure PATH composition kernel shared by the install shell and the runtime/probe paths.
|
||||
///
|
||||
/// Merges already-split PATH entries in precedence order:
|
||||
/// 1. `managed` — Buzz-controlled dirs (highest precedence, e.g. managed Node/npm bins)
|
||||
/// 2. `login` — login-shell PATH entries (split before calling)
|
||||
/// 3. `inherited` — current-process PATH entries (split before calling), appended
|
||||
/// only when `use_inherited` is `true`
|
||||
///
|
||||
/// Callers are responsible for splitting raw PATH strings and for prepending any
|
||||
/// additional prefix entries (e.g. `home/.local/bin`, `nvm`, `exe_parent`) before
|
||||
/// passing them in `managed`. `split_paths`/`join_paths` are kept at the wrapper
|
||||
/// boundaries so this function remains fully pure and testable on any host.
|
||||
pub(crate) fn compose_path_entries(
|
||||
managed: Vec<PathBuf>,
|
||||
login: Vec<PathBuf>,
|
||||
inherited: Vec<PathBuf>,
|
||||
use_inherited: bool,
|
||||
) -> Vec<PathBuf> {
|
||||
let mut parts = managed;
|
||||
parts.extend(login);
|
||||
if use_inherited {
|
||||
parts.extend(inherited);
|
||||
}
|
||||
parts
|
||||
}
|
||||
|
||||
/// Assemble the augmented `PATH` for a launched managed-agent child process.
|
||||
///
|
||||
/// Concatenates, in priority order:
|
||||
@@ -11,6 +91,10 @@ use std::path::PathBuf;
|
||||
/// 4. `nvm_bin` — nvm's default Node.js bin dir (if the user uses nvm)
|
||||
/// 5. exe parent dir — DMG sidecars under `Contents/MacOS/`
|
||||
/// 6. user's login-shell `PATH` — runtimes like node/python from other managers
|
||||
/// 7. Windows only: the current process `PATH` (appended when no login-shell
|
||||
/// PATH exists, because callers use `Command::env("PATH", …)` which
|
||||
/// *replaces* the child's PATH — without this, the child loses node/npm/git
|
||||
/// and every npm `.cmd` shim fails with `'node' is not recognized`)
|
||||
///
|
||||
/// `shell_path` is the raw colon-delimited string from a login shell, so it is
|
||||
/// split into individual entries before joining. Pushing it as a single segment
|
||||
@@ -24,31 +108,46 @@ pub(in crate::managed_agents) fn build_augmented_path(
|
||||
shell_path: Option<String>,
|
||||
nvm_bin: Option<PathBuf>,
|
||||
) -> Option<String> {
|
||||
let mut parts: Vec<PathBuf> = Vec::new();
|
||||
let home_added = home.is_some();
|
||||
let exe_added = exe_parent.is_some();
|
||||
let has_local_context = home_added || exe_added;
|
||||
|
||||
// Build the managed/prefix entries (everything before login-shell PATH).
|
||||
let mut managed: Vec<PathBuf> = Vec::new();
|
||||
if let Some(home) = home {
|
||||
parts.push(home.join(".local").join("bin"));
|
||||
managed.push(home.join(".local").join("bin"));
|
||||
}
|
||||
// Only add managed runtime dirs when a home or executable context exists.
|
||||
// This keeps tests/utility callers that intentionally pass no local context
|
||||
// from manufacturing a PATH out of ambient platform dirs alone.
|
||||
if home_added || exe_parent.is_some() {
|
||||
if has_local_context {
|
||||
if let Some(managed_npm_bin) = crate::managed_agents::buzz_managed_npm_bin_dir() {
|
||||
parts.push(managed_npm_bin);
|
||||
managed.push(managed_npm_bin);
|
||||
}
|
||||
if let Some(managed_node_bin) = crate::managed_agents::buzz_managed_node_bin_dir() {
|
||||
parts.push(managed_node_bin);
|
||||
managed.push(managed_node_bin);
|
||||
}
|
||||
}
|
||||
if let Some(nvm_bin) = nvm_bin {
|
||||
parts.push(nvm_bin);
|
||||
managed.push(nvm_bin);
|
||||
}
|
||||
if let Some(parent) = exe_parent {
|
||||
parts.push(parent);
|
||||
}
|
||||
if let Some(shell_path) = shell_path {
|
||||
parts.extend(std::env::split_paths(&shell_path));
|
||||
managed.push(parent);
|
||||
}
|
||||
|
||||
// Split the login-shell PATH into individual entries.
|
||||
let had_shell_path = shell_path.is_some();
|
||||
let login: Vec<PathBuf> = shell_path
|
||||
.as_deref()
|
||||
.map(|s| std::env::split_paths(s).collect())
|
||||
.unwrap_or_default();
|
||||
|
||||
let inherited: Vec<PathBuf> = std::env::var_os("PATH")
|
||||
.map(|p| std::env::split_paths(&p).collect())
|
||||
.unwrap_or_default();
|
||||
let use_inherited = should_use_inherited(had_shell_path, has_local_context, cfg!(windows));
|
||||
|
||||
let parts = compose_path_entries(managed, login, inherited, use_inherited);
|
||||
if parts.is_empty() {
|
||||
return None;
|
||||
}
|
||||
@@ -134,4 +233,374 @@ mod tests {
|
||||
assert!(result.starts_with("/home/user/.local/bin:"), "{result}");
|
||||
assert!(result.ends_with(":/usr/local/bin"), "{result}");
|
||||
}
|
||||
|
||||
/// On Unix, supplying a `shell_path` must NOT trigger the Windows process-PATH
|
||||
/// fallback — the output must be byte-identical to what it was before this
|
||||
/// fix.
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn unix_shell_path_output_unchanged_by_windows_fallback_logic() {
|
||||
let result = build_augmented_path(
|
||||
Some(PathBuf::from("/home/user")),
|
||||
None,
|
||||
Some("/usr/local/bin:/usr/bin:/bin".to_string()),
|
||||
None,
|
||||
);
|
||||
let result = result.expect("path");
|
||||
assert!(
|
||||
result.ends_with(":/usr/local/bin:/usr/bin:/bin"),
|
||||
"Unix output must not append process PATH: {result}"
|
||||
);
|
||||
}
|
||||
|
||||
/// On Windows: when no login-shell PATH is available, `build_augmented_path`
|
||||
/// must append the inherited process PATH so node/npm remain visible.
|
||||
#[cfg(windows)]
|
||||
#[test]
|
||||
fn windows_appends_process_path_when_no_shell_path() {
|
||||
let _guard = crate::managed_agents::lock_path_mutex();
|
||||
let previous = std::env::var_os("PATH");
|
||||
std::env::set_var("PATH", r"C:\Program Files\nodejs");
|
||||
|
||||
let result = build_augmented_path(Some(PathBuf::from(r"C:\Users\agent")), None, None, None);
|
||||
|
||||
match previous {
|
||||
Some(value) => std::env::set_var("PATH", value),
|
||||
None => std::env::remove_var("PATH"),
|
||||
}
|
||||
|
||||
let result = result.expect("path must not be None with a home dir");
|
||||
assert!(
|
||||
result.starts_with(r"C:\Users\agent\.local\bin;"),
|
||||
"home/.local/bin must be first: {result}"
|
||||
);
|
||||
assert!(
|
||||
result.ends_with(r";C:\Program Files\nodejs"),
|
||||
"process PATH must be last: {result}"
|
||||
);
|
||||
}
|
||||
|
||||
/// On Windows: when a login-shell PATH IS supplied, the process PATH must
|
||||
/// NOT also be appended.
|
||||
#[cfg(windows)]
|
||||
#[test]
|
||||
fn windows_does_not_append_process_path_when_shell_path_present() {
|
||||
let _guard = crate::managed_agents::lock_path_mutex();
|
||||
let previous = std::env::var_os("PATH");
|
||||
std::env::set_var("PATH", r"C:\ShouldNotAppear");
|
||||
|
||||
let result = build_augmented_path(
|
||||
Some(PathBuf::from(r"C:\Users\agent")),
|
||||
None,
|
||||
Some(r"C:\Program Files\nodejs".to_string()),
|
||||
None,
|
||||
);
|
||||
|
||||
match previous {
|
||||
Some(value) => std::env::set_var("PATH", value),
|
||||
None => std::env::remove_var("PATH"),
|
||||
}
|
||||
|
||||
let result = result.expect("path");
|
||||
assert!(
|
||||
!result.contains("ShouldNotAppear"),
|
||||
"process PATH must not be appended when shell_path is present: {result}"
|
||||
);
|
||||
}
|
||||
|
||||
/// On Windows: when no local context is provided, the function must return
|
||||
/// None even if the process PATH is set.
|
||||
#[cfg(windows)]
|
||||
#[test]
|
||||
fn windows_no_process_path_without_local_context() {
|
||||
let _guard = crate::managed_agents::lock_path_mutex();
|
||||
let previous = std::env::var_os("PATH");
|
||||
std::env::set_var("PATH", r"C:\Windows\System32");
|
||||
|
||||
let result = build_augmented_path(None, None, None, None);
|
||||
|
||||
match previous {
|
||||
Some(value) => std::env::set_var("PATH", value),
|
||||
None => std::env::remove_var("PATH"),
|
||||
}
|
||||
|
||||
assert_eq!(
|
||||
result, None,
|
||||
"must return None when no local context and no shell_path"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// ── Pure policy and composition tests — run on every host ────────────────────
|
||||
//
|
||||
// These test `should_use_inherited` and `compose_path_entries` with explicit
|
||||
// inputs, so they run on macOS/Linux CI and validate the Windows policy
|
||||
// behavior without touching process state or requiring a Windows target.
|
||||
#[cfg(test)]
|
||||
mod compose_tests {
|
||||
use super::{compose_path_entries, is_batch_shim, should_use_inherited};
|
||||
use std::path::{Path, PathBuf};
|
||||
|
||||
fn p(s: &str) -> PathBuf {
|
||||
PathBuf::from(s)
|
||||
}
|
||||
|
||||
// ── should_use_inherited policy matrix ────────────────────────────────────
|
||||
|
||||
/// Windows + no shell path + has local context → must use inherited.
|
||||
#[test]
|
||||
fn policy_windows_no_shell_with_context_uses_inherited() {
|
||||
assert!(
|
||||
should_use_inherited(false, true, true),
|
||||
"Windows, no shell path, has context → must append inherited"
|
||||
);
|
||||
}
|
||||
|
||||
/// Windows + shell path present → must NOT use inherited (login path covers it).
|
||||
#[test]
|
||||
fn policy_windows_shell_path_present_suppresses_inherited() {
|
||||
assert!(
|
||||
!should_use_inherited(true, true, true),
|
||||
"Windows, shell path present → must not append inherited"
|
||||
);
|
||||
}
|
||||
|
||||
/// Windows + no local context → must NOT use inherited (no ambient state).
|
||||
#[test]
|
||||
fn policy_windows_no_local_context_suppresses_inherited() {
|
||||
assert!(
|
||||
!should_use_inherited(false, false, true),
|
||||
"Windows, no local context → must not append inherited"
|
||||
);
|
||||
}
|
||||
|
||||
/// Non-Windows → never use inherited, regardless of other flags.
|
||||
#[test]
|
||||
fn policy_non_windows_never_uses_inherited() {
|
||||
assert!(
|
||||
!should_use_inherited(false, true, false),
|
||||
"non-Windows must never append inherited PATH"
|
||||
);
|
||||
assert!(
|
||||
!should_use_inherited(false, false, false),
|
||||
"non-Windows + no context must never append inherited PATH"
|
||||
);
|
||||
}
|
||||
|
||||
// ── compose_path_entries ordering ─────────────────────────────────────────
|
||||
|
||||
#[test]
|
||||
fn managed_entries_appear_first() {
|
||||
let managed = vec![p("/buzz/node/bin"), p("/buzz/npm/bin")];
|
||||
let login = vec![p("/usr/local/bin"), p("/usr/bin")];
|
||||
let result = compose_path_entries(managed, login, vec![], false);
|
||||
assert_eq!(result[0], p("/buzz/node/bin"), "managed[0] must be first");
|
||||
assert_eq!(result[1], p("/buzz/npm/bin"), "managed[1] must be second");
|
||||
assert_eq!(
|
||||
result[2],
|
||||
p("/usr/local/bin"),
|
||||
"login[0] must follow managed"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn login_path_suppresses_inherited_when_use_inherited_false() {
|
||||
let login = vec![p("/usr/local/bin")];
|
||||
let inherited = vec![p("/should/not/appear")];
|
||||
let result = compose_path_entries(vec![], login, inherited, false);
|
||||
assert!(
|
||||
!result.contains(&p("/should/not/appear")),
|
||||
"inherited must not appear when use_inherited=false"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn inherited_appended_last_when_use_inherited_true() {
|
||||
let managed = vec![p("/buzz/npm/bin")];
|
||||
let inherited = vec![p("C:/windows/node"), p("C:/windows/npm")];
|
||||
let result = compose_path_entries(managed, vec![], inherited.clone(), true);
|
||||
assert_eq!(result[0], p("/buzz/npm/bin"), "managed must be first");
|
||||
assert_eq!(
|
||||
&result[1..],
|
||||
&inherited[..],
|
||||
"inherited entries must be appended last"
|
||||
);
|
||||
}
|
||||
|
||||
/// Windows policy ON + empty inherited PATH — should produce just managed
|
||||
/// entries, not None and not a phantom segment.
|
||||
#[test]
|
||||
fn windows_policy_on_empty_inherited_produces_managed_only() {
|
||||
let managed = vec![p("/buzz/npm/bin")];
|
||||
let result = compose_path_entries(managed.clone(), vec![], vec![], true);
|
||||
assert_eq!(
|
||||
result, managed,
|
||||
"empty inherited must not add phantom entries"
|
||||
);
|
||||
}
|
||||
|
||||
/// Windows policy ON + unset/absent inherited (empty vec from var_os None) —
|
||||
/// same result as above; no crash, no phantom.
|
||||
#[test]
|
||||
fn windows_policy_on_unset_inherited_path_produces_managed_only() {
|
||||
// Simulates std::env::var_os("PATH") returning None → empty vec.
|
||||
let managed = vec![p("/buzz/npm/bin")];
|
||||
let inherited: Vec<PathBuf> = vec![]; // empty, as if PATH is unset
|
||||
let result = compose_path_entries(managed.clone(), vec![], inherited, true);
|
||||
assert_eq!(result, managed);
|
||||
}
|
||||
|
||||
/// No local context + Windows policy ON — compose_path_entries itself still
|
||||
/// works (no crash), and the caller is responsible for not calling it.
|
||||
/// Specifically: all-empty inputs with use_inherited=true still returns empty.
|
||||
#[test]
|
||||
fn all_empty_with_use_inherited_true_returns_empty() {
|
||||
let result = compose_path_entries(vec![], vec![], vec![], true);
|
||||
assert!(
|
||||
result.is_empty(),
|
||||
"all-empty inputs must produce empty output"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn empty_all_inputs_use_inherited_false_returns_empty() {
|
||||
let result = compose_path_entries(vec![], vec![], vec![], false);
|
||||
assert!(
|
||||
result.is_empty(),
|
||||
"all-empty inputs must produce empty output"
|
||||
);
|
||||
}
|
||||
|
||||
/// Non-Windows behavior: `use_inherited=false` must produce byte-identical
|
||||
/// output to before this fix. Inherited entries are collected but dropped.
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn unix_use_inherited_false_output_unchanged() {
|
||||
let managed = vec![p("/buzz/npm/bin")];
|
||||
let login = vec![p("/usr/local/bin"), p("/usr/bin"), p("/bin")];
|
||||
let inherited = vec![p("/proc/ambient/PATH")]; // would be real proc PATH on Unix
|
||||
let result = compose_path_entries(managed, login, inherited, false);
|
||||
assert_eq!(
|
||||
result,
|
||||
vec![
|
||||
p("/buzz/npm/bin"),
|
||||
p("/usr/local/bin"),
|
||||
p("/usr/bin"),
|
||||
p("/bin")
|
||||
],
|
||||
"Unix output must not include inherited entries when use_inherited=false"
|
||||
);
|
||||
}
|
||||
|
||||
// ── Structural wrapper-alignment test ──────────────────────────────────────
|
||||
//
|
||||
// Verifies that both `build_augmented_path` and `install_shell_command`
|
||||
// compute the same `should_use_inherited` decision for equivalent inputs.
|
||||
// Tests the policy function directly to confirm the wrappers can't drift.
|
||||
|
||||
/// Exhaustive truth-table for `should_use_inherited` — all four input
|
||||
/// combinations that affect real callers. Confirms the policy is correct
|
||||
/// before either wrapper binds to it.
|
||||
#[test]
|
||||
fn should_use_inherited_policy_truth_table() {
|
||||
// (had_shell, has_context, is_windows) → expected
|
||||
let cases = [
|
||||
(false, true, true, true), // Windows, no shell, context → USE
|
||||
(true, true, true, false), // Windows, shell present → NO
|
||||
(false, false, true, false), // Windows, no context → NO
|
||||
(false, true, false, false), // non-Windows → NO
|
||||
];
|
||||
for (had_shell, has_ctx, is_win, expected) in cases {
|
||||
let result = should_use_inherited(had_shell, has_ctx, is_win);
|
||||
assert_eq!(
|
||||
result, expected,
|
||||
"policy mismatch: had_shell={had_shell} has_ctx={has_ctx} is_win={is_win}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// ── is_batch_shim extension tests ─────────────────────────────────────────
|
||||
|
||||
#[test]
|
||||
fn batch_shim_cmd_lower() {
|
||||
assert!(is_batch_shim(Path::new("claude.cmd")));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn batch_shim_cmd_upper() {
|
||||
assert!(is_batch_shim(Path::new("claude.CMD")));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn batch_shim_bat_lower() {
|
||||
assert!(is_batch_shim(Path::new("claude.bat")));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn batch_shim_bat_upper() {
|
||||
assert!(is_batch_shim(Path::new("claude.BAT")));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn batch_shim_exe_not_shim() {
|
||||
assert!(!is_batch_shim(Path::new("claude.exe")));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn batch_shim_no_extension_not_shim() {
|
||||
assert!(!is_batch_shim(Path::new("claude")));
|
||||
}
|
||||
|
||||
// ── should_skip_claude_executable policy tests ────────────────────────────
|
||||
//
|
||||
// Cross-host policy: shim + Windows → skip; shim + non-Windows → assign;
|
||||
// non-shim either OS → assign. Mirrors the `should_use_inherited` pattern.
|
||||
|
||||
#[test]
|
||||
fn skip_claude_executable_shim_windows_returns_true() {
|
||||
assert!(
|
||||
super::should_skip_claude_executable(Path::new("claude.cmd"), true),
|
||||
"shim + windows=true must skip"
|
||||
);
|
||||
assert!(
|
||||
super::should_skip_claude_executable(Path::new("claude.BAT"), true),
|
||||
"shim + windows=true must skip"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn skip_claude_executable_shim_non_windows_returns_false() {
|
||||
assert!(
|
||||
!super::should_skip_claude_executable(Path::new("claude.cmd"), false),
|
||||
"shim + windows=false must NOT skip (valid executable on non-Windows)"
|
||||
);
|
||||
assert!(
|
||||
!super::should_skip_claude_executable(Path::new("claude.bat"), false),
|
||||
"shim + windows=false must NOT skip"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn skip_claude_executable_exe_both_platforms_returns_false() {
|
||||
assert!(
|
||||
!super::should_skip_claude_executable(Path::new("claude.exe"), true),
|
||||
"non-shim + windows=true must NOT skip"
|
||||
);
|
||||
assert!(
|
||||
!super::should_skip_claude_executable(Path::new("claude.exe"), false),
|
||||
"non-shim + windows=false must NOT skip"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn skip_claude_executable_no_ext_both_platforms_returns_false() {
|
||||
assert!(
|
||||
!super::should_skip_claude_executable(Path::new("claude"), true),
|
||||
"no-ext + windows=true must NOT skip"
|
||||
);
|
||||
assert!(
|
||||
!super::should_skip_claude_executable(Path::new("claude"), false),
|
||||
"no-ext + windows=false must NOT skip"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -618,6 +618,63 @@ fn codex_spawn_does_not_set_a_claude_executable() {
|
||||
.any(|(key, _)| key == "CLAUDE_CODE_EXECUTABLE"));
|
||||
}
|
||||
|
||||
/// On Windows, `.cmd` and `.bat` batch shims must NOT be assigned to
|
||||
/// `CLAUDE_CODE_EXECUTABLE` — `CreateProcess` cannot exec them directly and
|
||||
/// returns EINVAL (issue #2397). The adapter must fall back to its own PATH
|
||||
/// lookup instead.
|
||||
///
|
||||
/// These tests exercise `is_batch_shim` directly — a pure path predicate with
|
||||
/// no global PATH or resolve_command cache involvement — so they run on every
|
||||
/// host and cannot be poisoned by the `claude_spawn_uses_the_probed_cli_executable`
|
||||
/// test that runs before them.
|
||||
#[test]
|
||||
fn batch_shim_cmd_extension_is_rejected() {
|
||||
assert!(
|
||||
super::path::is_batch_shim(std::path::Path::new("claude.cmd")),
|
||||
"claude.cmd must be identified as a batch shim"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn batch_shim_cmd_extension_uppercase_is_rejected() {
|
||||
assert!(
|
||||
super::path::is_batch_shim(std::path::Path::new("claude.CMD")),
|
||||
"claude.CMD must be identified as a batch shim (case-insensitive)"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn batch_shim_bat_extension_is_rejected() {
|
||||
assert!(
|
||||
super::path::is_batch_shim(std::path::Path::new("claude.bat")),
|
||||
"claude.bat must be identified as a batch shim"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn batch_shim_bat_extension_uppercase_is_rejected() {
|
||||
assert!(
|
||||
super::path::is_batch_shim(std::path::Path::new("claude.BAT")),
|
||||
"claude.BAT must be identified as a batch shim (case-insensitive)"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn batch_shim_exe_extension_is_not_rejected() {
|
||||
assert!(
|
||||
!super::path::is_batch_shim(std::path::Path::new("claude.exe")),
|
||||
"claude.exe must not be identified as a batch shim"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn batch_shim_no_extension_is_not_rejected() {
|
||||
assert!(
|
||||
!super::path::is_batch_shim(std::path::Path::new("claude")),
|
||||
"claude (no extension) must not be identified as a batch shim"
|
||||
);
|
||||
}
|
||||
|
||||
// ── PGID-based orphan sweep tests ───────────────────────────────────────
|
||||
|
||||
/// Validates the kernel invariant that the orphan sweep PGID fix relies on:
|
||||
|
||||
Reference in New Issue
Block a user