From 039431f5ea2209f42f82be222079408415bbf9da Mon Sep 17 00:00:00 2001 From: Max Hsu Date: Mon, 15 Jun 2026 14:14:48 +0800 Subject: [PATCH] fix(mcp): detect npx cache entries before probing (#4034) --- src/builtin_mcp.py | 79 ++++++++++++++++++++++++++--- tests/test_builtin_mcp_npx_cache.py | 39 +++++++++++++- 2 files changed, 110 insertions(+), 8 deletions(-) diff --git a/src/builtin_mcp.py b/src/builtin_mcp.py index cf528c10d..0154d2fb9 100644 --- a/src/builtin_mcp.py +++ b/src/builtin_mcp.py @@ -5,12 +5,13 @@ Auto-registration of built-in MCP servers on startup. Each server runs as a stdio subprocess managed by McpManager. """ +import asyncio +import json import logging import os import shutil import subprocess import sys -import asyncio from core.platform_compat import IS_WINDOWS, which_tool @@ -197,12 +198,13 @@ def _npx_package_from_args(args): async def _is_npx_package_cached(npx_path, package_spec, timeout_s=5): """Probe whether an npx package is already in the local cache. - Runs `npx --no-install --version`. --no-install tells npx to - fail instead of downloading, so a cache miss returns fast. We treat - "exited 0 with non-empty stdout" as proof of a working cached copy. - Anything else (non-zero exit, empty stdout, timeout, missing npx, - network error) means we should skip the server. + First checks the local `_npx` cache for an installed package. If the + package is not found there, falls back to `npx --no-install + --version` so older npm layouts still work without downloading. """ + if _is_package_in_npx_cache(package_spec): + return True + try: proc = await asyncio.create_subprocess_exec( npx_path, "--no-install", package_spec, "--version", @@ -231,3 +233,68 @@ async def _is_npx_package_cached(npx_path, package_spec, timeout_s=5): pass return False return proc.returncode == 0 and bool(stdout.strip()) + + +def _is_package_in_npx_cache(package_spec): + """Return True when npm's `_npx` cache already contains package_spec.""" + package_name = _npx_package_name(package_spec) + if not package_name: + return False + + for cache_root in _npm_cache_roots(): + npx_root = os.path.join(cache_root, "_npx") + if _npx_cache_contains_package(npx_root, package_name): + return True + return False + + +def _npx_package_name(package_spec): + """Strip a version/range suffix from an npm package spec.""" + if not package_spec: + return "" + if package_spec.startswith("@"): + parts = package_spec.split("@", 2) + if len(parts) >= 3: + return f"@{parts[1]}" + return package_spec + return package_spec.split("@", 1)[0] + + +def _npm_cache_roots(): + roots = [] + configured = os.environ.get("npm_config_cache") + if configured: + roots.append(os.path.expanduser(configured)) + roots.append(os.path.join(os.path.expanduser("~"), ".npm")) + local_app_data = os.environ.get("LOCALAPPDATA") + if local_app_data: + roots.append(os.path.join(local_app_data, "npm-cache")) + return list(dict.fromkeys(roots)) + + +def _npx_cache_contains_package(npx_root, package_name): + if not os.path.isdir(npx_root): + return False + package_path = os.path.join("node_modules", *package_name.split("/"), "package.json") + try: + entries = list(os.scandir(npx_root)) + except OSError: + return False + for entry in entries: + try: + is_dir = entry.is_dir() + except OSError: + continue + cached_name = _cached_package_name(os.path.join(entry.path, package_path)) + if is_dir and cached_name == package_name: + return True + return False + + +def _cached_package_name(package_json_path): + try: + with open(package_json_path, encoding="utf-8") as fh: + data = json.load(fh) + except (OSError, ValueError): + return "" + return str(data.get("name", "")).strip() diff --git a/tests/test_builtin_mcp_npx_cache.py b/tests/test_builtin_mcp_npx_cache.py index bed77df70..a320c056a 100644 --- a/tests/test_builtin_mcp_npx_cache.py +++ b/tests/test_builtin_mcp_npx_cache.py @@ -36,7 +36,38 @@ def test_npx_package_from_args_prefers_package_after_y_flag(monkeypatch): ) == "@playwright/mcp@latest" -def test_npx_cache_check_falls_back_when_async_subprocess_is_unsupported(monkeypatch): +def test_npx_cache_check_detects_scoped_package_in_npx_cache(monkeypatch, tmp_path): + builtin_mcp = _load_builtin_mcp(monkeypatch) + package_json = ( + tmp_path + / ".npm" + / "_npx" + / "9833c18b2d85bc59" + / "node_modules" + / "@playwright" + / "mcp" + / "package.json" + ) + package_json.parent.mkdir(parents=True) + package_json.write_text('{"name":"@playwright/mcp","version":"0.0.76"}', encoding="utf-8") + + async def unexpected_exec(*args, **kwargs): + raise AssertionError("cache hit should not shell out to npx") + + monkeypatch.setenv("HOME", str(tmp_path)) + monkeypatch.delenv("npm_config_cache", raising=False) + monkeypatch.setattr(builtin_mcp.asyncio, "create_subprocess_exec", unexpected_exec) + + assert asyncio.run( + builtin_mcp._is_npx_package_cached( + "npx", + "@playwright/mcp@latest", + timeout_s=2, + ) + ) is True + + +def test_npx_cache_check_falls_back_when_async_subprocess_is_unsupported(monkeypatch, tmp_path): builtin_mcp = _load_builtin_mcp(monkeypatch) async def unsupported_exec(*args, **kwargs): @@ -51,6 +82,8 @@ def test_npx_cache_check_falls_back_when_async_subprocess_is_unsupported(monkeyp monkeypatch.setattr(builtin_mcp.asyncio, "create_subprocess_exec", unsupported_exec) monkeypatch.setattr(builtin_mcp.subprocess, "run", fake_run) + monkeypatch.setenv("HOME", str(tmp_path)) + monkeypatch.delenv("npm_config_cache", raising=False) assert asyncio.run( builtin_mcp._is_npx_package_cached( @@ -69,7 +102,7 @@ def test_npx_cache_check_falls_back_when_async_subprocess_is_unsupported(monkeyp assert captured["kwargs"]["timeout"] == 2 -def test_npx_cache_check_fallback_treats_timeout_as_cache_miss(monkeypatch): +def test_npx_cache_check_fallback_treats_timeout_as_cache_miss(monkeypatch, tmp_path): builtin_mcp = _load_builtin_mcp(monkeypatch) async def unsupported_exec(*args, **kwargs): @@ -80,6 +113,8 @@ def test_npx_cache_check_fallback_treats_timeout_as_cache_miss(monkeypatch): monkeypatch.setattr(builtin_mcp.asyncio, "create_subprocess_exec", unsupported_exec) monkeypatch.setattr(builtin_mcp.subprocess, "run", fake_run) + monkeypatch.setenv("HOME", str(tmp_path)) + monkeypatch.delenv("npm_config_cache", raising=False) assert asyncio.run( builtin_mcp._is_npx_package_cached(