Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 55 additions & 0 deletions tests/tools/test_mcp_tool_issue_948.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
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

Expand Down Expand Up @@ -126,3 +127,57 @@ async def _test():
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"
45 changes: 32 additions & 13 deletions tools/mcp_tool.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 {})
Expand All @@ -413,25 +420,37 @@ 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"}:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add $HERMES_HOME/bin/<cmd> to this fallback list before ~/.local/bin. The PR description promises managed uv preference, and hermes_cli/managed_uv.py:1-50 defines $HERMES_HOME/bin/uv as Hermes’s authoritative managed location; it is absent from this PR head’s candidates.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hope to solve asap,when config mcp, it is difficult.

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),
# 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:
Expand Down