From 73159ed6efa37ddf7df3669e365c20a386944304 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 25 May 2026 02:20:09 +0000 Subject: [PATCH 1/2] Unify subprocess providers as 'package' type with requirements & setup_commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Rename npx_runner.py → process_runner.py; class NpxSession → ProcessSession (NpxSession alias kept for backward compat) - Add SUBPROCESS_KEYS = ('package', 'npx') so existing npx: YAML files keep working unchanged; saving through the UI rewrites them to package: format - Add _get_package_command() helper that checks both keys in priority order - Add run_provider_setup() that pip-installs requirements and runs setup_commands on every server startup (safe no-op when already installed) - Add requirements: and setup_commands: fields to both code and package providers in YAML, structured JSON, validation, and editor UI - Merge all introspect endpoints into a single POST /api/introspect that accepts command + requirements + setup_commands; removes /api/introspect-npx - Remove the ad-hoc 'Run Command' modal; setup_commands replace it persistently - Simplify the wizard to 2 cards: Python Code and Package - Package card accepts any command (npx, uvx, python -m, installed binary); package manager auto-detected from the command prefix for display purposes - Update provider list badges: 'pkg' (purple) for package, 'code' (blue) for code - Add provider_type field to GET /api/tools list; is_npx kept as backward-compat alias - Install uv via pip in Dockerfile so uvx-based providers work out of the box; add /root/.local/bin to PATH - Add pip install uv step to GitHub Actions CI workflow - 150/150 tests pass covering all new behavior and backward compat https://claude.ai/code/session_016EMphWWMguzwVrfAYvJFSX --- .github/workflows/tests.yml | 3 + Dockerfile | 7 +- README.md | 134 +++++--- frontend/app.py | 595 +++++++++++++++++++----------------- process_runner.py | 163 ++++++++++ server.py | 97 ++++-- tests/test_frontend.py | 325 ++++++++++++++++---- tests/test_server.py | 134 +++++++- 8 files changed, 1042 insertions(+), 416 deletions(-) create mode 100644 process_runner.py diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 313cbad..7916189 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -30,6 +30,9 @@ jobs: pip install --upgrade pip pip install -r requirements.txt -r requirements-dev.txt + - name: Install uv (for uvx-based provider support) + run: pip install uv + - name: Run tests run: pytest tests/ -v --tb=short diff --git a/Dockerfile b/Dockerfile index cd22204..6a0dfa1 100755 --- a/Dockerfile +++ b/Dockerfile @@ -12,7 +12,12 @@ RUN apt-get update && apt-get install -y --no-install-recommends \ COPY requirements.txt . RUN pip install --no-cache-dir -r requirements.txt -COPY server.py config.py npx_runner.py ./ +# Install uv so that uvx-based MCP package providers work out of the box. +# uv installs its binaries to /root/.local/bin; add that to PATH. +RUN pip install --no-cache-dir uv +ENV PATH="/root/.local/bin:$PATH" + +COPY server.py config.py process_runner.py ./ COPY frontend/ ./frontend/ COPY handlers/ ./handlers/ diff --git a/README.md b/README.md index 49e58e3..5fa9697 100755 --- a/README.md +++ b/README.md @@ -11,11 +11,12 @@ Each tool **provider** is a single YAML file under `tools/`. The YAML contains: - The Python code for all tool functions (embedded directly in the file) - One or more tool declarations that reference those functions - Per-tool input schemas, secrets, and auth metadata -- Or an `npx:` block to delegate to an existing MCP npm package +- Or a `package:` block to delegate to any existing MCP subprocess server — launched + via `npx`, `uvx`, `python -m`, or any installed binary -`server.py` loads every YAML at startup, executes its `code` block (or spawns the -declared `npx` process), and registers each declared tool automatically — no Python -files to maintain separately, no changes to `server.py` needed when adding new tools. +`server.py` loads every YAML at startup, installs declared `requirements` (pip packages), +runs `setup_commands`, then registers each tool automatically — no Python files to +maintain separately, no changes to `server.py` needed when adding new tools. ## Ports @@ -36,7 +37,7 @@ files to maintain separately, no changes to `server.py` needed when adding new t ├── requirements-dev.txt ← test dependencies ├── server.py ├── config.py ← shared env-var config (imported by all modules) -├── npx_runner.py ← spawns & proxies npx-based MCP providers +├── process_runner.py ← spawns & proxies any stdio MCP subprocess ├── frontend/ │ └── app.py ← FastAPI UI server (port 8889) ├── .env.example @@ -76,7 +77,7 @@ Click **+ New Provider** and choose a provider type: | Type | Description | |---|---| | **Python code** | Write `async def` functions; declare tools that reference them | -| **npx package** | Enter an `npx` command (e.g. `npx @playwright/mcp@latest`); the UI auto-introspects the MCP server and populates tool definitions — no code needed | +| **Package** | Enter any command that launches a stdio MCP server (`npx`, `uvx`, `python -m`, or an installed binary); the UI auto-introspects tools — no code needed | After the provider step, the wizard shows a **Secrets** step: any `secrets.env` entries in the provider are listed, and you can fill in their values to save them directly to `.env`. @@ -87,39 +88,24 @@ The **🔑 Secrets** button (also available in the wizard's final step) reads al entries from the selected provider, shows which variables are already set in `.env`, and lets you fill in or update missing values — all without leaving the browser. -### Run Command +### Setup Commands -The **🛠 Run Command** button (in the editor toolbar when a provider is open) lets you run -any shell command inside the server environment — whether that's the local process or inside -the Docker container — and streams the output live in a terminal panel. - -This is particularly useful for npx-based providers that need browser binaries or other -one-time setup steps. For example, after adding a Playwright provider, install the Chrome -browser with: +Each provider has a **Setup Commands** list (editable in the editor panel, saved to YAML). +These shell commands run automatically every time the MCP server starts — perfect for +installing browser binaries, downloading data, or any one-time setup that must survive a +Docker restart. +Example — for a Playwright package provider: ``` npx playwright install chrome ``` -Or install Chromium together with its system dependencies (recommended inside Docker): - -``` -npx playwright install --with-deps chromium -``` - -The modal auto-suggests relevant commands based on the open provider's npx command (e.g. it -pre-fills the playwright install variants when a Playwright provider is selected). Press -**Enter** or click **▶ Run** to stream output; click **⏹ Stop** to kill the process. +Commands run in order before the server accepts connections. The subprocess package is +launched lazily on the first tool call, not at startup, so the browser binary is always +ready when needed. -> **How npx providers start:** the npx process is launched *lazily* — on the first tool call -> after server startup, not at startup itself. This means a `playwright install` run in **Run -> Command** installs the binaries into the server environment; the actual browser process only -> starts when a tool like `browser_navigate` is first called. -> -> When you edit an npx command in the UI and save, the YAML on disk is updated but the running -> process keeps the old command. Click **Restart MCP Server** (the yellow bar that appears -> after saving) to apply the new command — the updated process is then started on the next -> tool call. +> **After editing and saving** a provider's command or setup steps, click **Restart MCP Server** +> (the yellow bar that appears after saving) to apply the changes. ## Secrets @@ -644,14 +630,20 @@ Add `WEATHER_API_KEY=replace-me` to `.env.example` and `.env` (or use the Secret --- -### Part 3 — an npx-based provider (no code required) +### Part 3 — a package provider (no code required) -Use the **+ New Provider → npx package** wizard in the web UI, or create the YAML manually: +Use the **+ New Provider → Package** wizard in the web UI, or create the YAML manually. +Any command that spawns a stdio MCP server works — `npx`, `uvx`, `python -m`, or an +installed binary: ```yaml -npx: +# ── npx (Node.js, no install needed) ───────────────────────────────────────── +package: command: npx @playwright/mcp@latest --headless --isolated +setup_commands: + - npx playwright install chrome # installs browser on every startup + tools: # Populated automatically by the UI's Introspect button — or fill manually - name: browser_navigate @@ -665,20 +657,54 @@ tools: required: [url] ``` +```yaml +# ── uvx (Python package, no install needed) ─────────────────────────────────── +package: + command: uvx mcp-server-fetch + +tools: [] # auto-populated by Introspect +``` + +```yaml +# ── pip-installed Python module ─────────────────────────────────────────────── +package: + command: python -m mcp_server_github + +requirements: + - mcp-server-github # installed by pip before the server starts + +tools: [] +``` + +```yaml +# ── globally installed npm binary ───────────────────────────────────────────── +package: + command: mcp-server-github + +setup_commands: + - npm install -g @modelcontextprotocol/server-github + +tools: [] +``` + > **`--headless`** runs Chromium without a visible window — required inside Docker or any > headless server environment. Remove it if you want to watch the browser on a desktop. > **`--isolated`** gives each session its own browser context (no shared cookies/storage). -The server spawns the `npx` process, performs the MCP handshake once, then forwards -every tool call to it. The process is reused across calls (and started lazily on the -first tool call, not at server startup). +The server spawns the process, performs the MCP handshake once, then forwards every tool +call to it. The process is reused across calls (started lazily on the first tool call). -If the provider requires browser binaries (e.g. Playwright), install them via the -**🛠 Run Command** panel in the web UI before making the first tool call: +**Backward compatibility:** The legacy `npx:` key still works and is treated identically +to `package:`. Saving the provider through the UI rewrites it to `package:` format. -``` -npx playwright install chrome -``` +### pip Requirements vs setup_commands + +| Feature | Use for | +|---|---| +| `requirements:` | pip packages to install in the Python environment (`httpx`, `requests`, etc.) | +| `setup_commands:` | Any other one-time setup — browser binaries, npm installs, data downloads | + +Both run on every server startup (pip is a no-op if the package is already installed). --- @@ -736,10 +762,24 @@ documentation: | # optional — shown in the web UI; markdown code: | # Python source — executed once at startup # Import anything, define helpers and async tool functions. -# ── npx provider (mutually exclusive with code) ─────────────────────────────── +# ── Package provider (mutually exclusive with code) ─────────────────────────── +# Supports any command: npx, uvx, python -m, or an installed binary. -npx: +package: command: string # e.g. "npx @playwright/mcp@latest --isolated" + # "uvx mcp-server-fetch" + # "python -m mcp_server_github" + # "mcp-server-github" + +# ── Shared optional fields (both provider types) ────────────────────────────── + +requirements: # pip packages installed before the server starts + - package-name + - package-name==1.2.3 + +setup_commands: # shell commands run on every server startup + - npx playwright install chrome # (e.g. browser binaries, npm global installs) + - echo "server ready" # ── Tool declarations (required) ────────────────────────────────────────────── @@ -762,3 +802,7 @@ tools: auth: # optional — forwarded to context["auth"] any_key: any_value ``` + +> **Legacy `npx:` key:** still accepted for backward compatibility. Existing configs +> with `npx: {command: ...}` continue to work unchanged; saving through the UI rewrites +> them to `package:` format. diff --git a/frontend/app.py b/frontend/app.py index d433916..f63fd0a 100644 --- a/frontend/app.py +++ b/frontend/app.py @@ -10,12 +10,11 @@ PUT /api/tools/{name} — update provider from structured JSON DELETE /api/tools/{name} — delete provider YAML POST /api/validate — validate structured provider {provider} -POST /api/introspect-npx — run npx command, return tools list +POST /api/introspect — spawn command, run requirements/setup, return tools list POST /api/extract-functions — parse Python code for async functions GET /api/env — list .env vars (values masked) POST /api/env — upsert vars into .env {vars: {KEY: VALUE}} POST /api/restart — send SIGTERM to restart server -POST /api/run-command — stream a shell command's output as SSE """ import ast @@ -23,7 +22,10 @@ import json import os import re +import shlex import signal +import subprocess +import sys import textwrap import threading import traceback @@ -87,10 +89,41 @@ def _extract_secret_env_keys(spec: dict[str, Any]) -> list[str]: return keys +# --------------------------------------------------------------------------- +# Package manager detection (for logging / display — execution is identical) +# --------------------------------------------------------------------------- + +def _detect_package_manager(command: str) -> str: + """Identify the package manager from the first token of a command string.""" + first = command.strip().split()[0] if command.strip() else "" + if first == "npx": + return "npx" + if first == "uvx": + return "uvx" + if first in ("python", "python3"): + return "pip" + if first == "npm": + return "npm" + return "command" + + # --------------------------------------------------------------------------- # Structured ↔ YAML conversion # --------------------------------------------------------------------------- +# Legacy "npx:" key is kept for backward compatibility. +_SUBPROCESS_KEYS = ("package", "npx") + + +def _get_package_spec(spec: dict[str, Any]) -> dict[str, Any] | None: + """Return the subprocess sub-dict (package: or npx:), or None for code providers.""" + for key in _SUBPROCESS_KEYS: + sub = spec.get(key) + if sub: + return sub + return None + + def _provider_to_structured(name: str, spec: dict[str, Any]) -> dict[str, Any]: """Convert a loaded YAML spec into the structured JSON the UI works with.""" tools_out = [] @@ -119,12 +152,22 @@ def _provider_to_structured(name: str, spec: dict[str, Any]) -> dict[str, Any]: "secrets": secrets, }) + pkg_sub = _get_package_spec(spec) + if pkg_sub is not None: + ptype = "package" + command = (pkg_sub.get("command") or "").strip() + else: + ptype = "code" + command = "" + return { "name": name, "documentation": spec.get("documentation", ""), - "type": "npx" if spec.get("npx") else "code", - "npx_command": (spec.get("npx") or {}).get("command", ""), + "type": ptype, + "command": command, "code": spec.get("code", ""), + "requirements": list(spec.get("requirements") or []), + "setup_commands": list(spec.get("setup_commands") or []), "tools": tools_out, } @@ -139,13 +182,21 @@ def _structured_to_yaml(provider: dict[str, Any]) -> str: ptype = provider.get("type", "code") - if ptype == "npx": - spec["npx"] = {"command": (provider.get("npx_command") or "").strip()} + if ptype == "package": + spec["package"] = {"command": (provider.get("command") or "").strip()} else: code = (provider.get("code") or "").strip() if code: spec["code"] = code + "\n" + requirements = [r for r in (provider.get("requirements") or []) if r] + if requirements: + spec["requirements"] = requirements + + setup_commands = [c for c in (provider.get("setup_commands") or []) if c] + if setup_commands: + spec["setup_commands"] = setup_commands + tools_out = [] for t in provider.get("tools", []): props: dict[str, Any] = {} @@ -188,13 +239,21 @@ def _validate_provider(provider: dict[str, Any]) -> dict[str, Any]: errors: list[str] = [] ptype = provider.get("type", "code") - if ptype == "npx": - if not (provider.get("npx_command") or "").strip(): - errors.append("npx_command is required for npx providers") + if ptype == "package": + if not (provider.get("command") or "").strip(): + errors.append("command is required for package providers") else: if not (provider.get("code") or "").strip(): errors.append("code is required for code providers") + requirements = provider.get("requirements") + if requirements is not None and not isinstance(requirements, list): + errors.append("requirements must be a list") + + setup_commands = provider.get("setup_commands") + if setup_commands is not None and not isinstance(setup_commands, list): + errors.append("setup_commands must be a list") + tools = provider.get("tools", []) if not tools: errors.append("At least one tool is required") @@ -274,12 +333,15 @@ async def list_tools() -> list[dict]: missing_secrets = [k for k in secret_keys if not env_vars.get(k)] structured = _provider_to_structured(path.stem, spec) validation = _validate_provider(structured) + is_package = bool(_get_package_spec(spec)) out.append({ "name": path.stem, "file": path.name, "tool_count": len(tool_entries), "tool_names": [t.get("name") for t in tool_entries], - "is_npx": bool(spec.get("npx")), + "provider_type": structured["type"], + "is_package": is_package, + "is_npx": is_package, # backward-compat alias "secret_keys": secret_keys, "missing_secrets": missing_secrets, "validation_errors": validation["errors"], @@ -345,20 +407,52 @@ async def validate_provider(request: Request) -> dict: provider = body.get("provider") or body return _validate_provider(provider) - # ── npx introspection ──────────────────────────────────────────────────── + # ── Package introspection ───────────────────────────────────────────────── + + @app.post("/api/introspect") + async def introspect_package(request: Request) -> dict: + """Introspect any stdio MCP server command. - @app.post("/api/introspect-npx") - async def introspect_npx(request: Request) -> dict: + Accepts: + command — the command to run (required) + requirements — list of pip packages to install first (optional) + setup_commands — list of shell commands to run before spawning (optional) + + Auto-detects the package manager from the command prefix (npx, uvx, + python, etc.) for logging purposes; execution is identical for all. + """ body = await request.json() command = (body.get("command") or "").strip() + requirements: list[str] = body.get("requirements") or [] + setup_commands: list[str] = body.get("setup_commands") or [] if not command: raise HTTPException(400, "command is required") + + pm = _detect_package_manager(command) + try: - from npx_runner import introspect + # 1. Install pip requirements + for req in requirements: + if not req: + continue + subprocess.run( + [sys.executable, "-m", "pip", "install", req], + check=True, + ) + + # 2. Run setup commands + for cmd in setup_commands: + if not cmd: + continue + subprocess.run(shlex.split(cmd), check=True) + + # 3. Introspect the MCP server + from process_runner import introspect tools = await introspect(command) - return {"ok": True, "tools": tools} + return {"ok": True, "tools": tools, "package_manager": pm} except Exception as exc: - return {"ok": False, "error": str(exc), "tools": []} + traceback.print_exc() + return {"ok": False, "error": str(exc), "tools": [], "package_manager": pm} # ── Function extractor (code providers) ────────────────────────────────── @@ -403,50 +497,6 @@ def _send(): threading.Thread(target=_send, daemon=True).start() return {"ok": True} - # ── Command runner (SSE streaming) ──────────────────────────────────────── - - @app.post("/api/run-command") - async def run_command(request: Request) -> StreamingResponse: - body = await request.json() - command = (body.get("command") or "").strip() - if not command: - raise HTTPException(400, "command is required") - - async def _generate(): - proc = None - try: - proc = await asyncio.create_subprocess_shell( - command, - stdout=asyncio.subprocess.PIPE, - stderr=asyncio.subprocess.STDOUT, - ) - assert proc.stdout is not None - while True: - try: - line = await asyncio.wait_for(proc.stdout.readline(), timeout=300.0) - except asyncio.TimeoutError: - yield f"data: {json.dumps({'error': 'Timed out after 5 min', 'done': True, 'returncode': -1})}\n\n" - proc.kill() - return - if not line: - break - yield f"data: {json.dumps({'line': line.decode(errors='replace').rstrip()})}\n\n" - await proc.wait() - yield f"data: {json.dumps({'done': True, 'returncode': proc.returncode})}\n\n" - except Exception as exc: - if proc is not None: - try: - proc.kill() - except Exception: - pass - yield f"data: {json.dumps({'error': str(exc), 'done': True, 'returncode': -1})}\n\n" - - return StreamingResponse( - _generate(), - media_type="text/event-stream", - headers={"Cache-Control": "no-cache", "X-Accel-Buffering": "no"}, - ) - # ── HTML UI ─────────────────────────────────────────────────────────────── @app.get("/", response_class=HTMLResponse) @@ -499,12 +549,13 @@ async def index(): /* param rows */ .param-row{display:grid;grid-template-columns:1fr 120px 2fr auto;gap:8px;align-items:start;margin-bottom:6px;background:var(--surface);border:1px solid var(--border);border-radius:6px;padding:8px} .secret-row{display:grid;grid-template-columns:1fr 1fr auto;gap:8px;align-items:center;margin-bottom:6px;background:var(--surface);border:1px solid var(--border);border-radius:6px;padding:8px} +.list-row{display:grid;grid-template-columns:1fr auto;gap:8px;align-items:center;margin-bottom:6px} /* CodeMirror */ .CodeMirror{height:260px;font-size:13px;border-radius:0 0 6px 6px;font-family:'JetBrains Mono',Consolas,monospace;border:1px solid #45475a;border-top:none} .cm-wrap{border:1px solid #45475a;border-radius:6px;overflow:hidden} .cm-label{background:#313244;padding:4px 10px;font-size:.75em;color:var(--muted);border:1px solid #45475a;border-bottom:none;border-radius:6px 6px 0 0;font-weight:600;text-transform:uppercase;letter-spacing:.4px} /* badges */ -.badge-npx{background:#cba6f7;color:#1e1e2e;font-size:.65em;padding:2px 6px;border-radius:3px;font-weight:700} +.badge-pkg{background:#cba6f7;color:#1e1e2e;font-size:.65em;padding:2px 6px;border-radius:3px;font-weight:700} .badge-code{background:#89b4fa;color:#1e1e2e;font-size:.65em;padding:2px 6px;border-radius:3px;font-weight:700} .badge-count{background:#45475a;color:#cdd6f4;font-size:.65em;padding:2px 6px;border-radius:3px} /* modal */ @@ -580,7 +631,6 @@ async def index():
-
@@ -612,11 +662,13 @@ async def index(): - - + +
+
+ 📦 pip Requirements optional + +
+
+
pip packages installed before the server starts (e.g. httpx, requests==2.32.0)
+
+ + +
+
+ ⚙ Setup Commands optional + +
+
+
Shell commands run automatically on every server startup (e.g. npx playwright install chrome)
+
+
@@ -682,32 +754,45 @@ async def index():
🐍
Python Code
- Write async Python functions — each one becomes an MCP tool + Write async def functions — each one becomes an MCP tool
-
+
📦
-
NPX Package
- Supply an npx command — the server introspects the MCP tools automatically +
Package
+ Run an existing MCP server via npx, uvx, python -m, or any command — tools are auto-detected
- -
+ +
- + +
+
+ + +
Any command that spawns a stdio MCP server (npx, uvx, python -m, or an installed binary).
+
+
+ +
+ +
pip packages installed before introspection and on every server restart.
- - -
The full command used to start the MCP server process.
+ +
+ +
Shell commands run on every server startup (e.g. npx playwright install chrome).
@@ -724,6 +809,16 @@ async def index():
+
+ +
+ +
+
+ +
+ +
@@ -746,35 +841,6 @@ async def index(): - - - @@ -785,13 +851,11 @@ async def index(): let currentName = null; let currentProvider = null; // the structured JSON object being edited let codeEditor = null; // CodeMirror instance for the code block -let secretsModal = null, wizModal = null, cmdModal = null; -let wzType = null; // 'code' | 'npx' +let secretsModal = null, wizModal = null; +let wzType = null; // 'code' | 'package' let wzStep = 'type'; -let wzIntrospectedTools = []; // tools returned by introspect-npx +let wzIntrospectedTools = []; // tools returned by introspect let providersMeta = {}; // name → {missing_secrets, validation_errors} -let cmdRunning = false; -let cmdAbortController = null; // ───────────────────────────────────────────────────────────────────────────── // Boot @@ -803,7 +867,6 @@ async def index(): }); secretsModal = new bootstrap.Modal('#secrets-modal'); wizModal = new bootstrap.Modal('#wizard-modal'); - cmdModal = new bootstrap.Modal('#cmd-modal'); loadList(); }); @@ -861,6 +924,7 @@ async def index(): const alertRow = (warnBadge || errBadge) ? `
${warnBadge}${errBadge}
` : ''; + const isPkg = p.is_package || p.is_npx; return `
@@ -871,7 +935,7 @@ async def index(): ${alertRow}
- ${p.is_npx ? 'npx' : 'code'} + ${isPkg ? 'pkg' : 'code'} ${p.tool_count}
`; @@ -926,36 +990,99 @@ async def index(): } function renderProvider(p) { - document.getElementById('editor-title').textContent = p.name + (p.type === 'npx' ? ' (npx)' : ' (code)'); + const isPkg = p.type === 'package'; + document.getElementById('editor-title').textContent = p.name + (isPkg ? ' (package)' : ' (code)'); document.getElementById('f-documentation').value = p.documentation || ''; - const isNpx = p.type === 'npx'; - document.getElementById('npx-box').style.display = isNpx ? '' : 'none'; - document.getElementById('code-box').style.display = isNpx ? 'none' : ''; + document.getElementById('package-box').style.display = isPkg ? '' : 'none'; + document.getElementById('code-box').style.display = isPkg ? 'none' : ''; - if (isNpx) { - document.getElementById('f-npx-command').value = p.npx_command || ''; + if (isPkg) { + document.getElementById('f-command').value = p.command || ''; } else { codeEditor.setValue(p.code || ''); setTimeout(() => codeEditor.refresh(), 50); } - renderTools(p.tools || [], isNpx); + renderRequirements(p.requirements || []); + renderSetupCommands(p.setup_commands || []); + renderTools(p.tools || [], isPkg); +} + +// ───────────────────────────────────────────────────────────────────────────── +// Requirements and Setup Commands rendering +// ───────────────────────────────────────────────────────────────────────────── +function renderRequirements(reqs) { + const c = document.getElementById('requirements-container'); + if (!reqs.length) { c.innerHTML = ''; return; } + c.innerHTML = reqs.map((r, i) => ` +
+ + +
`).join(''); +} + +function renderSetupCommands(cmds) { + const c = document.getElementById('setup-commands-container'); + if (!cmds.length) { c.innerHTML = ''; return; } + c.innerHTML = cmds.map((cmd, i) => ` +
+ + +
`).join(''); +} + +function addRequirement() { + ensureProvider(); + currentProvider.requirements = currentProvider.requirements || []; + currentProvider.requirements.push(''); + renderRequirements(currentProvider.requirements); +} + +function removeRequirement(i) { + ensureProvider(); + currentProvider.requirements.splice(i, 1); + renderRequirements(currentProvider.requirements); +} + +function updateRequirement(i, val) { + ensureProvider(); + currentProvider.requirements[i] = val; +} + +function addSetupCommand() { + ensureProvider(); + currentProvider.setup_commands = currentProvider.setup_commands || []; + currentProvider.setup_commands.push(''); + renderSetupCommands(currentProvider.setup_commands); +} + +function removeSetupCommand(i) { + ensureProvider(); + currentProvider.setup_commands.splice(i, 1); + renderSetupCommands(currentProvider.setup_commands); +} + +function updateSetupCommand(i, val) { + ensureProvider(); + currentProvider.setup_commands[i] = val; } // ───────────────────────────────────────────────────────────────────────────── // Tools rendering // ───────────────────────────────────────────────────────────────────────────── -function renderTools(tools, isNpx) { +function renderTools(tools, isPkg) { const container = document.getElementById('tools-container'); if (!tools.length) { container.innerHTML = '
No tools yet — click + Add Tool.
'; return; } - container.innerHTML = tools.map((t, i) => renderToolCard(t, i, isNpx)).join(''); + container.innerHTML = tools.map((t, i) => renderToolCard(t, i, isPkg)).join(''); } -function renderToolCard(t, i, isNpx) { +function renderToolCard(t, i, isPkg) { const params = (t.parameters || []).map((p, j) => `
✕
`).join(''); - const fnField = isNpx ? '' : ` + const fnField = isPkg ? '' : `
r.trim()); + p.setup_commands = (currentProvider.setup_commands || []).filter(c => c.trim()); return p; } @@ -1072,43 +1202,42 @@ async def index(): function addTool() { ensureProvider(); - const isNpx = currentProvider.type === 'npx'; + const isPkg = currentProvider.type === 'package'; currentProvider.tools.push({ name: '', function: '', description: '', documentation: '', parameters: [], secrets: [], }); - renderTools(currentProvider.tools, isNpx); + renderTools(currentProvider.tools, isPkg); } function removeTool(i) { ensureProvider(); currentProvider.tools.splice(i, 1); - renderTools(currentProvider.tools, currentProvider.type === 'npx'); + renderTools(currentProvider.tools, currentProvider.type === 'package'); } function addParam(ti) { ensureProvider(); currentProvider.tools[ti].parameters.push({name:'',type:'string',description:'',required:false,default:null}); - const isNpx = currentProvider.type === 'npx'; - renderTools(currentProvider.tools, isNpx); + renderTools(currentProvider.tools, currentProvider.type === 'package'); } function removeParam(ti, pi) { ensureProvider(); currentProvider.tools[ti].parameters.splice(pi, 1); - renderTools(currentProvider.tools, currentProvider.type === 'npx'); + renderTools(currentProvider.tools, currentProvider.type === 'package'); } function addSecret(ti) { ensureProvider(); currentProvider.tools[ti].secrets.push({arg:'',env:''}); - renderTools(currentProvider.tools, currentProvider.type === 'npx'); + renderTools(currentProvider.tools, currentProvider.type === 'package'); } function removeSecret(ti, si) { ensureProvider(); currentProvider.tools[ti].secrets.splice(si, 1); - renderTools(currentProvider.tools, currentProvider.type === 'npx'); + renderTools(currentProvider.tools, currentProvider.type === 'package'); } // ───────────────────────────────────────────────────────────────────────────── @@ -1152,7 +1281,6 @@ async def index(): document.getElementById('secrets-provider-name').textContent = currentName; const env = await api('GET', '/api/env').catch(() => ({vars:{}, env_file:'.env'})); document.getElementById('secrets-env-path').textContent = env.env_file || '.env'; - // Collect all declared secret env var names from in-memory tools const keys = []; for (const t of currentProvider.tools) { for (const s of (t.secrets || [])) { @@ -1199,7 +1327,7 @@ async def index(): // ───────────────────────────────────────────────────────────────────────────── // Wizard // ───────────────────────────────────────────────────────────────────────────── -const WZ_STEPS = ['type','npx','code','secrets']; +const WZ_STEPS = ['type','package','code','secrets']; function wzShowStep(step) { WZ_STEPS.forEach(s => { @@ -1214,10 +1342,14 @@ async def index(): function openWizard() { wzType = null; wzStep = 'type'; wzIntrospectedTools = []; - document.getElementById('wz-npx-name').value = ''; - document.getElementById('wz-npx-cmd').value = ''; + document.getElementById('wz-pkg-name').value = ''; + document.getElementById('wz-pkg-cmd').value = ''; + document.getElementById('wz-pkg-reqs-container').innerHTML = ''; + document.getElementById('wz-pkg-cmds-container').innerHTML = ''; document.getElementById('wz-code-name').value = ''; document.getElementById('wz-code-input').value = ''; + document.getElementById('wz-code-reqs-container').innerHTML = ''; + document.getElementById('wz-code-cmds-container').innerHTML = ''; document.getElementById('wz-introspect-result').innerHTML = ''; document.getElementById('wz-analyze-result').innerHTML = ''; wzShowStep('type'); @@ -1231,21 +1363,43 @@ async def index(): setTimeout(() => wzShowStep(type), 120); } +// Wizard requirement/setup-command list helpers +function _wzListAdd(containerId, placeholder) { + const c = document.getElementById(containerId); + const idx = c.children.length; + const div = document.createElement('div'); + div.className = 'list-row mt-1'; + div.innerHTML = ` + `; + c.appendChild(div); +} +function wzAddReq() { _wzListAdd('wz-pkg-reqs-container', 'requests==2.32.0'); } +function wzAddSetupCmd() { _wzListAdd('wz-pkg-cmds-container', 'npx playwright install chrome'); } +function wzAddCodeReq() { _wzListAdd('wz-code-reqs-container', 'httpx'); } +function wzAddCodeSetupCmd() { _wzListAdd('wz-code-cmds-container', 'python -m playwright install chromium'); } + +function _wzGetListValues(containerId) { + return Array.from(document.querySelectorAll(`#${containerId} input`)) + .map(el => el.value.trim()).filter(Boolean); +} + async function wzNext() { const errEl = document.getElementById('wz-error'); errEl.textContent = ''; - if (wzStep === 'type') return; // handled by card click + if (wzStep === 'type') return; - if (wzStep === 'npx') { - const name = document.getElementById('wz-npx-name').value.trim(); - const cmd = document.getElementById('wz-npx-cmd').value.trim(); + if (wzStep === 'package') { + const name = document.getElementById('wz-pkg-name').value.trim(); + const cmd = document.getElementById('wz-pkg-cmd').value.trim(); if (!name) { errEl.textContent = 'Provider name is required.'; return; } - if (!cmd) { errEl.textContent = 'NPX command is required.'; return; } + if (!cmd) { errEl.textContent = 'Command is required.'; return; } if (!wzIntrospectedTools.length) { errEl.textContent = 'Click "Introspect Tools" first.'; return; } - // Create the provider + const requirements = _wzGetListValues('wz-pkg-reqs-container'); + const setup_commands = _wzGetListValues('wz-pkg-cmds-container'); const provider = { - name, type: 'npx', npx_command: cmd, documentation: '', code: '', + name, type: 'package', command: cmd, documentation: '', code: '', + requirements, setup_commands, tools: wzIntrospectedTools.map(t => ({ name: t.name, function: '', @@ -1269,10 +1423,12 @@ async def index(): const code = document.getElementById('wz-code-input').value; if (!name) { errEl.textContent = 'Provider name is required.'; return; } if (!code.trim()) { errEl.textContent = 'Code is required.'; return; } - // Build tools from analyzed functions + const requirements = _wzGetListValues('wz-code-reqs-container'); + const setup_commands = _wzGetListValues('wz-code-cmds-container'); const fns = await _analyzeFns(code); const provider = { - name, type: 'code', npx_command: '', documentation: '', code, + name, type: 'code', command: '', documentation: '', code, + requirements, setup_commands, tools: fns.map(fn => ({ name: fn.name, function: fn.name, description: `TODO — describe what ${fn.name} does`, @@ -1296,19 +1452,21 @@ async def index(): } function wzBack() { - const map = {npx:'type', code:'type', secrets: wzType||'type'}; + const map = {package:'type', code:'type', secrets: wzType||'type'}; wzShowStep(map[wzStep] || 'type'); } async function wzIntrospect() { - const cmd = document.getElementById('wz-npx-cmd').value.trim(); - const el = document.getElementById('wz-introspect-result'); - const btn = document.getElementById('wz-introspect-btn'); - if (!cmd) { el.innerHTML = 'Enter an npx command first.'; return; } + const cmd = document.getElementById('wz-pkg-cmd').value.trim(); + const el = document.getElementById('wz-introspect-result'); + const btn = document.getElementById('wz-introspect-btn'); + if (!cmd) { el.innerHTML = 'Enter a command first.'; return; } + const requirements = _wzGetListValues('wz-pkg-reqs-container'); + const setup_commands = _wzGetListValues('wz-pkg-cmds-container'); btn.disabled = true; btn.textContent = '⏳ Introspecting…'; - el.innerHTML = 'Running npx — this may take a moment on first use…'; + el.innerHTML = 'Running — this may take a moment on first use…'; try { - const r = await api('POST', '/api/introspect-npx', {command: cmd}); + const r = await api('POST', '/api/introspect', {command: cmd, requirements, setup_commands}); if (!r.ok) throw new Error(r.error || 'Introspection failed'); wzIntrospectedTools = r.tools || []; el.innerHTML = `
✓ Found ${wzIntrospectedTools.length} tool(s): ${wzIntrospectedTools.map(t=>t.name).join(', ')}
`; @@ -1375,7 +1533,7 @@ async def index(): catch(e) { toast(e.message, false); } } wizModal.hide(); - await loadList(); // refresh providersMeta before openProvider reads it + await loadList(); await openProvider(currentName); } @@ -1397,133 +1555,6 @@ async def index(): default: def.default ?? null, })); } - -// ───────────────────────────────────────────────────────────────────────────── -// Command runner -// ───────────────────────────────────────────────────────────────────────────── -function openCmdModal() { - // Build context-aware suggestions - const suggestions = []; - if (currentProvider) { - const npxCmd = (currentProvider.npx_command || '').toLowerCase(); - if (npxCmd.includes('playwright')) { - suggestions.push('npx playwright install chrome'); - suggestions.push('npx playwright install --with-deps chromium'); - suggestions.push('npx playwright install'); - } - // Generic: show current npx command with --help - if (currentProvider.type === 'npx' && currentProvider.npx_command) { - const base = currentProvider.npx_command.trim(); - suggestions.push(base + ' --help'); - } - } - suggestions.push('node --version'); - suggestions.push('npx --version'); - suggestions.push('which npx'); - - const suggestEl = document.getElementById('cmd-suggestions'); - suggestEl.innerHTML = suggestions.map(s => - `` - ).join(''); - - clearCmdOutput(); - cmdModal.show(); - setTimeout(() => document.getElementById('cmd-input').focus(), 300); -} - -function clearCmdOutput() { - document.getElementById('cmd-output').textContent = ''; - document.getElementById('cmd-status').textContent = ''; - document.getElementById('cmd-status').style.color = 'var(--muted)'; -} - -async function runCommand() { - // If already running, cancel it - if (cmdRunning) { - if (cmdAbortController) cmdAbortController.abort(); - return; - } - - const command = document.getElementById('cmd-input').value.trim(); - if (!command) return; - - const outEl = document.getElementById('cmd-output'); - const statusEl = document.getElementById('cmd-status'); - const btn = document.getElementById('cmd-run-btn'); - - outEl.textContent = `$ ${command}\n`; - statusEl.textContent = 'Running…'; - statusEl.style.color = 'var(--muted)'; - btn.textContent = '⏹ Stop'; - btn.classList.replace('btn-primary', 'btn-danger'); - cmdRunning = true; - cmdAbortController = new AbortController(); - - try { - const response = await fetch('/api/run-command', { - method: 'POST', - headers: {'Content-Type': 'application/json'}, - body: JSON.stringify({command}), - signal: cmdAbortController.signal, - }); - - if (!response.ok) { - const data = await response.json().catch(() => ({})); - outEl.textContent += `Error: ${data.detail || response.statusText}\n`; - return; - } - - const reader = response.body.getReader(); - const decoder = new TextDecoder(); - let buffer = ''; - - while (true) { - const {done, value} = await reader.read(); - if (done) break; - buffer += decoder.decode(value, {stream: true}); - const parts = buffer.split('\n'); - buffer = parts.pop(); // keep any incomplete line - - for (const part of parts) { - if (!part.startsWith('data: ')) continue; - let data; - try { data = JSON.parse(part.slice(6)); } catch { continue; } - - if (data.line !== undefined) { - outEl.textContent += data.line + '\n'; - outEl.scrollTop = outEl.scrollHeight; - } - if (data.error && data.line === undefined) { - outEl.textContent += `Error: ${data.error}\n`; - } - if (data.done) { - const ok = data.returncode === 0; - statusEl.textContent = ok - ? `✓ Exited 0` - : `✗ Exited ${data.returncode}`; - statusEl.style.color = ok ? 'var(--green)' : 'var(--red)'; - outEl.scrollTop = outEl.scrollHeight; - } - } - } - } catch (e) { - if (e.name === 'AbortError') { - outEl.textContent += '\n[Cancelled]\n'; - statusEl.textContent = 'Cancelled'; - statusEl.style.color = 'var(--yellow)'; - } else { - outEl.textContent += `\n[Connection error: ${e.message}]\n`; - statusEl.textContent = 'Error'; - statusEl.style.color = 'var(--red)'; - } - } finally { - cmdRunning = false; - cmdAbortController = null; - btn.textContent = '▶ Run'; - btn.classList.replace('btn-danger', 'btn-primary'); - } -} """ diff --git a/process_runner.py b/process_runner.py new file mode 100644 index 0000000..36979d1 --- /dev/null +++ b/process_runner.py @@ -0,0 +1,163 @@ +""" +process_runner.py — Spawn and talk to any stdio-based MCP server subprocess. + +Supports npx, uvx, pip-installed commands, npm-installed binaries, or any other +command that speaks the MCP stdio transport (one JSON-RPC object per line on +stdout, stdin for requests). + +Each provider YAML that has a ``package:`` block (instead of a ``code:`` block) +is handled here. + +Two use-cases +───────────── +1. Introspection (frontend wizard): spawn → initialize → tools/list → kill. +2. Tool calls (server): one persistent session per command string; + process is (re-)started on demand and reused across calls. +""" + +import asyncio +import json +import shlex +import traceback +from typing import Any + + +class ProcessSession: + """A long-lived connection to a single stdio MCP server process.""" + + def __init__(self, command: str) -> None: + self.command = command + self._parts: list[str] = shlex.split(command) + self._proc: asyncio.subprocess.Process | None = None + self._lock = asyncio.Lock() + self._next_id = 0 + + # ── internal ────────────────────────────────────────────────────────────── + + def _new_id(self) -> int: + self._next_id += 1 + return self._next_id + + async def _send(self, msg: dict[str, Any]) -> None: + assert self._proc and self._proc.stdin + data = json.dumps(msg, separators=(",", ":")) + "\n" + self._proc.stdin.write(data.encode()) + await self._proc.stdin.drain() + + async def _recv(self, timeout: float = 30.0) -> dict[str, Any]: + assert self._proc and self._proc.stdout + line = await asyncio.wait_for(self._proc.stdout.readline(), timeout=timeout) + if not line: + raise EOFError("MCP process closed stdout") + return json.loads(line) + + async def _start(self) -> None: + self._proc = await asyncio.create_subprocess_exec( + *self._parts, + stdin=asyncio.subprocess.PIPE, + stdout=asyncio.subprocess.PIPE, + stderr=asyncio.subprocess.PIPE, + ) + # initialize handshake + rid = self._new_id() + await self._send({ + "jsonrpc": "2.0", "id": rid, "method": "initialize", + "params": { + "protocolVersion": "2024-11-05", + "capabilities": {}, + "clientInfo": {"name": "mcpproxy", "version": "1.0"}, + }, + }) + await self._recv(timeout=60) # initialize response + # notifications/initialized (no response expected) + await self._send({"jsonrpc": "2.0", "method": "notifications/initialized"}) + + def _alive(self) -> bool: + return self._proc is not None and self._proc.returncode is None + + # ── public ──────────────────────────────────────────────────────────────── + + async def list_tools(self) -> list[dict[str, Any]]: + async with self._lock: + if not self._alive(): + await self._start() + rid = self._new_id() + await self._send({"jsonrpc": "2.0", "id": rid, "method": "tools/list", "params": {}}) + resp = await self._recv() + return resp.get("result", {}).get("tools", []) + + async def call_tool(self, tool_name: str, arguments: dict[str, Any]) -> Any: + async with self._lock: + if not self._alive(): + await self._start() + rid = self._new_id() + await self._send({ + "jsonrpc": "2.0", "id": rid, "method": "tools/call", + "params": {"name": tool_name, "arguments": arguments}, + }) + resp = await self._recv(timeout=120) + + if "error" in resp: + err = resp["error"] + return {"ok": False, "error": err.get("message", str(err))} + + result = resp.get("result", {}) + content: list[dict] = result.get("content", []) + if not content: + return {"ok": True, **result} + + parts: list[Any] = [] + for item in content: + if item.get("type") == "text": + text = item["text"] + try: + parts.append(json.loads(text)) + except json.JSONDecodeError: + parts.append(text) + else: + parts.append(item) + + return {"ok": True, "result": parts[0] if len(parts) == 1 else parts} + + async def close(self) -> None: + if self._proc: + try: + self._proc.stdin.close() # type: ignore[union-attr] + await asyncio.wait_for(self._proc.wait(), timeout=5) + except Exception: + self._proc.kill() + self._proc = None + + +# Backward-compatible alias +NpxSession = ProcessSession + + +# --------------------------------------------------------------------------- +# Module-level session registry (one session per command string) +# --------------------------------------------------------------------------- + +_sessions: dict[str, ProcessSession] = {} + + +def get_session(command: str) -> ProcessSession: + """Return (creating if needed) the persistent session for *command*.""" + if command not in _sessions: + _sessions[command] = ProcessSession(command) + return _sessions[command] + + +async def introspect(command: str) -> list[dict[str, Any]]: + """ + Spawn a *fresh* process, fetch its tools/list, then shut it down. + Used by the frontend wizard — does not affect the persistent session registry. + """ + session = ProcessSession(command) + try: + await session._start() + return await session.list_tools() + except Exception as exc: + traceback.print_exc() + raise RuntimeError(f"Failed to introspect '{command}': {exc}") from exc + finally: + await session.close() diff --git a/server.py b/server.py index c391314..6218964 100755 --- a/server.py +++ b/server.py @@ -3,15 +3,25 @@ Each YAML file in MCP_TOOL_CONFIG_DIR defines one *provider*. Two kinds: - Code provider — has a ``code:`` block with async Python functions. - npx provider — has an ``npx:`` block; tool calls are proxied to the - npx process over stdio (no code block needed). + Code provider — has a ``code:`` block with async Python functions. + Package provider — has a ``package:`` block; tool calls are proxied to + the subprocess over stdio (no code block needed). + Supports any command: npx, uvx, python -m, or an + installed binary. + Legacy ``npx:`` key is also accepted (backward compat). Provider YAML keys: - code: Python source executed once at startup (code providers). - npx: - command: npx command string, e.g. "npx @playwright/mcp@latest" - tools: List of tool declarations: + code: Python source executed once at startup (code providers). + package: + command: Full command to spawn the MCP server, e.g.: + "npx @playwright/mcp@latest --isolated" + "uvx mcp-server-fetch" + "python -m mcp_server_github" + "mcp-server-github" + requirements: List of pip packages installed before the server starts. + setup_commands: List of shell commands run on every server startup + (e.g. "npx playwright install chrome"). + tools: List of tool declarations: name — unique MCP tool name function — async function name from code block (code providers only) description — shown to the LLM @@ -26,6 +36,9 @@ import builtins import inspect import os +import shlex +import subprocess +import sys import threading import traceback from pathlib import Path @@ -57,6 +70,10 @@ "array": list, } +# Keys that indicate a subprocess/package provider (in priority order). +# "npx" is kept for backward compatibility with existing YAML files. +SUBPROCESS_KEYS = ("package", "npx") + # --------------------------------------------------------------------------- # Pure helpers @@ -209,11 +226,25 @@ async def dynamic_tool(ctx: Context, **kwargs: Any) -> Any: raise -def _make_npx_handler(command: str, tool_name: str) -> Callable[..., Any]: - """Return an async handler that proxies calls to an npx MCP process.""" - from npx_runner import get_session +def _get_package_command(spec: dict[str, Any]) -> str | None: + """Return the spawn command for package providers, or None for code providers. + + Checks ``package:`` first, then ``npx:`` for backward compatibility with + existing YAML configs that used the old key name. + """ + for key in SUBPROCESS_KEYS: + sub = spec.get(key) + if sub: + command = (sub.get("command") or "").strip() + return command or None + return None + + +def _make_process_handler(command: str, tool_name: str) -> Callable[..., Any]: + """Return an async handler that proxies calls to a subprocess MCP process.""" + from process_runner import get_session - async def npx_handler(context: dict[str, Any], **kwargs: Any) -> Any: + async def process_handler(context: dict[str, Any], **kwargs: Any) -> Any: try: session = get_session(command) return await session.call_tool(tool_name, kwargs) @@ -221,25 +252,21 @@ async def npx_handler(context: dict[str, Any], **kwargs: Any) -> Any: traceback.print_exc() return {"ok": False, "error": str(exc), "tool": tool_name} - npx_handler.__name__ = tool_name - return npx_handler + process_handler.__name__ = tool_name + return process_handler def register_provider(spec: dict[str, Any]) -> None: """Register all tools declared in one provider spec.""" source_path = spec.get("_config_path", "") try: - npx_spec = spec.get("npx") - - if npx_spec: - # ── npx provider ───────────────────────────────────────────────── - command = (npx_spec.get("command") or "").strip() - if not command: - raise ValueError(f"npx provider in {source_path} has no 'command' field") + command = _get_package_command(spec) + if command is not None: + # ── package provider (npx / uvx / python -m / any binary) ────────── for tool_spec in spec.get("tools", []): tool_name = tool_spec.get("name", "") - handler = _make_npx_handler(command, tool_name) + handler = _make_process_handler(command, tool_name) register_tool(tool_spec, handler) else: @@ -271,12 +298,40 @@ def register_provider(spec: dict[str, Any]) -> None: raise +def run_provider_setup(spec: dict[str, Any]) -> None: + """Install requirements and run setup commands declared in a provider spec. + + Runs synchronously at startup so that every ``docker restart`` re-executes + the setup steps. pip is a no-op when packages are already installed. + """ + source_path = spec.get("_config_path", "") + try: + for req in spec.get("requirements", []): + if not req: + continue + print(f"Installing requirement '{req}' for {source_path}") + subprocess.run( + [sys.executable, "-m", "pip", "install", req], + check=True, + ) + for cmd in spec.get("setup_commands", []): + if not cmd: + continue + print(f"Running setup command: {cmd}") + subprocess.run(shlex.split(cmd), check=True) + except Exception as exc: + print(f"run_provider_setup error in {source_path}: {exc}") + traceback.print_exc() + raise + + # --------------------------------------------------------------------------- # Load all providers at import time # --------------------------------------------------------------------------- for provider_spec in load_provider_specs(CONFIG_DIR): register_provider(provider_spec) + run_provider_setup(provider_spec) # --------------------------------------------------------------------------- diff --git a/tests/test_frontend.py b/tests/test_frontend.py index ce2b49c..6f1a1f2 100644 --- a/tests/test_frontend.py +++ b/tests/test_frontend.py @@ -1,11 +1,13 @@ """Unit tests for the HTTP frontend (frontend/app.py).""" from pathlib import Path +from unittest.mock import AsyncMock, patch import pytest import yaml from fastapi.testclient import TestClient from frontend.app import ( + _detect_package_manager, _extract_functions, _extract_secret_env_keys, _provider_to_structured, @@ -48,20 +50,24 @@ def client(app): "name": "myprovider", "type": "code", "documentation": "", - "npx_command": "", + "command": "", "code": "async def ping(context, msg='hi'):\n return {'ok': True}\n", + "requirements": [], + "setup_commands": [], "tools": [{ "name": "ping", "function": "ping", "description": "Ping tool", "documentation": "", "parameters": [], "secrets": [], }], } -NPX_PROVIDER = { +PACKAGE_PROVIDER = { "name": "playwright", - "type": "npx", + "type": "package", "documentation": "", - "npx_command": "npx @playwright/mcp@latest --isolated", + "command": "npx @playwright/mcp@latest --isolated", "code": "", + "requirements": [], + "setup_commands": [], "tools": [{ "name": "playwright_navigate", "function": "", "description": "Navigate", "documentation": "", "parameters": [ @@ -87,13 +93,34 @@ def test_lists_code_provider(self, app, tools_dir): data = r.json() assert len(data) == 1 assert data[0]["name"] == "myprovider" - assert data[0]["is_npx"] is False + assert data[0]["is_package"] is False + assert data[0]["provider_type"] == "code" - def test_lists_npx_provider(self, app, tools_dir): - content = _structured_to_yaml(NPX_PROVIDER) + def test_lists_package_provider(self, app, tools_dir): + content = _structured_to_yaml(PACKAGE_PROVIDER) (tools_dir / "playwright.yaml").write_text(content) r = TestClient(app).get("/api/tools") - assert r.json()[0]["is_npx"] is True + data = r.json() + assert data[0]["is_package"] is True + assert data[0]["provider_type"] == "package" + + def test_is_npx_backward_compat_alias(self, app, tools_dir): + """is_npx must still be present and equal is_package.""" + content = _structured_to_yaml(PACKAGE_PROVIDER) + (tools_dir / "playwright.yaml").write_text(content) + r = TestClient(app).get("/api/tools") + d = r.json()[0] + assert d["is_npx"] == d["is_package"] + + def test_legacy_npx_key_listed_as_package(self, app, tools_dir): + """A YAML with the legacy 'npx:' key is shown as a package provider.""" + spec = { + "npx": {"command": "npx @playwright/mcp@latest"}, + "tools": [{"name": "nav", "description": "Navigate", "input_schema": {"type": "object", "properties": {}, "required": []}}], + } + (tools_dir / "legacy.yaml").write_text(yaml.dump(spec)) + r = TestClient(app).get("/api/tools") + assert r.json()[0]["is_package"] is True # --------------------------------------------------------------------------- @@ -113,11 +140,37 @@ def test_existing(self, app, tools_dir): def test_not_found(self, client): assert client.get("/api/tools/nope").status_code == 404 - def test_npx_provider(self, app, tools_dir): - (tools_dir / "playwright.yaml").write_text(_structured_to_yaml(NPX_PROVIDER)) + def test_package_provider(self, app, tools_dir): + (tools_dir / "playwright.yaml").write_text(_structured_to_yaml(PACKAGE_PROVIDER)) r = TestClient(app).get("/api/tools/playwright") - assert r.json()["type"] == "npx" - assert "npx_command" in r.json() + data = r.json() + assert data["type"] == "package" + assert "command" in data + assert data["command"] == "npx @playwright/mcp@latest --isolated" + + def test_legacy_npx_key_returns_package_type(self, app, tools_dir): + """GET on a YAML with legacy 'npx:' key returns type='package'.""" + spec = { + "npx": {"command": "npx @playwright/mcp@latest"}, + "tools": [{"name": "nav", "description": "Nav", "input_schema": {"type": "object", "properties": {}, "required": []}}], + } + (tools_dir / "legacy.yaml").write_text(yaml.dump(spec)) + r = TestClient(app).get("/api/tools/legacy") + data = r.json() + assert data["type"] == "package" + assert data["command"] == "npx @playwright/mcp@latest" + + def test_requirements_and_setup_commands_returned(self, app, tools_dir): + provider = { + **CODE_PROVIDER, + "requirements": ["httpx", "requests"], + "setup_commands": ["echo hello"], + } + (tools_dir / "myprovider.yaml").write_text(_structured_to_yaml(provider)) + r = TestClient(app).get("/api/tools/myprovider") + data = r.json() + assert data["requirements"] == ["httpx", "requests"] + assert data["setup_commands"] == ["echo hello"] # --------------------------------------------------------------------------- @@ -152,10 +205,35 @@ def test_missing_code_400(self, client): r = client.post("/api/tools", json={"name": "x", "provider": p}) assert r.status_code == 400 - def test_create_npx_provider(self, client): - r = client.post("/api/tools", json={"name": "playwright", "provider": NPX_PROVIDER}) + def test_create_package_provider(self, client): + r = client.post("/api/tools", json={"name": "playwright", "provider": PACKAGE_PROVIDER}) assert r.status_code == 200 + def test_package_yaml_uses_package_key(self, client, tools_dir): + """Saved YAML must use 'package:' key, not 'npx:'.""" + client.post("/api/tools", json={"name": "pw", "provider": PACKAGE_PROVIDER}) + spec = yaml.safe_load((tools_dir / "pw.yaml").read_text()) + assert "package" in spec + assert "npx" not in spec + + def test_requirements_saved_to_yaml(self, client, tools_dir): + provider = {**CODE_PROVIDER, "requirements": ["httpx"]} + client.post("/api/tools", json={"name": "myprovider", "provider": provider}) + spec = yaml.safe_load((tools_dir / "myprovider.yaml").read_text()) + assert spec.get("requirements") == ["httpx"] + + def test_setup_commands_saved_to_yaml(self, client, tools_dir): + provider = {**PACKAGE_PROVIDER, "setup_commands": ["npx playwright install chrome"]} + client.post("/api/tools", json={"name": "playwright", "provider": provider}) + spec = yaml.safe_load((tools_dir / "playwright.yaml").read_text()) + assert spec.get("setup_commands") == ["npx playwright install chrome"] + + def test_empty_requirements_not_written(self, client, tools_dir): + """Empty requirements list should be omitted from YAML.""" + client.post("/api/tools", json={"name": "myprovider", "provider": CODE_PROVIDER}) + spec = yaml.safe_load((tools_dir / "myprovider.yaml").read_text()) + assert "requirements" not in spec or spec["requirements"] == [] + # --------------------------------------------------------------------------- # PUT /api/tools/{name} @@ -198,8 +276,8 @@ def test_valid_code_provider(self, client): r = client.post("/api/validate", json={"provider": CODE_PROVIDER}) assert r.json()["ok"] is True - def test_valid_npx_provider(self, client): - r = client.post("/api/validate", json={"provider": NPX_PROVIDER}) + def test_valid_package_provider(self, client): + r = client.post("/api/validate", json={"provider": PACKAGE_PROVIDER}) assert r.json()["ok"] is True def test_missing_tools(self, client): @@ -210,8 +288,8 @@ def test_missing_code(self, client): r = client.post("/api/validate", json={"provider": {**CODE_PROVIDER, "code": ""}}) assert not r.json()["ok"] - def test_missing_npx_command(self, client): - r = client.post("/api/validate", json={"provider": {**NPX_PROVIDER, "npx_command": ""}}) + def test_missing_command(self, client): + r = client.post("/api/validate", json={"provider": {**PACKAGE_PROVIDER, "command": ""}}) assert not r.json()["ok"] def test_tool_missing_description(self, client): @@ -241,6 +319,60 @@ def test_syntax_error(self, client): assert not r.json()["ok"] +# --------------------------------------------------------------------------- +# POST /api/introspect +# --------------------------------------------------------------------------- + +class TestIntrospect: + def test_missing_command_400(self, client): + r = client.post("/api/introspect", json={}) + assert r.status_code == 400 + + def test_introspect_returns_tools(self, client): + fake_tools = [{"name": "nav", "description": "Navigate", "inputSchema": {}}] + with patch("process_runner.introspect", new=AsyncMock(return_value=fake_tools)): + r = client.post("/api/introspect", json={"command": "npx @playwright/mcp@latest"}) + assert r.status_code == 200 + data = r.json() + assert data["ok"] is True + assert len(data["tools"]) == 1 + assert data["tools"][0]["name"] == "nav" + + def test_introspect_detects_package_manager(self, client): + with patch("process_runner.introspect", new=AsyncMock(return_value=[])): + r = client.post("/api/introspect", json={"command": "uvx mcp-server-fetch"}) + assert r.json().get("package_manager") == "uvx" + + def test_introspect_error_returns_ok_false(self, client): + with patch("process_runner.introspect", new=AsyncMock(side_effect=RuntimeError("failed"))): + r = client.post("/api/introspect", json={"command": "bad-command"}) + assert r.status_code == 200 + assert r.json()["ok"] is False + + def test_requirements_installed_before_introspect(self, client): + """pip install is called for each requirement before introspection.""" + with patch("process_runner.introspect", new=AsyncMock(return_value=[])), \ + patch("frontend.app.subprocess.run") as mock_run: + r = client.post("/api/introspect", json={ + "command": "python -m mcp_server_github", + "requirements": ["mcp-server-github"], + }) + mock_run.assert_called_once() + args = mock_run.call_args[0][0] + assert "pip" in args + assert "mcp-server-github" in args + + def test_old_introspect_npx_path_not_found(self, client): + """The old /api/introspect-npx endpoint must no longer exist.""" + r = client.post("/api/introspect-npx", json={"command": "npx something"}) + assert r.status_code == 404 + + def test_run_command_endpoint_not_found(self, client): + """The /api/run-command endpoint has been removed.""" + r = client.post("/api/run-command", json={"command": "echo hi"}) + assert r.status_code == 404 + + # --------------------------------------------------------------------------- # GET & POST /api/env # --------------------------------------------------------------------------- @@ -283,7 +415,6 @@ def test_returns_html(self, client): def test_no_raw_yaml_editor(self, client): r = client.get("/") - # The UI should not contain a raw YAML CodeMirror editor referencing 'yaml' mode assert "mode/yaml" not in r.text def test_no_discover_tab(self, client): @@ -292,6 +423,24 @@ def test_no_discover_tab(self, client): def test_contains_api_calls(self, client): assert "/api/tools" in client.get("/").text + def test_no_run_command_modal(self, client): + """Run Command modal has been replaced by setup_commands field.""" + assert "cmd-modal" not in client.get("/").text + + def test_contains_introspect_endpoint(self, client): + assert "/api/introspect" in client.get("/").text + + def test_contains_setup_commands_ui(self, client): + assert "setup_commands" in client.get("/").text or "setup-commands" in client.get("/").text + + def test_wizard_has_package_type(self, client): + assert "wzSelectType('package')" in client.get("/").text + + def test_wizard_has_two_type_cards(self, client): + text = client.get("/").text + assert "wzSelectType('code')" in text + assert "wzSelectType('package')" in text + # --------------------------------------------------------------------------- # Pure function tests @@ -324,21 +473,39 @@ def test_code_round_trip(self): yaml_str = _structured_to_yaml(CODE_PROVIDER) spec = yaml.safe_load(yaml_str) assert "code" in spec + assert not spec.get("package") assert not spec.get("npx") structured = _provider_to_structured("myprovider", spec) assert structured["type"] == "code" assert len(structured["tools"]) == 1 - def test_npx_round_trip(self): - yaml_str = _structured_to_yaml(NPX_PROVIDER) + def test_package_round_trip(self): + yaml_str = _structured_to_yaml(PACKAGE_PROVIDER) spec = yaml.safe_load(yaml_str) - assert spec.get("npx", {}).get("command") == "npx @playwright/mcp@latest --isolated" + assert "package" in spec + assert spec["package"]["command"] == "npx @playwright/mcp@latest --isolated" structured = _provider_to_structured("playwright", spec) - assert structured["type"] == "npx" - assert structured["npx_command"] == "npx @playwright/mcp@latest --isolated" + assert structured["type"] == "package" + assert structured["command"] == "npx @playwright/mcp@latest --isolated" + + def test_package_yaml_uses_package_key_not_npx(self): + yaml_str = _structured_to_yaml(PACKAGE_PROVIDER) + spec = yaml.safe_load(yaml_str) + assert "package" in spec + assert "npx" not in spec + + def test_legacy_npx_key_reads_as_package(self): + """Loading a YAML with the old 'npx:' key returns type='package'.""" + spec = { + "npx": {"command": "npx @playwright/mcp@latest"}, + "tools": [], + } + structured = _provider_to_structured("legacy", spec) + assert structured["type"] == "package" + assert structured["command"] == "npx @playwright/mcp@latest" def test_parameters_preserved(self): - yaml_str = _structured_to_yaml(NPX_PROVIDER) + yaml_str = _structured_to_yaml(PACKAGE_PROVIDER) spec = yaml.safe_load(yaml_str) structured = _provider_to_structured("playwright", spec) params = structured["tools"][0]["parameters"] @@ -346,6 +513,43 @@ def test_parameters_preserved(self): assert params[0]["name"] == "url" assert params[0]["required"] is True + def test_requirements_round_trip(self): + provider = {**CODE_PROVIDER, "requirements": ["httpx", "requests"]} + yaml_str = _structured_to_yaml(provider) + spec = yaml.safe_load(yaml_str) + assert spec["requirements"] == ["httpx", "requests"] + structured = _provider_to_structured("p", spec) + assert structured["requirements"] == ["httpx", "requests"] + + def test_setup_commands_round_trip(self): + provider = {**PACKAGE_PROVIDER, "setup_commands": ["npx playwright install chrome"]} + yaml_str = _structured_to_yaml(provider) + spec = yaml.safe_load(yaml_str) + assert spec["setup_commands"] == ["npx playwright install chrome"] + structured = _provider_to_structured("p", spec) + assert structured["setup_commands"] == ["npx playwright install chrome"] + + def test_empty_requirements_omitted_from_yaml(self): + yaml_str = _structured_to_yaml(CODE_PROVIDER) # requirements = [] + spec = yaml.safe_load(yaml_str) + assert "requirements" not in spec + + def test_empty_setup_commands_omitted_from_yaml(self): + yaml_str = _structured_to_yaml(CODE_PROVIDER) # setup_commands = [] + spec = yaml.safe_load(yaml_str) + assert "setup_commands" not in spec + + def test_requirements_defaults_to_empty_list(self): + """Specs without requirements return [] not None.""" + spec = {"code": "pass\n", "tools": []} + structured = _provider_to_structured("p", spec) + assert structured["requirements"] == [] + + def test_setup_commands_defaults_to_empty_list(self): + spec = {"code": "pass\n", "tools": []} + structured = _provider_to_structured("p", spec) + assert structured["setup_commands"] == [] + class TestExtractSecretEnvKeys: def test_empty(self): @@ -367,8 +571,8 @@ class TestValidateProvider: def test_valid_code(self): assert _validate_provider(CODE_PROVIDER)["ok"] is True - def test_valid_npx(self): - assert _validate_provider(NPX_PROVIDER)["ok"] is True + def test_valid_package(self): + assert _validate_provider(PACKAGE_PROVIDER)["ok"] is True def test_no_tools(self): assert not _validate_provider({**CODE_PROVIDER, "tools": []})["ok"] @@ -376,8 +580,22 @@ def test_no_tools(self): def test_code_required_for_code_type(self): assert not _validate_provider({**CODE_PROVIDER, "code": ""})["ok"] - def test_command_required_for_npx_type(self): - assert not _validate_provider({**NPX_PROVIDER, "npx_command": ""})["ok"] + def test_command_required_for_package_type(self): + assert not _validate_provider({**PACKAGE_PROVIDER, "command": ""})["ok"] + + def test_requirements_must_be_list_if_present(self): + p = {**CODE_PROVIDER, "requirements": "httpx"} # string, not list + assert not _validate_provider(p)["ok"] + + def test_setup_commands_must_be_list_if_present(self): + p = {**CODE_PROVIDER, "setup_commands": "echo hi"} # string, not list + assert not _validate_provider(p)["ok"] + + def test_empty_requirements_list_is_valid(self): + assert _validate_provider({**CODE_PROVIDER, "requirements": []})["ok"] is True + + def test_non_empty_requirements_list_is_valid(self): + assert _validate_provider({**CODE_PROVIDER, "requirements": ["httpx"]})["ok"] is True class TestExtractFunctionsPure: @@ -392,35 +610,24 @@ def test_syntax_error(self): assert not r["ok"] -# --------------------------------------------------------------------------- -# POST /api/run-command -# --------------------------------------------------------------------------- +class TestDetectPackageManager: + def test_npx(self): + assert _detect_package_manager("npx @playwright/mcp@latest") == "npx" -class TestRunCommand: - def test_missing_command_400(self, client): - r = client.post("/api/run-command", json={}) - assert r.status_code == 400 + def test_uvx(self): + assert _detect_package_manager("uvx mcp-server-fetch") == "uvx" + + def test_python(self): + assert _detect_package_manager("python -m mcp_server_github") == "pip" + + def test_python3(self): + assert _detect_package_manager("python3 -m something") == "pip" + + def test_npm(self): + assert _detect_package_manager("npm run serve") == "npm" + + def test_installed_binary(self): + assert _detect_package_manager("mcp-server-github") == "command" - def test_simple_command_streams_sse(self, client): - """A fast echo command should return SSE with a 'done' event.""" - with client.stream("POST", "/api/run-command", json={"command": "echo hello"}) as r: - assert r.status_code == 200 - assert "text/event-stream" in r.headers["content-type"] - text = r.read().decode() - # Should have at least one data line and a done event - assert "hello" in text - assert '"done": true' in text or '"done":true' in text - - def test_exit_code_nonzero(self, client): - """A failing command should report a non-zero returncode.""" - with client.stream("POST", "/api/run-command", json={"command": "exit 42"}) as r: - text = r.read().decode() - import json as _json - events = [ - _json.loads(line[6:]) - for line in text.splitlines() - if line.startswith("data: ") - ] - done_events = [e for e in events if e.get("done")] - assert done_events, "Expected a done event" - assert done_events[-1]["returncode"] != 0 + def test_empty_string(self): + assert _detect_package_manager("") == "command" diff --git a/tests/test_server.py b/tests/test_server.py index d8b3995..e6c1a9c 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -1,30 +1,33 @@ """Unit tests for server.py pure helper functions. -Note: server.py has module-level side effects (load_provider_specs + register_provider). -conftest.py sets MCP_TOOL_CONFIG_DIR to an empty temp dir before import so zero tools -are registered and no npx processes are started. +Note: server.py has module-level side effects (load_provider_specs + register_provider + +run_provider_setup). conftest.py sets MCP_TOOL_CONFIG_DIR to an empty temp dir before +import so zero tools are registered and no processes are started. """ import inspect import os import textwrap from pathlib import Path from typing import Any -from unittest.mock import AsyncMock, MagicMock, patch +from unittest.mock import AsyncMock, MagicMock, call, patch import pytest import yaml -# server.py has module-level side effects (load_provider_specs + register_provider). -# conftest.py has already set MCP_TOOL_CONFIG_DIR to an empty temp dir, so the -# import is safe and results in zero tools being registered. +# server.py has module-level side effects (load_provider_specs + register_provider + +# run_provider_setup). conftest.py has already set MCP_TOOL_CONFIG_DIR to an empty +# temp dir, so the import is safe and results in zero tools being registered. from server import ( + SUBPROCESS_KEYS, _build_typed_signature, + _get_package_command, build_runtime_context, exec_provider_code, load_provider_specs, redact_secrets, register_tool, resolve_env_defaults, + run_provider_setup, ) @@ -257,7 +260,6 @@ def test_handler_called_with_context_and_kwargs(self): "required": ["msg"], }, } - # We patch mcp.tool to avoid touching the real FastMCP registry with patch("server.mcp") as mock_mcp: mock_mcp.tool.return_value = lambda fn: fn register_tool(tool_spec, handler) @@ -328,3 +330,119 @@ def decorator(fn): assert result["ok"] is False assert "something broke" in result["error"] assert result["tool"] == "bad_tool" + + +# --------------------------------------------------------------------------- +# _get_package_command +# --------------------------------------------------------------------------- + +class TestGetPackageCommand: + def test_package_key_returns_command(self): + spec = {"package": {"command": "npx @playwright/mcp@latest --isolated"}} + assert _get_package_command(spec) == "npx @playwright/mcp@latest --isolated" + + def test_npx_key_backward_compat(self): + """Legacy 'npx:' key must still be recognised.""" + spec = {"npx": {"command": "npx @playwright/mcp@latest"}} + assert _get_package_command(spec) == "npx @playwright/mcp@latest" + + def test_package_key_takes_priority_over_npx(self): + spec = { + "package": {"command": "uvx mcp-server-fetch"}, + "npx": {"command": "npx something"}, + } + assert _get_package_command(spec) == "uvx mcp-server-fetch" + + def test_uvx_command(self): + spec = {"package": {"command": "uvx mcp-server-fetch"}} + assert _get_package_command(spec) == "uvx mcp-server-fetch" + + def test_python_module_command(self): + spec = {"package": {"command": "python -m mcp_server_github"}} + assert _get_package_command(spec) == "python -m mcp_server_github" + + def test_installed_binary_command(self): + spec = {"package": {"command": "mcp-server-github"}} + assert _get_package_command(spec) == "mcp-server-github" + + def test_code_provider_returns_none(self): + spec = {"code": "async def f(ctx): pass", "tools": []} + assert _get_package_command(spec) is None + + def test_empty_spec_returns_none(self): + assert _get_package_command({}) is None + + def test_missing_command_field_returns_none(self): + spec = {"package": {}} + assert _get_package_command(spec) is None + + def test_empty_command_field_returns_none(self): + spec = {"package": {"command": " "}} + assert _get_package_command(spec) is None + + def test_subprocess_keys_constant_includes_both(self): + assert "package" in SUBPROCESS_KEYS + assert "npx" in SUBPROCESS_KEYS + + +# --------------------------------------------------------------------------- +# run_provider_setup +# --------------------------------------------------------------------------- + +class TestRunProviderSetup: + def test_empty_spec_no_subprocess_calls(self): + with patch("server.subprocess.run") as mock_run: + run_provider_setup({}) + mock_run.assert_not_called() + + def test_installs_requirements(self): + spec = {"requirements": ["httpx", "requests"]} + with patch("server.subprocess.run") as mock_run: + run_provider_setup(spec) + calls = mock_run.call_args_list + assert len(calls) == 2 + # Both calls should be pip install + for c in calls: + args = c[0][0] + assert "-m" in args + assert "pip" in args + assert "install" in args + + def test_installs_correct_package_names(self): + spec = {"requirements": ["httpx==0.27.0", "requests"]} + with patch("server.subprocess.run") as mock_run: + run_provider_setup(spec) + calls = mock_run.call_args_list + assert calls[0][0][0][-1] == "httpx==0.27.0" + assert calls[1][0][0][-1] == "requests" + + def test_runs_setup_commands(self): + spec = {"setup_commands": ["npx playwright install chrome"]} + with patch("server.subprocess.run") as mock_run: + run_provider_setup(spec) + calls = mock_run.call_args_list + assert len(calls) == 1 + args = calls[0][0][0] + assert args == ["npx", "playwright", "install", "chrome"] + + def test_runs_both_requirements_and_setup_commands(self): + spec = { + "requirements": ["httpx"], + "setup_commands": ["echo hello"], + } + with patch("server.subprocess.run") as mock_run: + run_provider_setup(spec) + assert mock_run.call_count == 2 + + def test_skips_empty_strings(self): + spec = {"requirements": ["", "httpx", ""], "setup_commands": ["", "echo hi"]} + with patch("server.subprocess.run") as mock_run: + run_provider_setup(spec) + assert mock_run.call_count == 2 # only "httpx" + "echo hi" + + def test_check_true_passed_to_subprocess(self): + spec = {"requirements": ["httpx"]} + with patch("server.subprocess.run") as mock_run: + run_provider_setup(spec) + _, kwargs = mock_run.call_args + assert kwargs.get("check") is True From c3e1666536c3a57b05112b111514d921c6ed8596 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 25 May 2026 02:23:10 +0000 Subject: [PATCH 2/2] =?UTF-8?q?Remove=20npx:=20backward=20compat=20?= =?UTF-8?q?=E2=80=94=20package:=20is=20the=20sole=20YAML=20key?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - SUBPROCESS_KEYS reduced to ('package',); npx: key no longer read - _get_package_command() simplified to a direct spec.get('package') lookup - _get_package_spec() in frontend/app.py simplified to spec.get('package') - Removed is_npx alias from GET /api/tools list response - Removed is_npx || is_package JS fallback in provider list rendering - Removed all backward-compat test cases for npx: key - Updated SUBPROCESS_KEYS test to assert ('package',) exactly - Removed backward-compat notes from README - 144/144 tests pass https://claude.ai/code/session_016EMphWWMguzwVrfAYvJFSX --- README.md | 6 ------ frontend/app.py | 15 +++------------ server.py | 18 +++++------------- tests/test_frontend.py | 39 --------------------------------------- tests/test_server.py | 17 ++--------------- 5 files changed, 10 insertions(+), 85 deletions(-) diff --git a/README.md b/README.md index 5fa9697..0bdc785 100755 --- a/README.md +++ b/README.md @@ -694,9 +694,6 @@ tools: [] The server spawns the process, performs the MCP handshake once, then forwards every tool call to it. The process is reused across calls (started lazily on the first tool call). -**Backward compatibility:** The legacy `npx:` key still works and is treated identically -to `package:`. Saving the provider through the UI rewrites it to `package:` format. - ### pip Requirements vs setup_commands | Feature | Use for | @@ -803,6 +800,3 @@ tools: any_key: any_value ``` -> **Legacy `npx:` key:** still accepted for backward compatibility. Existing configs -> with `npx: {command: ...}` continue to work unchanged; saving through the UI rewrites -> them to `package:` format. diff --git a/frontend/app.py b/frontend/app.py index f63fd0a..75fe595 100644 --- a/frontend/app.py +++ b/frontend/app.py @@ -111,17 +111,9 @@ def _detect_package_manager(command: str) -> str: # Structured ↔ YAML conversion # --------------------------------------------------------------------------- -# Legacy "npx:" key is kept for backward compatibility. -_SUBPROCESS_KEYS = ("package", "npx") - - def _get_package_spec(spec: dict[str, Any]) -> dict[str, Any] | None: - """Return the subprocess sub-dict (package: or npx:), or None for code providers.""" - for key in _SUBPROCESS_KEYS: - sub = spec.get(key) - if sub: - return sub - return None + """Return the subprocess sub-dict (package:), or None for code providers.""" + return spec.get("package") or None def _provider_to_structured(name: str, spec: dict[str, Any]) -> dict[str, Any]: @@ -341,7 +333,6 @@ async def list_tools() -> list[dict]: "tool_names": [t.get("name") for t in tool_entries], "provider_type": structured["type"], "is_package": is_package, - "is_npx": is_package, # backward-compat alias "secret_keys": secret_keys, "missing_secrets": missing_secrets, "validation_errors": validation["errors"], @@ -924,7 +915,7 @@ async def index(): const alertRow = (warnBadge || errBadge) ? `
${warnBadge}${errBadge}
` : ''; - const isPkg = p.is_package || p.is_npx; + const isPkg = p.is_package; return `
diff --git a/server.py b/server.py index 6218964..bb63161 100755 --- a/server.py +++ b/server.py @@ -70,9 +70,7 @@ "array": list, } -# Keys that indicate a subprocess/package provider (in priority order). -# "npx" is kept for backward compatibility with existing YAML files. -SUBPROCESS_KEYS = ("package", "npx") +SUBPROCESS_KEYS = ("package",) # --------------------------------------------------------------------------- @@ -227,16 +225,10 @@ async def dynamic_tool(ctx: Context, **kwargs: Any) -> Any: def _get_package_command(spec: dict[str, Any]) -> str | None: - """Return the spawn command for package providers, or None for code providers. - - Checks ``package:`` first, then ``npx:`` for backward compatibility with - existing YAML configs that used the old key name. - """ - for key in SUBPROCESS_KEYS: - sub = spec.get(key) - if sub: - command = (sub.get("command") or "").strip() - return command or None + """Return the spawn command for package providers, or None for code providers.""" + sub = spec.get("package") + if sub: + return (sub.get("command") or "").strip() or None return None diff --git a/tests/test_frontend.py b/tests/test_frontend.py index 6f1a1f2..c86fa00 100644 --- a/tests/test_frontend.py +++ b/tests/test_frontend.py @@ -104,23 +104,6 @@ def test_lists_package_provider(self, app, tools_dir): assert data[0]["is_package"] is True assert data[0]["provider_type"] == "package" - def test_is_npx_backward_compat_alias(self, app, tools_dir): - """is_npx must still be present and equal is_package.""" - content = _structured_to_yaml(PACKAGE_PROVIDER) - (tools_dir / "playwright.yaml").write_text(content) - r = TestClient(app).get("/api/tools") - d = r.json()[0] - assert d["is_npx"] == d["is_package"] - - def test_legacy_npx_key_listed_as_package(self, app, tools_dir): - """A YAML with the legacy 'npx:' key is shown as a package provider.""" - spec = { - "npx": {"command": "npx @playwright/mcp@latest"}, - "tools": [{"name": "nav", "description": "Navigate", "input_schema": {"type": "object", "properties": {}, "required": []}}], - } - (tools_dir / "legacy.yaml").write_text(yaml.dump(spec)) - r = TestClient(app).get("/api/tools") - assert r.json()[0]["is_package"] is True # --------------------------------------------------------------------------- @@ -148,18 +131,6 @@ def test_package_provider(self, app, tools_dir): assert "command" in data assert data["command"] == "npx @playwright/mcp@latest --isolated" - def test_legacy_npx_key_returns_package_type(self, app, tools_dir): - """GET on a YAML with legacy 'npx:' key returns type='package'.""" - spec = { - "npx": {"command": "npx @playwright/mcp@latest"}, - "tools": [{"name": "nav", "description": "Nav", "input_schema": {"type": "object", "properties": {}, "required": []}}], - } - (tools_dir / "legacy.yaml").write_text(yaml.dump(spec)) - r = TestClient(app).get("/api/tools/legacy") - data = r.json() - assert data["type"] == "package" - assert data["command"] == "npx @playwright/mcp@latest" - def test_requirements_and_setup_commands_returned(self, app, tools_dir): provider = { **CODE_PROVIDER, @@ -494,16 +465,6 @@ def test_package_yaml_uses_package_key_not_npx(self): assert "package" in spec assert "npx" not in spec - def test_legacy_npx_key_reads_as_package(self): - """Loading a YAML with the old 'npx:' key returns type='package'.""" - spec = { - "npx": {"command": "npx @playwright/mcp@latest"}, - "tools": [], - } - structured = _provider_to_structured("legacy", spec) - assert structured["type"] == "package" - assert structured["command"] == "npx @playwright/mcp@latest" - def test_parameters_preserved(self): yaml_str = _structured_to_yaml(PACKAGE_PROVIDER) spec = yaml.safe_load(yaml_str) diff --git a/tests/test_server.py b/tests/test_server.py index e6c1a9c..d86ba68 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -341,18 +341,6 @@ def test_package_key_returns_command(self): spec = {"package": {"command": "npx @playwright/mcp@latest --isolated"}} assert _get_package_command(spec) == "npx @playwright/mcp@latest --isolated" - def test_npx_key_backward_compat(self): - """Legacy 'npx:' key must still be recognised.""" - spec = {"npx": {"command": "npx @playwright/mcp@latest"}} - assert _get_package_command(spec) == "npx @playwright/mcp@latest" - - def test_package_key_takes_priority_over_npx(self): - spec = { - "package": {"command": "uvx mcp-server-fetch"}, - "npx": {"command": "npx something"}, - } - assert _get_package_command(spec) == "uvx mcp-server-fetch" - def test_uvx_command(self): spec = {"package": {"command": "uvx mcp-server-fetch"}} assert _get_package_command(spec) == "uvx mcp-server-fetch" @@ -380,9 +368,8 @@ def test_empty_command_field_returns_none(self): spec = {"package": {"command": " "}} assert _get_package_command(spec) is None - def test_subprocess_keys_constant_includes_both(self): - assert "package" in SUBPROCESS_KEYS - assert "npx" in SUBPROCESS_KEYS + def test_subprocess_keys_constant(self): + assert SUBPROCESS_KEYS == ("package",) # ---------------------------------------------------------------------------