diff --git a/Cargo.lock b/Cargo.lock index 5a5c3ac6..73663321 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -456,6 +456,16 @@ dependencies = [ "unicode-segmentation", ] +[[package]] +name = "core-foundation" +version = "0.9.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "91e195e091a93c46f7102ec7818a2aa394e1e1771c3ab4825963fa03e45afb8f" +dependencies = [ + "core-foundation-sys", + "libc", +] + [[package]] name = "core-foundation" version = "0.10.1" @@ -598,7 +608,11 @@ dependencies = [ name = "doctor" version = "0.1.0" dependencies = [ + "dirs", + "futures", + "reqwest", "serde", + "serde_json", "tokio", ] @@ -620,6 +634,15 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "34aa73646ffb006b8f5147f3dc182bd4bcb190227ce861fc4a4844bf8e3cb2c0" +[[package]] +name = "encoding_rs" +version = "0.8.35" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "75030f3c4f45dafd7586dd6780965a8c7e8e285a5ecb86713e63a79c5b2766f3" +dependencies = [ + "cfg-if", +] + [[package]] name = "equivalent" version = "1.0.2" @@ -669,6 +692,12 @@ version = "0.1.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5baebc0774151f905a1a2cc41989300b1e6fbb29aff0ceffa1064fdd3088d582" +[[package]] +name = "fnv" +version = "1.0.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3f9eec918d3f24069decb9af1554cad7c880e2da24a9afd88aca000531ab82c1" + [[package]] name = "foldhash" version = "0.1.5" @@ -858,6 +887,25 @@ dependencies = [ "regex-syntax", ] +[[package]] +name = "h2" +version = "0.4.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "171fefbc92fe4a4de27e0698d6a5b392d6a0e333506bc49133760b3bcf948733" +dependencies = [ + "atomic-waker", + "bytes", + "fnv", + "futures-core", + "futures-sink", + "http", + "indexmap", + "slab", + "tokio", + "tokio-util", + "tracing", +] + [[package]] name = "hashbrown" version = "0.15.5" @@ -943,6 +991,7 @@ dependencies = [ "bytes", "futures-channel", "futures-core", + "h2", "http", "http-body", "httparse", @@ -987,9 +1036,11 @@ dependencies = [ "percent-encoding", "pin-project-lite", "socket2", + "system-configuration", "tokio", "tower-service", "tracing", + "windows-registry", ] [[package]] @@ -1771,7 +1822,11 @@ checksum = "ab3f43e3283ab1488b624b44b0e988d0acea0b3214e694730a055cb6b2efa801" dependencies = [ "base64", "bytes", + "encoding_rs", + "futures-channel", "futures-core", + "futures-util", + "h2", "http", "http-body", "http-body-util", @@ -1780,6 +1835,7 @@ dependencies = [ "hyper-util", "js-sys", "log", + "mime", "percent-encoding", "pin-project-lite", "quinn", @@ -1885,7 +1941,7 @@ version = "0.6.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1d99feebc72bae7ab76ba994bb5e121b8d83d910ca40b36e0921f53becc41784" dependencies = [ - "core-foundation", + "core-foundation 0.10.1", "core-foundation-sys", "jni", "log", @@ -1986,7 +2042,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b7f4bc775c73d9a02cde8bf7b2ec4c9d12743edf609006c7facc23998404cd1d" dependencies = [ "bitflags", - "core-foundation", + "core-foundation 0.10.1", "core-foundation-sys", "libc", "security-framework-sys", @@ -2233,6 +2289,27 @@ dependencies = [ "syn", ] +[[package]] +name = "system-configuration" +version = "0.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a13f3d0daba03132c0aa9767f98351b3488edc2c100cda2d2ec2b04f3d8d3c8b" +dependencies = [ + "bitflags", + "core-foundation 0.9.4", + "system-configuration-sys", +] + +[[package]] +name = "system-configuration-sys" +version = "0.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8e1d1b10ced5ca923a1fcb8d03e96b8d3268065d724548c0211415ff6ac6bac4" +dependencies = [ + "core-foundation-sys", + "libc", +] + [[package]] name = "tempfile" version = "3.27.0" @@ -2732,6 +2809,17 @@ version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f0805222e57f7521d6a62e36fa9163bc891acd422f971defe97d64e70d0a4fe5" +[[package]] +name = "windows-registry" +version = "0.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "02752bf7fbdcce7f2a27a742f798510f3e5ad88dbe84871e5168e2120c3d5720" +dependencies = [ + "windows-link", + "windows-result", + "windows-strings", +] + [[package]] name = "windows-result" version = "0.4.1" diff --git a/apps/staged/src-tauri/Cargo.lock b/apps/staged/src-tauri/Cargo.lock index 2f914d9b..c3e2a7ce 100644 --- a/apps/staged/src-tauri/Cargo.lock +++ b/apps/staged/src-tauri/Cargo.lock @@ -1274,7 +1274,11 @@ dependencies = [ name = "doctor" version = "0.1.0" dependencies = [ + "dirs", + "futures", + "reqwest", "serde", + "serde_json", "tokio", ] @@ -4162,6 +4166,7 @@ dependencies = [ "base64 0.22.1", "bytes", "encoding_rs", + "futures-channel", "futures-core", "futures-util", "h2", diff --git a/crates/doctor/Cargo.toml b/crates/doctor/Cargo.toml index 46ddff82..a4b2600f 100644 --- a/crates/doctor/Cargo.toml +++ b/crates/doctor/Cargo.toml @@ -6,4 +6,8 @@ description = "Health-check system for verifying external tool dependencies" [dependencies] serde = { version = "1.0", features = ["derive"] } +serde_json = "1" tokio = { version = "1.0", features = ["rt", "macros"] } +futures = "0.3" +reqwest = { version = "0.13", features = ["blocking", "json"] } +dirs = "6" diff --git a/crates/doctor/src/agents.rs b/crates/doctor/src/agents.rs index eccfa4cd..d7d85ce9 100644 --- a/crates/doctor/src/agents.rs +++ b/crates/doctor/src/agents.rs @@ -1,10 +1,14 @@ //! AI agent checks and fix command lookup. +use std::path::{Path, PathBuf}; use std::process::Command; use crate::checks::CLONEFILE_FIX_COMMAND; use crate::resolve::format_command_output; -use crate::types::{CheckStatus, DoctorCheck, FixType, ResolvedBinary}; +use crate::types::{ + AgentVersionInfo, AuthStatus, CheckStatus, DoctorCheck, FixType, InstallSource, ResolvedBinary, +}; +use crate::{execute_command_with_path_prefix, ExecOutcome}; /// Metadata for an individual AI agent check. pub struct AgentCheckInfo { @@ -24,6 +28,17 @@ pub struct AgentCheckInfo { pub bridge_install_url: Option<&'static str>, /// Shell command to install the ACP bridge (used as fix_command for partial installs). pub bridge_install_command: Option<&'static str>, + /// Shell command that triggers an interactive authentication flow for this agent. + pub auth_command: Option<&'static str>, + /// Shell command that reports whether the user is currently authenticated. + pub auth_status_command: Option<&'static str>, + /// Per-tool override declaring this agent's install source when generic path + /// and filesystem-fingerprint heuristics fall through to + /// [`InstallSource::Unknown`]. Lets a curl/native-only agent (Claude native, + /// Cursor, Amp) report its true source instead of `Unknown`. Applied only on + /// `Unknown`; a positively-detected source (Brew/Npm/…) and a missing binary + /// (`None`) are left untouched. + pub install_source_override: Option, } /// All AI agents we check for individually. @@ -37,6 +52,9 @@ pub const AI_AGENT_CHECKS: &[AgentCheckInfo] = &[ install_command: None, bridge_install_url: None, bridge_install_command: None, + auth_command: None, + auth_status_command: None, + install_source_override: None, }, AgentCheckInfo { id: "ai-agent-claude", @@ -46,7 +64,12 @@ pub const AI_AGENT_CHECKS: &[AgentCheckInfo] = &[ install_url: Some("https://code.claude.com/docs/en/overview"), install_command: Some("curl -fsSL https://claude.ai/install.sh | bash"), bridge_install_url: Some("https://github.com/zed-industries/claude-agent-acp#installation"), - bridge_install_command: Some("npm install -g @zed-industries/claude-agent-acp"), + bridge_install_command: Some("npm install -g @agentclientprotocol/claude-agent-acp"), + auth_command: Some("claude auth login"), + auth_status_command: Some("claude auth status"), + // Main `claude` is a curl/native install; the bridge is npm and is + // detected positively, so this only takes effect on the main-only path. + install_source_override: Some(InstallSource::CurlPipe), }, AgentCheckInfo { id: "ai-agent-codex", @@ -57,6 +80,9 @@ pub const AI_AGENT_CHECKS: &[AgentCheckInfo] = &[ install_command: Some("brew install --cask codex"), bridge_install_url: Some("https://github.com/zed-industries/codex-acp#installation"), bridge_install_command: Some("npm install -g @zed-industries/codex-acp"), + auth_command: Some("codex login"), + auth_status_command: Some("codex login status"), + install_source_override: None, }, AgentCheckInfo { id: "ai-agent-pi", @@ -67,6 +93,9 @@ pub const AI_AGENT_CHECKS: &[AgentCheckInfo] = &[ install_command: None, bridge_install_url: None, bridge_install_command: None, + auth_command: None, + auth_status_command: None, + install_source_override: None, }, AgentCheckInfo { id: "ai-agent-amp", @@ -77,15 +106,177 @@ pub const AI_AGENT_CHECKS: &[AgentCheckInfo] = &[ install_command: Some("curl -fsSL https://ampcode.com/install.sh | bash"), bridge_install_url: Some("https://www.npmjs.com/package/amp-acp"), bridge_install_command: Some("npm install -g amp-acp"), + auth_command: Some("amp login"), + auth_status_command: Some("amp usage"), + // Main `amp` curl installer; bridge `amp-acp` is npm (detected positively). + install_source_override: Some(InstallSource::CurlPipe), + }, + AgentCheckInfo { + id: "ai-agent-copilot", + label: "GitHub Copilot", + commands: &["copilot"], + main_command: None, + install_url: Some("https://github.com/github/copilot-cli"), + install_command: Some("npm install -g @github/copilot"), + bridge_install_url: None, + bridge_install_command: None, + auth_command: Some("copilot login"), + auth_status_command: None, + install_source_override: None, + }, + AgentCheckInfo { + id: "ai-agent-cursor", + label: "Cursor Agent", + commands: &["cursor-agent"], + main_command: None, + install_url: Some("https://cursor.com/"), + install_command: Some("curl -fsSL https://cursor.com/install | bash"), + bridge_install_url: None, + bridge_install_command: None, + auth_command: Some("cursor-agent login"), + auth_status_command: Some("cursor-agent status"), + // Cursor has only a curl/native installer and no ACP bridge, so the + // resolved binary is the curl install — the override's primary use case. + install_source_override: Some(InstallSource::CurlPipe), }, ]; +/// Derive a source-aware update command from a readout's install source and +/// package id. Returns `None` for self-updating sources (`CurlPipe`), sources +/// with no canonical update recipe (`Mise`/`Asdf`/`Unknown`/`System`), or when +/// the package id is unknown. +/// +/// The caller is responsible for gating on `update_available == Some(true)` — +/// this function only knows how to update, not whether to. `apply_npm_registry` +/// runs over the final command string downstream, so npm commands automatically +/// pick up a registry override when one is configured. +pub fn derive_update_command( + install_source: Option<&InstallSource>, + package_id: Option<&str>, +) -> Option { + let pkg = package_id?; + match install_source? { + InstallSource::Npm => Some(format!("npm install -g {pkg}@latest")), + InstallSource::Brew => Some(format!("brew upgrade {pkg}")), + InstallSource::Cargo => Some(format!("cargo install --force {pkg}")), + InstallSource::CurlPipe + | InstallSource::Mise + | InstallSource::Asdf + | InstallSource::Unknown + | InstallSource::System => None, + } +} + +/// Append `--registry=` to `command` when a registry override is supplied +/// and the command is npm-backed. Non-npm commands (curl-pipe installers, auth +/// commands, the git-clonefile fix, …) and the `None` registry case return the +/// command unchanged. +/// +/// The registry URL is always caller-supplied — the crate never bakes in a +/// Block-specific (or any other) registry. +pub(crate) fn apply_npm_registry(command: &str, registry: Option<&str>) -> String { + match registry { + Some(url) if is_npm_command(command) => format!("{command} --registry={url}"), + _ => command.to_string(), + } +} + +/// Heuristic for whether `command` shells out to npm. Matches a leading `npm ` +/// invocation as well as the `npm install` / `npm view` forms that may be +/// preceded by other tokens. +fn is_npm_command(command: &str) -> bool { + command.starts_with("npm ") || command.contains("npm install") || command.contains("npm view") +} + +/// Parent directories of every resolved binary behind a check, deduplicated. +/// Used to build a PATH prefix for the auth probe so the agent is findable +/// even when the parent process has a restricted PATH (Finder/launchd case). +/// Order: main first, then bridges, in the order they were resolved. +fn collect_resolved_bin_dirs( + main: Option<&ResolvedBinary>, + bridges: &[ResolvedBinary], +) -> Vec { + let mut out: Vec = Vec::new(); + let mut push = |p: &Path| { + if let Some(parent) = p.parent() { + let pb = parent.to_path_buf(); + if !out.contains(&pb) { + out.push(pb); + } + } + }; + if let Some(m) = main { + if let Some(p) = m.path.as_deref() { + push(p); + } + } + for b in bridges { + if let Some(p) = b.path.as_deref() { + push(p); + } + } + out +} + +/// Build the resolve-trace portion of an agent check's `raw_output`. When the +/// agent has a separate `main_command`, prepends the main CLI's resolve trace +/// so the happy path explains *both* binaries' resolution, not just the +/// bridge's. Single-binary agents (no `main_command`) leave only the bridge +/// trace, which is the binary that was searched for. +fn build_raw_with_main_search( + header: &str, + info: &AgentCheckInfo, + resolved_main: Option<&ResolvedBinary>, + bridge_search: &str, +) -> String { + match (info.main_command, resolved_main) { + (Some(main_cmd), Some(rm)) => { + let bridge_cmd = info.commands.first().copied().unwrap_or(""); + format!( + "{header}\n# main CLI ({main_cmd}):\n{}\n# ACP bridge ({bridge_cmd}):\n{bridge_search}", + rm.search_output, + ) + } + _ => format!("{header}\n{bridge_search}"), + } +} + +/// Resolve the effective install source for an agent binary. Generic path and +/// filesystem fingerprinting (in [`crate::resolve`]) win when they identify a +/// source; only when they fall through to [`InstallSource::Unknown`] does the +/// agent's declared `override_hint` take effect. A missing binary (`None`) is +/// never fabricated into a source. +fn apply_install_source_override( + detected: Option, + override_hint: Option<&InstallSource>, +) -> Option { + match detected { + Some(InstallSource::Unknown) => override_hint.cloned().or(Some(InstallSource::Unknown)), + other => other, + } +} + +/// Build a per-binary version readout from a resolved install source. Returns +/// `Some` only when the binary was resolved (i.e. a source was detected); the +/// version fields stay `None` until the freshness pass fills them in. A binary +/// that wasn't found (`None`) yields `None` — no empty readout is fabricated. +fn version_readout(source: Option) -> Option { + source.map(|src| AgentVersionInfo { + install_source: Some(src), + ..AgentVersionInfo::default() + }) +} + /// Check whether a single AI agent is installed. +/// +/// `npm_registry`, when `Some`, routes any npm-backed install/bridge fix +/// command through that registry (see [`apply_npm_registry`]). pub fn check_single_ai_agent( info: &AgentCheckInfo, any_agent_found: bool, resolved_cmds: &[ResolvedBinary], resolved_main: Option<&ResolvedBinary>, + npm_registry: Option<&str>, ) -> DoctorCheck { let header = format!( "# Check: {} — verify {} agent is installed", @@ -97,10 +288,18 @@ pub fn check_single_ai_agent( .collect(); let search = search_lines.join("\n"); - let resolved_path = resolved_cmds - .iter() - .find_map(|rb| rb.path.as_ref()) + let resolved_bridge: Option<&ResolvedBinary> = + resolved_cmds.iter().find(|rb| rb.path.is_some()); + let resolved_path = resolved_bridge + .and_then(|rb| rb.path.as_ref()) .map(|p| p.to_string_lossy().to_string()); + // Prefer the bridge's install_source — that's the binary the UI cares about + // for "how did your acp adapter get installed". For goose (no separate + // main_command) the bridge is the main binary itself, so this is correct. + let bridge_install_source = apply_install_source_override( + resolved_bridge.and_then(|rb| rb.install_source.clone()), + info.install_source_override.as_ref(), + ); if let Some(ref path_str) = resolved_path { if info.id == "ai-agent-goose" { @@ -122,6 +321,16 @@ pub fn check_single_ai_agent( path: resolved_path, bridge_path: None, raw_output: Some(raw), + auth_status: None, + installed_version: None, + latest_version: None, + update_available: None, + install_source: bridge_install_source.clone(), + self_updating: None, + // Goose has no separate ACP bridge — the single binary + // is the agent CLI, reported under `main`. + main: version_readout(bridge_install_source.clone()), + bridge: None, } } Ok(output) => { @@ -142,6 +351,16 @@ pub fn check_single_ai_agent( path: resolved_path, bridge_path: None, raw_output: Some(raw), + auth_status: None, + installed_version: None, + latest_version: None, + update_available: None, + install_source: bridge_install_source.clone(), + self_updating: None, + // Goose has no separate ACP bridge — the single binary + // is the agent CLI, reported under `main`. + main: version_readout(bridge_install_source.clone()), + bridge: None, } } Err(e) => DoctorCheck { @@ -157,6 +376,14 @@ pub fn check_single_ai_agent( raw_output: Some(format!( "{header}\n$ goose acp --help\nerror: {e}\n{search}" )), + auth_status: None, + installed_version: None, + latest_version: None, + update_available: None, + install_source: bridge_install_source.clone(), + self_updating: None, + main: version_readout(bridge_install_source.clone()), + bridge: None, }, } } else { @@ -168,17 +395,115 @@ pub fn check_single_ai_agent( } else { (resolved_path, None) }; + + // Surface the agent CLI and its ACP bridge as two independent + // readouts. When the agent has a separate `main_command`, the + // install-source override (a curl/native hint) applies to the main + // CLI; the bridge keeps its positively-detected source. When there + // is no separate bridge, the single resolved binary is the agent + // CLI itself, reported under `main` (bridge stays `None`). + let (main_readout, bridge_readout) = if info.main_command.is_some() { + let main_src = apply_install_source_override( + resolved_main.and_then(|rb| rb.install_source.clone()), + info.install_source_override.as_ref(), + ); + let bridge_src = resolved_bridge.and_then(|rb| rb.install_source.clone()); + (version_readout(main_src), version_readout(bridge_src)) + } else { + (version_readout(bridge_install_source.clone()), None) + }; + + // Build a PATH prefix from the resolved binary directories so the + // auth probe can find the agent even when the parent process was + // launched with a restricted PATH (Finder/launchd case). The probe + // command itself is left intact — only PATH is augmented. + let auth_path_prefix = collect_resolved_bin_dirs(resolved_main, resolved_cmds); + + let (auth_status, auth_output) = match info.auth_status_command { + Some(cmd) => match execute_command_with_path_prefix(cmd, &auth_path_prefix) { + ExecOutcome::Ok => ( + Some(AuthStatus::Authenticated), + Some(format!("$ {cmd}\n(exit 0)")), + ), + // Shell itself couldn't be spawned — we have no signal at + // all about auth state. + ExecOutcome::Spawn(e) => ( + Some(AuthStatus::Unknown), + Some(format!("$ {cmd}\n(spawn failure: {e})")), + ), + // Exit 127 means the shell couldn't find the command — + // PATH-shadowed or uninstalled. Not the same as "signed + // out"; report Unknown so the UI doesn't offer a useless + // `... auth login` fix. + ExecOutcome::Exit { + code: Some(127), + stderr, + } => ( + Some(AuthStatus::Unknown), + Some(format!( + "$ {cmd}\n(command not found / exit 127)\n{}", + stderr.trim_end(), + )), + ), + // Genuine non-zero exit — the command ran and rejected us. + ExecOutcome::Exit { code, stderr } => ( + Some(AuthStatus::NotAuthenticated), + Some(format!( + "$ {cmd}\n(exit {})\n{}", + code.map(|c| c.to_string()) + .unwrap_or_else(|| "signal".to_string()), + stderr.trim_end(), + )), + ), + }, + None if info.auth_command.is_some() => (Some(AuthStatus::NotApplicable), None), + None => (None, None), + }; + + let mut raw = build_raw_with_main_search(&header, info, resolved_main, &search); + if let Some(extra) = auth_output { + raw.push('\n'); + raw.push_str(&extra); + } + + let (status, message, fix_type, fix_command) = match auth_status { + Some(AuthStatus::NotAuthenticated) => ( + CheckStatus::Warn, + "Installed, not authenticated".to_string(), + info.auth_command.map(|_| FixType::Auth), + info.auth_command.map(|s| s.to_string()), + ), + // PATH-shadowed / command not found: we can't tell. Surface + // the state but don't offer an auth fix — `claude auth login` + // won't help if `claude` isn't on PATH. + Some(AuthStatus::Unknown) => ( + CheckStatus::Warn, + "Installed, auth status unknown".to_string(), + None, + None, + ), + _ => (CheckStatus::Pass, "Installed".to_string(), None, None), + }; + DoctorCheck { id: info.id.to_string(), label: info.label.to_string(), - status: CheckStatus::Pass, - message: "Installed".to_string(), + status, + message, fix_url: None, - fix_command: None, - fix_type: None, + fix_command, + fix_type, path: main_path, bridge_path, - raw_output: Some(format!("{header}\n{search}")), + raw_output: Some(raw), + auth_status, + installed_version: None, + latest_version: None, + update_available: None, + install_source: bridge_install_source, + self_updating: None, + main: main_readout, + bridge: bridge_readout, } } } else { @@ -187,6 +512,14 @@ pub fn check_single_ai_agent( if let Some(main_path) = resolved_main.as_ref().and_then(|rm| rm.path.as_ref()) { let bridge_cmd = info.commands[0]; let main_search = &resolved_main.as_ref().unwrap().search_output; + // Bridge wasn't found, so fall back to the main binary's + // install_source — the only resolved path we have to describe. + let main_install_source = apply_install_source_override( + resolved_main + .as_ref() + .and_then(|rm| rm.install_source.clone()), + info.install_source_override.as_ref(), + ); return DoctorCheck { id: info.id.to_string(), label: info.label.to_string(), @@ -199,11 +532,22 @@ pub fn check_single_ai_agent( .bridge_install_url .or(info.install_url) .map(|s| s.to_string()), - fix_command: info.bridge_install_command.map(|s| s.to_string()), + fix_command: info + .bridge_install_command + .map(|s| apply_npm_registry(s, npm_registry)), fix_type: info.bridge_install_command.map(|_| FixType::Bridge), path: Some(main_path.to_string_lossy().to_string()), bridge_path: None, raw_output: Some(format!("{header}\n{search}\n{main_search}")), + auth_status: None, + installed_version: None, + latest_version: None, + update_available: None, + install_source: main_install_source.clone(), + self_updating: None, + // Bridge is absent; only the agent CLI resolved. + main: version_readout(main_install_source), + bridge: None, }; } @@ -223,11 +567,21 @@ pub fn check_single_ai_agent( "Not installed — at least one AI agent is needed".to_string() }, fix_url: info.install_url.map(|s| s.to_string()), - fix_command: info.install_command.map(|s| s.to_string()), + fix_command: info + .install_command + .map(|s| apply_npm_registry(s, npm_registry)), fix_type: info.install_command.map(|_| FixType::Command), path: None, bridge_path: None, raw_output: Some(format!("{header}\n{search}{extra_search}")), + auth_status: None, + installed_version: None, + latest_version: None, + update_available: None, + install_source: None, + self_updating: None, + main: None, + bridge: None, } } } @@ -247,9 +601,341 @@ pub fn lookup_fix_command(check_id: &str, fix_type: &FixType) -> Option return match fix_type { FixType::Command => info.install_command.map(|s| s.to_string()), FixType::Bridge => info.bridge_install_command.map(|s| s.to_string()), + FixType::Auth => info.auth_command.map(|s| s.to_string()), + // Update commands are derived per-readout from + // `(install_source, package_id)` and supplied to the executor + // via `command_override`; there is no static fallback. + FixType::UpdateMain | FixType::UpdateBridge => None, }; } } None } + +#[cfg(test)] +mod tests { + use super::*; + use std::path::PathBuf; + + fn resolved(path: Option<&str>, source: Option) -> ResolvedBinary { + ResolvedBinary { + path: path.map(PathBuf::from), + search_output: String::new(), + install_source: source, + } + } + + fn agent(id: &str) -> &'static AgentCheckInfo { + AI_AGENT_CHECKS.iter().find(|i| i.id == id).unwrap() + } + + /// An agent with a separate ACP bridge surfaces both binaries as independent + /// readouts, each carrying its own install source. Uses Pi (no auth + /// commands) so the check runs without shelling out. + #[test] + fn agent_check_populates_main_and_bridge_readouts() { + let bridge = resolved(Some("/n/bin/pi-acp"), Some(InstallSource::Npm)); + let main = resolved(Some("/c/bin/pi"), Some(InstallSource::Cargo)); + let check = check_single_ai_agent( + agent("ai-agent-pi"), + true, + std::slice::from_ref(&bridge), + Some(&main), + None, + ); + + let m = check.main.expect("main readout present"); + assert_eq!(m.install_source, Some(InstallSource::Cargo)); + let b = check.bridge.expect("bridge readout present"); + assert_eq!(b.install_source, Some(InstallSource::Npm)); + // Flat field mirrors the bridge (headline) readout for back-compat. + assert_eq!(check.install_source, Some(InstallSource::Npm)); + // Version fields stay empty on the cheap (no-freshness) path. + assert!(m.installed_version.is_none()); + assert!(b.installed_version.is_none()); + assert!(m.self_updating.is_none()); + } + + /// When only the agent CLI resolves (no bridge), the main readout carries + /// the CLI's source — upgraded from `Unknown` via the per-agent override — + /// and the bridge readout is `None`. Claude's main-only path returns before + /// any auth probe, so this runs without shelling out. + #[test] + fn agent_check_main_only_applies_override_and_omits_bridge() { + let bridge_missing = resolved(None, None); + let main = resolved(Some("/h/.local/bin/claude"), Some(InstallSource::Unknown)); + let check = check_single_ai_agent( + agent("ai-agent-claude"), + true, + std::slice::from_ref(&bridge_missing), + Some(&main), + None, + ); + + let m = check.main.expect("main readout present"); + assert_eq!( + m.install_source, + Some(InstallSource::CurlPipe), + "Unknown main CLI upgraded to CurlPipe via override", + ); + assert!(check.bridge.is_none(), "no bridge resolved"); + assert_eq!(check.install_source, Some(InstallSource::CurlPipe)); + } + + /// An agent with no separate bridge reports its single binary under `main`, + /// leaving `bridge` as `None`. Copilot has an auth command but no + /// auth-status command, so it reports NotApplicable without shelling out. + #[test] + fn agent_without_bridge_reports_single_binary_as_main() { + let single = resolved(Some("/n/bin/copilot"), Some(InstallSource::Npm)); + let check = check_single_ai_agent( + agent("ai-agent-copilot"), + true, + std::slice::from_ref(&single), + None, + None, + ); + + let m = check.main.expect("main readout present"); + assert_eq!(m.install_source, Some(InstallSource::Npm)); + assert!(check.bridge.is_none()); + assert_eq!(check.install_source, Some(InstallSource::Npm)); + } + + /// A fully unresolved agent carries neither readout. + #[test] + fn unresolved_agent_has_no_readouts() { + let missing = resolved(None, None); + let check = check_single_ai_agent( + agent("ai-agent-pi"), + false, + std::slice::from_ref(&missing), + Some(&missing), + None, + ); + assert!(check.main.is_none()); + assert!(check.bridge.is_none()); + } + + #[test] + fn auth_fix_lookup_returns_agent_auth_command() { + assert_eq!( + lookup_fix_command("ai-agent-claude", &FixType::Auth).as_deref(), + Some("claude auth login"), + ); + assert_eq!( + lookup_fix_command("ai-agent-copilot", &FixType::Auth).as_deref(), + Some("copilot login"), + ); + } + + #[test] + fn auth_fix_lookup_returns_none_for_agents_without_auth_command() { + assert_eq!(lookup_fix_command("ai-agent-goose", &FixType::Auth), None); + } + + #[test] + fn override_applies_only_to_unknown() { + // Unknown + declared override => override wins. + assert_eq!( + apply_install_source_override( + Some(InstallSource::Unknown), + Some(&InstallSource::CurlPipe), + ), + Some(InstallSource::CurlPipe), + ); + // Unknown + no override => stays Unknown. + assert_eq!( + apply_install_source_override(Some(InstallSource::Unknown), None), + Some(InstallSource::Unknown), + ); + // A positively-detected source is never overridden. + assert_eq!( + apply_install_source_override(Some(InstallSource::Npm), Some(&InstallSource::CurlPipe),), + Some(InstallSource::Npm), + ); + // A missing binary is never fabricated into a source. + assert_eq!( + apply_install_source_override(None, Some(&InstallSource::CurlPipe)), + None, + ); + } + + #[test] + fn curl_native_agents_declare_curl_pipe_override() { + for id in ["ai-agent-claude", "ai-agent-amp", "ai-agent-cursor"] { + let info = AI_AGENT_CHECKS.iter().find(|i| i.id == id).unwrap(); + assert_eq!( + info.install_source_override, + Some(InstallSource::CurlPipe), + "{id} should declare a CurlPipe override", + ); + } + // Registry-installed agents declare no override. + let codex = AI_AGENT_CHECKS + .iter() + .find(|i| i.id == "ai-agent-codex") + .unwrap(); + assert_eq!(codex.install_source_override, None); + } + + #[test] + fn apply_npm_registry_appends_to_npm_install() { + assert_eq!( + apply_npm_registry("npm install -g amp-acp", Some("https://artifactory/npm")), + "npm install -g amp-acp --registry=https://artifactory/npm", + ); + } + + #[test] + fn apply_npm_registry_appends_to_npm_view() { + assert_eq!( + apply_npm_registry("npm view amp-acp version", Some("https://artifactory/npm")), + "npm view amp-acp version --registry=https://artifactory/npm", + ); + } + + #[test] + fn apply_npm_registry_leaves_non_npm_commands_unchanged() { + let curl = "curl -fsSL https://cursor.com/install | bash"; + assert_eq!( + apply_npm_registry(curl, Some("https://artifactory/npm")), + curl, + ); + assert_eq!( + apply_npm_registry("claude auth login", Some("https://artifactory/npm")), + "claude auth login", + ); + } + + /// On the happy path (both main and bridge resolved), `raw_output` includes + /// the main CLI's resolve trace alongside the bridge's, with clear labels + /// and a stable main-first ordering — diagnoses PATH-shadowed agents from + /// the Copy details payload alone. + #[test] + fn raw_output_includes_main_and_bridge_resolve_traces() { + let bridge = ResolvedBinary { + path: Some(PathBuf::from("/n/bin/pi-acp")), + search_output: "BRIDGE-SEARCH-MARKER".to_string(), + install_source: Some(InstallSource::Npm), + }; + let main = ResolvedBinary { + path: Some(PathBuf::from("/c/bin/pi")), + search_output: "MAIN-SEARCH-MARKER".to_string(), + install_source: Some(InstallSource::Cargo), + }; + let check = check_single_ai_agent( + agent("ai-agent-pi"), + true, + std::slice::from_ref(&bridge), + Some(&main), + None, + ); + let raw = check.raw_output.expect("raw_output set"); + let main_at = raw + .find("MAIN-SEARCH-MARKER") + .expect("main trace in raw_output"); + let bridge_at = raw + .find("BRIDGE-SEARCH-MARKER") + .expect("bridge trace in raw_output"); + assert!( + main_at < bridge_at, + "main should appear before bridge; raw_output:\n{raw}", + ); + assert!( + raw.contains("# main CLI") && raw.contains("# ACP bridge"), + "expected labeled sections; raw_output:\n{raw}", + ); + } + + #[test] + fn collect_resolved_bin_dirs_dedupes_and_preserves_order() { + let main = ResolvedBinary { + path: Some(PathBuf::from("/tmp/a/main")), + search_output: String::new(), + install_source: None, + }; + let b1 = ResolvedBinary { + path: Some(PathBuf::from("/tmp/b/bridge1")), + search_output: String::new(), + install_source: None, + }; + // Same dir as main — should dedupe. + let b2 = ResolvedBinary { + path: Some(PathBuf::from("/tmp/a/another")), + search_output: String::new(), + install_source: None, + }; + let dirs = collect_resolved_bin_dirs(Some(&main), &[b1, b2]); + assert_eq!(dirs, vec![PathBuf::from("/tmp/a"), PathBuf::from("/tmp/b")],); + } + + #[test] + fn apply_npm_registry_none_is_passthrough() { + assert_eq!( + apply_npm_registry("npm install -g amp-acp", None), + "npm install -g amp-acp", + ); + } + + #[test] + fn derive_update_command_npm_emits_at_latest() { + assert_eq!( + derive_update_command(Some(&InstallSource::Npm), Some("@anthropic-ai/claude-code"),) + .as_deref(), + Some("npm install -g @anthropic-ai/claude-code@latest"), + ); + } + + #[test] + fn derive_update_command_brew_emits_upgrade() { + assert_eq!( + derive_update_command(Some(&InstallSource::Brew), Some("codex")).as_deref(), + Some("brew upgrade codex"), + ); + } + + #[test] + fn derive_update_command_cargo_emits_install_force() { + assert_eq!( + derive_update_command(Some(&InstallSource::Cargo), Some("some-crate")).as_deref(), + Some("cargo install --force some-crate"), + ); + } + + #[test] + fn derive_update_command_self_updating_and_opaque_sources_return_none() { + for src in [ + InstallSource::CurlPipe, + InstallSource::Mise, + InstallSource::Asdf, + InstallSource::Unknown, + InstallSource::System, + ] { + assert_eq!( + derive_update_command(Some(&src), Some("pkg")), + None, + "expected None for {src:?}", + ); + } + } + + #[test] + fn derive_update_command_returns_none_without_package_id() { + assert_eq!(derive_update_command(Some(&InstallSource::Npm), None), None,); + } + + #[test] + fn update_fix_lookup_returns_none_for_update_variants() { + // Update commands are derived per-readout, not statically registered. + assert_eq!( + lookup_fix_command("ai-agent-claude", &FixType::UpdateMain), + None, + ); + assert_eq!( + lookup_fix_command("ai-agent-claude", &FixType::UpdateBridge), + None, + ); + } +} diff --git a/crates/doctor/src/checks.rs b/crates/doctor/src/checks.rs index 7aff2e3d..897f942a 100644 --- a/crates/doctor/src/checks.rs +++ b/crates/doctor/src/checks.rs @@ -29,6 +29,14 @@ pub fn check_git(resolved: &ResolvedBinary) -> DoctorCheck { path: None, bridge_path: None, raw_output: Some(format!("{header}\nnot found via resolve_binary\n{search}")), + auth_status: None, + installed_version: None, + latest_version: None, + update_available: None, + install_source: None, + self_updating: None, + main: None, + bridge: None, }; } }; @@ -53,6 +61,14 @@ pub fn check_git(resolved: &ResolvedBinary) -> DoctorCheck { path: Some(path_str), bridge_path: None, raw_output: Some(raw), + auth_status: None, + installed_version: None, + latest_version: None, + update_available: None, + install_source: resolved.install_source.clone(), + self_updating: None, + main: None, + bridge: None, } } Ok(output) => { @@ -72,6 +88,14 @@ pub fn check_git(resolved: &ResolvedBinary) -> DoctorCheck { path: Some(path_str), bridge_path: None, raw_output: Some(raw), + auth_status: None, + installed_version: None, + latest_version: None, + update_available: None, + install_source: resolved.install_source.clone(), + self_updating: None, + main: None, + bridge: None, } } Err(e) => DoctorCheck { @@ -85,6 +109,14 @@ pub fn check_git(resolved: &ResolvedBinary) -> DoctorCheck { path: Some(path_str), bridge_path: None, raw_output: Some(format!("{header}\n$ git --version\nerror: {e}\n{search}")), + auth_status: None, + installed_version: None, + latest_version: None, + update_available: None, + install_source: resolved.install_source.clone(), + self_updating: None, + main: None, + bridge: None, }, } } @@ -110,6 +142,14 @@ pub fn check_gh(resolved: &ResolvedBinary) -> DoctorCheck { path: None, bridge_path: None, raw_output: Some(format!("{header}\nnot found via resolve_binary\n{search}")), + auth_status: None, + installed_version: None, + latest_version: None, + update_available: None, + install_source: None, + self_updating: None, + main: None, + bridge: None, }; } }; @@ -135,6 +175,14 @@ pub fn check_gh(resolved: &ResolvedBinary) -> DoctorCheck { path: Some(path_str), bridge_path: None, raw_output: Some(raw), + auth_status: None, + installed_version: None, + latest_version: None, + update_available: None, + install_source: resolved.install_source.clone(), + self_updating: None, + main: None, + bridge: None, } } Ok(output) => { @@ -154,6 +202,14 @@ pub fn check_gh(resolved: &ResolvedBinary) -> DoctorCheck { path: Some(path_str), bridge_path: None, raw_output: Some(raw), + auth_status: None, + installed_version: None, + latest_version: None, + update_available: None, + install_source: resolved.install_source.clone(), + self_updating: None, + main: None, + bridge: None, } } Err(e) => DoctorCheck { @@ -167,6 +223,14 @@ pub fn check_gh(resolved: &ResolvedBinary) -> DoctorCheck { path: Some(path_str), bridge_path: None, raw_output: Some(format!("{header}\n$ gh --version\nerror: {e}\n{search}")), + auth_status: None, + installed_version: None, + latest_version: None, + update_available: None, + install_source: resolved.install_source.clone(), + self_updating: None, + main: None, + bridge: None, }, } } @@ -191,6 +255,14 @@ pub fn check_gh_auth(gh: &ResolvedBinary) -> DoctorCheck { path: None, bridge_path: None, raw_output: Some(format!("{header}\ngh not found via resolve_binary")), + auth_status: None, + installed_version: None, + latest_version: None, + update_available: None, + install_source: None, + self_updating: None, + main: None, + bridge: None, }; } }; @@ -213,6 +285,14 @@ pub fn check_gh_auth(gh: &ResolvedBinary) -> DoctorCheck { path: None, bridge_path: None, raw_output: Some(raw), + auth_status: None, + installed_version: None, + latest_version: None, + update_available: None, + install_source: None, + self_updating: None, + main: None, + bridge: None, } } else { let stderr = String::from_utf8_lossy(&output.stderr); @@ -233,6 +313,14 @@ pub fn check_gh_auth(gh: &ResolvedBinary) -> DoctorCheck { path: None, bridge_path: None, raw_output: Some(raw), + auth_status: None, + installed_version: None, + latest_version: None, + update_available: None, + install_source: None, + self_updating: None, + main: None, + bridge: None, } } } @@ -247,6 +335,14 @@ pub fn check_gh_auth(gh: &ResolvedBinary) -> DoctorCheck { path: None, bridge_path: None, raw_output: Some(format!("{header}\n$ gh auth status\nerror: {e}")), + auth_status: None, + installed_version: None, + latest_version: None, + update_available: None, + install_source: None, + self_updating: None, + main: None, + bridge: None, }, } } @@ -275,6 +371,14 @@ pub fn check_git_lfs(git: &ResolvedBinary, git_lfs: &ResolvedBinary) -> DoctorCh raw_output: Some(format!( "{header}\ngit not found via resolve_binary\n{search}" )), + auth_status: None, + installed_version: None, + latest_version: None, + update_available: None, + install_source: None, + self_updating: None, + main: None, + bridge: None, }; } }; @@ -302,6 +406,14 @@ pub fn check_git_lfs(git: &ResolvedBinary, git_lfs: &ResolvedBinary) -> DoctorCh path, bridge_path: None, raw_output: Some(raw), + auth_status: None, + installed_version: None, + latest_version: None, + update_available: None, + install_source: git_lfs.install_source.clone(), + self_updating: None, + main: None, + bridge: None, } } Ok(output) => { @@ -321,6 +433,14 @@ pub fn check_git_lfs(git: &ResolvedBinary, git_lfs: &ResolvedBinary) -> DoctorCh path: None, bridge_path: None, raw_output: Some(raw), + auth_status: None, + installed_version: None, + latest_version: None, + update_available: None, + install_source: None, + self_updating: None, + main: None, + bridge: None, } } Err(e) => DoctorCheck { @@ -334,6 +454,14 @@ pub fn check_git_lfs(git: &ResolvedBinary, git_lfs: &ResolvedBinary) -> DoctorCh path: None, bridge_path: None, raw_output: Some(format!("{header}\n$ git lfs version\nerror: {e}\n{search}")), + auth_status: None, + installed_version: None, + latest_version: None, + update_available: None, + install_source: None, + self_updating: None, + main: None, + bridge: None, }, } } @@ -359,6 +487,14 @@ pub fn check_clonefile(git: &ResolvedBinary) -> DoctorCheck { path: None, bridge_path: None, raw_output: Some(format!("{header}\ngit not found via resolve_binary")), + auth_status: None, + installed_version: None, + latest_version: None, + update_available: None, + install_source: None, + self_updating: None, + main: None, + bridge: None, }; } }; @@ -387,6 +523,14 @@ pub fn check_clonefile(git: &ResolvedBinary) -> DoctorCheck { path: None, bridge_path: None, raw_output: Some(raw), + auth_status: None, + installed_version: None, + latest_version: None, + update_available: None, + install_source: git.install_source.clone(), + self_updating: None, + main: None, + bridge: None, } } else { DoctorCheck { @@ -401,6 +545,14 @@ pub fn check_clonefile(git: &ResolvedBinary) -> DoctorCheck { path: None, bridge_path: None, raw_output: Some(raw), + auth_status: None, + installed_version: None, + latest_version: None, + update_available: None, + install_source: git.install_source.clone(), + self_updating: None, + main: None, + bridge: None, } } } @@ -421,6 +573,14 @@ pub fn check_clonefile(git: &ResolvedBinary) -> DoctorCheck { path: None, bridge_path: None, raw_output: Some(raw), + auth_status: None, + installed_version: None, + latest_version: None, + update_available: None, + install_source: git.install_source.clone(), + self_updating: None, + main: None, + bridge: None, } } Err(e) => DoctorCheck { @@ -436,6 +596,14 @@ pub fn check_clonefile(git: &ResolvedBinary) -> DoctorCheck { raw_output: Some(format!( "{header}\n$ git config --global core.clonefile\nerror: {e}" )), + auth_status: None, + installed_version: None, + latest_version: None, + update_available: None, + install_source: git.install_source.clone(), + self_updating: None, + main: None, + bridge: None, }, } } diff --git a/crates/doctor/src/freshness.rs b/crates/doctor/src/freshness.rs new file mode 100644 index 00000000..5c548f9d --- /dev/null +++ b/crates/doctor/src/freshness.rs @@ -0,0 +1,824 @@ +//! Version-freshness lookups: installed version (local subprocess) + +//! latest available version (brew/npm/crates.io) with a disk cache. +//! +//! Only runs when `RunChecksOptions::check_freshness` is set — the default +//! `run_checks()` path is unaffected. + +use std::collections::HashMap; +use std::path::{Path, PathBuf}; +use std::process::Command; +use std::sync::{Arc, Mutex}; +use std::time::{SystemTime, UNIX_EPOCH}; + +use serde::{Deserialize, Serialize}; +use serde_json::Value; + +use crate::package_ids::LatestSource; +use crate::types::InstallSource; + +/// One hour: long enough that repeated `run_checks_with_options` calls in the +/// same session don't re-hit registries; short enough that real upgrades +/// surface within a reasonable window. +const CACHE_TTL_SECONDS: i64 = 60 * 60; + +/// Result of a single version-freshness probe. +#[derive(Debug, Clone, Default)] +pub(crate) struct VersionInfo { + pub installed: Option, + pub latest: Option, + pub update_available: Option, +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +struct CacheEntry { + value: String, + fetched_at: i64, +} + +/// In-memory mirror of the on-disk cache. Read once at the start of a freshness +/// run, mutated as fetches happen, persisted once at the end. +#[derive(Debug, Default)] +pub(crate) struct FreshnessCache { + entries: HashMap, +} + +impl FreshnessCache { + fn cache_key(source: LatestSource, package_id: &str) -> String { + format!("{:?}:{}", source, package_id) + } + + fn get_fresh(&self, source: LatestSource, package_id: &str, now: i64) -> Option { + let key = Self::cache_key(source, package_id); + let entry = self.entries.get(&key)?; + if now.saturating_sub(entry.fetched_at) <= CACHE_TTL_SECONDS { + Some(entry.value.clone()) + } else { + None + } + } + + fn insert(&mut self, source: LatestSource, package_id: &str, value: String, now: i64) { + let key = Self::cache_key(source, package_id); + self.entries.insert( + key, + CacheEntry { + value, + fetched_at: now, + }, + ); + } +} + +/// Resolve the on-disk cache file path. Prefers `dirs::cache_dir()`; the +/// `/doctor` parent is created lazily on save. +pub(crate) fn cache_file_path() -> Option { + let base = dirs::cache_dir()?; + Some(base.join("doctor").join("freshness.json")) +} + +/// Read the on-disk cache. Missing-or-malformed → empty (and we'll overwrite +/// on next save). Errors are logged, never propagated. +pub(crate) fn load_cache() -> FreshnessCache { + let Some(path) = cache_file_path() else { + return FreshnessCache::default(); + }; + match std::fs::read(&path) { + Ok(bytes) => match serde_json::from_slice::>(&bytes) { + Ok(entries) => FreshnessCache { entries }, + Err(e) => { + eprintln!( + "doctor: freshness cache malformed at {}: {e}", + path.display() + ); + FreshnessCache::default() + } + }, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => FreshnessCache::default(), + Err(e) => { + eprintln!("doctor: failed to read freshness cache: {e}"); + FreshnessCache::default() + } + } +} + +/// Atomically persist the cache: write to `.tmp`, then rename. +pub(crate) fn save_cache(cache: &FreshnessCache) { + let Some(path) = cache_file_path() else { + return; + }; + if let Some(parent) = path.parent() { + if let Err(e) = std::fs::create_dir_all(parent) { + eprintln!( + "doctor: failed to create freshness cache dir {}: {e}", + parent.display(), + ); + return; + } + } + let json = match serde_json::to_vec_pretty(&cache.entries) { + Ok(j) => j, + Err(e) => { + eprintln!("doctor: failed to serialize freshness cache: {e}"); + return; + } + }; + let tmp = path.with_extension("json.tmp"); + if let Err(e) = std::fs::write(&tmp, &json) { + eprintln!("doctor: failed to write freshness cache tmp file: {e}"); + return; + } + if let Err(e) = std::fs::rename(&tmp, &path) { + eprintln!("doctor: failed to rename freshness cache tmp -> final: {e}"); + } +} + +fn now_epoch_seconds() -> i64 { + SystemTime::now() + .duration_since(UNIX_EPOCH) + .map(|d| d.as_secs() as i64) + .unwrap_or(0) +} + +/// Locate the first semver-shaped token (`X.Y.Z`) on any line of `text`. +/// +/// We accept an optional leading `v` and ignore anything after the patch +/// component (pre-release / build metadata is fine to include in the returned +/// string — it's compared as a string only). +pub(crate) fn extract_version(text: &str) -> Option { + for line in text.lines() { + if let Some(v) = find_semver_in_line(line) { + return Some(v); + } + } + None +} + +fn find_semver_in_line(line: &str) -> Option { + let bytes = line.as_bytes(); + let mut i = 0; + while i < bytes.len() { + if bytes[i].is_ascii_digit() { + // Greedy: extend through digits.dots; require at least two dots. + let start = i; + let mut dots = 0; + while i < bytes.len() { + let b = bytes[i]; + if b.is_ascii_digit() { + i += 1; + } else if b == b'.' && i + 1 < bytes.len() && bytes[i + 1].is_ascii_digit() { + dots += 1; + i += 1; + } else { + break; + } + } + if dots >= 2 { + return Some(line[start..i].to_string()); + } + } else { + i += 1; + } + } + None +} + +/// Lightweight parse of `X.Y.Z[...]` into `(major, minor, patch)`. Returns +/// `None` if any of the first three numeric components is missing. +fn parse_semver_triple(v: &str) -> Option<(u64, u64, u64)> { + let trimmed = v.trim().trim_start_matches('v'); + let core = trimmed.split(['-', '+', ' ']).next()?; + let mut parts = core.split('.'); + let major = parts.next()?.parse::().ok()?; + let minor = parts.next()?.parse::().ok()?; + let patch_str = parts.next()?; + let patch = patch_str + .chars() + .take_while(|c| c.is_ascii_digit()) + .collect::() + .parse::() + .ok()?; + Some((major, minor, patch)) +} + +/// Compute `update_available` if both sides parse as semver triples; otherwise +/// `None` (so the UI doesn't render a misleading "outdated" badge based on +/// string inequality of unparseable versions). +fn compute_update_available(installed: &Option, latest: &Option) -> Option { + let i = installed.as_deref()?; + let l = latest.as_deref()?; + let it = parse_semver_triple(i)?; + let lt = parse_semver_triple(l)?; + Some(lt > it) +} + +/// How to read a target's *installed* version. The mechanism depends on how the +/// binary was installed: a CLI `--version` probe works for native/brew/cargo +/// binaries, but npm-distributed ACP bridges are `node` scripts that don't +/// implement `--version` (e.g. `codex-acp --version` errors, `amp-acp +/// --version` prints nothing), so those are read straight from the package's +/// `package.json` instead — no subprocess, no network. +pub(crate) enum InstalledProbe<'a> { + /// Run ` ` and extract a semver (the default behavior). + Cli(&'a [&'a str]), + /// Walk up from the canonicalized binary to the owning + /// `node_modules//package.json` and read its `version`. + NpmPackageJson { package_id: Option<&'a str> }, +} + +/// Pick the installed-version probe for a readout from its install source. +/// `Npm` installs read `package.json`; everything else uses the CLI +/// `--version` probe. +pub(crate) fn select_installed_probe<'a>( + install_source: Option<&InstallSource>, + package_id: Option<&'a str>, +) -> InstalledProbe<'a> { + if matches!(install_source, Some(InstallSource::Npm)) { + InstalledProbe::NpmPackageJson { package_id } + } else { + InstalledProbe::Cli(&["--version"]) + } +} + +/// Owned mirror of [`InstalledProbe`] so the probe can cross the +/// `spawn_blocking` boundary (which requires `'static`). +enum OwnedProbe { + Cli(Vec), + NpmPackageJson { package_id: Option }, +} + +impl InstalledProbe<'_> { + fn to_owned_probe(&self) -> OwnedProbe { + match self { + InstalledProbe::Cli(args) => { + OwnedProbe::Cli(args.iter().map(|s| s.to_string()).collect()) + } + InstalledProbe::NpmPackageJson { package_id } => OwnedProbe::NpmPackageJson { + package_id: package_id.map(|s| s.to_string()), + }, + } + } +} + +/// Read an npm package's installed version from its `package.json`. Pure +/// filesystem: canonicalize the binary (resolving the symlink npm leaves in +/// `bin/`), then walk up a bounded number of levels looking for the first +/// `package.json`. When the package id is known, the file's `name` must match +/// it — otherwise we keep walking, so a dependency's `package.json` nested +/// below the real one is never mistaken for the target. +fn installed_version_from_package_json( + binary_path: &Path, + expected_pkg: Option<&str>, +) -> Option { + let resolved = std::fs::canonicalize(binary_path).ok()?; + let mut dir = resolved.parent(); + for _ in 0..6 { + let d = dir?; + let pj = d.join("package.json"); + if pj.is_file() { + if let Ok(bytes) = std::fs::read(&pj) { + if let Ok(v) = serde_json::from_slice::(&bytes) { + let name_ok = expected_pkg + .map(|p| v.get("name").and_then(|n| n.as_str()) == Some(p)) + .unwrap_or(true); + if name_ok { + return v + .get("version") + .and_then(|x| x.as_str()) + .map(str::to_string); + } + } + } + } + dir = d.parent(); + } + None +} + +/// Run ` ` and parse the first semver-shaped token out of the +/// combined stdout/stderr. Errors and missing tokens both yield `None`. +fn installed_version(binary_path: &Path, version_args: &[&str]) -> Option { + let output = Command::new(binary_path).args(version_args).output().ok()?; + let combined = format!( + "{}\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr), + ); + extract_version(&combined) +} + +/// Whether a tool keeps itself up to date and therefore should never raise an +/// "update available" nag. Curl/native installers (Claude native, Cursor, +/// Amp-curl) self-update; those installs are fingerprinted as +/// [`InstallSource::CurlPipe`] (directly or via a per-agent override). Registry +/// installs (brew/npm/cargo) are user-managed and remain actionable, which is +/// why a brew/npm install of the same agent is *not* treated as self-updating. +pub(crate) fn is_self_updating(source: Option<&InstallSource>) -> bool { + matches!(source, Some(InstallSource::CurlPipe)) +} + +/// Dispatch a latest-version probe per source. +fn latest_version( + source: LatestSource, + package_id: &str, + npm_registry: Option<&str>, +) -> Option { + match source { + LatestSource::Brew => latest_brew(package_id), + LatestSource::Npm => latest_npm(package_id, npm_registry), + LatestSource::CratesIo => latest_crates_io(package_id), + LatestSource::GitHubReleases => latest_github_releases(package_id), + } +} + +fn latest_brew(package_id: &str) -> Option { + let output = Command::new("brew") + .args(["info", "--json=v2", package_id]) + .output() + .ok()?; + if !output.status.success() { + return None; + } + parse_brew_info_v2(&output.stdout, package_id) +} + +/// Pull `formulae[0].versions.stable` or `casks[0].version` out of a +/// `brew info --json=v2` payload. Public to the crate so unit tests can +/// drive it with a fixture without shelling out. +pub(crate) fn parse_brew_info_v2(bytes: &[u8], _package_id: &str) -> Option { + let root: Value = serde_json::from_slice(bytes).ok()?; + if let Some(formulae) = root.get("formulae").and_then(|v| v.as_array()) { + if let Some(first) = formulae.first() { + if let Some(v) = first + .get("versions") + .and_then(|v| v.get("stable")) + .and_then(|v| v.as_str()) + { + return Some(v.to_string()); + } + } + } + if let Some(casks) = root.get("casks").and_then(|v| v.as_array()) { + if let Some(first) = casks.first() { + if let Some(v) = first.get("version").and_then(|v| v.as_str()) { + return Some(v.to_string()); + } + } + } + None +} + +fn latest_npm(package_id: &str, npm_registry: Option<&str>) -> Option { + let mut cmd = Command::new("npm"); + cmd.args(["view", package_id, "version"]); + if let Some(registry) = npm_registry { + cmd.args(["--registry", registry]); + } + let output = cmd.output().ok()?; + if !output.status.success() { + return None; + } + let raw = String::from_utf8_lossy(&output.stdout).trim().to_string(); + if raw.is_empty() { + None + } else { + Some(raw) + } +} + +fn latest_crates_io(package_id: &str) -> Option { + let url = format!("https://crates.io/api/v1/crates/{package_id}"); + let client = reqwest::blocking::Client::builder() + .user_agent("block-builderbot-doctor/0.1") + .timeout(std::time::Duration::from_secs(10)) + .build() + .ok()?; + let resp = client.get(&url).send().ok()?; + if !resp.status().is_success() { + return None; + } + let json: Value = resp.json().ok()?; + json.get("crate") + .and_then(|c| c.get("max_stable_version")) + .and_then(|v| v.as_str()) + .map(|s| s.to_string()) +} + +/// Fetch the latest release tag for a `owner/repo` slug via the GitHub REST API. +/// +/// Used for tools published only through GitHub releases (no brew/npm/crates +/// presence), e.g. Cursor's curl install. Degrades gracefully to `None` on any +/// error — no token, rate-limited, network failure, or unparseable payload — +/// and never returns a hard error. A `GITHUB_TOKEN`/`GH_TOKEN` in the +/// environment is used as a bearer credential to relax the unauthenticated +/// rate limit, but is entirely optional. +fn latest_github_releases(repo: &str) -> Option { + let url = format!("https://api.github.com/repos/{repo}/releases/latest"); + let client = reqwest::blocking::Client::builder() + // GitHub rejects requests without a User-Agent. + .user_agent("block-builderbot-doctor/0.1") + .timeout(std::time::Duration::from_secs(10)) + .build() + .ok()?; + let mut req = client + .get(&url) + .header("Accept", "application/vnd.github+json"); + if let Some(token) = github_token() { + req = req.bearer_auth(token); + } + let resp = req.send().ok()?; + if !resp.status().is_success() { + return None; + } + let bytes = resp.bytes().ok()?; + parse_github_release_tag(&bytes) +} + +/// Read GitHub's optional release-auth token from the environment. Empty values +/// are treated as absent. Kept separate so the fetcher stays testable. +fn github_token() -> Option { + std::env::var("GITHUB_TOKEN") + .ok() + .or_else(|| std::env::var("GH_TOKEN").ok()) + .filter(|s| !s.is_empty()) +} + +/// Pull `tag_name` out of a GitHub `releases/latest` payload, stripping a +/// leading `v` so it lines up with the semver shapes the rest of the module +/// produces. Crate-public so unit tests can drive it without a network call. +pub(crate) fn parse_github_release_tag(bytes: &[u8]) -> Option { + let root: Value = serde_json::from_slice(bytes).ok()?; + root.get("tag_name") + .and_then(|v| v.as_str()) + .map(|s| s.trim_start_matches('v').to_string()) +} + +/// Top-level: returns installed (always, when binary present), latest (only +/// if `package_id` is `Some` and we're not offline), and `update_available` +/// (only if both parse as semver triples). +pub(crate) async fn fetch_version_info( + latest_source: Option, + package_id: Option<&str>, + binary_path: &Path, + probe: InstalledProbe<'_>, + offline: bool, + npm_registry: Option<&str>, + cache: Arc>, +) -> VersionInfo { + let path = binary_path.to_path_buf(); + let probe = probe.to_owned_probe(); + let pkg = package_id.map(|s| s.to_string()); + let npm_registry = npm_registry.map(|s| s.to_string()); + + let result = tokio::task::spawn_blocking(move || { + let installed = if path.as_os_str().is_empty() { + None + } else { + match &probe { + OwnedProbe::Cli(args) => installed_version( + &path, + &args.iter().map(|s| s.as_str()).collect::>(), + ), + OwnedProbe::NpmPackageJson { package_id } => { + installed_version_from_package_json(&path, package_id.as_deref()) + } + } + }; + + let latest = if offline { + None + } else if let (Some(source), Some(pkg)) = (latest_source, pkg) { + let now = now_epoch_seconds(); + // Cache lookup first. + let cached = { + let guard = cache.lock().ok(); + guard.as_ref().and_then(|c| c.get_fresh(source, &pkg, now)) + }; + if let Some(v) = cached { + Some(v) + } else if let Some(v) = latest_version(source, &pkg, npm_registry.as_deref()) { + if let Ok(mut guard) = cache.lock() { + guard.insert(source, &pkg, v.clone(), now); + } + Some(v) + } else { + None + } + } else { + None + }; + + let update_available = compute_update_available(&installed, &latest); + VersionInfo { + installed, + latest, + update_available, + } + }) + .await; + + result.unwrap_or_default() +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn extract_version_finds_git_style_string() { + assert_eq!( + extract_version("git version 2.39.5 (Apple Git-154)"), + Some("2.39.5".to_string()), + ); + } + + #[test] + fn extract_version_finds_gh_multiline() { + let s = "gh version 2.62.0 (2024-11-12)\nhttps://github.com/cli/cli/releases/tag/v2.62.0"; + assert_eq!(extract_version(s), Some("2.62.0".to_string())); + } + + #[test] + fn extract_version_returns_none_when_no_semver() { + assert_eq!(extract_version("no version here"), None); + assert_eq!(extract_version("1.2"), None); // need 3 components + } + + #[test] + fn extract_version_handles_leading_garbage() { + assert_eq!( + extract_version("Codex CLI 0.21.4-beta"), + Some("0.21.4".to_string()), + ); + } + + #[test] + fn parse_semver_triple_basic() { + assert_eq!(parse_semver_triple("1.2.3"), Some((1, 2, 3))); + assert_eq!(parse_semver_triple("v1.2.3"), Some((1, 2, 3))); + assert_eq!(parse_semver_triple("1.2.3-rc1"), Some((1, 2, 3))); + assert_eq!(parse_semver_triple("1.2"), None); + } + + #[test] + fn compute_update_available_handles_parse_failure() { + assert_eq!( + compute_update_available( + &Some("not-a-version".to_string()), + &Some("1.0.0".to_string()), + ), + None, + ); + assert_eq!( + compute_update_available(&Some("1.0.0".to_string()), &Some("1.0.0".to_string())), + Some(false), + ); + assert_eq!( + compute_update_available(&Some("1.0.0".to_string()), &Some("1.0.1".to_string())), + Some(true), + ); + } + + #[test] + fn brew_info_v2_parses_formula_versions_stable() { + let json = br#"{ + "formulae": [{ + "name": "git", + "versions": {"stable": "2.50.1", "head": null, "bottle": true} + }], + "casks": [] + }"#; + assert_eq!(parse_brew_info_v2(json, "git").as_deref(), Some("2.50.1"),); + } + + #[test] + fn brew_info_v2_parses_cask_version() { + let json = br#"{ + "formulae": [], + "casks": [{"token": "codex", "version": "0.21.4"}] + }"#; + assert_eq!(parse_brew_info_v2(json, "codex").as_deref(), Some("0.21.4"),); + } + + #[test] + fn brew_info_v2_returns_none_on_empty_payload() { + let json = br#"{"formulae": [], "casks": []}"#; + assert_eq!(parse_brew_info_v2(json, "ghost"), None); + } + + #[test] + fn cache_ttl_treats_old_entries_as_stale() { + let mut cache = FreshnessCache::default(); + let now = 1_000_000; + cache.insert(LatestSource::Brew, "git", "2.50.0".to_string(), now); + + // Within TTL. + assert_eq!( + cache.get_fresh(LatestSource::Brew, "git", now + 60), + Some("2.50.0".to_string()), + ); + + // 2 hours later — stale. + let two_hours_later = now + 2 * 60 * 60; + assert_eq!( + cache.get_fresh(LatestSource::Brew, "git", two_hours_later), + None, + ); + } + + #[test] + fn cache_key_uses_source_and_package_id() { + let k1 = FreshnessCache::cache_key(LatestSource::Brew, "git"); + let k2 = FreshnessCache::cache_key(LatestSource::Npm, "git"); + assert_ne!(k1, k2, "different sources should map to different keys"); + } + + #[test] + fn github_release_tag_strips_leading_v() { + let json = br#"{"tag_name": "v1.2.3", "name": "1.2.3"}"#; + assert_eq!( + parse_github_release_tag(json).as_deref(), + Some("1.2.3"), + "leading v should be stripped", + ); + } + + #[test] + fn github_release_tag_without_v_prefix() { + let json = br#"{"tag_name": "2025.06.01"}"#; + assert_eq!( + parse_github_release_tag(json).as_deref(), + Some("2025.06.01") + ); + } + + #[test] + fn github_release_tag_missing_field_is_none() { + assert_eq!( + parse_github_release_tag(br#"{"name": "no tag here"}"#), + None + ); + assert_eq!(parse_github_release_tag(b"not json"), None); + } + + #[test] + fn self_updating_only_for_curl_pipe() { + assert!(is_self_updating(Some(&InstallSource::CurlPipe))); + assert!(!is_self_updating(Some(&InstallSource::Npm))); + assert!(!is_self_updating(Some(&InstallSource::Brew))); + assert!(!is_self_updating(None)); + } + + /// Fresh per-test scratch dir under the system temp dir, unique by name + + /// process id (matches the pattern resolve.rs's tests use — no extra dev + /// dependency). + fn scratch_dir(name: &str) -> PathBuf { + let dir = + std::env::temp_dir().join(format!("doctor-freshness-{name}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + dir + } + + #[test] + fn package_json_version_read_through_bin_symlink() { + let root = scratch_dir("pj-symlink"); + let pkg = root.join("lib/node_modules/amp-acp"); + std::fs::create_dir_all(pkg.join("dist")).unwrap(); + let index = pkg.join("dist/index.js"); + std::fs::write(&index, "// node script\n").unwrap(); + std::fs::write( + pkg.join("package.json"), + br#"{"name": "amp-acp", "version": "0.4.2"}"#, + ) + .unwrap(); + + // npm leaves a symlink in bin/ pointing at the package's entrypoint. + std::fs::create_dir_all(root.join("bin")).unwrap(); + let bin = root.join("bin/amp-acp"); + #[cfg(unix)] + std::os::unix::fs::symlink(&index, &bin).unwrap(); + + assert_eq!( + installed_version_from_package_json(&bin, Some("amp-acp")).as_deref(), + Some("0.4.2"), + ); + // No expected name → first package.json found still wins. + assert_eq!( + installed_version_from_package_json(&bin, None).as_deref(), + Some("0.4.2"), + ); + } + + #[test] + fn package_json_skips_mismatched_name_and_keeps_walking() { + let root = scratch_dir("pj-mismatch"); + let pkg = root.join("node_modules/amp-acp"); + // A nested dependency's package.json sits closer to the binary; its + // name doesn't match, so the walk must continue up to amp-acp's. + let dep = pkg.join("node_modules/inner-dep"); + std::fs::create_dir_all(&dep).unwrap(); + std::fs::write( + dep.join("package.json"), + br#"{"name": "inner-dep", "version": "9.9.9"}"#, + ) + .unwrap(); + let index = dep.join("index.js"); + std::fs::write(&index, "// dep\n").unwrap(); + std::fs::write( + pkg.join("package.json"), + br#"{"name": "amp-acp", "version": "1.0.0"}"#, + ) + .unwrap(); + + assert_eq!( + installed_version_from_package_json(&index, Some("amp-acp")).as_deref(), + Some("1.0.0"), + "should skip inner-dep and find amp-acp", + ); + } + + #[test] + fn package_json_resolves_scoped_package() { + let root = scratch_dir("pj-scoped"); + let pkg = root.join("node_modules/@zed-industries/codex-acp"); + std::fs::create_dir_all(pkg.join("dist")).unwrap(); + let index = pkg.join("dist/index.js"); + std::fs::write(&index, "// node script\n").unwrap(); + std::fs::write( + pkg.join("package.json"), + br#"{"name": "@zed-industries/codex-acp", "version": "0.7.1"}"#, + ) + .unwrap(); + + assert_eq!( + installed_version_from_package_json(&index, Some("@zed-industries/codex-acp")) + .as_deref(), + Some("0.7.1"), + ); + } + + /// Exercises the Claude main CLI's new npm package id end-to-end at the + /// freshness layer: when claude is npm-installed under nvm, its main + /// readout walks up to `@anthropic-ai/claude-code`'s `package.json`. + #[test] + fn package_json_resolves_claude_main_npm_layout() { + let root = scratch_dir("pj-claude-main"); + let pkg = root.join("node_modules/@anthropic-ai/claude-code"); + std::fs::create_dir_all(pkg.join("cli")).unwrap(); + let entry = pkg.join("cli/cli.js"); + std::fs::write(&entry, "// node script\n").unwrap(); + std::fs::write( + pkg.join("package.json"), + br#"{"name": "@anthropic-ai/claude-code", "version": "2.1.0"}"#, + ) + .unwrap(); + + // npm leaves a `claude` symlink in `bin/`. + std::fs::create_dir_all(root.join("bin")).unwrap(); + let bin = root.join("bin/claude"); + #[cfg(unix)] + std::os::unix::fs::symlink(&entry, &bin).unwrap(); + + assert_eq!( + installed_version_from_package_json(&bin, Some("@anthropic-ai/claude-code")).as_deref(), + Some("2.1.0"), + ); + } + + #[test] + fn package_json_missing_returns_none() { + let root = scratch_dir("pj-missing"); + let bin = root.join("bin/whatever"); + std::fs::create_dir_all(root.join("bin")).unwrap(); + std::fs::write(&bin, "x\n").unwrap(); + assert_eq!( + installed_version_from_package_json(&bin, Some("nope")), + None + ); + } + + #[test] + fn select_probe_npm_reads_package_json() { + match select_installed_probe(Some(&InstallSource::Npm), Some("amp-acp")) { + InstalledProbe::NpmPackageJson { package_id } => { + assert_eq!(package_id, Some("amp-acp")); + } + _ => panic!("npm install source should select NpmPackageJson probe"), + } + } + + #[test] + fn select_probe_non_npm_uses_cli_version() { + for src in [ + Some(&InstallSource::Brew), + Some(&InstallSource::CurlPipe), + Some(&InstallSource::Cargo), + None, + ] { + match select_installed_probe(src, Some("amp-acp")) { + InstalledProbe::Cli(args) => assert_eq!(args, &["--version"]), + _ => panic!("non-npm source {src:?} should select Cli probe"), + } + } + } +} diff --git a/crates/doctor/src/lib.rs b/crates/doctor/src/lib.rs index b3608e73..30f9b519 100644 --- a/crates/doctor/src/lib.rs +++ b/crates/doctor/src/lib.rs @@ -6,17 +6,25 @@ pub mod agents; pub mod checks; +pub(crate) mod freshness; +pub(crate) mod package_ids; pub mod resolve; pub mod types; -pub use types::{CheckStatus, DoctorCheck, DoctorReport, FixType}; +pub use types::{AgentVersionInfo, CheckStatus, DoctorCheck, DoctorReport, FixType}; use std::collections::HashMap; +use std::path::PathBuf; +use std::sync::{Arc, Mutex}; -use agents::{check_single_ai_agent, lookup_fix_command, AI_AGENT_CHECKS}; +use agents::{check_single_ai_agent, derive_update_command, lookup_fix_command, AI_AGENT_CHECKS}; use checks::{check_clonefile, check_gh, check_gh_auth, check_git, check_git_lfs}; +use freshness::{ + fetch_version_info, is_self_updating, load_cache, save_cache, select_installed_probe, +}; +use package_ids::{lookup_package_id, LatestSource, Role}; use resolve::resolve_binary; -use types::ResolvedBinary; +use types::{InstallSource, ResolvedBinary}; /// Fallback check returned when a spawn_blocking task panics. fn empty_check(id: &str, label: &str) -> DoctorCheck { @@ -31,11 +39,56 @@ fn empty_check(id: &str, label: &str) -> DoctorCheck { path: None, bridge_path: None, raw_output: None, + auth_status: None, + installed_version: None, + latest_version: None, + update_available: None, + install_source: None, + self_updating: None, + main: None, + bridge: None, } } -/// Run all health checks and return the report. +/// Options controlling optional, slower passes layered on top of the core +/// check set. Defaults preserve the original [`run_checks`] behavior — no +/// network, no extra subprocess fan-out. +#[derive(Debug, Clone, Default)] +pub struct RunChecksOptions { + /// If true, populate `installed_version`/`latest_version`/`update_available` + /// on each check by probing the binary and the relevant registry. + pub check_freshness: bool, + /// If true (in combination with `check_freshness`), skip the remote + /// registry lookups — only the local installed-version probe runs. + pub offline: bool, + /// Optional npm registry override. When `Some`, every npm-backed + /// install/probe command (`npm install -g …`, `npm view …`) is routed + /// through this registry via `--registry=`. The URL is always + /// caller-supplied; the crate bakes in no registry of its own. `None` + /// (the default) reproduces the original commands exactly. + pub npm_registry: Option, +} + +/// Run all health checks and return the report. Equivalent to +/// `run_checks_with_options(RunChecksOptions::default())`. pub async fn run_checks() -> DoctorReport { + run_checks_with_options(RunChecksOptions::default()).await +} + +/// Run all health checks with explicit options. Existing callers that want +/// the cheap, no-network path should keep using [`run_checks`]. +pub async fn run_checks_with_options(opts: RunChecksOptions) -> DoctorReport { + let npm_registry = opts.npm_registry.as_deref(); + let report = collect_base_report(npm_registry).await; + + if opts.check_freshness { + populate_freshness(report, opts.offline, npm_registry).await + } else { + report + } +} + +async fn collect_base_report(npm_registry: Option<&str>) -> DoctorReport { let mut binary_names: Vec<&'static str> = vec!["git", "gh", "git-lfs"]; for info in AI_AGENT_CHECKS { for cmd in info.commands { @@ -65,6 +118,7 @@ pub async fn run_checks() -> DoctorReport { let fallback = ResolvedBinary { path: None, search_output: "resolution task panicked".to_string(), + install_source: None, }; let r_git = resolved .get("git") @@ -98,10 +152,12 @@ pub async fn run_checks() -> DoctorReport { let c_git_lfs = tokio::task::spawn_blocking(move || check_git_lfs(&git_r2, &git_lfs_r)); let c_clonefile = tokio::task::spawn_blocking(move || check_clonefile(&git_r3)); + let npm_registry_owned = npm_registry.map(|s| s.to_string()); let agent_handles: Vec<_> = AI_AGENT_CHECKS .iter() .map(|info| { let found = any_agent_found; + let registry = npm_registry_owned.clone(); let cmds: Vec = info .commands .iter() @@ -114,7 +170,7 @@ pub async fn run_checks() -> DoctorReport { .collect(); let main = info.main_command.and_then(|cmd| resolved.get(cmd).cloned()); tokio::task::spawn_blocking(move || { - check_single_ai_agent(info, found, &cmds, main.as_ref()) + check_single_ai_agent(info, found, &cmds, main.as_ref(), registry.as_deref()) }) }) .collect(); @@ -142,51 +198,795 @@ pub async fn run_checks() -> DoctorReport { DoctorReport { checks } } +/// Which binary behind a check a freshness probe targets. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +enum ReadoutSlot { + /// The agent's own CLI (or, for agents without a separate bridge, the + /// single resolved binary). Maps to `DoctorCheck.path` / `DoctorCheck.main`. + Main, + /// The agent's ACP bridge. Maps to `DoctorCheck.bridge_path` / + /// `DoctorCheck.bridge`. + Bridge, + /// A non-agent check (git, gh, …) with no main/bridge split. Maps directly + /// to the flat version fields on `DoctorCheck`. + Flat, +} + +/// Resolve the `(package_id, latest_source)` pair for a check + install source. +/// `None`/`None` when the source is missing or the check has no matching entry. +fn resolve_package( + check_id: &str, + source: Option<&InstallSource>, + role: Role, +) -> (Option, Option) { + source + .cloned() + .and_then(|src| lookup_package_id(check_id, src, role)) + .map(|(pkg, latest)| (Some(pkg.to_string()), Some(latest))) + .unwrap_or((None, None)) +} + +/// Fold a freshness [`freshness::VersionInfo`] into a per-binary readout, +/// applying the self-updating suppression rule for `update_available`. When an +/// update is actionable (and the slot is `Main`/`Bridge`), derive the +/// source-aware `update_command` + `update_fix_type` from the readout's install +/// source and the supplied package id. The flat (non-agent) slot never gets an +/// update command — non-agent updates are out of scope. +fn apply_freshness( + readout: &mut AgentVersionInfo, + info: &freshness::VersionInfo, + slot: ReadoutSlot, + package_id: Option<&str>, +) { + readout.installed_version = info.installed.clone(); + readout.latest_version = info.latest.clone(); + // Self-updating tools (curl/native installers) manage their own freshness: + // report installed/latest for display, but never raise an "update available" + // nag — the update isn't the user's to action. + let self_updating = is_self_updating(readout.install_source.as_ref()); + readout.self_updating = Some(self_updating); + readout.update_available = if self_updating { + None + } else { + info.update_available + }; + + let actionable = readout.update_available == Some(true) && !self_updating; + let slot_fix_type = match slot { + ReadoutSlot::Main => Some(FixType::UpdateMain), + ReadoutSlot::Bridge => Some(FixType::UpdateBridge), + ReadoutSlot::Flat => None, + }; + if let (true, Some(fix_type)) = (actionable, slot_fix_type) { + if let Some(cmd) = derive_update_command(readout.install_source.as_ref(), package_id) { + readout.update_command = Some(cmd); + readout.update_fix_type = Some(fix_type); + } + } +} + +/// Post-hoc pass: for every check that has a usable binary path and a known +/// package id, run the installed/latest version probes in parallel and update +/// the corresponding fields on the report. The on-disk cache is read once at +/// the start and written once at the end. +/// +/// Agent checks front up to two independent binaries — the agent CLI (`main`) +/// and its ACP bridge (`bridge`) — and each is probed and reported separately. +/// The flat version fields are kept in sync for backward compatibility: they +/// mirror the bridge readout when a bridge exists, otherwise the main readout +/// (the same headline the pre-split pass produced). Non-agent checks keep +/// writing straight to the flat fields. +async fn populate_freshness( + mut report: DoctorReport, + offline: bool, + npm_registry: Option<&str>, +) -> DoctorReport { + let cache = Arc::new(Mutex::new(load_cache())); + let npm_registry = npm_registry.map(|s| s.to_string()); + + let mut targets: Vec = Vec::new(); + for check in &report.checks { + let is_agent = check.main.is_some() || check.bridge.is_some(); + if is_agent { + if let (Some(readout), Some(path)) = (&check.main, check.path.as_deref()) { + let (package_id, latest_source) = + resolve_package(&check.id, readout.install_source.as_ref(), Role::Main); + targets.push(FreshnessTarget { + id: check.id.clone(), + slot: ReadoutSlot::Main, + path: PathBuf::from(path), + latest_source, + package_id, + install_source: readout.install_source.clone(), + }); + } + if let (Some(readout), Some(path)) = (&check.bridge, check.bridge_path.as_deref()) { + let (package_id, latest_source) = + resolve_package(&check.id, readout.install_source.as_ref(), Role::Bridge); + targets.push(FreshnessTarget { + id: check.id.clone(), + slot: ReadoutSlot::Bridge, + path: PathBuf::from(path), + latest_source, + package_id, + install_source: readout.install_source.clone(), + }); + } + } else { + // Non-agent check: prefer the bridge path when present (matches the + // install_source the check carries — see check_single_ai_agent). + let path_str = check.bridge_path.as_deref().or(check.path.as_deref()); + let Some(path_str) = path_str else { continue }; + let (package_id, latest_source) = + resolve_package(&check.id, check.install_source.as_ref(), Role::Any); + targets.push(FreshnessTarget { + id: check.id.clone(), + slot: ReadoutSlot::Flat, + path: PathBuf::from(path_str), + latest_source, + package_id, + install_source: check.install_source.clone(), + }); + } + } + + let futures = targets.into_iter().map(|t| { + let cache = cache.clone(); + let npm_registry = npm_registry.clone(); + async move { + // npm-distributed bridges don't honor `--version`; read their + // installed version straight from the owning `package.json`. + let probe = select_installed_probe(t.install_source.as_ref(), t.package_id.as_deref()); + let info = fetch_version_info( + t.latest_source, + t.package_id.as_deref(), + &t.path, + probe, + offline, + npm_registry.as_deref(), + cache, + ) + .await; + ((t.id, t.slot), (info, t.package_id)) + } + }); + + let results = futures::future::join_all(futures).await; + let mut by_target: HashMap<(String, ReadoutSlot), (freshness::VersionInfo, Option)> = + HashMap::new(); + for (key, payload) in results { + by_target.insert(key, payload); + } + + for check in &mut report.checks { + let is_agent = check.main.is_some() || check.bridge.is_some(); + if is_agent { + if let (Some(readout), Some((info, pkg))) = ( + check.main.as_mut(), + by_target.remove(&(check.id.clone(), ReadoutSlot::Main)), + ) { + apply_freshness(readout, &info, ReadoutSlot::Main, pkg.as_deref()); + } + if let (Some(readout), Some((info, pkg))) = ( + check.bridge.as_mut(), + by_target.remove(&(check.id.clone(), ReadoutSlot::Bridge)), + ) { + apply_freshness(readout, &info, ReadoutSlot::Bridge, pkg.as_deref()); + } + // Mirror the headline readout (bridge if present, else main) into the + // flat fields for backward-compatible consumers. + let headline = check.bridge.as_ref().or(check.main.as_ref()).map(|r| { + ( + r.installed_version.clone(), + r.latest_version.clone(), + r.update_available, + r.self_updating, + ) + }); + if let Some((installed, latest, update_available, self_updating)) = headline { + check.installed_version = installed; + check.latest_version = latest; + check.update_available = update_available; + check.self_updating = self_updating; + } + } else if let Some((info, _pkg)) = by_target.remove(&(check.id.clone(), ReadoutSlot::Flat)) + { + check.installed_version = info.installed; + check.latest_version = info.latest; + let self_updating = is_self_updating(check.install_source.as_ref()); + check.self_updating = Some(self_updating); + check.update_available = if self_updating { + None + } else { + info.update_available + }; + } + } + + if let Ok(guard) = cache.lock() { + save_cache(&guard); + } + + report +} + +struct FreshnessTarget { + id: String, + slot: ReadoutSlot, + path: PathBuf, + latest_source: Option, + package_id: Option, + install_source: Option, +} + /// Run a fix command for a doctor check, identified by check ID and fix type. /// /// The actual shell command is looked up from the static check definitions — /// the caller never sends a raw command string. pub async fn execute_fix(check_id: String, fix_type: FixType) -> Result<(), String> { - let command = lookup_fix_command(&check_id, &fix_type) - .ok_or_else(|| format!("Unknown check '{check_id}' or fix type '{fix_type:?}'"))?; + execute_fix_streaming(check_id, fix_type, |_| {}).await +} - execute_command(command).await +/// Like [`execute_fix`], but routes npm-backed fix commands through an optional +/// caller-supplied registry (see [`RunChecksOptions::npm_registry`]) and +/// optionally accepts a `command_override` — the exact shell command to run, +/// bypassing the static [`lookup_fix_command`] lookup. The override is the only +/// way to dispatch the per-readout `FixType::UpdateMain` / `UpdateBridge` +/// commands, which aren't in the static table. +/// +/// When `command_override` is `Some`, `fix_type` is informational only; the +/// override is always used. When `None`, the command is looked up exactly as +/// before. `apply_npm_registry` runs over the final command string regardless +/// of where it came from. +pub async fn execute_fix_with_options( + check_id: String, + fix_type: FixType, + command_override: Option, + npm_registry: Option<&str>, +) -> Result<(), String> { + execute_fix_streaming_with_options(check_id, fix_type, command_override, npm_registry, |_| {}) + .await } -/// Run an arbitrary shell command in a login shell. +/// Run a fix command and stream its output line-by-line to `on_line`. +/// +/// `on_line` is invoked once per output line, with the trailing newline +/// stripped. Both stdout and stderr lines are delivered through the same +/// callback; ordering across the two streams is best-effort (each stream is +/// in order internally, but interleaving between them depends on scheduling). /// -/// Internal primitive used by [`execute_fix`]. Not exposed publicly — callers -/// should use `execute_fix` which looks up the command from static definitions. -pub(crate) async fn execute_command(command: String) -> Result<(), String> { - tokio::task::spawn_blocking(move || { - let (shell, args) = if std::path::Path::new("/bin/zsh").exists() { - ("/bin/zsh", vec!["-l", "-c", &command]) +/// The callback runs on the tokio runtime's blocking pool — do not block in it +/// for unbounded periods, but emitting Tauri events / writing to a channel is +/// fine. +pub async fn execute_fix_streaming( + check_id: String, + fix_type: FixType, + on_line: F, +) -> Result<(), String> +where + F: FnMut(&str) + Send + 'static, +{ + execute_fix_streaming_with_options(check_id, fix_type, None, None, on_line).await +} + +/// Like [`execute_fix_streaming`], but accepts a `command_override` to run an +/// exact shell command (skipping [`lookup_fix_command`]) and routes npm-backed +/// commands through an optional caller-supplied registry. See +/// [`execute_fix_with_options`] for the semantics of `command_override`. +pub async fn execute_fix_streaming_with_options( + check_id: String, + fix_type: FixType, + command_override: Option, + npm_registry: Option<&str>, + mut on_line: F, +) -> Result<(), String> +where + F: FnMut(&str) + Send + 'static, +{ + let command = match command_override { + Some(cmd) => cmd, + None => lookup_fix_command(&check_id, &fix_type) + .ok_or_else(|| format!("Unknown check '{check_id}' or fix type '{fix_type:?}'"))?, + }; + let command = agents::apply_npm_registry(&command, npm_registry); + + // Echo the resolved command as a preamble line so downstream callers (e.g. + // goose-internal's `run_fix`, which `info!`s every callback line and emits + // it via the `agent-setup:output` Tauri event) record *what* ran, not just + // its output. Matches the `$ ` phrasing the auth probe already + // writes into `raw_output`. + on_line(&format!("$ {command}")); + + run_command_streaming(command, on_line).await +} + +/// Async wrapper that runs `run_command_streaming_blocking` on the blocking pool. +pub(crate) async fn run_command_streaming(command: String, on_line: F) -> Result<(), String> +where + F: FnMut(&str) + Send + 'static, +{ + tokio::task::spawn_blocking(move || run_command_streaming_blocking(&command, on_line)) + .await + .unwrap_or_else(|e| Err(format!("Task failed: {e}"))) +} + +enum StreamLine { + Stdout(String), + Stderr(String), +} + +/// Build the login-shell `Command` used by every doctor exec path. Optionally +/// prepends `path_prefix` directories to the child `PATH` so a binary that's +/// only findable in the resolved location (e.g. an nvm bin dir) is visible to +/// the spawned shell even when the parent process was launched with a +/// restricted `PATH` (the macOS Finder/launchd case). When `path_prefix` is +/// empty the env layout is byte-identical to the previous implementation, so +/// existing call sites are unaffected. +fn build_shell_command(command: &str, path_prefix: &[PathBuf]) -> std::process::Command { + let (shell, args) = if std::path::Path::new("/bin/zsh").exists() { + ("/bin/zsh", vec!["-l", "-c", command]) + } else { + ("/bin/bash", vec!["-l", "-c", command]) + }; + let home = std::env::var("HOME").unwrap_or_else(|_| "/".to_string()); + let user = std::env::var("USER").unwrap_or_default(); + + let mut cmd = std::process::Command::new(shell); + cmd.args(&args) + .env_clear() + .env("HOME", &home) + .env("USER", &user) + .env("TERM", "xterm-256color") + .current_dir(&home); + if !path_prefix.is_empty() { + let mut seen: std::collections::HashSet = std::collections::HashSet::new(); + let mut path = String::new(); + for p in path_prefix { + let s = p.to_string_lossy().to_string(); + if !seen.insert(s.clone()) { + continue; + } + if !path.is_empty() { + path.push(':'); + } + path.push_str(&s); + } + // Conservative fallback ~ what login zsh on macOS sees before + // /etc/zprofile augments it. Keeps the rest of the resolved-binary + // dir's command graph reachable (e.g. node, npm) without depending on + // the parent process's PATH. + path.push_str(":/usr/local/bin:/opt/homebrew/bin:/usr/bin:/bin:/usr/sbin:/sbin"); + cmd.env("PATH", path); + } + cmd +} + +/// Detailed outcome of running a command. Lets the caller distinguish +/// `command not found` (exit 127 from the shell, or a spawn failure on the +/// shell itself) from a genuine non-zero exit — important for the auth probe, +/// where the former means "we can't tell" and the latter means "not signed in". +#[derive(Debug)] +pub(crate) enum ExecOutcome { + Ok, + /// The shell itself couldn't be spawned (or `wait()` failed). Rare; means + /// we have no signal at all from the inner command. + Spawn(std::io::Error), + /// The shell ran, the inner command exited non-zero. Code is `Some(127)` + /// when the shell reports "command not found". + Exit { + code: Option, + stderr: String, + }, +} + +/// Run `command` through a login shell, with the caller-supplied `path_prefix` +/// prepended to the child `PATH`. Returns the detailed exec outcome. No +/// streaming — used by the auth probe which wants a single sync result. +pub(crate) fn execute_command_with_path_prefix( + command: &str, + path_prefix: &[PathBuf], +) -> ExecOutcome { + let mut cmd = build_shell_command(command, path_prefix); + cmd.stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()); + let child = match cmd.spawn() { + Ok(c) => c, + Err(e) => return ExecOutcome::Spawn(e), + }; + let output = match child.wait_with_output() { + Ok(o) => o, + Err(e) => return ExecOutcome::Spawn(e), + }; + if output.status.success() { + ExecOutcome::Ok + } else { + ExecOutcome::Exit { + code: output.status.code(), + stderr: String::from_utf8_lossy(&output.stderr).to_string(), + } + } +} + +/// Spawn `command` through a login shell, stream stdout/stderr lines to +/// `on_line`, and return based on the process exit status. Stderr lines are +/// also accumulated so a non-zero exit can surface a useful error message +/// (matching the non-streaming behavior of the previous `execute_command`). +fn run_command_streaming_blocking(command: &str, mut on_line: F) -> Result<(), String> +where + F: FnMut(&str), +{ + use std::io::{BufRead, BufReader}; + + let mut child = build_shell_command(command, &[]) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .spawn() + .map_err(|e| format!("Failed to run command: {e}"))?; + + let stdout = child.stdout.take().expect("stdout was piped"); + let stderr = child.stderr.take().expect("stderr was piped"); + + let (tx, rx) = std::sync::mpsc::channel::(); + let tx_err = tx.clone(); + + let stdout_thread = std::thread::spawn(move || { + let reader = BufReader::new(stdout); + for line in reader.lines().map_while(Result::ok) { + if tx.send(StreamLine::Stdout(line)).is_err() { + break; + } + } + }); + let stderr_thread = std::thread::spawn(move || { + let reader = BufReader::new(stderr); + for line in reader.lines().map_while(Result::ok) { + if tx_err.send(StreamLine::Stderr(line)).is_err() { + break; + } + } + }); + + let mut stderr_accum = String::new(); + for msg in rx.iter() { + match msg { + StreamLine::Stdout(s) => { + on_line(&s); + } + StreamLine::Stderr(s) => { + on_line(&s); + if !stderr_accum.is_empty() { + stderr_accum.push('\n'); + } + stderr_accum.push_str(&s); + } + } + } + + let _ = stdout_thread.join(); + let _ = stderr_thread.join(); + + let status = child + .wait() + .map_err(|e| format!("Failed to wait for command: {e}"))?; + + if status.success() { + Ok(()) + } else { + let trimmed = stderr_accum.trim().to_string(); + Err(if trimmed.is_empty() { + format!("Command failed with exit code {status}") } else { - ("/bin/bash", vec!["-l", "-c", &command]) + trimmed + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + use std::sync::{Arc, Mutex}; + + /// Streaming helper must invoke `on_line` for each output line of a + /// successful command. Lines from `.zshrc` etc. may also appear; we only + /// assert that our expected payload showed up. + #[tokio::test] + async fn run_command_streaming_emits_each_stdout_line() { + let lines: Arc>> = Arc::new(Mutex::new(Vec::new())); + let lines_clone = lines.clone(); + + let result = run_command_streaming( + "echo doctor-streaming-marker-hello && echo doctor-streaming-marker-world".to_string(), + move |line| { + lines_clone.lock().unwrap().push(line.to_string()); + }, + ) + .await; + + assert!(result.is_ok(), "streaming command failed: {result:?}"); + let captured = lines.lock().unwrap().clone(); + assert!( + captured + .iter() + .any(|l| l == "doctor-streaming-marker-hello"), + "did not see 'hello' marker; captured: {captured:?}", + ); + assert!( + captured + .iter() + .any(|l| l == "doctor-streaming-marker-world"), + "did not see 'world' marker; captured: {captured:?}", + ); + } + + /// `execute_fix(|_| {})` and `execute_fix_streaming(.., |_| {})` must + /// produce identical results for the same fix lookup — `execute_fix` is + /// supposed to be a thin delegate. + #[tokio::test] + async fn execute_fix_delegates_to_streaming() { + let direct = execute_fix("ai-agent-nonexistent".to_string(), FixType::Auth).await; + let streamed = + execute_fix_streaming("ai-agent-nonexistent".to_string(), FixType::Auth, |_| {}).await; + assert_eq!(direct, streamed); + } + + /// A command not found by the shell must surface as `Exit { code: 127, .. }` + /// — the auth probe maps that to `AuthStatus::Unknown` instead of + /// `NotAuthenticated`. Using an unambiguously-nonexistent command name so + /// the shell hits its "command not found" path regardless of rc files. + #[tokio::test] + async fn exec_command_not_found_reports_exit_127() { + let outcome = tokio::task::spawn_blocking(|| { + execute_command_with_path_prefix("doctor-nonexistent-xyz-12345", &[]) + }) + .await + .unwrap(); + match outcome { + ExecOutcome::Exit { code, .. } => assert_eq!( + code, + Some(127), + "expected exit 127 for command-not-found; got {code:?}", + ), + other => panic!("expected Exit(127); got {other:?}"), + } + } + + /// PATH prefix lets the spawned shell find a command that's only in the + /// supplied dir — without it, the same command would exit 127. This is the + /// PATH-shadowing fix in miniature. + #[tokio::test] + async fn exec_command_with_path_prefix_finds_command_in_prefix_dir() { + use std::os::unix::fs::PermissionsExt; + + let tmp = std::env::temp_dir().join(format!( + "doctor-pathprefix-{}-{}", + std::process::id(), + // Coarse uniqueness — enough for this test. + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos(), + )); + std::fs::create_dir_all(&tmp).unwrap(); + // Distinct, unguessable name so no real PATH entry could shadow it. + let script_name = "doctor-pathprefix-probe-abcdef"; + let script = tmp.join(script_name); + std::fs::write(&script, "#!/bin/sh\nexit 0\n").unwrap(); + let mut perms = std::fs::metadata(&script).unwrap().permissions(); + perms.set_mode(0o755); + std::fs::set_permissions(&script, perms).unwrap(); + + let prefix = vec![tmp.clone()]; + let outcome = tokio::task::spawn_blocking(move || { + execute_command_with_path_prefix(script_name, &prefix) + }) + .await + .unwrap(); + let _ = std::fs::remove_dir_all(&tmp); + match outcome { + ExecOutcome::Ok => {} + other => { + panic!("expected Ok with the script reachable via path prefix; got {other:?}",) + } + } + } + + /// `apply_freshness` derives a source-aware update command for a Main slot + /// when the npm-installed agent has an actionable update. + #[tokio::test] + async fn apply_freshness_npm_main_emits_update_main_command() { + let mut readout = AgentVersionInfo { + install_source: Some(InstallSource::Npm), + ..AgentVersionInfo::default() }; - let home = std::env::var("HOME").unwrap_or_else(|_| "/".to_string()); - let user = std::env::var("USER").unwrap_or_default(); - let output = std::process::Command::new(shell) - .args(&args) - .env_clear() - .env("HOME", &home) - .env("USER", &user) - .env("TERM", "xterm-256color") - .current_dir(&home) - .output() - .map_err(|e| format!("Failed to run command: {e}"))?; - - if output.status.success() { - Ok(()) - } else { - let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string(); - Err(if stderr.is_empty() { - format!("Command failed with exit code {}", output.status) - } else { - stderr - }) + let info = freshness::VersionInfo { + installed: Some("0.1.0".into()), + latest: Some("0.2.0".into()), + update_available: Some(true), + }; + apply_freshness( + &mut readout, + &info, + ReadoutSlot::Main, + Some("@anthropic-ai/claude-code"), + ); + assert_eq!( + readout.update_command.as_deref(), + Some("npm install -g @anthropic-ai/claude-code@latest"), + ); + assert_eq!(readout.update_fix_type, Some(FixType::UpdateMain)); + } + + /// Bridge slot with a brew install upgrades via `brew upgrade ` and + /// is tagged `UpdateBridge`. + #[tokio::test] + async fn apply_freshness_brew_bridge_emits_update_bridge_command() { + let mut readout = AgentVersionInfo { + install_source: Some(InstallSource::Brew), + ..AgentVersionInfo::default() + }; + let info = freshness::VersionInfo { + installed: Some("0.1.0".into()), + latest: Some("0.2.0".into()), + update_available: Some(true), + }; + apply_freshness(&mut readout, &info, ReadoutSlot::Bridge, Some("amp")); + assert_eq!(readout.update_command.as_deref(), Some("brew upgrade amp"),); + assert_eq!(readout.update_fix_type, Some(FixType::UpdateBridge)); + } + + /// Self-updating (CurlPipe) readouts never get an update command, even when + /// upstream reports a newer version — `is_self_updating` suppresses both + /// `update_available` and the derived update command. + #[tokio::test] + async fn apply_freshness_curl_pipe_never_emits_update_command() { + let mut readout = AgentVersionInfo { + install_source: Some(InstallSource::CurlPipe), + ..AgentVersionInfo::default() + }; + let info = freshness::VersionInfo { + installed: Some("1.0.0".into()), + latest: Some("2.0.0".into()), + update_available: Some(true), + }; + apply_freshness( + &mut readout, + &info, + ReadoutSlot::Main, + Some("getcursor/cursor"), + ); + assert!(readout.update_command.is_none()); + assert!(readout.update_fix_type.is_none()); + assert_eq!(readout.self_updating, Some(true)); + assert!( + readout.update_available.is_none(), + "self-updating readout suppresses update_available too", + ); + } + + /// No update available -> no update command, even on a registry source. + #[tokio::test] + async fn apply_freshness_no_update_available_emits_no_command() { + let mut readout = AgentVersionInfo { + install_source: Some(InstallSource::Npm), + ..AgentVersionInfo::default() + }; + let info = freshness::VersionInfo { + installed: Some("0.2.0".into()), + latest: Some("0.2.0".into()), + update_available: Some(false), + }; + apply_freshness(&mut readout, &info, ReadoutSlot::Main, Some("amp-acp")); + assert!(readout.update_command.is_none()); + assert!(readout.update_fix_type.is_none()); + } + + /// `command_override` makes the executor run the exact string supplied, + /// bypassing `lookup_fix_command`. We test by overriding for the + /// `UpdateMain` variant — which has no static lookup — so a successful + /// run can only be the override path. + #[tokio::test] + async fn execute_fix_with_options_runs_command_override() { + let result = execute_fix_with_options( + "ai-agent-claude".to_string(), + FixType::UpdateMain, + Some("true".to_string()), + None, + ) + .await; + assert!( + result.is_ok(), + "override should execute `true` and exit 0; got {result:?}", + ); + } + + /// The streaming executor must emit a `$ ` preamble line + /// through the callback *before* any subprocess output, so downstream + /// callers record what ran. Exercises the `command_override` branch + /// (the codepath `UpdateMain`/`UpdateBridge` clicks take, since they have + /// no static recipe). + #[tokio::test] + async fn execute_fix_streaming_emits_command_preamble_first() { + let lines: Arc>> = Arc::new(Mutex::new(Vec::new())); + let lines_clone = lines.clone(); + + let result = execute_fix_streaming_with_options( + "ai-agent-claude".to_string(), + FixType::UpdateMain, + Some("echo hello".to_string()), + None, + move |line| { + lines_clone.lock().unwrap().push(line.to_string()); + }, + ) + .await; + + assert!(result.is_ok(), "override should execute; got {result:?}"); + let captured = lines.lock().unwrap().clone(); + assert_eq!( + captured.first().map(String::as_str), + Some("$ echo hello"), + "first callback line must be the command preamble; captured: {captured:?}", + ); + assert!( + captured.iter().any(|l| l == "hello"), + "subprocess output line should follow the preamble; captured: {captured:?}", + ); + } + + /// Without a `command_override`, `UpdateMain` has no static recipe and + /// must surface as the standard "Unknown check / fix type" error. + #[tokio::test] + async fn execute_fix_with_options_update_without_override_errors() { + let result = execute_fix_with_options( + "ai-agent-claude".to_string(), + FixType::UpdateMain, + None, + None, + ) + .await; + assert!(result.is_err()); + } + + /// Default `run_checks()` must not populate any of the version-freshness + /// fields — guards against accidentally flipping the default to on + /// (which would slow down every staged Tauri app launch). + #[tokio::test] + async fn run_checks_default_leaves_freshness_fields_empty() { + let report = run_checks().await; + for check in &report.checks { + assert!( + check.installed_version.is_none(), + "check {} unexpectedly populated installed_version = {:?}", + check.id, + check.installed_version, + ); + assert!( + check.latest_version.is_none(), + "check {} unexpectedly populated latest_version = {:?}", + check.id, + check.latest_version, + ); + assert!( + check.update_available.is_none(), + "check {} unexpectedly populated update_available = {:?}", + check.id, + check.update_available, + ); + // The split main/bridge readouts may carry an install source on the + // cheap path, but never version fields — those are freshness-only. + for (slot, readout) in [("main", &check.main), ("bridge", &check.bridge)] { + if let Some(r) = readout { + assert!( + r.installed_version.is_none() + && r.latest_version.is_none() + && r.update_available.is_none() + && r.self_updating.is_none(), + "check {} {slot} readout unexpectedly populated version fields: {r:?}", + check.id, + ); + } + } } - }) - .await - .unwrap_or_else(|e| Err(format!("Task failed: {e}"))) + } } diff --git a/crates/doctor/src/package_ids.rs b/crates/doctor/src/package_ids.rs new file mode 100644 index 00000000..34ded056 --- /dev/null +++ b/crates/doctor/src/package_ids.rs @@ -0,0 +1,281 @@ +//! Per-check package identifiers for version-freshness lookups. +//! +//! Maps a doctor check ID to one or more `(InstallSource, package_id, +//! LatestSource, Role)` entries. The freshness module picks the entry whose +//! `InstallSource` matches the binary's detected install source AND whose +//! `Role` matches the readout being built (the agent's main CLI vs. its ACP +//! bridge), then fetches the latest version via the entry's `LatestSource`. +//! Checks not listed here (or with no matching source) skip the latest-version +//! probe. + +use crate::types::InstallSource; + +/// How to fetch the "latest available" version for a package. +/// +/// For registry installs this mirrors the install source 1:1 (a brew binary +/// checks brew, an npm binary checks npm). Non-registry installs (curl/native) +/// need an explicit mechanism that doesn't follow from the install source — +/// e.g. Cursor's curl install has no registry presence and is tracked via +/// GitHub releases, so its `CurlPipe` entry fetches from `GitHubReleases`. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum LatestSource { + Brew, + Npm, + /// Retained so a future cargo-installed tool can reach `latest_crates_io`; + /// no entry in `PACKAGE_IDS` selects it yet (no agent ships via crates.io). + #[allow(dead_code)] + CratesIo, + GitHubReleases, +} + +/// Which binary an entry describes. An AI-agent check fronts up to two distinct +/// binaries — the agent's own CLI (`Main`) and its ACP bridge (`Bridge`). When +/// both are installed from the same registry (e.g. both via npm, as with +/// Claude) the install source alone is ambiguous and the role is what +/// disambiguates them. Non-agent checks (and agents whose two binaries already +/// have distinct install sources) use `Any`. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum Role { + Main, + Bridge, + /// Entry applies to either readout (and to flat, non-agent checks). + Any, +} + +impl Role { + /// `query` matches `entry` when: + /// - the entry's role is `Any` (it applies anywhere), or + /// - the query is `Any` (the caller doesn't care which role), or + /// - both roles are equal. + fn matches(entry: Role, query: Role) -> bool { + matches!(entry, Role::Any) || matches!(query, Role::Any) || entry == query + } +} + +/// One freshness entry: the install source it applies to, the package id to +/// query, how to fetch that package's latest version, and which readout (main +/// CLI vs ACP bridge) it describes. +type PackageEntry = (InstallSource, &'static str, LatestSource, Role); + +/// Static table of `check_id -> &[PackageEntry]`. +/// +/// A single check can have multiple entries when the same agent ships through +/// different registries (e.g. brew for the main binary, npm for the ACP bridge) +/// or when both binaries share an install source but have distinct package ids +/// (Claude: `@anthropic-ai/claude-code` for the main CLI, `@agentclientprotocol +/// /claude-agent-acp` for the bridge — both npm). `lookup_package_id` picks the +/// first entry whose `(install_source, role)` matches the query. +pub(crate) const PACKAGE_IDS: &[(&str, &[PackageEntry])] = &[ + ( + "git", + &[(InstallSource::Brew, "git", LatestSource::Brew, Role::Any)], + ), + ( + "gh", + &[(InstallSource::Brew, "gh", LatestSource::Brew, Role::Any)], + ), + ( + "git-lfs", + &[( + InstallSource::Brew, + "git-lfs", + LatestSource::Brew, + Role::Any, + )], + ), + // ai-agent-goose: brew tap exists but no canonical formula yet — skip. + // TODO: revisit when block/goose lands a stable brew formula. + ( + "ai-agent-claude", + &[ + // Main CLI when installed via npm (e.g. under nvm). The native + // curl-pipe install is fingerprinted as `CurlPipe` (no registry + // entry here, self-updating) so this only applies when Claude + // landed via `npm i -g @anthropic-ai/claude-code`. + ( + InstallSource::Npm, + "@anthropic-ai/claude-code", + LatestSource::Npm, + Role::Main, + ), + // ACP bridge — separate npm package. + ( + InstallSource::Npm, + "@agentclientprotocol/claude-agent-acp", + LatestSource::Npm, + Role::Bridge, + ), + ], + // TODO: the main `claude` native (CurlPipe) install has no registry + // entry — its latest is published via the native installer's channel + // manifest, which we don't parse yet. Claude native is self-updating + // (see `freshness::is_self_updating`), so it stays report-only for now. + ), + ( + "ai-agent-codex", + &[ + // Bridge ships via npm; main CLI via brew — distinct install + // sources so role disambiguation isn't strictly needed, but tag it + // anyway for clarity. + ( + InstallSource::Npm, + "@zed-industries/codex-acp", + LatestSource::Npm, + Role::Bridge, + ), + (InstallSource::Brew, "codex", LatestSource::Brew, Role::Main), + ], + ), + ( + "ai-agent-pi", + &[( + InstallSource::Npm, + "pi-acp", + LatestSource::Npm, + Role::Bridge, + )], + ), + ( + "ai-agent-amp", + &[ + // Bridge: npm. Main: brew. Main curl-pipe install is `CurlPipe`, + // not present in the table (self-updating, report-only). + ( + InstallSource::Npm, + "amp-acp", + LatestSource::Npm, + Role::Bridge, + ), + (InstallSource::Brew, "amp", LatestSource::Brew, Role::Main), + ], + ), + ( + "ai-agent-copilot", + &[( + InstallSource::Npm, + "@github/copilot", + LatestSource::Npm, + Role::Any, + )], + ), + // ai-agent-cursor: curl-pipe installer with no registry presence; its + // releases are published on GitHub. The repo slug is a best-effort default + // and the GitHub fetcher degrades to `None` on a miss. Cursor is + // self-updating, so this latest is report-only (no update nag). + ( + "ai-agent-cursor", + &[( + InstallSource::CurlPipe, + "getcursor/cursor", + LatestSource::GitHubReleases, + Role::Any, + )], + ), +]; + +/// Pick the package id and its latest-version source for the entry whose +/// `(install_source, role)` matches. Returns `None` if the check isn't in the +/// table, or if no entry matches both the source and the role. +pub(crate) fn lookup_package_id( + check_id: &str, + source: InstallSource, + role: Role, +) -> Option<(&'static str, LatestSource)> { + for (id, entries) in PACKAGE_IDS { + if *id == check_id { + for (entry_source, pkg, latest, entry_role) in entries.iter() { + if entry_source == &source && Role::matches(*entry_role, role) { + return Some((*pkg, *latest)); + } + } + return None; + } + } + None +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn lookup_matches_source_with_any_role() { + assert_eq!( + lookup_package_id("git", InstallSource::Brew, Role::Any), + Some(("git", LatestSource::Brew)), + ); + } + + #[test] + fn lookup_returns_none_for_mismatched_source() { + // git is registered under Brew only — Npm should miss. + assert_eq!( + lookup_package_id("git", InstallSource::Npm, Role::Any), + None + ); + } + + #[test] + fn lookup_returns_none_for_unknown_check() { + assert_eq!( + lookup_package_id("nonexistent", InstallSource::Brew, Role::Any), + None + ); + } + + #[test] + fn codex_has_role_tagged_entries() { + assert_eq!( + lookup_package_id("ai-agent-codex", InstallSource::Npm, Role::Bridge), + Some(("@zed-industries/codex-acp", LatestSource::Npm)), + ); + assert_eq!( + lookup_package_id("ai-agent-codex", InstallSource::Brew, Role::Main), + Some(("codex", LatestSource::Brew)), + ); + } + + #[test] + fn cursor_curl_pipe_resolves_to_github_releases() { + let (_, latest) = lookup_package_id("ai-agent-cursor", InstallSource::CurlPipe, Role::Any) + .expect("cursor entry"); + assert_eq!(latest, LatestSource::GitHubReleases); + } + + /// The whole point of the role split: claude's main CLI under npm must + /// resolve to `@anthropic-ai/claude-code`, not the bridge package. + #[test] + fn claude_main_npm_resolves_to_main_package() { + assert_eq!( + lookup_package_id("ai-agent-claude", InstallSource::Npm, Role::Main), + Some(("@anthropic-ai/claude-code", LatestSource::Npm)), + ); + } + + /// And the bridge readout still resolves to the bridge package even though + /// they share an install source. + #[test] + fn claude_bridge_npm_resolves_to_bridge_package() { + assert_eq!( + lookup_package_id("ai-agent-claude", InstallSource::Npm, Role::Bridge), + Some(("@agentclientprotocol/claude-agent-acp", LatestSource::Npm)), + ); + } + + /// A `Role::Any` query on a role-tagged check returns the first matching + /// entry — used by non-agent (flat) lookups and as a permissive fallback. + #[test] + fn claude_npm_with_any_role_returns_first_match() { + // The Main entry appears first in the table; Any should hit it. + let (pkg, _) = lookup_package_id("ai-agent-claude", InstallSource::Npm, Role::Any).unwrap(); + assert_eq!(pkg, "@anthropic-ai/claude-code"); + } + + /// `Role::Any` entries match any query role — confirms copilot's single + /// untagged entry is reachable from both Main and Bridge queries. + #[test] + fn copilot_any_entry_matches_main_and_bridge_queries() { + assert!(lookup_package_id("ai-agent-copilot", InstallSource::Npm, Role::Main).is_some()); + assert!(lookup_package_id("ai-agent-copilot", InstallSource::Npm, Role::Bridge).is_some()); + } +} diff --git a/crates/doctor/src/resolve.rs b/crates/doctor/src/resolve.rs index 89482f4d..d0e1d368 100644 --- a/crates/doctor/src/resolve.rs +++ b/crates/doctor/src/resolve.rs @@ -3,9 +3,10 @@ use std::path::{Path, PathBuf}; use std::process::Command; -use super::types::ResolvedBinary; +use super::types::{InstallSource, ResolvedBinary}; -/// Resolve a binary by trying login shell path lookup then common install paths. +/// Resolve a binary by trying login shell path lookup, common install paths, +/// then npm global install dirs. pub fn resolve_binary(cmd: &str) -> ResolvedBinary { let mut lines = vec![format!("resolve '{cmd}':")]; @@ -30,9 +31,11 @@ pub fn resolve_binary(cmd: &str) -> ResolvedBinary { " {shell} -l -c '{lookup_cmd}' => {} (resolved)", path.display() )); + let install_source = Some(detect_install_source(path)); return ResolvedBinary { path: Some(path.clone()), search_output: lines.join("\n"), + install_source, }; } @@ -56,8 +59,8 @@ pub fn resolve_binary(cmd: &str) -> ResolvedBinary { } } - // Strategy 2: Common install paths (fallback) - lines.push(" strategy 2 — common install paths (fallback):".to_string()); + // Strategy 2: Common install paths (fallback). + lines.push(" strategy 2 — common install paths:".to_string()); for dir in &[ "/opt/homebrew/bin", "/usr/local/bin", @@ -67,9 +70,56 @@ pub fn resolve_binary(cmd: &str) -> ResolvedBinary { let path = PathBuf::from(dir).join(cmd); if is_executable_file(&path) { lines.push(format!(" {} => found (resolved)", path.display())); + let install_source = Some(detect_install_source(&path)); return ResolvedBinary { path: Some(path), search_output: lines.join("\n"), + install_source, + }; + } + lines.push(format!(" {} => not found", path.display())); + } + + // Strategy 3: npm global install dirs. + // + // Mirrors the dirs added by goose-internal's `services::path_env` (and the + // upstream goose `config::search_path::SearchPaths::with_npm`) so that + // bridges installed via `npm install -g` resolve here too. Without this, + // npm-only installs are "found" by goose-internal's ACP inventory but + // reported missing by doctor. + lines.push(" strategy 3 — npm global install dirs:".to_string()); + let home = std::env::home_dir(); + if let Some(home) = home.as_deref() { + for dir in npm_search_dirs(home) { + let path = dir.join(cmd); + if path.exists() { + lines.push(format!(" {} => found (resolved)", path.display())); + let install_source = Some(detect_install_source(&path)); + return ResolvedBinary { + path: Some(path), + search_output: lines.join("\n"), + install_source, + }; + } + lines.push(format!(" {} => not found", path.display())); + } + } else { + lines.push(" (could not determine HOME)".to_string()); + } + + // Strategy 3 last resort: ask npm itself (the most authoritative answer, + // but costs one subprocess — only invoked when the static probes above + // didn't find the binary). `npm prefix -g` is the version-stable + // equivalent of the older `npm bin -g`; the bin dir is `/bin`. + if let Some(npm_bin_dir) = npm_global_bin_dir(&mut lines) { + let path = npm_bin_dir.join(cmd); + if path.exists() { + lines.push(format!(" {} => found (resolved)", path.display())); + let install_source = Some(detect_install_source(&path)); + return ResolvedBinary { + path: Some(path), + search_output: lines.join("\n"), + install_source, }; } lines.push(format!(" {} => not found", path.display())); @@ -79,6 +129,7 @@ pub fn resolve_binary(cmd: &str) -> ResolvedBinary { ResolvedBinary { path: None, search_output: lines.join("\n"), + install_source: None, } } @@ -133,6 +184,239 @@ fn summarize_output(output: &str) -> String { format!("{}...", summary.replace('\n', "\\n")) } +/// Candidate npm global install dirs to probe for a given $HOME. +/// +/// Includes the standard per-user dirs plus all detected nvm node versions +/// (both `~/.nvm` and Homebrew's `/opt/homebrew/opt/nvm` layout). +fn npm_search_dirs(home: &Path) -> Vec { + let mut dirs = vec![home.join(".npm-global/bin"), home.join(".npm/bin")]; + + // ~/.nvm/versions/node/*/bin + for version_dir in read_subdirs(&home.join(".nvm/versions/node")) { + dirs.push(version_dir.join("bin")); + } + + // macOS Homebrew nvm: /opt/homebrew/opt/nvm/versions/node/*/bin + #[cfg(target_os = "macos")] + for version_dir in read_subdirs(Path::new("/opt/homebrew/opt/nvm/versions/node")) { + dirs.push(version_dir.join("bin")); + } + + dirs +} + +fn read_subdirs(parent: &Path) -> Vec { + match std::fs::read_dir(parent) { + Ok(entries) => entries + .filter_map(|e| e.ok()) + .filter(|e| e.file_type().map(|t| t.is_dir()).unwrap_or(false)) + .map(|e| e.path()) + .collect(), + Err(_) => Vec::new(), + } +} + +fn npm_global_bin_dir(lines: &mut Vec) -> Option { + let output = Command::new("npm").args(["prefix", "-g"]).output().ok()?; + if !output.status.success() { + lines.push(" npm prefix -g => failed".to_string()); + return None; + } + let prefix = String::from_utf8_lossy(&output.stdout).trim().to_string(); + if prefix.is_empty() { + lines.push(" npm prefix -g => empty output".to_string()); + return None; + } + let bin = PathBuf::from(prefix).join("bin"); + lines.push(format!(" npm prefix -g => {}", bin.display())); + Some(bin) +} + +/// Infer how a binary was installed. First applies path-prefix heuristics (no +/// subprocess or network probes) covering Brew, Cargo, Mise, Asdf, Npm +/// (mirroring the dirs in [`npm_search_dirs`]), and the System dirs. When those +/// fall through to [`InstallSource::Unknown`] for a binary in a user-local bin +/// dir, a cheap filesystem fingerprint (see [`fingerprint_curl_pipe`]) is +/// attempted to recognise curl/native installers (Claude native, Cursor, Amp), +/// which land in `~/.local/bin`/`~/bin`. [`InstallSource::Unknown`] remains the +/// honest fallback when no fingerprint matches. +pub(crate) fn detect_install_source(path: &Path) -> InstallSource { + let home = std::env::home_dir(); + let base = detect_install_source_with_home(path, home.as_deref()); + if base == InstallSource::Unknown { + if let Some(home) = home.as_deref() { + if fingerprint_curl_pipe(path, home) { + return InstallSource::CurlPipe; + } + } + } + base +} + +/// A known curl/native installer footprint for a binary that lands in a +/// user-local bin dir. Pairs the binary name with marker paths (relative to +/// `$HOME`) that the installer also creates; if the binary lives in a user-local +/// bin dir and any marker exists, the install is fingerprinted as a curl-pipe +/// install. +struct CurlInstallerFootprint { + /// Binary file name as it appears in `~/.local/bin` or `~/bin`. + binary: &'static str, + /// Marker paths relative to `$HOME`; if any exists the fingerprint matches. + markers: &'static [&'static str], +} + +/// Known footprints of curl/native installers whose binaries land in a +/// user-local bin dir. Conservative on purpose — only well-known data dirs are +/// listed so a bare `~/.local/bin/` with no installer footprint stays +/// [`InstallSource::Unknown`]. +const CURL_INSTALLER_FOOTPRINTS: &[CurlInstallerFootprint] = &[ + // Claude native installer — claude.ai/install.sh. + CurlInstallerFootprint { + binary: "claude", + markers: &[".local/share/claude", ".claude/local", ".claude/bin"], + }, + // Cursor CLI installer — cursor.com/install. + CurlInstallerFootprint { + binary: "cursor-agent", + markers: &[".local/share/cursor-agent/versions", ".cursor/bin"], + }, + // Amp installer — ampcode.com/install.sh. + CurlInstallerFootprint { + binary: "amp", + markers: &[".local/share/amp", ".cache/amp"], + }, +]; + +/// Cheap filesystem fingerprint for curl/native installs that path-prefix +/// heuristics can't classify. Only considers binaries inside a user-local bin +/// dir (`~/.local/bin`, `~/bin`) and uses two low-false-positive signals: +/// +/// 1. A known installer footprint marker (see [`CURL_INSTALLER_FOOTPRINTS`]) +/// exists under `$HOME` and the binary name matches that installer. +/// 2. The bin entry is a symlink into a *versioned* install dir under `$HOME` +/// (the layout Cursor's and Claude's native installers use: +/// `~/.local/bin/` → `…/versions//`). +/// +/// No subprocess or network access — only `read_link`/`exists`/`canonicalize`. +fn fingerprint_curl_pipe(path: &Path, home: &Path) -> bool { + let in_user_local_bin = + path.starts_with(home.join(".local/bin")) || path.starts_with(home.join("bin")); + if !in_user_local_bin { + return false; + } + + // Signal 1 — a known installer footprint marker exists under $HOME. + if let Some(name) = path.file_name().and_then(|n| n.to_str()) { + for fp in CURL_INSTALLER_FOOTPRINTS { + if fp.binary == name && fp.markers.iter().any(|m| home.join(m).exists()) { + return true; + } + } + } + + // Signal 2 — the bin entry is a symlink into a versioned install dir under + // $HOME. + if let Ok(target) = std::fs::read_link(path) { + let resolved = if target.is_absolute() { + target + } else if let Some(parent) = path.parent() { + parent.join(target) + } else { + target + }; + let resolved = resolved.canonicalize().unwrap_or(resolved); + if resolved.starts_with(home) && resolved.components().any(|c| c.as_os_str() == "versions") + { + return true; + } + } + + false +} + +/// Whether the canonicalized binary path lives inside a `node_modules/` +/// directory — a clean positive signal for `npm install -g`, regardless of +/// which node distribution (brew-shipped, nvm, fnm, asdf, mise) hosts the npm +/// prefix. Brew formulae and cargo installs never place binaries inside +/// `node_modules/`, so this dominates the path-prefix heuristics below. +/// +/// The npm global bin entry is a symlink (e.g. `/opt/homebrew/bin/claude` → +/// `../lib/node_modules/@anthropic-ai/claude-code/...`), so the check resolves +/// the symlink via [`fs::canonicalize`] before inspecting components. If +/// canonicalize fails (broken symlink, permissions), fall back to the path +/// as-is — better to attempt the check than skip it. No subprocess or network. +fn looks_like_npm_global(path: &Path) -> bool { + let resolved = std::fs::canonicalize(path); + let target = resolved.as_deref().unwrap_or(path); + target.components().any(|c| c.as_os_str() == "node_modules") +} + +/// Testable inner: same logic as [`detect_install_source`] but takes the home +/// directory as a parameter so unit tests can inject a fixed value. +fn detect_install_source_with_home(path: &Path, home: Option<&Path>) -> InstallSource { + // npm global install (any node distribution). Checked first: the bin entry + // is a symlink into `node_modules/`, which may live under a brew prefix + // (`npm config get prefix = /opt/homebrew`), so this must win over the + // `/opt/homebrew/` Brew path-prefix check below. + if looks_like_npm_global(path) { + return InstallSource::Npm; + } + + // Homebrew-managed nvm — checked before the generic `/opt/homebrew/` + // Brew prefix, since this path is a strict subset of it. + if path.starts_with("/opt/homebrew/opt/nvm/versions/node") { + return InstallSource::Npm; + } + + // Brew (path-prefix). `/usr/local/Cellar` covers Intel-mac brew; if a + // binary appears as `/usr/local/bin/` that's a symlink into Cellar, we + // only follow the chain if `canonicalize` succeeds cheaply. + if path.starts_with("/opt/homebrew/") + || path.starts_with("/usr/local/Cellar/") + || path.starts_with("/home/linuxbrew/.linuxbrew/") + { + return InstallSource::Brew; + } + if path.starts_with("/usr/local/bin/") { + if let Ok(canonical) = path.canonicalize() { + if canonical.starts_with("/usr/local/Cellar/") { + return InstallSource::Brew; + } + } + } + + if let Some(home) = home { + if path.starts_with(home.join(".cargo/bin")) { + return InstallSource::Cargo; + } + if path.starts_with(home.join(".local/share/mise")) || path.starts_with(home.join(".mise")) + { + return InstallSource::Mise; + } + if path.starts_with(home.join(".asdf")) { + return InstallSource::Asdf; + } + if path.starts_with(home.join(".npm-global/bin")) + || path.starts_with(home.join(".npm/bin")) + || path.starts_with(home.join(".nvm/versions/node")) + { + return InstallSource::Npm; + } + } + + // System dirs. Checked last so e.g. a Brew binary surfacing at + // `/usr/local/bin/x` was already classified above. + if path.starts_with("/usr/bin/") + || path.starts_with("/bin/") + || path.starts_with("/usr/sbin/") + || path.starts_with("/sbin/") + { + return InstallSource::System; + } + + InstallSource::Unknown +} + /// Format the raw output of a command invocation for debug diagnostics. pub fn format_command_output(cmd_desc: &str, output: &std::process::Output) -> String { let stdout = String::from_utf8_lossy(&output.stdout); @@ -149,9 +433,8 @@ pub fn format_command_output(cmd_desc: &str, output: &std::process::Output) -> S #[cfg(test)] mod tests { - use super::{candidate_paths_from_shell_output, is_executable_file, shell_quote}; + use super::*; use std::fs::{self, File}; - use std::path::PathBuf; #[test] fn candidate_accepts_single_absolute_path() { @@ -247,4 +530,288 @@ mod tests { assert!(!is_executable_file(&dir)); let _ = fs::remove_dir_all(&dir); } + + #[test] + fn npm_search_dirs_includes_expected_paths_for_fixed_home() { + let home = PathBuf::from("/home/test"); + let dirs = npm_search_dirs(&home); + + assert!( + dirs.iter().any(|p| p == &home.join(".npm-global/bin")), + "missing ~/.npm-global/bin in {dirs:?}" + ); + assert!( + dirs.iter().any(|p| p == &home.join(".npm/bin")), + "missing ~/.npm/bin in {dirs:?}" + ); + } + + #[test] + fn detect_install_source_classifies_brew() { + assert_eq!( + detect_install_source_with_home(Path::new("/opt/homebrew/bin/git"), None), + InstallSource::Brew, + ); + assert_eq!( + detect_install_source_with_home(Path::new("/home/linuxbrew/.linuxbrew/bin/git"), None), + InstallSource::Brew, + ); + } + + #[test] + fn detect_install_source_classifies_system() { + assert_eq!( + detect_install_source_with_home(Path::new("/usr/bin/git"), None), + InstallSource::System, + ); + assert_eq!( + detect_install_source_with_home(Path::new("/bin/sh"), None), + InstallSource::System, + ); + } + + #[test] + fn detect_install_source_classifies_cargo_under_home() { + let home = PathBuf::from("/home/test"); + assert_eq!( + detect_install_source_with_home(&home.join(".cargo/bin/cargo"), Some(home.as_path())), + InstallSource::Cargo, + ); + } + + #[test] + fn detect_install_source_classifies_npm_dirs() { + let home = PathBuf::from("/home/test"); + assert_eq!( + detect_install_source_with_home( + &home.join(".npm-global/bin/foo"), + Some(home.as_path()) + ), + InstallSource::Npm, + ); + assert_eq!( + detect_install_source_with_home( + &home.join(".nvm/versions/node/v20.10.0/bin/foo"), + Some(home.as_path()) + ), + InstallSource::Npm, + ); + assert_eq!( + detect_install_source_with_home( + Path::new("/opt/homebrew/opt/nvm/versions/node/v20.10.0/bin/foo"), + None, + ), + InstallSource::Npm, + ); + } + + #[test] + fn detect_install_source_classifies_mise_and_asdf() { + let home = PathBuf::from("/home/test"); + assert_eq!( + detect_install_source_with_home( + &home.join(".local/share/mise/installs/node/20/bin/foo"), + Some(home.as_path()), + ), + InstallSource::Mise, + ); + assert_eq!( + detect_install_source_with_home( + &home.join(".asdf/installs/nodejs/20/bin/foo"), + Some(home.as_path()), + ), + InstallSource::Asdf, + ); + } + + #[test] + fn detect_install_source_unknown_for_curl_pipe_and_other() { + let home = PathBuf::from("/home/test"); + // ~/.local/bin and ~/bin are common curl-pipe targets but unreliable to + // fingerprint from path alone — Unknown beats false positives. + assert_eq!( + detect_install_source_with_home(&home.join(".local/bin/foo"), Some(home.as_path())), + InstallSource::Unknown, + ); + assert_eq!( + detect_install_source_with_home(Path::new("/tmp/weird/foo"), Some(home.as_path())), + InstallSource::Unknown, + ); + } + + #[test] + fn npm_global_under_brew_prefix_classifies_as_npm() { + // `npm config get prefix = /opt/homebrew` lands the package under + // `/lib/node_modules/...` with a bin symlink at `/bin`. + // The brew path-prefix check must not win — node_modules in the + // canonicalized target is the authoritative npm signal. + let root = std::env::temp_dir().join(format!("doctor-npm-brew-{}", std::process::id())); + let _ = fs::remove_dir_all(&root); + let pkg = root.join("lib/node_modules/@anthropic-ai/claude-code/bin"); + fs::create_dir_all(&pkg).unwrap(); + let real = pkg.join("claude.exe"); + File::create(&real).unwrap(); + fs::create_dir_all(root.join("bin")).unwrap(); + let link = root.join("bin/claude"); + std::os::unix::fs::symlink( + "../lib/node_modules/@anthropic-ai/claude-code/bin/claude.exe", + &link, + ) + .unwrap(); + + assert!(looks_like_npm_global(&link)); + assert_eq!( + detect_install_source_with_home(&link, None), + InstallSource::Npm, + ); + let _ = fs::remove_dir_all(&root); + } + + #[test] + fn genuine_brew_cellar_not_misclassified_as_npm() { + // A real Cellar binary (no node_modules in its canonicalized path) must + // stay Brew — `looks_like_npm_global` returns false for it. + let root = std::env::temp_dir().join(format!("doctor-brew-cellar-{}", std::process::id())); + let _ = fs::remove_dir_all(&root); + let cellar = root.join("Cellar/git/2.44.0/bin"); + fs::create_dir_all(&cellar).unwrap(); + let bin = cellar.join("git"); + File::create(&bin).unwrap(); + + assert!(!looks_like_npm_global(&bin)); + // The real `/opt/homebrew` prefix check is exercised by + // `detect_install_source_classifies_brew`; here we only assert the new + // npm layer leaves non-npm paths to fall through. + let _ = fs::remove_dir_all(&root); + } + + #[test] + fn npm_global_under_nvm_classifies_as_npm() { + // The nvm layout: ~/.local/bin/ → ~/.nvm/versions/node//lib/ + // node_modules//... Path-prefix already handled ~/.nvm directly; + // the node_modules layer must keep classifying the symlinked bin too. + let home = std::env::temp_dir().join(format!("doctor-npm-nvm-{}", std::process::id())); + let _ = fs::remove_dir_all(&home); + let pkg = home.join(".nvm/versions/node/v20.10.0/lib/node_modules/@scope/tool/bin"); + fs::create_dir_all(&pkg).unwrap(); + let real = pkg.join("tool.js"); + File::create(&real).unwrap(); + fs::create_dir_all(home.join(".local/bin")).unwrap(); + let link = home.join(".local/bin/tool"); + std::os::unix::fs::symlink(&real, &link).unwrap(); + + assert!(looks_like_npm_global(&link)); + assert_eq!( + detect_install_source_with_home(&link, Some(home.as_path())), + InstallSource::Npm, + ); + let _ = fs::remove_dir_all(&home); + } + + #[test] + fn non_node_modules_path_falls_through_to_existing_detection() { + // A plain binary (no symlink, no node_modules) is not npm; the new layer + // returns false and existing path-prefix detection decides. + let root = std::env::temp_dir().join(format!("doctor-no-npm-{}", std::process::id())); + let _ = fs::remove_dir_all(&root); + let dir = root.join("usr/local/bin"); + fs::create_dir_all(&dir).unwrap(); + let bin = dir.join("foo"); + File::create(&bin).unwrap(); + + assert!(!looks_like_npm_global(&bin)); + // System dirs still classify correctly through the unchanged fall-through. + assert_eq!( + detect_install_source_with_home(Path::new("/usr/bin/foo"), None), + InstallSource::System, + ); + let _ = fs::remove_dir_all(&root); + } + + #[test] + fn fingerprint_curl_pipe_matches_known_installer_marker() { + let home = std::env::temp_dir().join(format!("doctor-fp-marker-{}", std::process::id())); + let _ = fs::remove_dir_all(&home); + // Claude native installer: ~/.local/bin/claude + ~/.local/share/claude. + fs::create_dir_all(home.join(".local/bin")).unwrap(); + fs::create_dir_all(home.join(".local/share/claude")).unwrap(); + let bin = home.join(".local/bin/claude"); + File::create(&bin).unwrap(); + + assert!(fingerprint_curl_pipe(&bin, &home)); + let _ = fs::remove_dir_all(&home); + } + + #[test] + fn fingerprint_curl_pipe_matches_versioned_symlink() { + #[cfg(unix)] + { + let home = + std::env::temp_dir().join(format!("doctor-fp-symlink-{}", std::process::id())); + let _ = fs::remove_dir_all(&home); + // Cursor layout: ~/.local/bin/cursor-agent -> ~/.local/share/cursor-agent/versions/1.0.0/cursor-agent + let versioned = home.join(".local/share/cursor-agent/versions/1.0.0"); + fs::create_dir_all(&versioned).unwrap(); + let real = versioned.join("cursor-agent"); + File::create(&real).unwrap(); + fs::create_dir_all(home.join(".local/bin")).unwrap(); + let link = home.join(".local/bin/cursor-agent"); + std::os::unix::fs::symlink(&real, &link).unwrap(); + + assert!(fingerprint_curl_pipe(&link, &home)); + let _ = fs::remove_dir_all(&home); + } + } + + #[test] + fn fingerprint_curl_pipe_no_match_without_footprint() { + let home = std::env::temp_dir().join(format!("doctor-fp-none-{}", std::process::id())); + let _ = fs::remove_dir_all(&home); + // A bare ~/.local/bin binary with no installer footprint stays Unknown. + fs::create_dir_all(home.join(".local/bin")).unwrap(); + let bin = home.join(".local/bin/mytool"); + File::create(&bin).unwrap(); + + assert!(!fingerprint_curl_pipe(&bin, &home)); + let _ = fs::remove_dir_all(&home); + } + + #[test] + fn fingerprint_curl_pipe_ignores_binaries_outside_user_local_bin() { + let home = std::env::temp_dir().join(format!("doctor-fp-outside-{}", std::process::id())); + let _ = fs::remove_dir_all(&home); + // Marker exists, but the binary is elsewhere — must not fingerprint. + fs::create_dir_all(home.join(".local/share/claude")).unwrap(); + let bin = PathBuf::from("/tmp/elsewhere/claude"); + + assert!(!fingerprint_curl_pipe(&bin, &home)); + let _ = fs::remove_dir_all(&home); + } + + #[test] + fn search_output_includes_npm_dirs_when_binary_not_found() { + let resolved = resolve_binary("definitely-not-a-real-binary-xyz-123abc"); + assert!( + resolved.path.is_none(), + "did not expect to find a real binary" + ); + assert!( + resolved + .search_output + .contains("strategy 3 — npm global install dirs"), + "expected strategy 3 marker in search_output:\n{}", + resolved.search_output + ); + if let Some(home) = std::env::home_dir() { + let expected = home.join(".npm-global/bin"); + assert!( + resolved + .search_output + .contains(&expected.display().to_string()), + "expected {} in search_output:\n{}", + expected.display(), + resolved.search_output + ); + } + } } diff --git a/crates/doctor/src/types.rs b/crates/doctor/src/types.rs index 24981b72..e5f947ab 100644 --- a/crates/doctor/src/types.rs +++ b/crates/doctor/src/types.rs @@ -14,12 +14,86 @@ pub enum CheckStatus { /// The type of fix available for a check. #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] -#[serde(rename_all = "lowercase")] +#[serde(rename_all = "camelCase")] pub enum FixType { /// A shell command to install or configure the dependency. Command, /// A shell command to install the ACP bridge binary. Bridge, + /// A shell command that triggers an authentication flow. + Auth, + /// A shell command to update the agent's main CLI. Source-aware: derived + /// from the readout's `(install_source, package_id)` so an npm-installed + /// agent is updated via `npm install -g …@latest`, not the install-time + /// curl-pipe recipe. The command is not in the static AI_AGENT_CHECKS + /// table; the executor receives it via `command_override`. + UpdateMain, + /// A shell command to update the agent's ACP bridge. Same shape as + /// `UpdateMain` — derived per-readout, passed via `command_override`. + UpdateBridge, +} + +/// Authentication state for a check that probes credentials. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +#[serde(rename_all = "camelCase")] +pub enum AuthStatus { + Authenticated, + NotAuthenticated, + NotApplicable, + /// Probe couldn't determine auth state because the agent's + /// `auth_status_command` failed to spawn or exited with 127 (command not + /// found). Distinct from `NotAuthenticated`: a PATH-shadowed agent isn't + /// signed out, we just can't tell. Downstream consumers should treat this + /// as informational (no auth fix to offer). + Unknown, +} + +/// How a binary was installed on disk. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +#[serde(rename_all = "camelCase")] +pub enum InstallSource { + Brew, + Npm, + Cargo, + Mise, + Asdf, + CurlPipe, + System, + Unknown, +} + +/// Version + install-source readout for one binary behind an agent check. +/// +/// An AI-agent check may front two distinct binaries — the agent's own CLI +/// (`main`) and its ACP bridge (`bridge`) — each installed and versioned +/// independently. This struct carries one binary's readout so the two can be +/// surfaced side by side instead of collapsed into a single source/version. +/// +/// All fields default to `None`; the cheap (no-freshness) path populates only +/// `install_source`, and the freshness pass fills in the version fields. +#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq, Eq)] +#[serde(rename_all = "camelCase")] +pub struct AgentVersionInfo { + /// How this binary was installed (brew, npm, curl/native, …), if detected. + pub install_source: Option, + /// Installed version string, if detected by the freshness pass. + pub installed_version: Option, + /// Latest available version string, if known. + pub latest_version: Option, + /// Whether a newer version is available. Suppressed (`None`) for + /// self-updating installs — see [`DoctorCheck::update_available`]. + pub update_available: Option, + /// Whether this binary keeps itself up to date (curl/native installers). + pub self_updating: Option, + /// Source-aware update command for this readout, derived from + /// `(install_source, package_id)`. `Some` only when an update is both + /// computable (the install source has a known update recipe and a + /// registered package id) and actionable (`update_available == Some(true)` + /// and the binary is not self-updating). Pairs with `update_fix_type`. + pub update_command: Option, + /// `FixType::UpdateMain` or `FixType::UpdateBridge` matching this readout's + /// slot. Always paired with `update_command`: both `Some` or both `None`. + pub update_fix_type: Option, } /// A single health-check result shown in the UI. @@ -48,6 +122,39 @@ pub struct DoctorCheck { /// Raw debug output: command stdout/stderr, search paths tried, etc. /// Used by the "Copy details" feature for support diagnostics. pub raw_output: Option, + /// Authentication status, when the check probes credentials. + pub auth_status: Option, + /// Installed version string, if detected. + pub installed_version: Option, + /// Latest available version string, if known. + pub latest_version: Option, + /// Whether a newer version is available than the installed one. + /// + /// For self-updating tools (see `self_updating`) this is never `Some(true)`: + /// the tool manages its own freshness, so we don't raise an update nag even + /// when a newer version exists upstream. + pub update_available: Option, + /// How the binary was installed (brew, npm, cargo, etc.), if detected. + pub install_source: Option, + /// Whether this tool keeps itself up to date (curl/native installers such as + /// Claude native, Cursor, and Amp-curl). When `Some(true)`, the freshness + /// pass reports the installed/latest versions for display but suppresses + /// `update_available` — the update is "managed by the tool", not actionable. + /// Only populated by the freshness pass; `None` on the default cheap path. + pub self_updating: Option, + /// Independent readout for the agent's own CLI (e.g. `claude`, `codex`). + /// + /// For AI-agent checks this carries the main CLI's install source and (when + /// freshness runs) its versions, separate from the ACP bridge. `None` for + /// non-agent checks and for agents with no resolvable main CLI. + /// + /// The flat fields above remain populated for backward compatibility: they + /// mirror the `bridge` readout when a bridge exists, otherwise `main`. + pub main: Option, + /// Independent readout for the agent's ACP bridge (e.g. `claude-agent-acp`, + /// `codex-acp`). `None` for non-agent checks and for agents that have no + /// separate bridge binary (the single binary is reported under `main`). + pub bridge: Option, } /// The full report returned to the frontend. @@ -62,4 +169,5 @@ pub struct DoctorReport { pub struct ResolvedBinary { pub path: Option, pub search_output: String, + pub install_source: Option, }