fix(acp): resolve npx agents from ~/.local/bin and trust the terminal probe

src-tauri/src/acp/preflight.rs

@@ -123,12 +123,13 @@ pub async fn run_preflight(agent_type: AgentType) -> PreflightResult {
 /// diagnostics report, which bounds every command with `DIAG_PROBE_TIMEOUT`.
 ///
 /// The two lookups run concurrently because either can fall through to
-/// `resolve_npx_command`'s `npm prefix -g` probe (only when `which` misses), and
-/// a FAILED prefix resolution is never cached — `cached_npm_global_prefix_with`
-/// short-circuits on `None` before it reaches `OnceCell::set`. On a machine
-/// where that probe stalls, running these in sequence would pay
-/// `NPM_PREFIX_TIMEOUT` twice per adapter agent; overlapping them costs the same
-/// two spawns but bounds the added Settings latency to one timeout.
+/// `resolve_npx_command`'s `npm prefix -g` probe (only when `which` and
+/// `~/.local/bin` both miss), and a FAILED prefix resolution is never cached —
+/// `cached_npm_global_prefix_with` short-circuits on `None` before it reaches
+/// `OnceCell::set`. On a machine where that probe stalls, running these in
+/// sequence would pay `NPM_PREFIX_TIMEOUT` twice per adapter agent; overlapping
+/// them costs the same two spawns but bounds the added Settings latency to one
+/// timeout.
 async fn probe_adapter(meta: &AcpAgentMeta) -> Option<AdapterInfo> {
     let relation = registry::acp_adapter_relation(meta.agent_type)?;
     let adapter_cmd = match &meta.distribution {

src-tauri/src/acp/registry.rs

@@ -2977,10 +2977,13 @@ pub fn get_agent_meta(agent_type: AgentType) -> AcpAgentMeta {
             // interrupted `hermes update` in the standalone `hermes-acp` entry,
             // a no-op under the `hermes acp` codeg launches.
             //
-            // Launch preference: `resolve_npx_command("hermes")` checks PATH
-            // first, so an official-installer `hermes` (which self-updates)
-            // naturally outranks the npm-managed copy; the npm global install
-            // is the managed/one-click channel codeg's Install button drives.
+            // Launch preference: `resolve_npx_command("hermes")` checks PATH,
+            // then `~/.local/bin` (the official installer's target), then the
+            // npm global prefix. Whatever is on the app's PATH wins first; past
+            // that, an official-installer `hermes` (which self-updates)
+            // outranks the npm-managed copy even when the app's PATH lacks
+            // `~/.local/bin`. The npm global install is the managed/one-click
+            // channel codeg's Install button drives.
             distribution: AgentDistribution::Npx {
                 version: "0.21.5",
                 package: "hermes-agent@0.21.5",
@@ -4234,8 +4237,9 @@ mod tests {
     // `acp` subcommand — the package's OTHER bins (`hermes-agent`,
     // `hermes-npm`) map to different console scripts (`run_agent:main` and
     // the bridge maintenance CLI), not the ACP adapter. `resolve_npx_command`
-    // checks PATH before the npm prefix, so an official-installer `hermes`
-    // keeps outranking the npm-managed copy without any policy bit.
+    // checks PATH and `~/.local/bin` before the npm prefix, so an
+    // official-installer `hermes` keeps outranking the npm-managed copy
+    // without any policy bit.
     #[test]
     fn hermes_launches_the_hermes_bin_with_acp_subcommand() {
         let meta = get_agent_meta(AgentType::Hermes);

src-tauri/src/commands/acp.rs

@@ -195,9 +195,10 @@ pub(crate) fn apply_custom_version_to_url(
     url.replace(registry_version, custom_version)
 }
 
-/// Check whether an NPX agent command is spawnable.
-/// Uses PATH first, then falls back to the current npm global prefix to handle
-/// GUI environments that don't inherit the user's shell PATH.
+/// Check whether an NPX agent command is spawnable, by the same lookup as
+/// [`resolve_npx_command`]: PATH, then `~/.local/bin`, then the current npm
+/// global prefix. The fallbacks handle GUI environments that don't inherit the
+/// user's shell PATH.
 pub(crate) async fn is_cmd_available(cmd: &str) -> bool {
     resolve_npx_command(cmd).await.is_some()
 }
@@ -212,7 +213,9 @@ pub(crate) fn resolve_command_on_path(cmd: &str) -> Option<PathBuf> {
 /// cached — the managed cache always wins, the system CLI only fills the
 /// gap, mirroring how npx agents already prefer a PATH install at launch.
 /// Checks PATH first, then `~/.local/bin` — a common install-script target
-/// a macOS GUI app's PATH typically lacks.
+/// a macOS GUI app's PATH typically lacks. [`resolve_npx_command`] runs these
+/// same two steps before the npm global prefix, so an npx agent's official
+/// installer copy (Hermes) is found the same way.
 pub(crate) fn resolve_system_agent_binary(cmd: &str) -> Option<PathBuf> {
     if let Some(path) = resolve_command_on_path(cmd) {
         return Some(path);
@@ -256,16 +259,13 @@ pub(crate) fn resolve_system_agent_binary_for(agent_type: AgentType, cmd: &str)
 /// it doesn't speak ACP" instead of a bare "not installed".
 ///
 /// Widest resolution of the three probes, because a false negative here loses
-/// the most convincing evidence: PATH + the npm global prefix (the GUI-PATH-gap
-/// fallback `resolve_npx_command` already implements), then `~/.local/bin` via
-/// [`resolve_system_agent_binary`], then the vendor installers' own dirs.
+/// the most convincing evidence: everything [`resolve_npx_command`] checks
+/// (PATH, `~/.local/bin`, the npm global prefix), then the vendor installers'
+/// own dirs.
 pub(crate) async fn resolve_vendor_cli(cmd: &str, extra_dirs: &[&str]) -> Option<PathBuf> {
     if let Some(path) = resolve_npx_command(cmd).await {
         return Some(path);
     }
-    if let Some(path) = resolve_system_agent_binary(cmd) {
-        return Some(path);
-    }
     let exe = if cfg!(windows) {
         format!("{cmd}.exe")
     } else {
@@ -405,8 +405,19 @@ async fn prewarm_uvx_agent(
     Ok(())
 }
 
+/// Resolve an npx agent's command the way connect, launch, status and
+/// diagnostics all see it: PATH, then `~/.local/bin` (both via
+/// [`resolve_system_agent_binary`]), then the current npm global prefix.
+///
+/// Both fallbacks cover an app PATH that lacks the user's shell PATH.
+/// `~/.local/bin` goes first on purpose: when neither copy is on the app's
+/// PATH, a vendor's official installer copy (which targets `~/.local/bin`)
+/// beats an npm copy, as it typically does in a terminal where the installer
+/// put `~/.local/bin` first on PATH. See the Hermes "Launch preference" note
+/// in `acp/registry.rs`. [`NpxCommandResolver::resolve_for_list`] must keep
+/// the same order.
 pub(crate) async fn resolve_npx_command(cmd: &str) -> Option<PathBuf> {
-    if let Some(path) = resolve_command_on_path(cmd) {
+    if let Some(path) = resolve_system_agent_binary(cmd) {
         return Some(path);
     }
     resolve_npx_command_from_current_npm_prefix(cmd).await
@@ -424,7 +435,7 @@ impl NpxCommandResolver {
             return cached.clone();
         }
 
-        let resolved = if let Some(path) = resolve_command_on_path(cmd) {
+        let resolved = if let Some(path) = resolve_system_agent_binary(cmd) {
             Some(path)
         } else {
             let prefix = if let Some(prefix) = &self.request_npm_prefix {
@@ -1236,6 +1247,16 @@ fn compute_verdict(inp: &DiagInputs) -> DiagnosticsVerdict {
     }
 
     if agent.resolve_npx.is_none() {
+        if agent.db_version.is_none()
+            && agent.detected_version.is_none()
+            && inp.terminal.cmd_resolved.is_some()
+        {
+            return diag_verdict(
+                DiagLevel::Fail,
+                "terminal_only_path",
+                "The command resolves in your terminal but not in the app process — a GUI PATH gap.",
+            );
+        }
         // An ACP adapter agent that was never installed is the single most
         // reported "bug": the user has the vendor CLI and reads "not installed"
         // as codeg failing to see it. Answer the question they're actually
@@ -1694,6 +1715,51 @@ mod diagnostics_tests {
         assert_eq!(compute_verdict(&inp).code, "terminal_only_path");
     }
 
+    #[test]
+    fn verdict_terminal_only_path_without_npm_evidence() {
+        let mut inp = base_inputs();
+        inp.agent = Some(AgentDiag {
+            name: "Hermes Agent".to_string(),
+            cmd: "hermes".to_string(),
+            distribution: "npx",
+            ..Default::default()
+        });
+        inp.terminal.cmd_resolved = Some("/Users/u/.local/bin/hermes".to_string());
+        let v = compute_verdict(&inp);
+        assert_eq!(v.code, "terminal_only_path");
+        assert_eq!(v.level, DiagLevel::Fail);
+    }
+
+    // That no-record check sits after the Node checks, so Node problems are
+    // still reported first, as for every npx agent...
+    #[test]
+    fn verdict_node_missing_outranks_terminal_only_path() {
+        let mut inp = DiagInputs {
+            agent: Some(AgentDiag {
+                name: "Hermes Agent".to_string(),
+                cmd: "hermes".to_string(),
+                distribution: "npx",
+                ..Default::default()
+            }),
+            ..Default::default()
+        };
+        inp.terminal.cmd_resolved = Some("/Users/u/.local/bin/hermes".to_string());
+        assert_eq!(compute_verdict(&inp).code, "node_missing");
+    }
+
+    // ...and before the adapter explainer: when the adapter's OWN command
+    // resolves in the login shell, the adapter is installed, so "the adapter
+    // is not installed" would be wrong.
+    #[test]
+    fn verdict_terminal_only_path_outranks_adapter_explainer() {
+        let mut inp = base_inputs();
+        inp.agent = Some(adapter_agent_never_installed(Some(
+            "/opt/homebrew/bin/codex",
+        )));
+        inp.terminal.cmd_resolved = Some("/Users/u/bin/codex-acp".to_string());
+        assert_eq!(compute_verdict(&inp).code, "terminal_only_path");
+    }
+
     #[test]
     fn verdict_node_too_old() {
         let mut inp = base_inputs();
@@ -2053,7 +2119,8 @@ async fn system_probed_version_with(
     let mut version = None;
     if let Some(probe) = declared_probe {
         // The declared probe is a full command line (`agent-cli --version`);
-        // its program resolves like an agent command (PATH, then npm prefix).
+        // its program resolves like an agent command (PATH, then
+        // `~/.local/bin`, then npm prefix).
         let mut parts = probe.split_whitespace();
         if let Some(program) = parts.next() {
             let args: Vec<String> = parts.map(str::to_string).collect();
@@ -7983,14 +8050,14 @@ fn shell_join(argv: &[String]) -> String {
 
 /// The argv for Hermes's `--setup` and `model` flows: a `hermes` CLI on PATH
 /// by bare name (official installer or an npm global bin on PATH — the short
-/// form reads best in guidance), else the npm-prefix-resolved absolute path
-/// (an npm install whose bin dir is NOT on the terminal's PATH must be
-/// addressed absolutely or the displayed command fails), else a documented
-/// `npx -y --package <pin> hermes …` form that bootstraps on demand.
-/// `--package` is required in that form: the package's default bin is
-/// `hermes-agent` (a different console script), not `hermes`. Returned as
-/// argv vectors so callers can shell-quote per platform for display or
-/// execute them.
+/// form reads best in guidance), else the absolute path `resolve_npx_command`
+/// finds in `~/.local/bin` or the npm prefix (a copy whose dir is NOT on the
+/// terminal's PATH must be addressed absolutely or the displayed command
+/// fails), else a documented `npx -y --package <pin> hermes …` form that
+/// bootstraps on demand. `--package` is required in that form: the package's
+/// default bin is `hermes-agent` (a different console script), not `hermes`.
+/// Returned as argv vectors so callers can shell-quote per platform for
+/// display or execute them.
 async fn hermes_setup_argvs() -> (Vec<String>, Vec<String>) {
     let meta = registry::get_agent_meta(AgentType::Hermes);
     let package = match meta.distribution {
@@ -13273,10 +13340,10 @@ pub(crate) async fn acp_prepare_npx_agent_core(
             // postinstall leaves a shim that exits "runtime is not ready".
             // Verify the binary a launch would resolve actually answers
             // `--version` BEFORE recording success — the same resolution
-            // order connect uses, so an official-installer CLI on PATH
-            // legitimately satisfies the check. Without this, a broken
-            // install is recorded as installed and only fails at connect
-            // time with an opaque error.
+            // order connect uses, so an official-installer CLI (on PATH or in
+            // `~/.local/bin`) legitimately satisfies the check. Without this,
+            // a broken install is recorded as installed and only fails at
+            // connect time with an opaque error.
             if npm_package_requires_scripts(&install_spec) {
                 emit_agent_install_event(
                     emitter,
@@ -18099,6 +18166,53 @@ wire_api = "chat"
         let _ = std::fs::remove_dir_all(prefix);
     }
 
+    #[cfg(not(windows))]
+    #[tokio::test]
+    async fn npx_command_resolution_checks_local_bin_before_npm_prefix() {
+        use std::os::unix::fs::PermissionsExt;
+
+        let home = tempfile::tempdir().expect("tempdir");
+        let bin_dir = home.path().join(".local").join("bin");
+        let prefix = home.path().join("npm-prefix");
+        let prefix_bin_dir = npm_prefix_bin_dir(&prefix);
+        let cmd = "codeg-test-local-bin-npx-agent";
+        let command_path = bin_dir.join(cmd);
+        // Only in the npm prefix: the list path must still fall through to it.
+        let prefix_only_cmd = "codeg-test-npm-prefix-only-npx-agent";
+        let prefix_only_path = prefix_bin_dir.join(prefix_only_cmd);
+        for path in [
+            bin_dir.join(cmd),
+            prefix_bin_dir.join(cmd),
+            prefix_only_path.clone(),
+        ] {
+            std::fs::create_dir_all(path.parent().expect("bin directory"))
+                .expect("create bin directory");
+            std::fs::write(&path, "").expect("write command");
+            std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o755))
+                .expect("mark command executable");
+        }
+
+        let (direct, listed, listed_prefix_only) =
+            temp_env::async_with_vars([("HOME", Some(home.path()))], async {
+                let direct = resolve_npx_command(cmd).await;
+                let mut resolver = NpxCommandResolver {
+                    request_npm_prefix: Some(Some(prefix.clone())),
+                    ..Default::default()
+                };
+                let listed = resolver.resolve_for_list(cmd).await;
+                let listed_prefix_only = resolver.resolve_for_list(prefix_only_cmd).await;
+                (direct, listed, listed_prefix_only)
+            })
+            .await;
+
+        assert_eq!(direct.as_deref(), Some(command_path.as_path()));
+        assert_eq!(listed.as_deref(), Some(command_path.as_path()));
+        assert_eq!(
+            listed_prefix_only.as_deref(),
+            Some(prefix_only_path.as_path())
+        );
+    }
+
     fn write_skill_md(name: &str, body: &str) -> (PathBuf, PathBuf) {
         let dir = unique_test_dir(name);
         let path = dir.join("SKILL.md");