From 94d209c99ca67c347b2d9f3bd375460d787a233e Mon Sep 17 00:00:00 2001 From: arookieofc Date: Thu, 29 Oct 2026 23:00:01 +0800 Subject: [PATCH] fix(agents): resolve agent commands through PATH x PATHEXT on Windows (#273) On Windows std/tokio Command only tries .exe for a bare program name, so agents installed as npm shims (codex.cmd, claude.cmd) failed with "program not found". Bare names are now resolved through PATH x PATHEXT before spawning. - ide-core::process gains find_executable / find_executable_in (moved from ide-run::which, which re-exports them). - ide-jsonrpc Peer::spawn_framed resolves against the child's PATH (caller env, login shell, then ours): covers ACP agents and the native Codex driver. - ide-agent newest_install builds candidates with executable_names, so the native Claude driver finds .cmd shims and multi-install selection works on Windows. - ide-agent replay recorder resolves the same way. Co-authored-by: a-rookie-of-C-language <186315048+a-rookie-of-C-language@users.noreply.github.com> --- crates/ide-agent/src/native/mod.rs | 69 ++++++++++++++++++++++-------- crates/ide-agent/src/replay.rs | 18 +++++++- crates/ide-core/src/process.rs | 38 ++++++++++++++++ crates/ide-jsonrpc/src/lib.rs | 24 ++++++++++- crates/ide-run/src/which.rs | 19 +------- 5 files changed, 129 insertions(+), 39 deletions(-) diff --git a/crates/ide-agent/src/native/mod.rs b/crates/ide-agent/src/native/mod.rs index 5f1983803..46de082e1 100644 --- a/crates/ide-agent/src/native/mod.rs +++ b/crates/ide-agent/src/native/mod.rs @@ -21,18 +21,26 @@ use std::sync::Arc; /// The newest install of `command` on the `PATH` a child sees, by its `--version`. /// Several installs are common (npm, Homebrew, the vendor's own installer), and the first /// on `PATH` is often an old one that the service no longer serves the current models to. -/// A path, or a name found once (or not at all), is used as it is. +/// A path, or a name not found on `PATH`, is used as it is; a single match returns its +/// resolved path so Windows `PATHEXT` shims (`claude.cmd`, `codex.cmd`) start. pub(crate) fn newest_install(command: &str) -> String { - if Path::new(command).components().count() > 1 { + let path = ide_core::process::host_var_os("PATH") + .or_else(|| ide_core::process::shell_env().find(|(k, _)| *k == "PATH").map(|(_, v)| v.to_owned())) + .unwrap_or_default(); + newest_install_in(command, &path) +} + +fn newest_install_in(command: &str, search: &std::ffi::OsStr) -> String { + if Path::new(command).components().count() > 1 || Path::new(command).is_absolute() { return command.to_owned(); } - let path = ide_core::process::host_var_os("PATH") - .or_else(|| ide_core::process::shell_env().find(|(k, _)| *k == "PATH").map(|(_, v)| v.to_owned())); + let pathext = ide_core::process::host_var_os("PATHEXT"); + let names = + ide_core::process::executable_names(command, cfg!(windows), pathext.as_deref().and_then(|p| p.to_str())); let mut seen = Vec::new(); - let candidates: Vec = path - .iter() - .flat_map(std::env::split_paths) - .map(|dir| dir.join(command)) + let candidates: Vec = std::env::split_paths(search) + .filter(|d| !d.as_os_str().is_empty()) + .flat_map(|dir| names.iter().map(move |n| dir.join(n))) .filter(|p| p.is_file()) .filter(|p| { let real = dunce::canonicalize(p).unwrap_or_else(|_| p.clone()); @@ -42,7 +50,7 @@ pub(crate) fn newest_install(command: &str) -> String { }) .collect(); if candidates.len() < 2 { - return command.to_owned(); + return candidates.into_iter().next().map_or_else(|| command.to_owned(), |p| p.to_string_lossy().into_owned()); } candidates .into_iter() @@ -66,16 +74,6 @@ fn parse_version(text: &str) -> Option> { }) } -#[cfg(test)] -#[test] -fn versions_are_read_from_either_side_of_the_name() { - assert_eq!(parse_version("codex-cli 0.164.0\n"), Some(vec![0, 164, 0])); - assert_eq!(parse_version("2.1.312 (Claude Code)"), Some(vec![2, 1, 312])); - assert_eq!(parse_version("v1.2"), Some(vec![1, 2])); - assert_eq!(parse_version("no version here"), None); - assert!(parse_version("codex-cli 0.164.0") > parse_version("codex-cli 0.147.0")); -} - /// The agent side of an in-process ACP connection. pub(crate) trait Shim: Handler { /// Called once, before any message arrives: the connection to the client, for @@ -94,3 +92,36 @@ pub(crate) fn serve(shim: Arc, client: Arc) -> Peer { shim.attach(to_client); Peer::connect_framed(or, ow, client, Framing::Lines) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn versions_are_read_from_either_side_of_the_name() { + assert_eq!(parse_version("codex-cli 0.164.0\n"), Some(vec![0, 164, 0])); + assert_eq!(parse_version("2.1.312 (Claude Code)"), Some(vec![2, 1, 312])); + assert_eq!(parse_version("v1.2"), Some(vec![1, 2])); + assert_eq!(parse_version("no version here"), None); + assert!(parse_version("codex-cli 0.164.0") > parse_version("codex-cli 0.147.0")); + } + + #[test] + fn newest_install_resolves_single_candidate_and_keeps_missing_or_paths() { + let dir = tempfile::tempdir().unwrap(); + let a = dir.path().join("a"); + let b = dir.path().join("b"); + std::fs::create_dir_all(&a).unwrap(); + std::fs::create_dir_all(&b).unwrap(); + std::fs::write(a.join("claude"), "#!/bin/sh\n").unwrap(); + std::fs::write(b.join("claude.cmd"), "@echo off\n").unwrap(); + let search = std::env::join_paths([&a, &b]).unwrap(); + if cfg!(windows) { + assert_eq!(newest_install_in("claude", &search), b.join("claude.cmd").to_string_lossy()); + } else { + assert_eq!(newest_install_in("claude", &search), a.join("claude").to_string_lossy()); + } + assert_eq!(newest_install_in("nope", &search), "nope"); + assert_eq!(newest_install_in("bin/claude", &search), "bin/claude"); + } +} diff --git a/crates/ide-agent/src/replay.rs b/crates/ide-agent/src/replay.rs index da2410837..170fc75a9 100644 --- a/crates/ide-agent/src/replay.rs +++ b/crates/ide-agent/src/replay.rs @@ -185,7 +185,23 @@ pub(crate) fn recorder(out: PathBuf) -> Connector { let file = Arc::new(Mutex::new(String::new())); let connections = Arc::new(Mutex::new(0u32)); Arc::new(move |spec, cwd, client, on_stderr| { - let mut cmd = tokio::process::Command::new(&spec.command); + let program = { + let path = Path::new(&spec.command); + if path.components().count() > 1 || path.is_absolute() { + path.to_path_buf() + } else { + let search = spec + .env + .iter() + .find(|(k, _)| k.eq_ignore_ascii_case("PATH")) + .map(|(_, v)| std::ffi::OsString::from(v)) + .or_else(|| ide_core::process::shell_env().find(|(k, _)| k == "PATH").map(|(_, v)| v)) + .or_else(|| ide_core::process::host_var_os("PATH")) + .unwrap_or_default(); + ide_core::process::find_executable(&spec.command, &search).unwrap_or_else(|| path.to_path_buf()) + } + }; + let mut cmd = tokio::process::Command::new(&program); ide_core::process::prepare(cmd.as_std_mut()); let mut child = cmd .args(&spec.args) diff --git a/crates/ide-core/src/process.rs b/crates/ide-core/src/process.rs index 9f789d857..f06ea74ff 100644 --- a/crates/ide-core/src/process.rs +++ b/crates/ide-core/src/process.rs @@ -218,6 +218,25 @@ pub fn executable_names(name: &str, windows: bool, pathext: Option<&str>) -> Vec exts.iter().map(|e| format!("{name}{}", e.to_ascii_lowercase())).collect() } +/// First `search` directory holding `name` (see [`executable_names`]). +pub fn find_executable_in( + name: &str, + search: &OsStr, + windows: bool, + pathext: Option<&str>, +) -> Option { + let names = executable_names(name, windows, pathext); + std::env::split_paths(search) + .filter(|d| !d.as_os_str().is_empty()) + .find_map(|dir| names.iter().map(|n| dir.join(n)).find(|p| p.is_file())) +} + +/// [`find_executable_in`] for this platform (`PATHEXT` from the environment). +pub fn find_executable(name: &str, search: &OsStr) -> Option { + let pathext = host_var_os("PATHEXT"); + find_executable_in(name, search, cfg!(windows), pathext.as_deref().and_then(|p| p.to_str())) +} + #[cfg(test)] mod tests { use super::*; @@ -363,4 +382,23 @@ mod tests { prepare(&mut cmd); assert_eq!(cmd.get_envs().find(|(k, _)| *k == "PATH").and_then(|(_, v)| v), Some(OsStr::new("/custom"))); } + + #[test] + fn find_executable_finds_bare_names_on_the_given_path() { + let dir = tempfile::tempdir().unwrap(); + let a = dir.path().join("a"); + let b = dir.path().join("b"); + std::fs::create_dir_all(&a).unwrap(); + std::fs::create_dir_all(&b).unwrap(); + std::fs::write(a.join("agent"), "#!/bin/sh\n").unwrap(); + std::fs::write(b.join("agent.cmd"), "@echo off\n").unwrap(); + let search = std::env::join_paths([&a, &b]).unwrap(); + if cfg!(windows) { + // The `.cmd` shim wins over a POSIX script `Command` can't start. + assert_eq!(find_executable("agent", &search), Some(b.join("agent.cmd"))); + } else { + assert_eq!(find_executable("agent", &search), Some(a.join("agent"))); + } + assert_eq!(find_executable("nope", &search), None); + } } diff --git a/crates/ide-jsonrpc/src/lib.rs b/crates/ide-jsonrpc/src/lib.rs index a6ee0c7cf..b7af9fcfc 100644 --- a/crates/ide-jsonrpc/src/lib.rs +++ b/crates/ide-jsonrpc/src/lib.rs @@ -332,6 +332,25 @@ impl Drop for KillOnDrop { } } +/// The program [`Peer::spawn_framed`] runs: a bare name resolved against the +/// child's `PATH` (a caller's `env` PATH, else the login shell's, else ours), so +/// that on Windows `PATHEXT` names (`codex` → `codex.cmd`) start; anything else +/// is kept as given. +fn resolve_spawn_program(command: &str, env: &[(String, String)]) -> std::path::PathBuf { + let path = std::path::Path::new(command); + if path.components().count() > 1 || path.is_absolute() { + return path.to_path_buf(); + } + let search = env + .iter() + .find(|(k, _)| k.eq_ignore_ascii_case("PATH")) + .map(|(_, v)| std::ffi::OsString::from(v)) + .or_else(|| ide_core::process::shell_env().find(|(k, _)| k == "PATH").map(|(_, v)| v)) + .or_else(|| ide_core::process::host_var_os("PATH")) + .unwrap_or_default(); + ide_core::process::find_executable(command, &search).unwrap_or_else(|| path.to_path_buf()) +} + impl Peer { /// Spawns `command` and speaks JSON-RPC over its stdio. Stderr lines, and /// stdout lines that are not part of a message (a launcher's own output), are @@ -360,7 +379,10 @@ impl Peer { framing: Framing, ) -> std::io::Result { use std::process::Stdio; - let mut cmd = tokio::process::Command::new(command); + // Windows resolves a bare name only as `.exe`; find the file a child + // would (`PATH` × `PATHEXT`, e.g. `codex` → `codex.cmd`) first. + let resolved = resolve_spawn_program(command, env); + let mut cmd = tokio::process::Command::new(&resolved); ide_core::process::prepare(cmd.as_std_mut()); cmd.args(args) .current_dir(cwd) diff --git a/crates/ide-run/src/which.rs b/crates/ide-run/src/which.rs index 8e790cc1f..7fec242b5 100644 --- a/crates/ide-run/src/which.rs +++ b/crates/ide-run/src/which.rs @@ -2,24 +2,7 @@ //! `PATHEXT` (`kotlinc` → `kotlinc.bat`, `npx` → `npx.cmd`), which //! `std::process::Command` does not do (it only appends `.exe`). -use std::ffi::OsStr; -use std::path::PathBuf; - -pub use ide_core::process::executable_names; - -/// First `search` directory holding `name` (see [`executable_names`]). -pub fn find_executable_in(name: &str, search: &OsStr, windows: bool, pathext: Option<&str>) -> Option { - let names = executable_names(name, windows, pathext); - std::env::split_paths(search) - .filter(|d| !d.as_os_str().is_empty()) - .find_map(|dir| names.iter().map(|n| dir.join(n)).find(|p| p.is_file())) -} - -/// [`find_executable_in`] for this platform (`PATHEXT` from the environment). -pub fn find_executable(name: &str, search: &OsStr) -> Option { - let pathext = std::env::var("PATHEXT").ok(); - find_executable_in(name, search, cfg!(windows), pathext.as_deref()) -} +pub use ide_core::process::{executable_names, find_executable, find_executable_in}; #[cfg(test)] mod tests {