From 05f54334bacf39e281de424a7671af2c13479016 Mon Sep 17 00:00:00 2001 From: dinos Date: Tue, 21 Apr 2026 17:59:06 +0200 Subject: [PATCH] fix(mcp): stdio env passthrough + durable package installs (#169) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(mcp): forward proxy and CA bundle env vars to stdio subprocesses The MCP SDK's stdio transport inherits only a minimal allowlist (HOME, PATH, USER, …) from the parent, stripping http_proxy/https_proxy and SSL_CERT_FILE/REQUESTS_CA_BUNDLE/etc. Behind a proxy or with a custom CA bundle, stdio MCP servers silently hang on outbound requests while the same server over HTTP transport works. Auto-forward the proxy and cert vars when present; user-configured env still takes precedence. * fix(mcp): use `uv tool install` so MCP packages survive uv sync Source installs previously used `uv pip install --python $VENV `, which lands in the evosci venv but is not recorded in pyproject.toml or uv.lock. A subsequent `uv sync` (typical after `git pull`) reconciles the venv to the lockfile and removes the MCP package, forcing users to re-run onboard. Prefer `uv tool install ` for the non-uv-tool install path: the binary symlink in ~/.local/bin survives uv sync and evosci upgrades, and the MCP server gets its own isolated env (no dep conflicts). Verify the expected CLI entry point resolves afterward; if not (package has no console-script), fall through to the old uv-pip path so command-less packages still work. The uv-tool-env path (`uv tool install evoscientist --with `) is unchanged — it was already durable via uv's receipt. * fix(mcp): gate standalone uv tool install on verify_command Previously `install_pip_package` would route every install through `uv tool install ` when `verify_command` was None, returning success as long as the uv subprocess exited 0. Library callers (`evoscientist[oauth]`, `lark-oapi`, etc.) expect the package to land in the active venv so they can import it — a standalone uv tool env is not importable, so the import fails at the next line. Gate the `uv tool install ` branch on `verify_command` being set: that signals the caller wants a durable CLI binary, which is what `uv tool install` produces. Library callers omit it and go straight to the pip-install-into-venv path. Also: log info messages on every fall-through so stale-binary and entry-point-missing failure modes are debuggable, and document the --with → standalone recovery path. * fix(mcp): resolve MCP binaries to `uv tool dir --bin`, not `.venv/bin` Under `uv run`, the project venv's `bin/` comes first on PATH, so `shutil.which("arxiv-mcp-server")` returns a stale `.venv/bin/` copy left over from an earlier install instead of the fresh symlink that `uv tool install` just placed in `~/.local/bin`. The venv copy gets written to mcp.yaml and is then wiped by the next `uv sync` — exactly the failure mode the durability fix was meant to prevent. Query `uv tool dir --bin` directly and prefer binaries found there over `shutil.which`. Same change to the post-install verify in `install_pip_package` so a venv shadow can't falsely short-circuit the fallback. * refactor(mcp): split install_pip_package into install_library + install_cli_tool `verify_command` was doing double duty: naming the CLI binary to check *and* signaling "this is a CLI install, use the standalone `uv tool install` path." Callers routed library installs through the CLI branch any time they forgot to pass it, and the resulting standalone uv tool env wasn't importable from the active venv. Separate the two use cases into distinct functions, each with one install strategy per environment shape. Shared logic lives in private `_install_with_uv_tool_env` / `_install_via_pip` helpers. - install_library(pkg): uv-tool-env --with → pip. Never uses standalone `uv tool install ` (not importable from active venv). - install_cli_tool(pkg, *, verify_command): uv-tool-env --with → standalone `uv tool install` → pip. `verify_command` is now required. Callers pick the right function at the call site: registry.py picks based on whether `entry.command` is set; onboard.py call sites all install libraries. --- EvoScientist/config/onboard.py | 12 +- EvoScientist/mcp/client.py | 29 +++- EvoScientist/mcp/registry.py | 222 +++++++++++++++++++++++----- tests/test_mcp_client.py | 263 ++++++++++++++++++++++++++++----- 4 files changed, 446 insertions(+), 80 deletions(-) diff --git a/EvoScientist/config/onboard.py b/EvoScientist/config/onboard.py index d2ae56c..fbab5b7 100644 --- a/EvoScientist/config/onboard.py +++ b/EvoScientist/config/onboard.py @@ -2210,9 +2210,9 @@ def _install_ccproxy() -> bool: True if installation succeeded and ccproxy is available. """ from ..ccproxy_manager import is_ccproxy_available - from ..mcp.registry import install_pip_package + from ..mcp.registry import install_library - ok = install_pip_package("evoscientist[oauth]") + ok = install_library("evoscientist[oauth]") if not ok: console.print(" [red]✗ Installation failed.[/red]") return False @@ -2456,7 +2456,7 @@ def _step_channels(config: EvoScientistConfig) -> dict[str, object]: updates["imessage_enabled"] = False return updates - from ..mcp.registry import install_pip_package, pip_install_hint + from ..mcp.registry import install_library, pip_install_hint # Build a lookup for channel definitions _ch_lookup = { @@ -2495,9 +2495,9 @@ def _step_channels(config: EvoScientistConfig) -> dict[str, object]: if install_now: console.print(f" [dim]Installing {_pkg_display}...[/dim]") if _pip_pkgs: - _ok = all(install_pip_package(p) for p in _pip_pkgs) + _ok = all(install_library(p) for p in _pip_pkgs) else: - _ok = install_pip_package(f"evoscientist[{pip_extra}]") + _ok = install_library(f"evoscientist[{pip_extra}]") if _ok: # Verify the import actually works now try: @@ -2603,7 +2603,7 @@ def _step_channels(config: EvoScientistConfig) -> dict[str, object]: raise KeyboardInterrupt() from None if install_sdk: console.print(' [dim]Installing "lark-oapi"...[/dim]') - if install_pip_package("lark-oapi>=1.4.0"): + if install_library("lark-oapi>=1.4.0"): console.print(" [green]✓ Installed successfully.[/green]") else: console.print(" [red]✗ Installation failed.[/red]") diff --git a/EvoScientist/mcp/client.py b/EvoScientist/mcp/client.py index 48ed9c1..96afcb8 100644 --- a/EvoScientist/mcp/client.py +++ b/EvoScientist/mcp/client.py @@ -33,6 +33,26 @@ VALID_TRANSPORTS = {"stdio", "http", "streamable_http", "sse", "websocket"} # URL-based transports (share the same connection shape) _URL_TRANSPORTS = {"http", "streamable_http", "sse", "websocket"} +# Env vars forwarded to stdio MCP subprocesses on top of the MCP SDK's +# minimal default set (HOME/PATH/USER/…). Without this, servers behind +# a proxy or with a custom CA bundle silently fail with long timeouts. +# User-provided ``env`` still wins via dict merge. +_STDIO_FORWARDED_ENV_VARS = ( + "http_proxy", + "https_proxy", + "HTTP_PROXY", + "HTTPS_PROXY", + "all_proxy", + "ALL_PROXY", + "no_proxy", + "NO_PROXY", + "SSL_CERT_FILE", + "SSL_CERT_DIR", + "REQUESTS_CA_BUNDLE", + "CURL_CA_BUNDLE", + "NODE_EXTRA_CA_CERTS", +) + def _get_mcp_config_dir() -> Path: """Get the MCP configuration directory, respecting XDG_CONFIG_HOME.""" @@ -527,8 +547,13 @@ def _build_connections(config: dict[str, Any]) -> dict[str, dict[str, Any]]: "command": _resolve_command(server.get("command", "")), "args": server.get("args", []), } - if "env" in server: - conn["env"] = server["env"] + forwarded = { + k: os.environ[k] for k in _STDIO_FORWARDED_ENV_VARS if k in os.environ + } + user_env = server.get("env") or {} + merged = {**forwarded, **user_env} + if merged: + conn["env"] = merged connections[name] = conn elif transport in _URL_TRANSPORTS: diff --git a/EvoScientist/mcp/registry.py b/EvoScientist/mcp/registry.py index 292927e..2660909 100644 --- a/EvoScientist/mcp/registry.py +++ b/EvoScientist/mcp/registry.py @@ -26,6 +26,9 @@ import yaml logger = logging.getLogger(__name__) +# Sentinel for function-local caches that legitimately store ``None``. +_UNSET = object() + # ============================================================================= # Data model @@ -156,50 +159,101 @@ def pip_install_hint() -> str: return "pip install" -def install_pip_package(package: str) -> bool: - """Silently install a pip package. +def _uv_tool_bin_dir() -> Path | None: + """Return uv's tool bin directory — where ``uv tool install`` places + symlinks (typically ``~/.local/bin``). ``None`` when uv isn't + available or the query fails. - In a **uv tool environment**, uses ``uv tool install --with`` - which records the dependency in uv's receipt so it survives - ``uv tool upgrade``. Existing ``--with`` packages are preserved. - - Otherwise, when ``uv`` is available, uses - ``uv pip install --python sys.executable``. - Falls back to ``python -m pip install`` when uv is not available. - - Returns True if installation succeeded. + Cached for the lifetime of the process. """ - # ---- uv tool env: durable install via `uv tool install --with` ---- - if _is_uv_tool_env() and shutil.which("uv"): - tool_name = _uv_tool_name() - if tool_name: - existing = _uv_tool_existing_requirements() - cmd = ["uv", "tool", "install", tool_name, "-q"] - for spec in existing.values(): - cmd += ["--with", spec] - if _bare_package_name(package) not in existing: - cmd += ["--with", package] - try: - result = subprocess.run( - cmd, capture_output=True, text=True, timeout=180 - ) - if result.returncode == 0: - import importlib + cached = getattr(_uv_tool_bin_dir, "_cached", _UNSET) + if cached is not _UNSET: + return cached # type: ignore[return-value] + result: Path | None = None + if shutil.which("uv"): + try: + proc = subprocess.run( + ["uv", "tool", "dir", "--bin"], + capture_output=True, + text=True, + timeout=10, + ) + if proc.returncode == 0: + path = proc.stdout.strip() + if path: + result = Path(path) + except (FileNotFoundError, subprocess.TimeoutExpired): + pass + _uv_tool_bin_dir._cached = result # type: ignore[attr-defined] + return result - importlib.invalidate_caches() - return True - except (FileNotFoundError, subprocess.TimeoutExpired): - pass - # Fall through to legacy methods if uv tool install failed. - # ---- Standard venv / conda / system Python ---- +def _uv_tool_bin(command: str) -> Path | None: + """Return the executable path for *command* in ``uv tool dir --bin`` + if it exists, else ``None``.""" + tool_bin = _uv_tool_bin_dir() + if tool_bin is None: + return None + cand = tool_bin / command + if cand.is_file() and os.access(cand, os.X_OK): + return cand + if os.name == "nt": + cand_exe = cand.with_suffix(".exe") + if cand_exe.is_file(): + return cand_exe + return None + + +def _install_with_uv_tool_env(package: str) -> bool: + """Install *package* into the evosci uv-tool env via ``--with``. + + Only does anything when the current interpreter is inside a uv tool + env; otherwise returns False immediately. On success the package is + recorded in uv's receipt and survives ``uv tool upgrade evoscientist``. + """ + if not (_is_uv_tool_env() and shutil.which("uv")): + return False + tool_name = _uv_tool_name() + if not tool_name: + return False + existing = _uv_tool_existing_requirements() + cmd = ["uv", "tool", "install", tool_name, "-q"] + for spec in existing.values(): + cmd += ["--with", spec] + if _bare_package_name(package) not in existing: + cmd += ["--with", package] + try: + result = subprocess.run(cmd, capture_output=True, text=True, timeout=180) + if result.returncode == 0: + import importlib + + importlib.invalidate_caches() + return True + logger.info( + "uv tool install %s --with %s failed (exit %d); falling back", + tool_name, + package, + result.returncode, + ) + except (FileNotFoundError, subprocess.TimeoutExpired) as exc: + logger.info( + "uv tool install --with %s errored (%s); falling back", package, exc + ) + return False + + +def _install_via_pip(package: str) -> bool: + """Install *package* into the active venv via ``uv pip`` or ``pip``. + + Importable from the current interpreter on success. Not durable under + ``uv sync`` — the reconcile will remove anything not in the lockfile. + """ commands: list[list[str]] = [] if shutil.which("uv"): commands.append( ["uv", "pip", "install", "--python", sys.executable, "-q", package] ) commands.append([sys.executable, "-m", "pip", "install", "-q", package]) - for cmd in commands: try: result = subprocess.run(cmd, capture_output=True, text=True, timeout=120) @@ -213,17 +267,105 @@ def install_pip_package(package: str) -> bool: return False +def install_library(package: str) -> bool: + """Install a package that will be imported from the active environment. + + Strategy: + + 1. **uv tool env**: ``uv tool install evoscientist --with `` — + package lands in evosci's isolated env and is importable from it. + 2. **Fallback**: ``uv pip install`` / ``pip install`` into the active + venv. Not durable under ``uv sync``, but there's no better option + for libraries outside a uv tool env. + + Does not use standalone ``uv tool install `` — that creates an + isolated env the caller can't import from. + """ + if _install_with_uv_tool_env(package): + return True + return _install_via_pip(package) + + +def install_cli_tool(package: str, *, verify_command: str) -> bool: + """Install a package whose primary deliverable is a CLI binary. + + Strategy: + + 1. **uv tool env**: ``uv tool install evoscientist --with `` — + durable via uv's receipt, binary resolvable via the tool's bin dir. + 2. **Standalone ``uv tool install ``**: binary symlinked under + ``~/.local/bin`` (``uv tool dir --bin``); survives ``uv sync``. + 3. **Fallback**: ``uv pip install`` / ``pip install`` into the active + venv. Not durable — wiped by ``uv sync`` — but covers the case + where ``uv`` isn't available or the package has no console-script. + + *verify_command* is the CLI name expected to appear after step 2; if + it's missing from ``uv tool dir --bin`` we fall through to step 3. + """ + # No bin-dir verify for step 1: in a uv-tool env the binary lands in + # evosci's own bin dir (next to sys.executable), not in `uv tool dir + # --bin`. `_resolve_command_path` picks it up via its sys.executable + # branch. + if _install_with_uv_tool_env(package): + return True + if shutil.which("uv"): + try: + result = subprocess.run( + ["uv", "tool", "install", "-q", package], + capture_output=True, + text=True, + timeout=180, + ) + if result.returncode == 0: + import importlib + + importlib.invalidate_caches() + # `uv tool install` silently produces no bin when the + # package lacks a console-script. Check the uv-tool bin + # dir directly rather than PATH, which under `uv run` + # resolves stale `.venv/bin/` copies first. + if _uv_tool_bin(verify_command) is not None: + return True + logger.info( + "uv tool install %s succeeded but %s missing from uv " + "tool bin dir; falling back to pip install into current venv", + package, + verify_command, + ) + else: + logger.info( + "uv tool install %s failed (exit %d); falling back to pip install", + package, + result.returncode, + ) + except (FileNotFoundError, subprocess.TimeoutExpired) as exc: + logger.info("uv tool install %s errored (%s); falling back", package, exc) + return _install_via_pip(package) + + def _resolve_command_path(command: str) -> str: """Resolve a command to its full path after pip installation. - Checks PATH first (standard case), then the bin directory of the current - Python interpreter — handles uv tool envs where a newly installed binary - is not on PATH but lives alongside the tool's Python executable. + Resolution order: + + 1. Absolute path — return as-is. + 2. ``uv tool dir --bin`` — prefer this over any ``.venv/bin/`` + shadow. ``uv run`` puts the project venv first on PATH, so a + stale venv copy of a package (from an earlier ``uv pip install`` + that's since been superseded by ``uv tool install``) would + otherwise mask the durable location. + 3. PATH — ``shutil.which``. + 4. Current interpreter's ``bin/`` — handles uv tool envs where a + newly installed binary isn't on PATH but lives alongside the + tool's Python executable. Returns the full absolute path if found, otherwise the original string. """ if os.path.isabs(command): return command + tool_bin = _uv_tool_bin(command) + if tool_bin is not None: + return str(tool_bin) found = shutil.which(command) if found: return found @@ -386,7 +528,11 @@ def install_mcp_server( # Pip package if entry.pip_package: print_fn(f" Installing {entry.pip_package}...", "dim") - if not install_pip_package(entry.pip_package): + if entry.command: + ok = install_cli_tool(entry.pip_package, verify_command=entry.command) + else: + ok = install_library(entry.pip_package) + if not ok: print_fn(f" Failed: {pip_install_hint()} {entry.pip_package}", "red") return False diff --git a/tests/test_mcp_client.py b/tests/test_mcp_client.py index 1677412..23cc9ca 100644 --- a/tests/test_mcp_client.py +++ b/tests/test_mcp_client.py @@ -164,7 +164,23 @@ class TestBuildConnections: assert conns["fs"]["command"].endswith("npx") assert conns["fs"]["args"] == ["-y", "server"] - def test_stdio_with_env(self): + def test_stdio_with_env(self, monkeypatch): + for var in ( + "http_proxy", + "https_proxy", + "HTTP_PROXY", + "HTTPS_PROXY", + "all_proxy", + "ALL_PROXY", + "no_proxy", + "NO_PROXY", + "SSL_CERT_FILE", + "SSL_CERT_DIR", + "REQUESTS_CA_BUNDLE", + "CURL_CA_BUNDLE", + "NODE_EXTRA_CA_CERTS", + ): + monkeypatch.delenv(var, raising=False) config = { "fs": { "transport": "stdio", @@ -176,6 +192,29 @@ class TestBuildConnections: conns = _build_connections(config) assert conns["fs"]["env"] == {"FOO": "bar"} + def test_stdio_forwards_proxy_and_cert_env(self, monkeypatch): + """Proxy and CA bundle env vars are forwarded to stdio subprocesses.""" + monkeypatch.setenv("https_proxy", "http://proxy:3128") + monkeypatch.setenv("SSL_CERT_FILE", "/etc/ssl/certs/ca.crt") + config = {"fs": {"transport": "stdio", "command": "npx", "args": []}} + conns = _build_connections(config) + assert conns["fs"]["env"]["https_proxy"] == "http://proxy:3128" + assert conns["fs"]["env"]["SSL_CERT_FILE"] == "/etc/ssl/certs/ca.crt" + + def test_stdio_user_env_overrides_forwarded(self, monkeypatch): + """User-configured env takes precedence over auto-forwarded values.""" + monkeypatch.setenv("https_proxy", "http://host-proxy:3128") + config = { + "fs": { + "transport": "stdio", + "command": "npx", + "args": [], + "env": {"https_proxy": "http://user-proxy:9000"}, + } + } + conns = _build_connections(config) + assert conns["fs"]["env"]["https_proxy"] == "http://user-proxy:9000" + def test_http_connection(self): config = { "api": { @@ -888,9 +927,9 @@ class TestUvToolCompat: monkeypatch.setattr(reg.shutil, "which", lambda x: None) assert reg.pip_install_hint() == "pip install" - # -- install_pip_package -- + # -- install_library / install_cli_tool -- - def test_install_pip_package_uv_tool_env_uses_uv_tool_install( + def test_install_library_uv_tool_env_uses_uv_tool_install( self, monkeypatch, tmp_path ): """In a uv tool env, should use ``uv tool install --with`` for durability.""" @@ -918,7 +957,7 @@ class TestUvToolCompat: reg.shutil, "which", lambda x: "/usr/bin/uv" if x == "uv" else None ) monkeypatch.setattr(reg.subprocess, "run", fake_run) - result = reg.install_pip_package("new-mcp-server") + result = reg.install_library("new-mcp-server") assert result is True assert len(captured) == 1 cmd = captured[0] @@ -930,9 +969,7 @@ class TestUvToolCompat: assert "existing-pkg" in with_args assert "new-mcp-server" in with_args - def test_install_pip_package_uv_tool_env_no_duplicate_with( - self, monkeypatch, tmp_path - ): + def test_install_library_uv_tool_env_no_duplicate_with(self, monkeypatch, tmp_path): """If the package is already in the receipt, don't add it twice.""" import EvoScientist.mcp.registry as reg @@ -957,14 +994,12 @@ class TestUvToolCompat: reg.shutil, "which", lambda x: "/usr/bin/uv" if x == "uv" else None ) monkeypatch.setattr(reg.subprocess, "run", fake_run) - reg.install_pip_package("arxiv-mcp-server") + reg.install_library("arxiv-mcp-server") cmd = captured[0] with_args = [cmd[i + 1] for i, v in enumerate(cmd) if v == "--with"] assert with_args.count("arxiv-mcp-server") == 1 - def test_install_pip_package_uv_tool_preserves_specifiers( - self, monkeypatch, tmp_path - ): + def test_install_library_uv_tool_preserves_specifiers(self, monkeypatch, tmp_path): """Existing --with specs with extras/versions must be preserved.""" import EvoScientist.mcp.registry as reg @@ -990,14 +1025,14 @@ class TestUvToolCompat: reg.shutil, "which", lambda x: "/usr/bin/uv" if x == "uv" else None ) monkeypatch.setattr(reg.subprocess, "run", fake_run) - reg.install_pip_package("new-pkg") + reg.install_library("new-pkg") cmd = captured[0] with_args = [cmd[i + 1] for i, v in enumerate(cmd) if v == "--with"] assert "rich>=13.0" in with_args assert "requests[socks]" in with_args assert "new-pkg" in with_args - def test_install_pip_package_uv_tool_dedup_with_version_spec( + def test_install_library_uv_tool_dedup_with_version_spec( self, monkeypatch, tmp_path ): """Dedup must match bare name even if package arg has version spec.""" @@ -1025,17 +1060,16 @@ class TestUvToolCompat: ) monkeypatch.setattr(reg.subprocess, "run", fake_run) # package arg has version constraint — should still dedup against "rich" - reg.install_pip_package("rich>=14.0") + reg.install_library("rich>=14.0") cmd = captured[0] with_args = [cmd[i + 1] for i, v in enumerate(cmd) if v == "--with"] # Should keep the existing spec, not add a duplicate assert with_args.count("rich>=13.0") == 1 assert "rich>=14.0" not in with_args - def test_install_pip_package_uv_tool_falls_back_on_failure( - self, monkeypatch, tmp_path - ): - """If ``uv tool install`` fails, fall back to ``uv pip install``.""" + def test_install_library_uv_tool_falls_back_on_failure(self, monkeypatch, tmp_path): + """install_library: if ``uv tool install --with`` fails, fall back + to ``uv pip install`` — standalone uv tool install is NEVER tried.""" import EvoScientist.mcp.registry as reg venv = tmp_path / "uv" / "tools" / "evoscientist" @@ -1046,11 +1080,10 @@ class TestUvToolCompat: ) monkeypatch.setenv("VIRTUAL_ENV", str(venv)) - call_count = 0 + commands: list[list[str]] = [] def fake_run(cmd, **kwargs): - nonlocal call_count - call_count += 1 + commands.append(cmd) if cmd[:3] == ["uv", "tool", "install"]: return type("R", (), {"returncode": 1})() return type("R", (), {"returncode": 0})() @@ -1059,12 +1092,53 @@ class TestUvToolCompat: reg.shutil, "which", lambda x: "/usr/bin/uv" if x == "uv" else None ) monkeypatch.setattr(reg.subprocess, "run", fake_run) - result = reg.install_pip_package("some-package") + result = reg.install_library("some-package") assert result is True - assert call_count == 2 # uv tool install (fail) + uv pip install (ok) + # Order: uv tool install --with (fail), uv pip install (ok). + assert len(commands) == 2 + assert commands[0][:3] == ["uv", "tool", "install"] + assert "--with" in commands[0] + assert commands[1][:3] == ["uv", "pip", "install"] - def test_install_pip_package_uses_python_flag_when_uv_available(self, monkeypatch): - """Non-uv-tool env should use ``uv pip install --python``.""" + def test_install_cli_tool_uv_tool_env_tries_standalone_on_failure( + self, monkeypatch, tmp_path + ): + """install_cli_tool: uv-tool env, --with fails → standalone uv tool + install is tried, then pip fallback.""" + import EvoScientist.mcp.registry as reg + + venv = tmp_path / "uv" / "tools" / "evoscientist" + venv.mkdir(parents=True) + receipt = venv / "uv-receipt.toml" + receipt.write_text( + '[tool]\nrequirements = [\n { name = "evoscientist" },\n]\n' + ) + monkeypatch.setenv("VIRTUAL_ENV", str(venv)) + + commands: list[list[str]] = [] + + def fake_run(cmd, **kwargs): + commands.append(cmd) + if cmd[:3] == ["uv", "tool", "install"]: + return type("R", (), {"returncode": 1})() + return type("R", (), {"returncode": 0})() + + monkeypatch.setattr( + reg.shutil, "which", lambda x: "/usr/bin/uv" if x == "uv" else None + ) + monkeypatch.setattr(reg.subprocess, "run", fake_run) + result = reg.install_cli_tool("some-cli", verify_command="some-cli") + assert result is True + assert len(commands) == 3 + assert commands[0][:3] == ["uv", "tool", "install"] + assert "--with" in commands[0] + assert commands[1][:3] == ["uv", "tool", "install"] + assert "--with" not in commands[1] + assert commands[2][:3] == ["uv", "pip", "install"] + + def test_install_library_goes_straight_to_pip_outside_uv_tool(self, monkeypatch): + """install_library outside a uv-tool env must skip ``uv tool install + `` entirely — standalone uv tools aren't importable.""" import sys import EvoScientist.mcp.registry as reg @@ -1073,23 +1147,118 @@ class TestUvToolCompat: def fake_run(cmd, **kwargs): captured.append(cmd) - ns = type("R", (), {"returncode": 0})() - return ns + return type("R", (), {"returncode": 0})() monkeypatch.setattr(reg, "_is_uv_tool_env", lambda: False) monkeypatch.setattr( reg.shutil, "which", lambda x: "/usr/bin/uv" if x == "uv" else None ) monkeypatch.setattr(reg.subprocess, "run", fake_run) - result = reg.install_pip_package("some-package") + result = reg.install_library("some-library") assert result is True assert len(captured) == 1 - cmd = captured[0] - assert "uv" in cmd[0] - assert "--python" in cmd - assert sys.executable in cmd + assert captured[0][:3] == ["uv", "pip", "install"] + assert sys.executable in captured[0] - def test_install_pip_package_falls_back_to_pip_when_no_uv(self, monkeypatch): + def test_install_cli_tool_prefers_standalone_uv_tool_install( + self, monkeypatch, tmp_path + ): + """install_cli_tool outside a uv-tool env: standalone ``uv tool + install `` is preferred so the binary survives uv sync.""" + import EvoScientist.mcp.registry as reg + + captured: list[list[str]] = [] + + def fake_run(cmd, **kwargs): + captured.append(cmd) + return type("R", (), {"returncode": 0})() + + fake_bin = tmp_path / "some-cli" + fake_bin.write_text("#!/bin/sh\n") + fake_bin.chmod(0o755) + + monkeypatch.setattr(reg, "_is_uv_tool_env", lambda: False) + monkeypatch.setattr( + reg.shutil, "which", lambda x: "/usr/bin/uv" if x == "uv" else None + ) + monkeypatch.setattr(reg.subprocess, "run", fake_run) + monkeypatch.setattr( + reg, "_uv_tool_bin", lambda cmd: fake_bin if cmd == "some-cli" else None + ) + result = reg.install_cli_tool("some-cli", verify_command="some-cli") + assert result is True + assert len(captured) == 1 + assert captured[0][:3] == ["uv", "tool", "install"] + assert "some-cli" in captured[0] + + def test_install_cli_tool_missing_bin_triggers_pip_fallback(self, monkeypatch): + """install_cli_tool: if the binary isn't in uv's tool bin dir after + ``uv tool install`` (e.g. package has no console-script), fall + through to ``uv pip install``.""" + import sys + + import EvoScientist.mcp.registry as reg + + captured: list[list[str]] = [] + + def fake_run(cmd, **kwargs): + captured.append(cmd) + return type("R", (), {"returncode": 0})() + + monkeypatch.setattr(reg, "_is_uv_tool_env", lambda: False) + monkeypatch.setattr( + reg.shutil, "which", lambda x: "/usr/bin/uv" if x == "uv" else None + ) + monkeypatch.setattr(reg.subprocess, "run", fake_run) + monkeypatch.setattr(reg, "_uv_tool_bin", lambda cmd: None) + result = reg.install_cli_tool( + "lib-without-entrypoint", verify_command="ghost-cli" + ) + assert result is True + assert len(captured) == 2 + assert captured[0][:3] == ["uv", "tool", "install"] + assert "--python" in captured[1] + assert sys.executable in captured[1] + + def test_install_cli_tool_bin_present_short_circuits(self, monkeypatch, tmp_path): + """install_cli_tool: ``uv tool install`` success + binary present in + uv-tool bin dir ⇒ no pip fallback, even if a stale copy exists in + the venv on PATH.""" + import EvoScientist.mcp.registry as reg + + captured: list[list[str]] = [] + + def fake_run(cmd, **kwargs): + captured.append(cmd) + return type("R", (), {"returncode": 0})() + + fake_bin = tmp_path / "arxiv-mcp-server" + fake_bin.write_text("#!/bin/sh\n") + fake_bin.chmod(0o755) + + def fake_which(x): + if x == "uv": + return "/usr/bin/uv" + if x == "arxiv-mcp-server": + return "/venv/bin/arxiv-mcp-server" # stale, should NOT be trusted + return None + + monkeypatch.setattr(reg, "_is_uv_tool_env", lambda: False) + monkeypatch.setattr(reg.shutil, "which", fake_which) + monkeypatch.setattr(reg.subprocess, "run", fake_run) + monkeypatch.setattr( + reg, + "_uv_tool_bin", + lambda cmd: fake_bin if cmd == "arxiv-mcp-server" else None, + ) + result = reg.install_cli_tool( + "arxiv-mcp-server", verify_command="arxiv-mcp-server" + ) + assert result is True + assert len(captured) == 1 + assert captured[0][:3] == ["uv", "tool", "install"] + + def test_install_library_falls_back_to_pip_when_no_uv(self, monkeypatch): import sys import EvoScientist.mcp.registry as reg @@ -1104,7 +1273,7 @@ class TestUvToolCompat: monkeypatch.setattr(reg, "_is_uv_tool_env", lambda: False) monkeypatch.setattr(reg.shutil, "which", lambda x: None) monkeypatch.setattr(reg.subprocess, "run", fake_run) - reg.install_pip_package("some-package") + reg.install_library("some-package") assert len(captured) == 1 assert sys.executable in captured[0] assert "-m" in captured[0] @@ -1166,6 +1335,32 @@ class TestUvToolCompat: import EvoScientist.mcp.registry as reg monkeypatch.setattr(reg.shutil, "which", lambda x: None) + monkeypatch.setattr(reg, "_uv_tool_bin", lambda cmd: None) monkeypatch.setattr(sys, "executable", str(tmp_path / "bin" / "python")) result = reg._resolve_command_path("nonexistent-tool") assert result == "nonexistent-tool" + + def test_resolve_command_path_prefers_uv_tool_over_venv_shadow( + self, monkeypatch, tmp_path + ): + """When both ``uv tool dir --bin`` and a venv's ``bin/`` contain the + command, prefer the uv-tool location so the path written to mcp.yaml + survives ``uv sync``.""" + import EvoScientist.mcp.registry as reg + + uv_bin_dir = tmp_path / "uv-bin" + uv_bin_dir.mkdir() + uv_copy = uv_bin_dir / "arxiv-mcp-server" + uv_copy.write_text("#!/bin/sh\n") + uv_copy.chmod(0o755) + + # Stale venv copy that `uv run` would surface first via PATH. + venv_copy = tmp_path / "venv-bin" / "arxiv-mcp-server" + venv_copy.parent.mkdir() + venv_copy.write_text("#!/bin/sh\n") + venv_copy.chmod(0o755) + + monkeypatch.setattr(reg, "_uv_tool_bin_dir", lambda: uv_bin_dir) + monkeypatch.setattr(reg.shutil, "which", lambda x: str(venv_copy)) + result = reg._resolve_command_path("arxiv-mcp-server") + assert result == str(uv_copy)