fix(mcp): detect npx cache entries before probing (#4034)

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 <pkg> --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 <pkg>
+    --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()

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(