diff --git a/crates/ai/src/ssh.rs b/crates/ai/src/ssh.rs index 41fbcd9d..086d91fe 100644 --- a/crates/ai/src/ssh.rs +++ b/crates/ai/src/ssh.rs @@ -81,24 +81,56 @@ fn includes(text: &str) -> Vec { .collect() } -/// Concrete `Host` names in an ssh config text. +/// Servers reached over SSH only to push and pull code. An alias for one (`github.com-work`, to +/// use a second account's key) is no computer to run an agent on. +const GIT_HOSTS: &[&str] = &[ + "github.com", + "ssh.github.com", + "gitlab.com", + "altssh.gitlab.com", + "bitbucket.org", + "altssh.bitbucket.org", + "codeberg.org", + "git.sr.ht", + "ssh.dev.azure.com", + "vs-ssh.visualstudio.com", +]; + +/// `name` is a known git server, or an alias made from one (`github.com-work`, `GitHub.com.alt`). +fn is_git_host(name: &str) -> bool { + let name = name.to_ascii_lowercase(); + GIT_HOSTS.iter().any(|g| name.strip_prefix(g).is_some_and(|rest| rest.is_empty() || rest.starts_with(['-', '.', '_']))) +} + +/// Concrete `Host` names in an ssh config text, without git servers: a host named after one, or +/// whose block has a git server's `HostName` or `User git`. pub fn parse_hosts(text: &str) -> Vec { - let mut out = Vec::new(); + // One entry per `Host` (or `Match`) line: its names, and whether it points at a git server. + let mut blocks: Vec<(Vec, bool)> = Vec::new(); for line in text.lines() { let l = line.trim(); if l.starts_with('#') { continue; } let Some((k, v)) = l.split_once(|c: char| c.is_whitespace() || c == '=') else { continue }; - if !k.eq_ignore_ascii_case("host") { - continue; + let v = v.trim_start().trim_start_matches('=').trim(); + if k.eq_ignore_ascii_case("host") { + let names: Vec = + v.split_whitespace().map(|n| n.trim_matches('"')).filter(|n| !n.is_empty() && !n.contains(['*', '?', '!'])).map(str::to_string).collect(); + let git = names.iter().any(|n| is_git_host(n)); + blocks.push((names, git)); + } else if k.eq_ignore_ascii_case("match") { + blocks.push((Vec::new(), false)); + } else if let Some((_, git)) = blocks.last_mut() { + let v = v.trim_matches('"'); + *git |= (k.eq_ignore_ascii_case("hostname") && is_git_host(v)) || (k.eq_ignore_ascii_case("user") && v == "git"); } - for name in v.split_whitespace() { - let name = name.trim_matches('"'); - if name.is_empty() || name.contains(['*', '?', '!']) || out.iter().any(|h: &String| h == name) { - continue; - } - out.push(name.to_string()); + } + let git: Vec<&String> = blocks.iter().filter(|b| b.1).flat_map(|b| &b.0).collect(); + let mut out: Vec = Vec::new(); + for name in blocks.iter().filter(|b| !b.1).flat_map(|b| &b.0) { + if !git.contains(&name) && !out.contains(name) { + out.push(name.clone()); } } out @@ -169,6 +201,14 @@ mod tests { assert_eq!(includes("Include config.d/*\ninclude a b\n"), vec!["config.d/*", "a", "b"]); } + #[test] + fn git_servers_are_not_hosts() { + let cfg = "Host github.com-second\n HostName github.com\n IdentityFile ~/.ssh/k\nHost work-gh\n HostName github.com\n\ + Host forge\n HostName git.example.org\n User git\nHost GitHub.com.alt gitlab.com\nHost box\n HostName 10.0.0.3\n User git2\n\ + Host = spaced\nHost github.community\nHost work-gh\n"; + assert_eq!(parse_hosts(cfg), vec!["box", "spaced", "github.community"]); + } + #[test] fn includes_are_followed() { let dir = tempfile::tempdir().unwrap(); diff --git a/crates/engines/src/lib.rs b/crates/engines/src/lib.rs index e653f8c4..0a04326a 100644 --- a/crates/engines/src/lib.rs +++ b/crates/engines/src/lib.rs @@ -79,7 +79,7 @@ pub fn well_known(id: &str) -> Vec { } pub(crate) fn find_on_path(exes: &[&str]) -> Option { - let path = std::env::var_os("PATH")?; + let path = craft_anything_platform::which::search_path()?; for dir in std::env::split_paths(&path) { for exe in exes { let p = dir.join(exe); diff --git a/crates/platform/src/process.rs b/crates/platform/src/process.rs index 76fee792..e099ea93 100644 --- a/crates/platform/src/process.rs +++ b/crates/platform/src/process.rs @@ -272,6 +272,8 @@ impl Process { cmd.args(&self.args); if self.env_clear { cmd.env_clear(); + } else { + use_search_path(cmd); } for (k, v) in &self.env { match v { @@ -354,6 +356,14 @@ impl Process { } } +/// The child gets [`search_path`](crate::which::search_path), which is also where a bare program +/// name is looked up on Unix, so an app opened from the Dock starts what a shell would. +fn use_search_path(cmd: &mut std::process::Command) { + if let Some(p) = crate::which::search_path() { + cmd.env("PATH", p); + } +} + fn spawn_error(name: &str, program: &OsStr, e: std::io::Error, remediation: Option<&str>) -> ToolError { match e.kind() { std::io::ErrorKind::NotFound => ToolError::new(ErrorCode::EngineMissing, format!("{name} isn't installed or isn't on your PATH.")) @@ -764,6 +774,7 @@ impl Launch { let name = self.display(); let mut cmd = std::process::Command::new(&self.program); cmd.args(&self.args).stdin(Stdio::null()); + use_search_path(&mut cmd); match &self.log { Some(path) => { let out = std::fs::File::create(path).map_err(|e| ToolError::io(path.display(), e))?; diff --git a/crates/platform/src/which.rs b/crates/platform/src/which.rs index f215f650..6fba4131 100644 --- a/crates/platform/src/which.rs +++ b/crates/platform/src/which.rs @@ -8,9 +8,42 @@ pub fn which(name: &str) -> Option { which_all(name).into_iter().next() } -/// Every match for `name` on `PATH`, in order (useful to spot duplicate installs). +/// Every match for `name` on [`search_path`], in order (useful to spot duplicate installs). pub fn which_all(name: &str) -> Vec { - which_in(name, std::env::var_os("PATH")) + which_in(name, search_path()) +} + +/// Folders, under the home folder and then system-wide, where coding agents, npm, Homebrew and +/// other installers put command-line programs. +#[cfg(unix)] +const USUAL_HOME_DIRS: &[&str] = &[".local/bin", ".claude/local", ".npm-global/bin", ".bun/bin", ".volta/bin", ".cargo/bin"]; +#[cfg(unix)] +const USUAL_DIRS: &[&str] = &["/opt/homebrew/bin", "/usr/local/bin"]; +#[cfg(not(unix))] +const USUAL_HOME_DIRS: &[&str] = &[]; +#[cfg(not(unix))] +const USUAL_DIRS: &[&str] = &[]; + +/// `PATH` plus the usual folders for command-line programs, after it so the person's own order +/// wins. An app opened from the Dock or a desktop launcher gets the system's short `PATH`, not the +/// shell's, and would miss a coding agent installed in `~/.local/bin`. Unchanged on Windows, where +/// apps get the full `PATH`. +pub fn search_path() -> Option { + with_usual_dirs(std::env::var_os("PATH"), std::env::var_os("HOME").map(PathBuf::from)) +} + +fn with_usual_dirs(path: Option, home: Option) -> Option { + let mut dirs: Vec = path.as_ref().map(|p| std::env::split_paths(p).collect()).unwrap_or_default(); + let before = dirs.len(); + for d in home.iter().flat_map(|h| USUAL_HOME_DIRS.iter().map(move |d| h.join(d))).chain(USUAL_DIRS.iter().map(PathBuf::from)) { + if !dirs.contains(&d) { + dirs.push(d); + } + } + if dirs.len() == before { + return path; + } + std::env::join_paths(&dirs).ok().or(path) } /// Like [`which_all`] with an explicit search path. @@ -116,6 +149,26 @@ mod tests { assert!(which("").is_none()); } + #[cfg(unix)] + #[test] + fn usual_dirs_come_after_path() { + let home = PathBuf::from("/home/u"); + let path = std::env::join_paths([PathBuf::from("/usr/bin"), home.join(".local/bin")]).unwrap(); + let dirs: Vec = std::env::split_paths(&with_usual_dirs(Some(path), Some(home.clone())).unwrap()).collect(); + assert_eq!(dirs[..3], [PathBuf::from("/usr/bin"), home.join(".local/bin"), home.join(".claude/local")]); + assert_eq!(dirs.len(), 1 + USUAL_HOME_DIRS.len() + USUAL_DIRS.len(), "nothing twice"); + assert!(dirs.contains(&PathBuf::from("/opt/homebrew/bin"))); + // No PATH and no home folder: the system-wide folders still. + assert_eq!(with_usual_dirs(None, None), std::env::join_paths(USUAL_DIRS).ok()); + } + + #[cfg(not(unix))] + #[test] + fn path_is_unchanged_off_unix() { + let path = OsString::from(r"C:\Windows;C:\tools"); + assert_eq!(with_usual_dirs(Some(path.clone()), Some(PathBuf::from(r"C:\Users\u"))), Some(path)); + } + #[cfg(unix)] #[test] fn explicit_search_path() { diff --git a/docs/pitfalls.md b/docs/pitfalls.md index cb80c5a3..9a1b2b0a 100644 --- a/docs/pitfalls.md +++ b/docs/pitfalls.md @@ -125,6 +125,12 @@ real Deep-mode run (the CLI never shows it: it already has a console). Every chi the windowed app gets `CREATE_NO_WINDOW`; check the desktop app, not only the CLI, after adding a program. +**A Mac app that couldn't find Claude Code, and offered a GitHub key instead.** An app opened from +the Dock gets the system's short `PATH`, not the shell's, so `claude` in `~/.local/bin` wasn't +found; "Built by" then fell back to "Claude Code on github.com-work", an `~/.ssh/config` alias +for a second GitHub key, read as a computer to build on. Look programs up and start them with +`which::search_path` (`Process` and `Launch` do), and leave git servers out of SSH hosts. + **A first analysis that waited for a lock it could never take.** Ghidra's analysis lock is created with `create_new` in `ghidra-cache/`, which only existed on machines that had used it before; on a fresh store the create failed and the loop read that as "someone else holds it" until the