diff --git a/tests/tools/test_mcp_tool_issue_948.py b/tests/tools/test_mcp_tool_issue_948.py index aefb32481df9..b11905ba4031 100644 --- a/tests/tools/test_mcp_tool_issue_948.py +++ b/tests/tools/test_mcp_tool_issue_948.py @@ -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 @@ -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" diff --git a/tools/mcp_tool.py b/tools/mcp_tool.py index b15fcdaecb1d..98fd50dcda89 100644 --- a/tools/mcp_tool.py +++ b/tools/mcp_tool.py @@ -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,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"}: 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: