Compare commits

...

2 Commits

Author SHA1 Message Date
ethernet
20617bc18a fix: prefer managed uv in MCP stdio command resolver
After #37660, Hermes owns its own uv/uvx at $HERMES_HOME/bin/. The
MCP resolver's candidate list for bare uv/uvx commands must check this
location before falling back to ~/.local/bin, /opt/homebrew/bin, etc.,
so MCP servers use the same managed uv as the CLI update path.

Adds test_resolve_stdio_command_prefers_managed_uv to verify ordering
against a stale ~/.local/bin/uv.
2026-06-02 18:47:06 -04:00
mohamedorigami-jpg
f4ce36cd47 fix(tools): resolve uv/uvx MCP commands under GUI-style PATHs (fixes #37589)
_tools/mcp_tool._resolve_stdio_command_ already fell back to ~/.local/bin
and /usr/local/bin for bare npx/npm/node MCP commands on
filtered PATHs (the docker sandbox case), but it did NOT cover uv
and uvx — the dominant Python MCP server runtime. On macOS,
Hermes Desktop inherits a minimal LaunchAgent PATH
(/usr/bin:/bin:/usr/sbin:/sbin) that omits ~/.local/bin (the
uv user installer target), /opt/homebrew/bin (Apple Silicon
Homebrew), and /usr/local/bin (Intel Homebrew / Linux from-source).
A bare command: uvx MCP server therefore fails with ENOENT at
execvp from Hermes Desktop even though it works from an interactive
terminal.

This adds uv and uvx to the candidate allowlist, plus a new
/opt/homebrew/bin candidate for the Apple Silicon case. Existing
ordering is preserved (~/.local/bin before /opt/homebrew/bin before
/usr/local/bin), so users with multiple installs continue to resolve
to whichever the user installed first.

Tests: 4 new tests in test_mcp_tool_issue_948.py cover the ~/.local/bin
fallback, shutil.which preemption, an unknown-command negative case
(catches the inverse regression of someone adding a too-broad allowlist
that rewrites bare my-tool to a coincidentally-named file), and
the existing npx tests continue to pass.

Closes #37589
2026-06-03 00:43:37 +03:00
2 changed files with 118 additions and 13 deletions

View File

@@ -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)

View File

@@ -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: