mirror of
https://github.com/hansjone/oclaw.git
synced 2026-10-19 05:33:20 +08:00
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 <cursoragent@cursor.com>
This commit is contained in:
parent
8787adc3cc
commit
4610989d72
5 changed files with 107 additions and 34 deletions
|
|
@ -36,6 +36,7 @@ from svc.config.passwords import load_expected_password
|
||||||
from svc.persistence.sqlite_store import SqliteStore
|
from svc.persistence.sqlite_store import SqliteStore
|
||||||
from svc.persistence.assistant_store import get_assistant_store
|
from svc.persistence.assistant_store import get_assistant_store
|
||||||
from runtime.agents.specialists import discover_specialist_ids, parse_agent_profile_bindings
|
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 (
|
from runtime.tools.mcp.installer import (
|
||||||
_safe_server_id,
|
_safe_server_id,
|
||||||
detect_local_dependencies,
|
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"}
|
detail = {"synced_tools": len(tools), "compat_mode": "bailian_webparser"}
|
||||||
store.set_mcp_server_health(server_id=sid, status="ok", detail=detail)
|
store.set_mcp_server_health(server_id=sid, status="ok", detail=detail)
|
||||||
return {"server_id": sid, "ok": True, "health": detail, "tools_synced": len(tools)}
|
return {"server_id": sid, "ok": True, "health": detail, "tools_synced": len(tools)}
|
||||||
rt = McpProcessRuntime(
|
rt = mcp_runtime_for_row(row, store=store)
|
||||||
build_mcp_process_command(cmd, args, store=store),
|
|
||||||
timeout_s=float(row.get("timeout_s") or 30.0),
|
|
||||||
)
|
|
||||||
try:
|
try:
|
||||||
health = rt.health()
|
health = rt.health()
|
||||||
health_ok = bool(health.get("ok"))
|
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}
|
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)
|
store.set_mcp_server_health(server_id=server_id, status="ok", detail=detail)
|
||||||
return {"ok": True, "response": detail}
|
return {"ok": True, "response": detail}
|
||||||
rt = McpProcessRuntime(
|
rt = mcp_runtime_for_row(row, store=store)
|
||||||
build_mcp_process_command(cmd, args, store=store),
|
|
||||||
timeout_s=float(row.get("timeout_s") or 30.0),
|
|
||||||
)
|
|
||||||
try:
|
try:
|
||||||
response = rt.health()
|
response = rt.health()
|
||||||
ok = bool(response.get("ok"))
|
ok = bool(response.get("ok"))
|
||||||
|
|
@ -3293,10 +3288,7 @@ def build_admin_router() -> APIRouter:
|
||||||
detail={"synced_tools": len(norm), "compat_mode": "bailian_webparser"},
|
detail={"synced_tools": len(norm), "compat_mode": "bailian_webparser"},
|
||||||
)
|
)
|
||||||
return {"ok": True, "server_id": server_id, "tools": norm, "compat_mode": "bailian_webparser"}
|
return {"ok": True, "server_id": server_id, "tools": norm, "compat_mode": "bailian_webparser"}
|
||||||
rt = McpProcessRuntime(
|
rt = mcp_runtime_for_row(row, store=store)
|
||||||
build_mcp_process_command(cmd, args, store=store),
|
|
||||||
timeout_s=float(row.get("timeout_s") or 30.0),
|
|
||||||
)
|
|
||||||
try:
|
try:
|
||||||
response = rt.tools_list()
|
response = rt.tools_list()
|
||||||
items = response.get("tools") if isinstance(response, dict) else None
|
items = response.get("tools") if isinstance(response, dict) else None
|
||||||
|
|
|
||||||
|
|
@ -5014,6 +5014,8 @@ async function renderPlugins() {
|
||||||
class: "btn btn--primary",
|
class: "btn btn--primary",
|
||||||
text: "Install MCP",
|
text: "Install MCP",
|
||||||
onclick: async () => {
|
onclick: async () => {
|
||||||
|
installStatus.textContent = "[install] running preflight...";
|
||||||
|
try {
|
||||||
const payload = {
|
const payload = {
|
||||||
source_type: sourceType.value,
|
source_type: sourceType.value,
|
||||||
source_ref: sourceRef.value.trim(),
|
source_ref: sourceRef.value.trim(),
|
||||||
|
|
@ -5063,6 +5065,9 @@ async function renderPlugins() {
|
||||||
installStatus.textContent = JSON.stringify(res);
|
installStatus.textContent = JSON.stringify(res);
|
||||||
markPrewarmReminder("mcp_installed");
|
markPrewarmReminder("mcp_installed");
|
||||||
router();
|
router();
|
||||||
|
} catch (err) {
|
||||||
|
installStatus.textContent = `[install] failed: ${String((err && err.message) || err || "install_failed")}`;
|
||||||
|
}
|
||||||
},
|
},
|
||||||
});
|
});
|
||||||
const jsonInstallBtn = el("button", {
|
const jsonInstallBtn = el("button", {
|
||||||
|
|
@ -5075,6 +5080,8 @@ async function renderPlugins() {
|
||||||
installStatus.textContent = "[json] empty payload";
|
installStatus.textContent = "[json] empty payload";
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
installStatus.textContent = "[json] installing...";
|
||||||
|
try {
|
||||||
let parsed;
|
let parsed;
|
||||||
try {
|
try {
|
||||||
parsed = JSON.parse(raw);
|
parsed = JSON.parse(raw);
|
||||||
|
|
@ -5237,6 +5244,9 @@ async function renderPlugins() {
|
||||||
}
|
}
|
||||||
markPrewarmReminder("mcp_batch_installed");
|
markPrewarmReminder("mcp_batch_installed");
|
||||||
router();
|
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;" });
|
const cliInstallModal = el("div", { class: "session-monitor-modal", style: "display:none;" });
|
||||||
|
|
|
||||||
|
|
@ -5,6 +5,7 @@ import json
|
||||||
import os
|
import os
|
||||||
from typing import Any
|
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.skills import SkillSpec, materialize_skills_from_tool_specs
|
||||||
from runtime.tools.base import ToolSpec
|
from runtime.tools.base import ToolSpec
|
||||||
from runtime.tools.mcp.filesystem_argv import build_mcp_process_command
|
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]]:
|
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``)."""
|
return mcp_row_env_config(row)
|
||||||
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
|
|
||||||
|
|
||||||
|
|
||||||
@dataclass
|
@dataclass
|
||||||
|
|
|
||||||
46
runtime/tools/mcp/env_config.py
Normal file
46
runtime/tools/mcp/env_config.py
Normal file
|
|
@ -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"]
|
||||||
|
|
@ -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}
|
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 {}
|
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"))]
|
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}
|
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 <mod>``, 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]]:
|
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"]}]
|
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]] = []
|
out: list[dict[str, Any]] = []
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue