car-external-agents 0.47.0

//! Detection — locate installed adapter binaries on `$PATH`, probe
//! version and auth state, build [`ExternalAgentSpec`] entries.
//!
//! Detection is best-effort and idempotent. Failure of a *soft*
//! sub-probe (version timeout, missing cred file, malformed JSON)
//! degrades the affected field rather than dropping the entry —
//! knowing "claude is on disk but I can't tell you the version" is
//! more useful than a silent omission.
//!
//! **Executability is not a soft probe.** Resolution used to stop at
//! the first `$PATH` hit with the `+x` bit set and report it as
//! installed even when the version probe had *already watched the
//! binary die*. A Homebrew-installed `codex` under macOS Gatekeeper
//! quarantine is killed with SIGKILL on every exec; detection called
//! it installed, `invoke` spawned it, and the run failed with no
//! diagnostic. Two rules follow from that:
//!
//! 1. A binary that provably cannot execute is reported with
//!    [`HealthStatus::NotExecutable`] and an actionable `reason`,
//!    never as a healthy install (see [`ProbeOutcome`]).
//! 2. Resolution collects *every* `$PATH` match and picks the first
//!    that actually runs, so a dead binary earlier on `$PATH` cannot
//!    permanently shadow a working one later (see
//!    [`resolve_candidates`]).
//!
//! An explicit pin via `$CAR_<ADAPTER>_BIN` overrides `$PATH`
//! entirely — see [`pinned_binary`]. The pin is still probed, so a
//! stale pin surfaces as `NotExecutable` rather than a silent kill.

use crate::adapters::{self, Adapter};
use crate::health::{ExternalAgentHealth, HealthStatus};
use crate::types::{ExecutableStatus, ExternalAgentSpec};
use std::path::{Path, PathBuf};
use std::time::{Duration, SystemTime, UNIX_EPOCH};

const VERSION_PROBE_TIMEOUT: Duration = Duration::from_secs(2);

/// Exclude world-writable scratch directories so a binary staged
/// under `/tmp` (Unix) or `%TEMP%` (Windows) is never the one
/// detection picks up. Matches `car_registry::supervisor`'s denylist
/// for the same reason — the 2026-05 audit walked an exploit chain
/// that staged under a world-writable dir before lifecycle-spawning,
/// and detection should not be a backdoor around that.
///
/// The prefixes are lower-cased with a trailing separator so the
/// match in [`resolve_candidates`] is case-insensitive on Windows (NTFS)
/// and separator-agnostic. Returned as owned `String`s because the
/// Windows temp dir is resolved dynamically, not a fixed literal.
fn production_scratch_prefixes() -> Vec<String> {
    #[cfg(not(windows))]
    {
        ["/tmp/", "/private/tmp/", "/var/tmp/", "/dev/shm/"]
            .iter()
            .map(|s| s.to_string())
            .collect()
    }
    #[cfg(windows)]
    {
        fn norm(s: &str) -> String {
            format!("{}\\", s.replace('/', "\\").trim_end_matches('\\')).to_ascii_lowercase()
        }
        let mut v = vec![norm(&std::env::temp_dir().to_string_lossy())];
        for var in ["TEMP", "TMP"] {
            if let Some(t) = std::env::var_os(var) {
                v.push(norm(&Path::new(&t).to_string_lossy()));
            }
        }
        let sysroot = std::env::var_os("SystemRoot")
            .map(|s| Path::new(&s).to_string_lossy().into_owned())
            .unwrap_or_else(|| r"C:\Windows".to_string());
        v.push(norm(&format!("{}\\Temp", sysroot.trim_end_matches('\\'))));
        let drive = std::env::var_os("SystemDrive")
            .map(|s| Path::new(&s).to_string_lossy().into_owned())
            .unwrap_or_else(|| "C:".to_string());
        v.push(norm(&format!(
            "{}\\Users\\Public",
            drive.trim_end_matches('\\')
        )));
        v
    }
}

/// True when `candidate` (a resolved absolute path) sits under one of
/// `scratch_prefixes`. Case-insensitive + separator-agnostic on Windows;
/// exact byte-prefix on Unix.
fn under_scratch(candidate: &str, scratch_prefixes: &[String]) -> bool {
    #[cfg(windows)]
    let candidate = candidate.replace('/', "\\").to_ascii_lowercase();
    #[cfg(windows)]
    let candidate = candidate.as_str();
    scratch_prefixes
        .iter()
        .any(|p| candidate.starts_with(p.as_str()))
}

fn now_secs() -> u64 {
    SystemTime::now()
        .duration_since(UNIX_EPOCH)
        .map(|d| d.as_secs())
        .unwrap_or(0)
}

/// Candidate file names to try for a bare `bin_name` in each PATH dir.
///
/// On Unix this is just `[bin_name]`. On Windows an adapter's bare name
/// (`"claude"`) is installed by npm as several shims — `claude.exe`/`.cmd`/
/// `.ps1` plus an extensionless bash shim — and Windows resolves a bare command
/// against `%PATHEXT%`. So unless `bin_name` already carries an extension, try
/// `bin_name` + each `PATHEXT` entry (executable extensions first) and the bare
/// name last. Without this, an agent installed only as `claude.cmd` is missed
/// entirely (car#511). Mirrors the host's PATHEXT-aware lookup in
/// `apps/host-windows/src/paths.rs`.
fn candidate_names(bin_name: &str) -> Vec<String> {
    #[cfg(not(windows))]
    {
        vec![bin_name.to_string()]
    }
    #[cfg(windows)]
    {
        // Already has an extension → use it verbatim.
        if Path::new(bin_name).extension().is_some() {
            return vec![bin_name.to_string()];
        }
        let pathext =
            std::env::var("PATHEXT").unwrap_or_else(|_| ".COM;.EXE;.BAT;.CMD".to_string());
        let mut names: Vec<String> = pathext
            .split(';')
            .filter(|e| !e.is_empty())
            .map(|ext| format!("{bin_name}{}", ext.to_ascii_lowercase()))
            .collect();
        // The extensionless npm shim (a bash script) last: a real .exe/.cmd is
        // preferred because it can actually be spawned/probed on Windows.
        names.push(bin_name.to_string());
        names
    }
}

/// Resolve `bin_name` against the supplied `$PATH`-style search list
/// (`:`-separated on POSIX, `;` on Windows). Returns **every**
/// executable match outside scratch directories, in `$PATH` order.
/// Empty when the binary isn't found or every match is in a denied
/// prefix.
///
/// Returning the full candidate list rather than the first hit is what
/// lets [`detect_one`] skip past a binary that resolves but won't run
/// (Gatekeeper quarantine, wrong architecture, dangling symlink,
/// broken npm shim). The `+x` bit is a necessary condition for a
/// candidate, never a sufficient one.
///
/// Duplicates are collapsed: a `$PATH` that repeats a directory (which
/// real shells accumulate) must not cost one subprocess spawn per
/// repeat.
///
/// `scratch_prefixes` is the list of path prefixes to refuse; in
/// production this is [`production_scratch_prefixes`]. Tests pass `&[]`
/// so `tempfile`-created scratch dirs (which land under `/tmp/` on
/// Linux CI) work as PATH entries — the production denylist still
/// has unit coverage via [`tests::detect_rejects_scratch_dir_binaries`].
fn resolve_candidates(bin_name: &str, path_var: &str, scratch_prefixes: &[String]) -> Vec<PathBuf> {
    let separator = if cfg!(windows) { ';' } else { ':' };
    let names = candidate_names(bin_name);
    let mut out: Vec<PathBuf> = Vec::new();
    let mut seen: std::collections::HashSet<PathBuf> = std::collections::HashSet::new();
    for dir in path_var.split(separator) {
        if dir.is_empty() {
            continue;
        }
        for name in &names {
            let candidate = Path::new(dir).join(name);
            // Reject scratch dirs before any FS work — the lossy
            // string conversion is fine for prefix matching.
            let candidate_str = candidate.to_string_lossy();
            if under_scratch(&candidate_str, scratch_prefixes) {
                continue;
            }
            // Keep anything that EXISTS as a directory entry, even if it
            // fails the executable-file gate. `symlink_metadata` does not
            // follow, so a link whose target is gone still passes here and
            // reaches `detect_one`, which classifies it as unrunnable with
            // a reason. Dropping it here instead would report "Codex is
            // not installed" to a user whose symlink merely dangles.
            // `detect_one` only ever *reports* such a candidate when no
            // working one is found, so this cannot shadow a real install.
            if std::fs::symlink_metadata(&candidate).is_err() {
                continue;
            }
            // Dedup on the canonical path so `/opt/homebrew/bin/codex`
            // reached via a repeated PATH entry — or via a symlink and
            // its target — is probed once, not N times. A dangling link
            // cannot be canonicalized; it falls back to its own path,
            // which is the right dedup key for it anyway.
            let key = std::fs::canonicalize(&candidate).unwrap_or_else(|_| candidate.clone());
            if seen.insert(key) {
                out.push(candidate);
            }
        }
    }
    out
}

/// `None` when `path` is an existing regular file with an executable bit
/// (Unix) — the cheap filesystem precondition every candidate must clear
/// before it is worth spawning. Otherwise a specific reason.
///
/// The reason is the point. `metadata` follows symlinks, so a
/// `/usr/local/bin/codex` pointing into an app bundle that has since
/// moved fails here — and reporting that as "no candidate found" tells
/// the user Codex is not installed when in fact their symlink dangles.
/// That is the same silent-omission failure this module exists to
/// eliminate, one layer earlier, so the classifier's answer is carried
/// forward instead of discarded.
fn precheck_reason(path: &Path) -> Option<String> {
    match std::fs::metadata(path) {
        Ok(meta) => {
            if !meta.is_file() {
                return Some("not a regular file".to_string());
            }
            #[cfg(unix)]
            {
                use std::os::unix::fs::PermissionsExt;
                if meta.permissions().mode() & 0o111 == 0 {
                    return Some("file is not executable (no +x bit)".to_string());
                }
            }
            None
        }
        Err(e) => {
            // `symlink_metadata` does NOT follow, so it still succeeds for
            // a link whose target is gone — that difference is exactly how
            // a dangling link is told apart from a path that simply is not
            // there.
            if std::fs::symlink_metadata(path).is_ok() {
                Some(format!(
                    "dangling symlink — the target no longer exists ({e}); \
                     an app bundle it pointed into was probably moved or updated"
                ))
            } else {
                Some(format!("cannot stat: {e}"))
            }
        }
    }
}

/// Environment-variable pin for an adapter's binary, e.g.
/// `CAR_CODEX_BIN=/Applications/ChatGPT.app/Contents/Resources/codex`.
/// Derived from the adapter id: `codex` → `CAR_CODEX_BIN`,
/// `claude-code` → `CAR_CLAUDE_CODE_BIN`, `gemini` → `CAR_GEMINI_BIN`.
///
/// Highest precedence — when set, `$PATH` is not consulted at all.
/// This is the deterministic escape hatch for two real cases: a CLI
/// that ships inside an app bundle and was never on `$PATH` (ChatGPT.app
/// vendors `codex`), and A/B reproducibility, where "which codex ran"
/// must not depend on ambient shell state.
///
/// A pin that doesn't resolve to an executable file is **not** silently
/// ignored — it yields a `NotExecutable` entry naming the pin, because
/// falling back to `$PATH` would defeat the entire point of pinning and
/// reintroduce the ambiguity the operator set the variable to remove.
pub fn pin_env_var(adapter_id: &str) -> String {
    format!("CAR_{}_BIN", adapter_id.replace('-', "_").to_uppercase())
}

/// Read the pin for `adapter_id`, if set to a non-empty value.
fn pinned_binary(adapter_id: &str) -> Option<PathBuf> {
    let raw = std::env::var(pin_env_var(adapter_id)).ok()?;
    let trimmed = raw.trim();
    if trimmed.is_empty() {
        return None;
    }
    Some(PathBuf::from(trimmed))
}

/// Run `<bin> --version` with a 2s timeout, return the trimmed stdout
/// if the probe succeeded with a zero exit code. Stderr is dropped —
/// some tools emit deprecation warnings there that pollute the parse
/// shape.
/// True when `bin` is a Windows `.cmd`/`.bat` batch shim (how npm installs
/// these CLIs). `CreateProcess` can't execute a batch file directly (os error
/// 193), so such a binary must be invoked through `cmd /C`. Always false off
/// Windows.
pub(crate) fn is_batch_shim(bin: &Path) -> bool {
    cfg!(windows)
        && bin
            .extension()
            .and_then(|e| e.to_str())
            .map(|e| {
                let e = e.to_ascii_lowercase();
                e == "cmd" || e == "bat"
            })
            .unwrap_or(false)
}

/// Build a `tokio::process::Command` that invokes `bin`, routing a Windows
/// `.cmd`/`.bat` npm shim through `cmd /C` (see [`is_batch_shim`]). A `.exe` (or
/// any Unix binary) is invoked directly. The caller appends the tool's own
/// arguments via `.arg`/`.args`; for the batch path they land after
/// `cmd /C <bin>`, which is the correct batch invocation. This is the one place
/// that decides how an external-agent binary is spawned — detection's version
/// probe and every runner invoker share it, so a shim can never be spawned
/// directly (which fails on Windows).
pub(crate) fn base_command(bin: &Path) -> tokio::process::Command {
    if is_batch_shim(bin) {
        let mut c = tokio::process::Command::new("cmd");
        c.arg("/C").arg(bin);
        // The shim resolves `node` (etc.) through PATH, and cmd.exe drops a PATH
        // over ~8191 chars — see car_engine::win_env. None = inherit unchanged.
        if let Some(path) = car_engine::win_env::cmd_path_override() {
            c.env("PATH", path);
        }
        c
    } else {
        tokio::process::Command::new(bin)
    }
}

/// What the `--version` probe learned about a candidate binary.
///
/// The distinction that matters is [`Unusable`](ProbeOutcome::Unusable)
/// vs [`Inconclusive`](ProbeOutcome::Inconclusive): the first is proof
/// the binary cannot run, the second means the binary ran (or might
/// have) but didn't answer usefully. Only the first is grounds for
/// refusing to hand the path to `invoke`. Collapsing them would
/// regress every tool whose `--version` is merely slow on a loaded
/// machine.
#[derive(Debug)]
enum ProbeOutcome {
    /// Probe succeeded; carries the raw `--version` stdout.
    Version(String),
    /// The binary provably cannot be executed. Carries an
    /// operator-actionable reason.
    Unusable(String),
    /// Ran but told us nothing usable — non-zero exit, empty output,
    /// non-UTF-8 output, or a timeout. Entry survives with no version,
    /// matching the long-standing best-effort contract.
    Inconclusive,
}

/// Run `<bin> --version` with a 2s timeout and classify the result.
///
/// Stderr is dropped — some tools emit deprecation warnings there that
/// pollute the parse shape.
async fn probe_version(bin: &Path) -> ProbeOutcome {
    let mut cmd = base_command(bin);
    cmd.arg("--version");
    cmd.stdin(std::process::Stdio::null());
    cmd.stdout(std::process::Stdio::piped());
    cmd.stderr(std::process::Stdio::null());
    cmd.kill_on_drop(true);

    // Spawn failure is unambiguous: ENOENT (raced deletion), EACCES,
    // ENOEXEC (wrong arch / bad interpreter line). The OS refused to
    // run this file; no amount of retrying changes that.
    let child = match cmd.spawn() {
        Ok(c) => c,
        Err(e) => return ProbeOutcome::Unusable(format!("cannot execute: {e}")),
    };

    let output = match tokio::time::timeout(VERSION_PROBE_TIMEOUT, child.wait_with_output()).await {
        Ok(Ok(out)) => out,
        // Waiting failed, or the probe outran its budget. Neither
        // proves the binary is broken.
        _ => return ProbeOutcome::Inconclusive,
    };

    // Death by signal means the process never got to run its own code.
    // The case that motivated this: macOS Gatekeeper SIGKILLs a
    // quarantined binary at exec, so `--version` returns signal 9 and
    // no output. Distinguishing this from a plain non-zero exit is the
    // whole reason `ProbeOutcome` exists.
    #[cfg(unix)]
    {
        use std::os::unix::process::ExitStatusExt;
        if let Some(sig) = output.status.signal() {
            // NOT every signal death proves the image was refused. The
            // probe runs in the caller's process group, so a Ctrl-C or a
            // terminal hangup during `car agent external` delivers
            // SIGINT/SIGHUP to a perfectly healthy binary; condemning it
            // for that would be worse than the bug being fixed.
            //
            // Only signals that indicate a refused or broken image count:
            // SIGKILL (Gatekeeper's exec-time kill — the motivating
            // case), SIGILL/SIGTRAP/SIGBUS/SIGSEGV/SIGSYS (a binary that
            // faulted before it could run). Everything else — including
            // externally-delivered termination — is inconclusive.
            //
            // SIGKILL (9) is the only member of that set, and it is
            // deliberately the ONLY one tested here. Signal numbers past
            // the first handful are not portable — 10 and 12 are
            // SIGBUS/SIGSYS on macOS but SIGUSR1/SIGUSR2 on Linux — so a
            // numeric allowlist covering fault signals silently means
            // something different per platform. 9 is identical
            // everywhere. Fault signals (SEGV/BUS/ILL) fall through to
            // `Inconclusive`, which is the honest answer: that binary DID
            // start executing its own code, so "the OS refused the image"
            // is precisely what it does not prove.
            //
            // SIGKILL carries an irreducible false positive: the Linux
            // OOM killer and macOS Jetsam also SIGKILL under memory
            // pressure. It has to stay or the Gatekeeper case is lost.
            // The mitigation is that the verdict must not be sticky —
            // detection re-runs, and the FFI presence cache keeps a
            // `not_executable` only until the next explicit re-detect.
            if sig != 9 {
                return ProbeOutcome::Inconclusive;
            }
            return ProbeOutcome::Unusable(format!(
                "killed by signal {sig} — binary was killed at exec; on macOS this is \
                 usually Gatekeeper quarantine (check `xattr -l <path>` for \
                 com.apple.quarantine)"
            ));
        }
    }

    if !output.status.success() {
        return ProbeOutcome::Inconclusive;
    }
    let Ok(stdout) = String::from_utf8(output.stdout) else {
        return ProbeOutcome::Inconclusive;
    };
    let trimmed = stdout.trim();
    if trimmed.is_empty() {
        ProbeOutcome::Inconclusive
    } else {
        ProbeOutcome::Version(trimmed.to_string())
    }
}

fn home_dir() -> Option<PathBuf> {
    std::env::var_os("HOME")
        .or_else(|| std::env::var_os("USERPROFILE"))
        .map(PathBuf::from)
}

/// Build a spec for one adapter, or `None` when the binary isn't
/// installed anywhere detection looks.
///
/// Candidate precedence, best first:
///
/// 1. **Ran and reported a version** — the only outcome that yields a
///    healthy entry. Search stops here.
/// 2. **Ran but said nothing useful** ([`ProbeOutcome::Inconclusive`])
///    — kept as a fallback with `version: None`, preserving the
///    best-effort contract for tools whose `--version` is slow or
///    unsupported.
/// 3. **Provably cannot execute** ([`ProbeOutcome::Unusable`]) — kept
///    only if nothing better exists, and then marked
///    [`HealthStatus::NotExecutable`] with the reason, so the caller
///    gets a diagnosis instead of a mystery SIGKILL at invoke time.
///
/// Walking past a dead candidate to a live one is what makes a
/// shadowing install (dead `codex` in `/opt/homebrew/bin`, working
/// `codex` in `/usr/local/bin`) self-healing rather than a support
/// ticket.
async fn detect_one(
    adapter: &Adapter,
    path_var: &str,
    home: &Path,
    scratch_prefixes: &[String],
) -> Option<ExternalAgentSpec> {
    let id = adapter.id.as_str();

    // An explicit pin bypasses `$PATH` entirely — including the scratch
    // denylist, which exists to stop *ambient* `$PATH` from reaching a
    // world-writable dir. An operator naming a path outright has
    // already made that decision.
    let (candidates, from_pin) = match pinned_binary(id) {
        Some(pinned) => (vec![pinned], true),
        None => (
            resolve_candidates(adapter.bin_name, path_var, scratch_prefixes),
            false,
        ),
    };
    if candidates.is_empty() {
        return None;
    }
    // A stale pin points somewhere the user's `$PATH` never mentions, so
    // naming only the path leaves them with no idea where it came from.
    // Name the variable that produced it.
    let pin_note = if from_pin {
        format!(" (pinned by ${})", pin_env_var(id))
    } else {
        String::new()
    };

    let mut inconclusive: Option<PathBuf> = None;
    let mut unusable: Option<(PathBuf, String)> = None;
    let mut chosen: Option<(PathBuf, Option<String>)> = None;

    for candidate in candidates {
        // A pinned path skips the `is_executable_file` gate that
        // `resolve_candidates` applies, so check it here — otherwise a
        // pin at a nonexistent path spawns and reports a confusing
        // ENOENT rather than "your pin points at nothing".
        if let Some(why) = precheck_reason(&candidate) {
            if unusable.is_none() {
                unusable = Some((candidate, format!("{why}{pin_note}")));
            }
            continue;
        }
        match probe_version(&candidate).await {
            ProbeOutcome::Version(raw) => {
                chosen = Some((candidate, (adapter.parse_version)(&raw)));
                break;
            }
            ProbeOutcome::Inconclusive => {
                if inconclusive.is_none() {
                    inconclusive = Some(candidate);
                }
            }
            ProbeOutcome::Unusable(reason) => {
                let reason = format!("{reason}{pin_note}");
                if unusable.is_none() {
                    unusable = Some((candidate, reason));
                }
            }
        }
    }

    let auth_kind = (adapter.probe_auth)(home);
    let mut spec = ExternalAgentSpec {
        id: id.to_string(),
        display_name: adapter.id.display_name().to_string(),
        binary_path: PathBuf::new(),
        version: None,
        auth_kind,
        capabilities: adapter.capabilities.clone(),
        detected_at: now_secs(),
        health: None,
        execution: ExecutableStatus::Runnable,
    };

    if let Some((path, version)) = chosen {
        spec.binary_path = path;
        spec.version = version;
        return Some(spec);
    }
    if let Some(path) = inconclusive {
        spec.binary_path = path;
        return Some(spec);
    }
    let (path, reason) = unusable?;
    let detail = format!("{} at {}", reason, path.display());
    let checked_at = now_secs();
    // The authoritative write (car#746). Nothing downstream may revise this.
    spec.execution = ExecutableStatus::Unusable {
        reason: detail.clone(),
        checked_at,
    };
    // Compatibility window (migration step 2): consumers that still read
    // `health.status == "not_executable"` keep working until they have moved to
    // `execution`. Dropped together with the fallback in `unusable_reason`.
    spec.health = Some(ExternalAgentHealth {
        id: id.to_string(),
        status: HealthStatus::NotExecutable,
        details: serde_json::json!({ "binary_path": path.to_string_lossy() }),
        reason: Some(detail),
        checked_at,
    });
    spec.binary_path = path;
    Some(spec)
}

