From 4610989d72c297345440316c1008911bac163fac Mon Sep 17 00:00:00 2001 From: oliver Date: Sat, 30 May 2026 23:42:44 +0800 Subject: [PATCH] fix(mcp): preflight python module check and surface install errors - Verify python -m module import in preflight with fix hints - Pass env_schema defaults in health/sync; catch JSON install exceptions in UI Co-authored-by: Cursor --- interfaces/admin/routes.py | 16 +++--------- interfaces/admin/static/app.js | 10 +++++++ runtime/tools/mcp/adapter.py | 24 ++--------------- runtime/tools/mcp/env_config.py | 46 +++++++++++++++++++++++++++++++++ runtime/tools/mcp/installer.py | 45 ++++++++++++++++++++++++++++++++ 5 files changed, 107 insertions(+), 34 deletions(-) create mode 100644 runtime/tools/mcp/env_config.py diff --git a/interfaces/admin/routes.py b/interfaces/admin/routes.py index df678fb5..b6e2732f 100644 --- a/interfaces/admin/routes.py +++ b/interfaces/admin/routes.py @@ -36,6 +36,7 @@ from svc.config.passwords import load_expected_password from svc.persistence.sqlite_store import SqliteStore from svc.persistence.assistant_store import get_assistant_store from runtime.agents.specialists import discover_specialist_ids, parse_agent_profile_bindings +from runtime.tools.mcp.env_config import mcp_runtime_for_row from runtime.tools.mcp.installer import ( _safe_server_id, detect_local_dependencies, @@ -130,10 +131,7 @@ def _mcp_health_and_sync_one(store: SqliteStore, row: dict[str, Any]) -> dict[st detail = {"synced_tools": len(tools), "compat_mode": "bailian_webparser"} store.set_mcp_server_health(server_id=sid, status="ok", detail=detail) return {"server_id": sid, "ok": True, "health": detail, "tools_synced": len(tools)} - rt = McpProcessRuntime( - build_mcp_process_command(cmd, args, store=store), - timeout_s=float(row.get("timeout_s") or 30.0), - ) + rt = mcp_runtime_for_row(row, store=store) try: health = rt.health() health_ok = bool(health.get("ok")) @@ -3237,10 +3235,7 @@ def build_admin_router() -> APIRouter: detail = {"ok": True, "status": "ok", "compat_mode": "bailian_webparser", "tools_count": 1} store.set_mcp_server_health(server_id=server_id, status="ok", detail=detail) return {"ok": True, "response": detail} - rt = McpProcessRuntime( - build_mcp_process_command(cmd, args, store=store), - timeout_s=float(row.get("timeout_s") or 30.0), - ) + rt = mcp_runtime_for_row(row, store=store) try: response = rt.health() ok = bool(response.get("ok")) @@ -3293,10 +3288,7 @@ def build_admin_router() -> APIRouter: detail={"synced_tools": len(norm), "compat_mode": "bailian_webparser"}, ) return {"ok": True, "server_id": server_id, "tools": norm, "compat_mode": "bailian_webparser"} - rt = McpProcessRuntime( - build_mcp_process_command(cmd, args, store=store), - timeout_s=float(row.get("timeout_s") or 30.0), - ) + rt = mcp_runtime_for_row(row, store=store) try: response = rt.tools_list() items = response.get("tools") if isinstance(response, dict) else None diff --git a/interfaces/admin/static/app.js b/interfaces/admin/static/app.js index eabef9aa..d6b641fe 100644 --- a/interfaces/admin/static/app.js +++ b/interfaces/admin/static/app.js @@ -5014,6 +5014,8 @@ async function renderPlugins() { class: "btn btn--primary", text: "Install MCP", onclick: async () => { + installStatus.textContent = "[install] running preflight..."; + try { const payload = { source_type: sourceType.value, source_ref: sourceRef.value.trim(), @@ -5063,6 +5065,9 @@ async function renderPlugins() { installStatus.textContent = JSON.stringify(res); markPrewarmReminder("mcp_installed"); router(); + } catch (err) { + installStatus.textContent = `[install] failed: ${String((err && err.message) || err || "install_failed")}`; + } }, }); const jsonInstallBtn = el("button", { @@ -5075,6 +5080,8 @@ async function renderPlugins() { installStatus.textContent = "[json] empty payload"; return; } + installStatus.textContent = "[json] installing..."; + try { let parsed; try { parsed = JSON.parse(raw); @@ -5237,6 +5244,9 @@ async function renderPlugins() { } markPrewarmReminder("mcp_batch_installed"); router(); + } catch (err) { + installStatus.textContent = `[json] failed: ${String((err && err.message) || err || "install_failed")}`; + } }, }); const cliInstallModal = el("div", { class: "session-monitor-modal", style: "display:none;" }); diff --git a/runtime/tools/mcp/adapter.py b/runtime/tools/mcp/adapter.py index 7510cb3a..9e336bad 100644 --- a/runtime/tools/mcp/adapter.py +++ b/runtime/tools/mcp/adapter.py @@ -5,6 +5,7 @@ import json import os from typing import Any +from runtime.tools.mcp.env_config import mcp_row_env_config from runtime.skills import SkillSpec, materialize_skills_from_tool_specs from runtime.tools.base import ToolSpec from runtime.tools.mcp.filesystem_argv import build_mcp_process_command @@ -13,28 +14,7 @@ from runtime.tools.public.bailian_webparser_tool import bailian_webparser_tool def _mcp_row_env_config(row: dict[str, Any]) -> tuple[list[str], dict[str, str]]: - """Per-server env allowlist + defaults from registry ``env_schema`` (e.g. Cursor ``mcpServers.env``).""" - from runtime.operations.mcp_env import mcp_env_allowlist_keys - - schema = row.get("env_schema") if isinstance(row.get("env_schema"), dict) else {} - defaults: dict[str, str] = {} - schema_keys: list[str] = [] - for k, spec in schema.items(): - key = str(k or "").strip() - if not key: - continue - schema_keys.append(key) - if isinstance(spec, dict) and spec.get("default") is not None: - dv = str(spec.get("default") or "").strip() - if dv: - defaults[key] = dv - seen: set[str] = set() - allowlist: list[str] = [] - for k in [*mcp_env_allowlist_keys(), *schema_keys]: - if k and k not in seen: - seen.add(k) - allowlist.append(k) - return allowlist, defaults + return mcp_row_env_config(row) @dataclass diff --git a/runtime/tools/mcp/env_config.py b/runtime/tools/mcp/env_config.py new file mode 100644 index 00000000..54cef85f --- /dev/null +++ b/runtime/tools/mcp/env_config.py @@ -0,0 +1,46 @@ +from __future__ import annotations + +from typing import Any + + +def mcp_row_env_config(row: dict[str, Any]) -> tuple[list[str], dict[str, str]]: + """Per-server env allowlist + defaults from registry ``env_schema``.""" + from runtime.operations.mcp_env import mcp_env_allowlist_keys + + schema = row.get("env_schema") if isinstance(row.get("env_schema"), dict) else {} + defaults: dict[str, str] = {} + schema_keys: list[str] = [] + for k, spec in schema.items(): + key = str(k or "").strip() + if not key: + continue + schema_keys.append(key) + if isinstance(spec, dict) and spec.get("default") is not None: + dv = str(spec.get("default") or "").strip() + if dv: + defaults[key] = dv + seen: set[str] = set() + allowlist: list[str] = [] + for k in [*mcp_env_allowlist_keys(), *schema_keys]: + if k and k not in seen: + seen.add(k) + allowlist.append(k) + return allowlist, defaults + + +def mcp_runtime_for_row(row: dict[str, Any], *, store: Any) -> Any: + from runtime.tools.mcp.filesystem_argv import build_mcp_process_command + from runtime.tools.mcp.runtime import McpProcessRuntime + + cmd = str(row.get("entry_command") or "").strip() + args = [str(x) for x in (row.get("entry_args") or []) if str(x).strip()] + allowlist, defaults = mcp_row_env_config(row) + return McpProcessRuntime( + build_mcp_process_command(cmd, args, store=store), + timeout_s=float(row.get("timeout_s") or 30.0), + env_allowlist=allowlist, + env_defaults=defaults, + ) + + +__all__ = ["mcp_row_env_config", "mcp_runtime_for_row"] diff --git a/runtime/tools/mcp/installer.py b/runtime/tools/mcp/installer.py index 685ba816..43033015 100644 --- a/runtime/tools/mcp/installer.py +++ b/runtime/tools/mcp/installer.py @@ -169,9 +169,54 @@ def preflight_mcp_server(manifest: McpServerManifest) -> dict[str, Any]: return {"ok": False, "error_code": "mcp_entry_not_found", "error": f"entry_command_not_found:{entry}", "warnings": warnings, "fix_suggestions": fix_suggestions} env_schema = manifest.env_schema if isinstance(manifest.env_schema, dict) else {} required_env = [str(k) for k, v in env_schema.items() if isinstance(v, dict) and bool(v.get("required"))] + mod_err = _preflight_python_module(entry, manifest.entry_args or []) + if mod_err is not None: + return {**mod_err, "warnings": warnings, "fix_suggestions": fix_suggestions} return {"ok": True, "error_code": "", "error": "", "entry_command_path": found, "required_env": required_env, "warnings": warnings, "fix_suggestions": fix_suggestions} +def _preflight_python_module(entry: str, entry_args: list[str]) -> dict[str, Any] | None: + """When entry is ``python -m ``, verify that interpreter can import ``mod``.""" + argv = [str(x or "").strip() for x in entry_args if str(x or "").strip()] + if str(entry or "").strip().lower() not in {"python", "python3", "py"}: + return None + if len(argv) < 2 or argv[0] != "-m": + return None + mod = argv[1].split(".")[0] + if not mod: + return None + exe = shutil.which(entry) or entry + try: + cp = _run_command([exe, "-c", f"import {mod}"], timeout=12) + except Exception as exc: + return { + "ok": False, + "error_code": "mcp_python_module_check_failed", + "error": str(exc), + "fix_suggestions": [ + { + "title": f"Install module for this Python ({exe})", + "command": f'"{exe}" -m pip install "git+https://github.com/hansjone/netx.git#subdirectory=packages/netx-mcp"', + } + ], + } + if cp.returncode == 0: + return None + err = (cp.stderr or cp.stdout or "").strip() + return { + "ok": False, + "error_code": "mcp_python_module_missing", + "error": err[:500] or f"cannot import {mod}", + "fix_suggestions": [ + { + "title": f"Install {mod} for the same Python oclaw uses", + "command": f'"{exe}" -m pip install "git+https://github.com/hansjone/netx.git#subdirectory=packages/netx-mcp"', + }, + {"title": "Verify import", "command": f'"{exe}" -c "import {mod}; print(\'ok\')"'}, + ], + } + + def detect_local_dependencies() -> list[dict[str, Any]]: deps = [{"name": "git", "version_args": ["--version"]}, {"name": "node", "version_args": ["--version"]}, {"name": "npm", "version_args": ["--version"]}, {"name": "npx", "version_args": ["--version"]}, {"name": "python", "version_args": ["--version"]}, {"name": "pip", "version_args": ["--version"]}] out: list[dict[str, Any]] = []