Skip to content

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

Merged
xintaofei merged 2 commits into
spacering-net:mainfrom
Sma1lboy:fix/npx-agent-local-bin
Oct 9, 2026
Merged

xintaofei merged 2 commits into
spacering-net:mainfrom
Sma1lboy:fix/npx-agent-local-bin

Conversation

@Sma1lboy

@Sma1lboy Sma1lboy commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

What does this PR do?

Hermes installed with the official installer (~/.local/bin/hermes, no npm hermes-agent) shows as not installed when codeg is launched from the GUI, and diagnostics report not_installed. Two causes, both in src-tauri/src/commands/acp.rs:

  1. resolve_npx_command() and NpxCommandResolver::resolve_for_list() (the agent-list path) only check PATH and the npm global prefix. A GUI app's PATH usually lacks ~/.local/bin, so Hermes resolves to None. Binary agents already fall back to ~/.local/bin through resolve_system_agent_binary(); npx agents did not.
  2. compute_verdict() only reaches terminal_only_path for an unresolved npx agent when db_version or detected_version is set. Both come from npm, so an install without an npm record falls through to not_installed, even when the terminal probe found the command.

Related Issue

Fixes #875

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)

Changes Made

  • src-tauri/src/commands/acp.rs: resolve_npx_command() and NpxCommandResolver::resolve_for_list() now call resolve_system_agent_binary(cmd) (PATH, then ~/.local/bin) before the npm global prefix. The order is PATH → ~/.local/bin → npm prefix, so when both an official install and an npm copy exist, a GUI launch picks the official one, the same as a terminal launch where it is on PATH. This matches the launch preference described in the Hermes entry in acp/registry.rs.
  • src-tauri/src/commands/acp.rs: in compute_verdict(), an unresolved npx agent with no npm record whose command resolves in the login shell now gets terminal_only_path instead of not_installed. The Node/npm checks still run first. The check sits before the adapter branch because the terminal probe looks up the agent's own command, so a hit means the adapter itself is installed. The existing npm-evidence branch is unchanged.
  • Two unit tests:
    • npx_command_resolution_checks_local_bin_before_npm_prefix: with HOME pointed at a temp dir, both npx lookup paths find ~/.local/bin/<cmd>. The list path also has the same command in an npm prefix and still returns the ~/.local/bin one. Unix only.
    • verdict_terminal_only_path_without_npm_evidence.

How to Test

  1. On macOS, install Hermes with the official installer so ~/.local/bin/hermes exists, and make sure the npm hermes-agent package is not installed.
  2. Start codeg-server with a GUI-like PATH that has node/npm but not ~/.local/bin, e.g. PATH=<nvm node bin>:/usr/bin:/bin:/usr/sbin:/sbin, and an empty CODEG_DATA_DIR.
  3. Call POST /api/acp_env_diagnostics with {"agentType":"hermes"}, plus acp_get_agent_status and acp_list_agents.

Result with Hermes 0.21.5 on macOS 15 (Apple Silicon), same environment for both builds:

main (592131c) this PR
hermes (resolve_npx_command) NOT RESOLVED ~/.local/bin/hermes
command -v <cmd> (login shell) ~/.local/bin/hermes ~/.local/bin/hermes
verdict not_installed ok
status / list installed_version null 0.21.5+5839.gbed0d53

The two new unit tests fail on main and pass on this branch.

Checklist

Code

  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate (fix: detect externally installed ACP agents #286 is an older, broader change in the same area)
  • My PR contains only changes related to this fix
  • cargo test --features test-utils --lib commands::acp passes
  • cargo clippy -- -D warnings (desktop and server): see the note below
  • I've added tests for my changes
  • I've tested on my platform: macOS 15 (Apple Silicon)

Documentation & Housekeeping

  • I've updated relevant documentation: N/A
  • I've considered cross-platform impact (Windows, macOS): the ~/.local/bin lookup is the existing resolve_system_agent_binary(), including its .exe handling; the resolver test is unix only

Screenshots / Logs

  • cargo test --features test-utils --lib commands::acp: all passed.
  • Desktop and server cargo clippy ... -- -D warnings with Rust 1.96 report one error, clippy::nonminimal_bool at src/computer/helper/mod.rs:667. That line exists on main and is not touched here. With -A clippy::nonminimal_bool both are clean.
  • cargo fmt --check already reports differences across the repo on main. Formatting acp.rs before and after this change gives the same diff as this PR, so the new lines are formatted.

@Sma1lboy
Sma1lboy marked this pull request as ready for review October 8, 2026 07:53
… probe

An official-installer CLI (Hermes' `~/.local/bin/hermes`) was invisible
to npx-agent resolution, which only checked PATH and the npm global
prefix, so a GUI-launched codeg reported it not installed. Both
`resolve_npx_command` and the list path's `NpxCommandResolver` now go
through `resolve_system_agent_binary` (PATH, then `~/.local/bin`) before
the npm prefix, so the official install outranks an npm copy in a GUI
launch as it already does on PATH.

Diagnostics also only reached `terminal_only_path` with npm evidence; a
login-shell hit with no npm record now reports the GUI PATH gap instead
of `not_installed`.

Fixes spacering-net#875
@Sma1lboy
Sma1lboy force-pushed the fix/npx-agent-local-bin branch from 094e7d3 to a46aba6 Compare October 8, 2026 08:18
…stale docs

Review fixes on top of the ~/.local/bin npx fallback (spacering-net#888).

- Document PATH -> ~/.local/bin -> npm prefix on resolve_npx_command, and
  update the comments that still described PATH -> npm prefix:
  is_cmd_available, resolve_system_agent_binary, resolve_vendor_cli, the
  declared version probe, hermes_setup_argvs, the post-install runtime
  check, the Hermes registry entry and its test, and preflight's adapter
  probe.
- Drop resolve_vendor_cli's second ~/.local/bin lookup; resolve_npx_command
  now performs it first, so the call could never find anything new.
- Add verdict tests that keep the no-record terminal check after the Node
  checks and before the ACP adapter explainer. Moving it either way left
  every existing test green.
- Extend the resolver test so the list path still falls back to the npm
  prefix for a command that is not in ~/.local/bin.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@xintaofei

Copy link
Copy Markdown
Collaborator

codeg work task 289 is done — #888 (3 files, +107/-48).

@xintaofei
xintaofei merged commit 8231b40 into spacering-net:main Oct 9, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants