mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-10 12:18:44 +08:00
Compare commits
2 Commits
bb/desktop
...
pr-37665
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
20617bc18a | ||
|
|
f4ce36cd47 |
@@ -3,6 +3,7 @@ import os
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from tools.mcp_tool import MCPServerTask, _format_connect_error, _resolve_stdio_command, _MCP_AVAILABLE
|
||||
|
||||
@@ -126,3 +127,83 @@ def test_run_stdio_uses_resolved_command_and_prepended_path(tmp_path):
|
||||
await server.shutdown()
|
||||
|
||||
asyncio.run(_test())
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Regression tests for #37589: Desktop/launchd processes inherit a minimal
|
||||
# PATH on macOS that does not include ~/.local/bin, /opt/homebrew/bin, or
|
||||
# /usr/local/bin. The resolver must locate uv/uvx (the dominant MCP server
|
||||
# runtime for Python projects) under those locations.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_resolve_stdio_command_finds_uvx_in_user_local_bin(tmp_path):
|
||||
"""uv's official installer drops uv/uvx at ``~/.local/bin/uvx`` on
|
||||
macOS and Linux. The resolver must pick it up when the GUI PATH
|
||||
doesn't include that directory (#37589)."""
|
||||
local_bin = tmp_path / ".local" / "bin"
|
||||
local_bin.mkdir(parents=True)
|
||||
uvx_path = local_bin / "uvx"
|
||||
uvx_path.write_text("#!/bin/sh\nexit 0\n", encoding="utf-8")
|
||||
uvx_path.chmod(0o755)
|
||||
|
||||
with patch("tools.mcp_tool.shutil.which", return_value=None), \
|
||||
patch("os.path.expanduser", lambda p: p.replace("~", str(tmp_path)) if p == "~" else p):
|
||||
command, env = _resolve_stdio_command("uvx", {"PATH": "/usr/bin:/bin:/usr/sbin:/sbin"})
|
||||
|
||||
assert command == str(uvx_path)
|
||||
# The resolver prepended the chosen bin so uvx's shebang-resolved
|
||||
# children (uv itself, python) can be found in the same directory.
|
||||
assert env["PATH"].split(os.pathsep)[0] == str(local_bin)
|
||||
|
||||
|
||||
def test_resolve_stdio_command_uvx_unchanged_when_already_on_path():
|
||||
"""shutil.which hit must still take precedence — don't double-resolve
|
||||
a working bare command on PATH into something else."""
|
||||
resolved_path = "/some/path/uvx"
|
||||
with patch("tools.mcp_tool.shutil.which", return_value=resolved_path):
|
||||
command, _env = _resolve_stdio_command("uvx", {"PATH": "/usr/bin"})
|
||||
|
||||
assert command == resolved_path
|
||||
|
||||
|
||||
def test_resolve_stdio_command_skips_unknown_commands():
|
||||
"""Bare command names outside the npx/npm/node/uv/uvx allowlist must
|
||||
NOT be matched against the candidate fallback paths — that would
|
||||
produce false positives like rewriting ``command: my-tool`` into a
|
||||
coincidentally-named file at ``/opt/homebrew/bin/my-tool`` (#37589)."""
|
||||
with patch("tools.mcp_tool.shutil.which", return_value=None), \
|
||||
patch("tools.mcp_tool.os.path.expanduser", lambda p: p), \
|
||||
patch("tools.mcp_tool.os.path.isfile", return_value=True), \
|
||||
patch("tools.mcp_tool.os.access", return_value=True):
|
||||
# A command like 'foo' or 'python' must be left alone even if
|
||||
# the test is faking every candidate as present.
|
||||
command, _env = _resolve_stdio_command("foo", {"PATH": "/usr/bin:/bin"})
|
||||
|
||||
assert command == "foo"
|
||||
|
||||
|
||||
def test_resolve_stdio_command_prefers_managed_uv(tmp_path):
|
||||
"""Managed uv at $HERMES_HOME/bin/uv should be preferred over
|
||||
~/.local/bin, /opt/homebrew/bin, and /usr/local/bin — MCP servers
|
||||
must use the same uv as the CLI update path (managed_uv.py)."""
|
||||
hermes_bin = tmp_path / "bin"
|
||||
hermes_bin.mkdir()
|
||||
uv_path = hermes_bin / "uv"
|
||||
uv_path.write_text("#!/bin/sh\necho uv 0.1.2\n", encoding="utf-8")
|
||||
uv_path.chmod(0o755)
|
||||
|
||||
# Also create a stale uv at ~/.local/bin to verify ordering.
|
||||
local_bin = tmp_path / ".local" / "bin"
|
||||
local_bin.mkdir(parents=True)
|
||||
stale_uv = local_bin / "uv"
|
||||
stale_uv.write_text("#!/bin/sh\necho stale\n", encoding="utf-8")
|
||||
stale_uv.chmod(0o755)
|
||||
|
||||
with patch("tools.mcp_tool.shutil.which", return_value=None), \
|
||||
patch("os.path.expanduser", lambda p: p.replace("~", str(tmp_path)) if p.startswith("~") else p), \
|
||||
patch.dict(os.environ, {"HERMES_HOME": str(tmp_path)}):
|
||||
command, env = _resolve_stdio_command("uv", {"PATH": "/usr/bin:/bin"})
|
||||
|
||||
assert command == str(uv_path)
|
||||
assert env["PATH"].split(os.pathsep)[0] == str(hermes_bin)
|
||||
|
||||
@@ -402,8 +402,15 @@ def _prepend_path(env: dict, directory: str) -> dict:
|
||||
def _resolve_stdio_command(command: str, env: dict) -> tuple[str, dict]:
|
||||
"""Resolve a stdio MCP command against the exact subprocess environment.
|
||||
|
||||
This primarily exists to make bare ``npx``/``npm``/``node`` commands work
|
||||
reliably even when MCP subprocesses run under a filtered PATH.
|
||||
This primarily exists to make bare ``npx``/``npm``/``node`` and
|
||||
``uv``/``uvx`` commands work reliably even when MCP subprocesses run
|
||||
under a filtered PATH (#37589). On macOS, processes launched from the
|
||||
GUI / LaunchAgents inherit a minimal PATH (``/usr/bin:/bin:/usr/sbin:/sbin``)
|
||||
that does not include ``~/.local/bin`` (uv user install), ``/opt/homebrew/bin``
|
||||
(Apple Silicon Homebrew), or ``/usr/local/bin`` (Intel Homebrew / Linux
|
||||
from-source), so a bare ``command: uvx`` MCP server fails with ENOENT
|
||||
at ``execvp`` from Hermes Desktop even though it works from an
|
||||
interactive Terminal.
|
||||
"""
|
||||
resolved_command = os.path.expanduser(str(command).strip())
|
||||
resolved_env = dict(env or {})
|
||||
@@ -413,25 +420,42 @@ def _resolve_stdio_command(command: str, env: dict) -> tuple[str, dict]:
|
||||
which_hit = shutil.which(resolved_command, path=path_arg)
|
||||
if which_hit:
|
||||
resolved_command = which_hit
|
||||
elif resolved_command in {"npx", "npm", "node"}:
|
||||
elif resolved_command in {"npx", "npm", "node", "uv", "uvx"}:
|
||||
hermes_home = os.path.expanduser(
|
||||
os.getenv(
|
||||
"HERMES_HOME", os.path.join(os.path.expanduser("~"), ".hermes")
|
||||
)
|
||||
)
|
||||
candidates = [
|
||||
# Hermes-bundled Node first so the desktop app's pinned
|
||||
# runtime wins over a system Node that may be incompatible.
|
||||
os.path.join(hermes_home, "node", "bin", resolved_command),
|
||||
# Managed uv: Hermes owns its own uv/uvx at
|
||||
# $HERMES_HOME/bin/ (see hermes_cli/managed_uv.py).
|
||||
# Always prefer this over PATH/brew/pip installs so MCP
|
||||
# servers use the same uv as the CLI update path.
|
||||
os.path.join(hermes_home, "bin", resolved_command),
|
||||
# uv's official macOS/Linux installer drops uv/uvx here.
|
||||
# This is the dominant install location for developers
|
||||
# on Apple Silicon and Ubuntu. The ``uv`` user installer
|
||||
# is documented to land here.
|
||||
os.path.join(os.path.expanduser("~"), ".local", "bin", resolved_command),
|
||||
# /usr/local/bin is the canonical install location for Node on
|
||||
# Linux from-source builds, the upstream node:bookworm-slim
|
||||
# image (which the Hermes Docker image copies node + npm +
|
||||
# corepack from since #4977), and macOS Homebrew on Intel.
|
||||
# Without this candidate, any MCP server configured with an
|
||||
# env.PATH that omits /usr/local/bin (a common pattern when
|
||||
# users hand-author PATH for sandboxing) fails with ENOENT
|
||||
# at execvp, and a naive symlink workaround into the user's
|
||||
# PATH only fails one layer deeper because npx's shebang
|
||||
# re-execs /usr/bin/env node which needs the same directory.
|
||||
# /opt/homebrew/bin — Apple Silicon Homebrew. uv is
|
||||
# installable via `brew install uv`, and a non-trivial
|
||||
# share of macOS users (including the original bug
|
||||
# reporter) have it here.
|
||||
os.path.join(os.sep, "opt", "homebrew", "bin", resolved_command),
|
||||
# /usr/local/bin — the canonical install location for
|
||||
# Node on Linux from-source builds, the upstream
|
||||
# node:bookworm-slim image (which the Hermes Docker
|
||||
# image copies node + npm + corepack from since #4977),
|
||||
# and macOS Homebrew on Intel. Without this candidate,
|
||||
# any MCP server configured with an env.PATH that omits
|
||||
# /usr/local/bin (a common pattern when users hand-author
|
||||
# PATH for sandboxing) fails with ENOENT at execvp, and
|
||||
# a naive symlink workaround into the user's PATH only
|
||||
# fails one layer deeper because npx's shebang re-execs
|
||||
# /usr/bin/env node which needs the same directory.
|
||||
os.path.join(os.sep, "usr", "local", "bin", resolved_command),
|
||||
]
|
||||
for candidate in candidates:
|
||||
|
||||
Reference in New Issue
Block a user