/// Detection, filtered to adapters that can actually be executed (car#746).
///
/// [`detect`] returns the full inventory on purpose — a user looking for a
/// broken install has to be able to find it — but that makes every consumer
/// responsible for remembering to filter, and one of four did not. Anything
/// that is about to *spawn* should call this instead, so the unusable entry is
/// not merely discouraged but absent.
///
/// Use [`detect`] when you are building a diagnostic view and want the broken
/// entries too.
pub async fn detect_runnable() -> Vec<ExternalAgentSpec> {
    detect()
        .await
        .into_iter()
        .filter(|spec| spec.unusable_reason().is_none())
        .collect()
}

/// Run detection for every known adapter against the current process
/// environment. Returns specs for installed adapters only — uninstalled
/// adapters are simply omitted from the list. Sorted by `id` for
/// deterministic UI ordering.
///
/// Best-effort: a failed *soft* sub-probe (auth shape, a slow or
/// unsupported `--version`) degrades the affected field rather than
/// dropping the entry.
///
/// Executability is not soft. A binary that provably cannot run is
/// still returned — omitting it would leave the user with no way to
/// find the broken install — but carries
/// `health: Some(NotExecutable)` with a reason naming the path.
/// **Callers must not invoke a spec in that state**; check
/// [`HealthStatus`] before spawning. `coder::router` already gates on
/// `HealthStatus::Ready`.
pub async fn detect() -> Vec<ExternalAgentSpec> {
    let path_var = std::env::var("PATH").unwrap_or_default();
    let Some(home) = home_dir() else {
        // No HOME → can't probe auth state. Still detect binaries on
        // PATH; auth_kind defaults to Unknown.
        return detect_with_paths(&path_var, Path::new("/")).await;
    };
    detect_with_paths(&path_var, &home).await
}

/// Same as [`detect`] but with explicit `$PATH` and `$HOME` overrides.
/// Production scratch denylist applies. Used by tests; not part of the
/// public API.
pub(crate) async fn detect_with_paths(path_var: &str, home: &Path) -> Vec<ExternalAgentSpec> {
    detect_with_paths_filtered(path_var, home, &production_scratch_prefixes()).await
}

/// Underlying implementation of [`detect_with_paths`] with an
/// overridable scratch-prefix list. Tests pass `&[]` so binaries
/// staged in `tempfile`-created dirs (which land under `/tmp/` on
/// Linux CI) survive the resolver. The production scratch denylist
/// keeps full unit coverage via [`tests::detect_rejects_scratch_dir_binaries`].
pub(crate) async fn detect_with_paths_filtered(
    path_var: &str,
    home: &Path,
    scratch_prefixes: &[String],
) -> Vec<ExternalAgentSpec> {
    // Probe adapters concurrently — each is an independent binary lookup +
    // `--version` subprocess (bounded by VERSION_PROBE_TIMEOUT). Sequential, the
    // worst case is N × that timeout, which on a slow machine overshoots
    // callers' own deadlines (e.g. discovery's per-provider bound) and starves
    // detection entirely.
    //
    // Adapters run concurrently; the CANDIDATES WITHIN one adapter are
    // probed serially, so the bound is
    // `max_over_adapters(candidates) × VERSION_PROBE_TIMEOUT`, not one
    // timeout. In practice candidates are 1 (the pre-spawn filesystem
    // gate rejects most), and only a `$PATH` carrying several
    // same-named binaries that each hang for 2s approaches the worst
    // case. That matters because `detect()` is on two hot paths —
    // `invoke` and `coder.start` — so if that bound is ever observed to
    // bite, probe candidates concurrently and take the first success in
    // `$PATH` order rather than raising the timeout.
    let probes = adapters::all()
        .iter()
        .map(|adapter| detect_one(adapter, path_var, home, scratch_prefixes));
    let mut specs: Vec<ExternalAgentSpec> = futures::future::join_all(probes)
        .await
        .into_iter()
        .flatten()
        .collect();
    // Deterministic order regardless of which probe finished first.
    specs.sort_by(|a, b| a.id.cmp(&b.id));
    specs
}

/// Run detection and immediately populate each spec's `health` field
/// with the ground-truth result from each tool's auth-status command.
/// Slower than plain [`detect`] (subprocess spawn per tool) but gives
/// host UIs a one-stop call for "what's installed AND ready to use."
///
/// Pass `force = true` to bypass the 30s per-tool health-check TTL
/// cache.
pub async fn detect_with_health(force: bool) -> Vec<ExternalAgentSpec> {
    let mut specs = detect().await;
    // Detection already proved these binaries can't run. The adapter's
    // status command would just be a second doomed spawn, and letting
    // its `Unknown` overwrite `NotExecutable` would erase the one
    // diagnosis that explains the failure.
    let runnable: Vec<ExternalAgentSpec> = specs
        .iter()
        .filter(|s| !is_not_executable(s))
        .cloned()
        .collect();
    let healths = crate::health::check_all(&runnable, force).await;
    let by_id: std::collections::HashMap<&str, &crate::health::ExternalAgentHealth> =
        healths.iter().map(|h| (h.id.as_str(), h)).collect();
    for spec in specs.iter_mut() {
        if is_not_executable(spec) {
            continue;
        }
        if let Some(h) = by_id.get(spec.id.as_str()) {
            spec.health = Some((*h).clone());
        }
    }
    specs
}

/// True when detection flagged this spec's binary as unrunnable.
/// Thin alias over the public [`ExternalAgentSpec::unusable_reason`] so
/// there is exactly one definition of the predicate — an in-crate copy
/// that drifted from the one callers use would be worse than none.
fn is_not_executable(spec: &ExternalAgentSpec) -> bool {
    spec.unusable_reason().is_some()
}

#[cfg(test)]
mod tests {
    use super::*;

    /// Build a fake executable for `name` that prints `version_output`.
    /// On Unix this is an extensionless `#!/bin/sh` script; on Windows it's a
    /// `name.cmd` npm-style batch shim (the real-world install shape), so
    /// detection exercises the PATHEXT resolution + `cmd /C` probe path.
    /// Returns the created path.
    fn make_fake_bin(dir: &Path, name: &str, version_output: &str) -> PathBuf {
        #[cfg(windows)]
        {
            let path = dir.join(format!("{name}.cmd"));
            std::fs::write(&path, format!("@echo off\r\necho {version_output}\r\n")).unwrap();
            path
        }
        #[cfg(not(windows))]
        {
            let path = dir.join(name);
            let script = format!("#!/bin/sh\necho '{version_output}'\n");
            std::fs::write(&path, script).unwrap();
            use std::os::unix::fs::PermissionsExt;
            std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o755)).unwrap();
            path
        }
    }

    /// Like [`make_fake_bin`], but the shim RUNS and exits non-zero — a failed
    /// version probe, not an unrunnable binary. Same platform split and for the
    /// same reason: an extensionless `#!/bin/sh` file is not executable on
    /// Windows at all, so a Unix-shaped fixture there tests "cannot exec"
    /// instead of "ran and failed" and inverts the assertion it was written to
    /// make (car#760).
    fn make_failing_bin(dir: &Path, name: &str) -> PathBuf {
        #[cfg(windows)]
        {
            let path = dir.join(format!("{name}.cmd"));
            std::fs::write(&path, "@echo off\r\nexit /b 1\r\n").unwrap();
            path
        }
        #[cfg(not(windows))]
        {
            let path = dir.join(name);
            std::fs::write(&path, "#!/bin/sh\nexit 1\n").unwrap();
            use std::os::unix::fs::PermissionsExt;
            std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o755)).unwrap();
            path
        }
    }

    #[cfg(windows)]
    #[test]
    fn candidate_names_expands_pathext_on_windows() {
        std::env::set_var("PATHEXT", ".COM;.EXE;.BAT;.CMD");
        let names = candidate_names("claude");
        // Executable extensions are tried, plus the bare npm shim last.
        assert!(names.contains(&"claude.exe".to_string()), "{names:?}");
        assert!(names.contains(&"claude.cmd".to_string()), "{names:?}");
        assert_eq!(names.last().unwrap(), "claude", "bare shim tried last");
        // A name that already has an extension is used verbatim.
        assert_eq!(
            candidate_names("claude.cmd"),
            vec!["claude.cmd".to_string()]
        );
    }

    #[cfg(windows)]
    #[test]
    fn batch_shim_routed_through_cmd() {
        assert!(is_batch_shim(Path::new(r"C:\x\claude.cmd")));
        assert!(is_batch_shim(Path::new(r"C:\x\claude.bat")));
        assert!(is_batch_shim(Path::new(r"C:\x\CLAUDE.CMD"))); // case-insensitive
        assert!(!is_batch_shim(Path::new(r"C:\x\claude.exe")));
        // A `.cmd` shim spawns `cmd` (…/C <shim>); a `.exe` spawns directly.
        let c = base_command(Path::new(r"C:\x\claude.cmd"));
        assert_eq!(c.as_std().get_program(), "cmd");
        let c = base_command(Path::new(r"C:\x\claude.exe"));
        assert_eq!(c.as_std().get_program(), r"C:\x\claude.exe");
    }

    #[cfg(not(windows))]
    #[test]
    fn batch_shim_never_on_unix() {
        assert!(!is_batch_shim(Path::new("/x/claude.cmd")));
        let c = base_command(Path::new("/x/claude"));
        assert_eq!(c.as_std().get_program(), "/x/claude");
    }

    #[cfg(windows)]
    #[tokio::test]
    async fn detect_finds_cmd_shim_on_path() {
        // The npm `.cmd` shim must be found via PATHEXT and version-probed
        // through `cmd /C` (car#511).
        let bin_dir = tempfile::TempDir::new().unwrap();
        let home_dir = tempfile::TempDir::new().unwrap();
        make_fake_bin(bin_dir.path(), "claude", "1.0.51 (Claude Code)");
        let path_var = bin_dir.path().to_string_lossy().to_string();
        let specs = detect_with_paths_filtered(&path_var, home_dir.path(), &[]).await;
        let claude = specs.iter().find(|s| s.id == "claude-code");
        assert!(
            claude.is_some(),
            "expected claude-code via .cmd shim in {specs:?}"
        );
        assert_eq!(claude.unwrap().version.as_deref(), Some("1.0.51"));
    }

    #[cfg(windows)]
    #[tokio::test]
    async fn detect_rejects_windows_temp_dir_binaries() {
        // A shim staged under %TEMP% (a world-writable staging dir) must be
        // ignored by the *production* denylist — the Windows analogue of the
        // `/tmp` exploit block. `tempfile` lands under %TEMP%, and unlike the
        // `&[]` tests above this goes through `detect_with_paths`, which applies
        // `production_scratch_prefixes()`.
        let bin_dir = tempfile::TempDir::new().unwrap();
        let home_dir = tempfile::TempDir::new().unwrap();
        make_fake_bin(bin_dir.path(), "claude", "1.0.51 (Claude Code)");
        let path_var = bin_dir.path().to_string_lossy().to_string();
        let specs = detect_with_paths(&path_var, home_dir.path()).await;
        assert!(
            specs.iter().all(|s| s.id != "claude-code"),
            "temp-dir binary must be rejected by the production denylist, got {specs:?}"
        );
    }

    #[tokio::test]
    async fn detect_finds_fake_binary_on_path() {
        let bin_dir = tempfile::TempDir::new().unwrap();
        let home_dir = tempfile::TempDir::new().unwrap();
        make_fake_bin(bin_dir.path(), "claude", "1.0.51 (Claude Code)");

        let path_var = bin_dir.path().to_string_lossy().to_string();
        // Empty scratch denylist — `tempfile` lands under /tmp/ on
        // Linux CI, which the production list rejects. The
        // production-denylist behavior is covered by
        // `detect_rejects_scratch_dir_binaries` below.
        let specs = detect_with_paths_filtered(&path_var, home_dir.path(), &[]).await;

        let claude = specs.iter().find(|s| s.id == "claude-code");
        assert!(claude.is_some(), "expected claude-code in {specs:?}");
        let claude = claude.unwrap();
        assert_eq!(claude.version.as_deref(), Some("1.0.51"));
        // No cred file → Unauthenticated, not Unknown.
        assert!(matches!(
            claude.auth_kind,
            crate::types::AuthKind::Unauthenticated
        ));
    }

    #[tokio::test]
    async fn detect_omits_uninstalled_binaries() {
        let bin_dir = tempfile::TempDir::new().unwrap();
        let home_dir = tempfile::TempDir::new().unwrap();
        // Empty PATH dir — no binaries installed.
        let path_var = bin_dir.path().to_string_lossy().to_string();
        let specs = detect_with_paths(&path_var, home_dir.path()).await;
        assert!(specs.is_empty(), "expected no detections, got {specs:?}");
    }

    #[tokio::test]
    async fn detect_picks_subscription_when_oauth_creds_present() {
        let bin_dir = tempfile::TempDir::new().unwrap();
        let home_dir = tempfile::TempDir::new().unwrap();
        make_fake_bin(bin_dir.path(), "claude", "1.0.51");

        // Write a fake credential file with an oauth-shaped key.
        let claude_dir = home_dir.path().join(".claude");
        std::fs::create_dir_all(&claude_dir).unwrap();
        std::fs::write(
            claude_dir.join(".credentials.json"),
            r#"{"oauthAccount": {"email": "[email protected]"}}"#,
        )
        .unwrap();

        let path_var = bin_dir.path().to_string_lossy().to_string();
        let specs = detect_with_paths_filtered(&path_var, home_dir.path(), &[]).await;
        let claude = specs.iter().find(|s| s.id == "claude-code").unwrap();
        assert!(matches!(
            claude.auth_kind,
            crate::types::AuthKind::Subscription
        ));
    }

    #[tokio::test]
    async fn detect_picks_apikey_when_only_apikey_present() {
        let bin_dir = tempfile::TempDir::new().unwrap();
        let home_dir = tempfile::TempDir::new().unwrap();
        make_fake_bin(bin_dir.path(), "claude", "1.0.51");

        let claude_dir = home_dir.path().join(".claude");
        std::fs::create_dir_all(&claude_dir).unwrap();
        std::fs::write(
            claude_dir.join(".credentials.json"),
            r#"{"apiKey": "sk-ant-..."}"#,
        )
        .unwrap();

        let path_var = bin_dir.path().to_string_lossy().to_string();
        let specs = detect_with_paths_filtered(&path_var, home_dir.path(), &[]).await;
        let claude = specs.iter().find(|s| s.id == "claude-code").unwrap();
        assert!(matches!(claude.auth_kind, crate::types::AuthKind::ApiKey));
    }

    #[tokio::test]
    async fn detect_rejects_scratch_dir_binaries() {
        // Skip on platforms where TMPDIR doesn't land under /tmp
        // (macOS resolves to /var/folders/... — outside the denylist
        // by design).
        let tmp = std::env::temp_dir();
        if !tmp.starts_with("/tmp") && !tmp.starts_with("/private/tmp") {
            return;
        }
        let bin_dir = tempfile::TempDir::new_in("/tmp").unwrap();
        let home_dir = tempfile::TempDir::new().unwrap();
        make_fake_bin(bin_dir.path(), "claude", "1.0.51");

        let path_var = bin_dir.path().to_string_lossy().to_string();
        let specs = detect_with_paths(&path_var, home_dir.path()).await;
        assert!(
            specs.iter().all(|s| s.id != "claude-code"),
            "scratch-dir binary must be rejected, got {specs:?}"
        );
    }

    #[tokio::test]
    async fn detect_keeps_entry_when_version_probe_fails() {
        let bin_dir = tempfile::TempDir::new().unwrap();
        let home_dir = tempfile::TempDir::new().unwrap();
        // A binary that RUNS and exits non-zero — version probe returns None.
        // Built platform-appropriately: the point of this test is a probe that
        // executed and failed, which an extensionless shell script cannot be on
        // Windows.
        make_failing_bin(bin_dir.path(), "claude");

        let path_var = bin_dir.path().to_string_lossy().to_string();
        let specs = detect_with_paths_filtered(&path_var, home_dir.path(), &[]).await;
        let claude = specs.iter().find(|s| s.id == "claude-code");
        assert!(claude.is_some(), "entry must survive failed version probe");
        let claude = claude.unwrap();
        assert!(claude.version.is_none());
        // A non-zero exit is Inconclusive, NOT Unusable — the binary
        // ran. Marking it NotExecutable would regress every tool with
        // a slow or unsupported `--version`.
        assert!(
            !is_not_executable(claude),
            "non-zero exit must not be classified as unrunnable"
        );
    }

    /// Build a file that resolves and has the +x bit but cannot be
    /// executed: an ENOEXEC binary (bad interpreter line). This is the
    /// portable stand-in for the Gatekeeper-quarantined binary that
    /// motivated the change — same observable shape, exec refused.
    #[cfg(not(windows))]
    fn make_unrunnable_bin(dir: &Path, name: &str) -> PathBuf {
        let path = dir.join(name);
        std::fs::write(&path, "#!/nonexistent/interpreter\n").unwrap();
        use std::os::unix::fs::PermissionsExt;
        std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o755)).unwrap();
        path
    }

    #[cfg(not(windows))]
    #[tokio::test]
    async fn unrunnable_binary_is_flagged_not_executable() {
        let bin_dir = tempfile::TempDir::new().unwrap();
        let home_dir = tempfile::TempDir::new().unwrap();
        make_unrunnable_bin(bin_dir.path(), "claude");

        let path_var = bin_dir.path().to_string_lossy().to_string();
        let specs = detect_with_paths_filtered(&path_var, home_dir.path(), &[]).await;
        let claude = specs
            .iter()
            .find(|s| s.id == "claude-code")
            .expect("broken install must still be reported so the user can find it");
        assert!(
            is_not_executable(claude),
            "a binary that cannot exec must be NotExecutable, got {:?}",
            claude.health
        );
        // The reason has to name the path — that file is what the user
        // must fix or delete.
        let reason = claude.health.as_ref().unwrap().reason.as_deref().unwrap();
        assert!(
            reason.contains("claude"),
            "reason must name the offending path, got {reason:?}"
        );
    }

    #[cfg(not(windows))]
    #[tokio::test]
    async fn working_binary_later_on_path_beats_dead_one_first() {
        // The exact production scenario: a quarantined Homebrew `codex`
        // shadowing a working one. Resolution must walk past the corpse.
        let dead_dir = tempfile::TempDir::new().unwrap();
        let live_dir = tempfile::TempDir::new().unwrap();
        let home_dir = tempfile::TempDir::new().unwrap();
        make_unrunnable_bin(dead_dir.path(), "claude");
        make_fake_bin(live_dir.path(), "claude", "1.0.51 (Claude Code)");

        let path_var = format!(
            "{}:{}",
            dead_dir.path().to_string_lossy(),
            live_dir.path().to_string_lossy()
        );
        let specs = detect_with_paths_filtered(&path_var, home_dir.path(), &[]).await;
        let claude = specs.iter().find(|s| s.id == "claude-code").unwrap();
        assert_eq!(
            claude.version.as_deref(),
            Some("1.0.51"),
            "must fall through to the working binary"
        );
        assert!(claude.binary_path.starts_with(live_dir.path()));
        assert!(!is_not_executable(claude));
    }

    #[cfg(not(windows))]
    #[tokio::test]
    async fn duplicate_path_entries_probed_once() {
        // Real shells accumulate repeated PATH entries; each repeat must
        // not cost a subprocess spawn.
        let bin_dir = tempfile::TempDir::new().unwrap();
        make_fake_bin(bin_dir.path(), "claude", "1.0.51");
        let dir = bin_dir.path().to_string_lossy().to_string();
        let path_var = format!("{dir}:{dir}:{dir}");
        let candidates = resolve_candidates("claude", &path_var, &[]);
        assert_eq!(candidates.len(), 1, "got {candidates:?}");
    }

    /// A symlink into an app bundle that has since moved must be
    /// DIAGNOSED, not silently dropped. Dropping it reports "not
    /// installed" to a user who has the tool installed and a broken link
    /// — the same silent omission this module exists to eliminate.
    #[cfg(unix)]
    #[tokio::test]
    async fn dangling_symlink_is_diagnosed_not_omitted() {
        let bin_dir = tempfile::TempDir::new().unwrap();
        let home_dir = tempfile::TempDir::new().unwrap();
        let gone = bin_dir.path().join("relocated-app-binary");
        std::os::unix::fs::symlink(&gone, bin_dir.path().join("claude")).unwrap();

        let path_var = bin_dir.path().to_string_lossy().to_string();
        let specs = detect_with_paths_filtered(&path_var, home_dir.path(), &[]).await;
        let claude = specs
            .iter()
            .find(|s| s.id == "claude-code")
            .expect("a dangling link must not read as 'not installed'");
        assert!(is_not_executable(claude), "got {:?}", claude.health);
        let reason = claude.health.as_ref().unwrap().reason.as_deref().unwrap();
        assert!(
            reason.contains("dangling symlink"),
            "reason must identify the dangling link, got {reason:?}"
        );
    }

    /// ...but a broken link must still lose to a working binary later on
    /// `$PATH`, or the diagnosis would become its own shadowing bug.
    #[cfg(unix)]
    #[tokio::test]
    async fn dangling_symlink_loses_to_a_working_binary() {
        let dead_dir = tempfile::TempDir::new().unwrap();
        let live_dir = tempfile::TempDir::new().unwrap();
        let home_dir = tempfile::TempDir::new().unwrap();
        std::os::unix::fs::symlink(dead_dir.path().join("gone"), dead_dir.path().join("claude"))
            .unwrap();
        make_fake_bin(live_dir.path(), "claude", "1.0.51 (Claude Code)");

        let path_var = format!(
            "{}:{}",
            dead_dir.path().to_string_lossy(),
            live_dir.path().to_string_lossy()
        );
        let specs = detect_with_paths_filtered(&path_var, home_dir.path(), &[]).await;
        let claude = specs.iter().find(|s| s.id == "claude-code").unwrap();
        assert_eq!(claude.version.as_deref(), Some("1.0.51"));
        assert!(!is_not_executable(claude));
    }

    /// A signal that merely *terminated* the probe — Ctrl-C into the
    /// process group, a supervisor's SIGTERM — is not evidence the image
    /// was refused. Only SIGKILL is, and only because Gatekeeper uses it.
    #[cfg(unix)]
    #[tokio::test]
    async fn non_kill_signal_death_is_inconclusive_not_unusable() {
        let bin_dir = tempfile::TempDir::new().unwrap();
        let home_dir = tempfile::TempDir::new().unwrap();
        let path = bin_dir.path().join("claude");
        // Raise SIGTERM on itself: ran fine, died by a signal that says
        // nothing about whether the binary is executable.
        std::fs::write(&path, "#!/bin/sh\nkill -TERM $$\n").unwrap();
        use std::os::unix::fs::PermissionsExt;
        std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o755)).unwrap();

        let path_var = bin_dir.path().to_string_lossy().to_string();
        let specs = detect_with_paths_filtered(&path_var, home_dir.path(), &[]).await;
        let claude = specs.iter().find(|s| s.id == "claude-code").unwrap();
        assert!(
            !is_not_executable(claude),
            "SIGTERM must not be read as 'cannot execute', got {:?}",
            claude.health
        );
    }

    #[test]
    fn pin_env_var_derives_from_adapter_id() {
        assert_eq!(pin_env_var("codex"), "CAR_CODEX_BIN");
        assert_eq!(pin_env_var("claude-code"), "CAR_CLAUDE_CODE_BIN");
        assert_eq!(pin_env_var("gemini"), "CAR_GEMINI_BIN");
    }
}