From 23e8f6aff67f8b1a73f8ef0f4d3ce9523420602d Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 25 May 2026 03:34:31 +0000 Subject: [PATCH] frontend: auto-discover tools, enable/disable switch, namespaced tool names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The UI no longer requires manually clicking Introspect / Analyze — both run automatically (introspect on provider open / package-command blur; analyze on every code edit, debounced). When discovery succeeds, the tool-name and function-name fields become a dropdown of legal values plus an "Other…" option for free-form entry. Failure is silent: the dropdown only shows "Other…" so users can always type a custom value, matching the previous behaviour. Each tool now has a per-card enable/disable switch persisted to YAML as `enabled: true|false` (always written; absent means true for backward compatibility with pre-existing YAML files). Disabled tools are kept in the file but skipped during MCP registration, so they never reach the LLM. Every tool is now advertised to MCP clients as `__`. The provider name is the YAML filename normalized to `[a-zA-Z0-9-]`. This makes tool-name collisions across providers impossible. Built-in tools follow the same convention: `mcpproxy__listfiles` and `mcpproxy__getfile`. The upstream tool name used when proxying to package subprocesses is still the unprefixed name from the YAML. Tests, README, and the integration shell script are updated accordingly; existing YAML files require no migration. https://claude.ai/code/session_01KRZQTKxffN7toaSDh71D6e --- README.md | 57 +++++--- builtin_tools.py | 4 +- config.py | 2 +- frontend/app.py | 253 ++++++++++++++++++++++++++++++++---- server.py | 95 +++++++++++--- tests/test_builtin_tools.py | 2 +- tests/test_frontend.py | 71 +++++++++- tests/test_mcp_client.sh | 6 +- tests/test_server.py | 201 +++++++++++++++++++++++++++- 9 files changed, 620 insertions(+), 71 deletions(-) diff --git a/README.md b/README.md index 9f597be..977b7f2 100755 --- a/README.md +++ b/README.md @@ -18,11 +18,24 @@ Each tool **provider** is a single YAML file under `tools/`. The YAML contains: runs `setup_commands`, then registers each tool automatically — no Python files to maintain separately, no changes to `server.py` needed when adding new tools. -Two **built-in tools** (`mcpproxy-listfiles` and `mcpproxy-getfile`) are always registered +Two **built-in tools** (`mcpproxy__listfiles` and `mcpproxy__getfile`) are always registered without any YAML config. They give LLMs read-only access to a configurable directory (default: `.playwright-mcp`) — useful for retrieving screenshots and snapshots produced by package providers such as Playwright MCP. +## Tool names advertised to the LLM + +Every tool is advertised to MCP clients as **`__`** — the provider +name (the YAML filename without the `.yaml` extension, normalized to `[a-zA-Z0-9-]`) +joined to the tool's own `name` by a double underscore. For example, a YAML file +`tools/playwright.yaml` declaring a tool `browser_navigate` is exposed to the LLM +as `playwright__browser_navigate`. This guarantees that tools from different +providers cannot collide, even if they happen to share a name. + +The two built-in tools follow the same convention: `mcpproxy__listfiles` and +`mcpproxy__getfile`. The `name:` field in your YAML stays unprefixed — the prefix +is added automatically when the tool is registered. + ## Ports | Port | Service | @@ -43,7 +56,7 @@ by package providers such as Playwright MCP. ├── server.py ├── config.py ← shared env-var config (imported by all modules) ├── process_runner.py ← spawns & proxies any stdio MCP subprocess -├── builtin_tools.py ← built-in mcpproxy-listfiles / mcpproxy-getfile tools +├── builtin_tools.py ← built-in mcpproxy__listfiles / mcpproxy__getfile tools ├── frontend/ │ └── app.py ← FastAPI UI server (port 8889) ├── .env.example @@ -72,6 +85,16 @@ Open **`http://localhost:8889`** in your browser after starting the server. - Click any provider to open its fields in a form editor - Edit documentation, code, and per-tool fields (name, description, parameters) - Add or remove tools with the **+ Add Tool** / **✕** buttons +- **Enable / disable** individual tools with the switch in each tool card's header. + A disabled tool is kept in YAML (as `enabled: false`) but not registered with MCP + and not shown to the LLM — toggle it back on later without re-typing the schema. +- **Function / Tool-name menu** — when mcpproxy can discover the legal set of names + (`async def` symbols in your code, or `tools/list` returned by a package's + stdio server), the field becomes a dropdown plus an **Other…** option so you can + pick from the menu or type a custom value. Discovery runs automatically when you + open a provider, when the code changes, and when the package command field loses + focus; the **↻ Re-scan** button forces a refresh. Failure is silent — the + dropdown just falls back to "Other…" so you can always free-type. - **Save** — write the file; restart MCP server to reload - **🔑 Secrets** — manage `.env` values for secrets declared in this provider - **Delete** — remove the provider YAML @@ -82,8 +105,8 @@ Click **+ New Provider** and choose a provider type: | Type | Description | |---|---| -| **Python code** | Write `async def` functions; declare tools that reference them | -| **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 | +| **Python code** | Write `async def` functions; the UI lists the ones it finds as you type. Each becomes a tool entry. | +| **Package** | Enter any command that launches a stdio MCP server (`npx`, `uvx`, `python -m`, or an installed binary). When you click **Next**, mcpproxy auto-introspects the command and pre-populates the tool list; if introspection fails you can still proceed and add tools by hand. | 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`. @@ -592,9 +615,10 @@ tools: ``` ```bash +# The provider file is tools/ping.yaml, so the advertised tool name is "ping__ping". curl -s -X POST http://localhost:8888/mcp \ -H "Content-Type: application/json" \ - -d '{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"ping","arguments":{"message":"world"}}}' + -d '{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"ping__ping","arguments":{"message":"world"}}}' ``` --- @@ -652,8 +676,8 @@ 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 + # Populated automatically when the wizard introspects the command — or fill manually + - name: browser_navigate # advertised as playwright__browser_navigate description: Navigate to a URL in a browser. input_schema: type: object @@ -669,7 +693,7 @@ tools: package: command: uvx mcp-server-fetch -tools: [] # auto-populated by Introspect +tools: [] # auto-populated by the wizard's introspection step ``` ```yaml @@ -766,8 +790,8 @@ config file required: | Tool | Description | |---|---| -| `mcpproxy-listfiles` | List files and subdirectories inside the files base directory | -| `mcpproxy-getfile` | Read a file from the files base directory (UTF-8 text or base64) | +| `mcpproxy__listfiles` | List files and subdirectories inside the files base directory | +| `mcpproxy__getfile` | Read a file from the files base directory (UTF-8 text or base64) | **Default base directory:** `.playwright-mcp` relative to the server's working directory (i.e. `/app/.playwright-mcp` inside Docker). Override with the `MCPPROXY_FILES_DIR` @@ -780,11 +804,11 @@ Only files **inside** the base directory are accessible — path-traversal attem 1. Ask the LLM to navigate to a page and take a screenshot via the Playwright MCP provider. 2. Playwright writes `screenshot.png` to `.playwright-mcp/`. -3. Ask the LLM to call `mcpproxy-listfiles` — it returns the file list. -4. Ask the LLM to call `mcpproxy-getfile` with `path="screenshot.png"` — it returns the +3. Ask the LLM to call `mcpproxy__listfiles` — it returns the file list. +4. Ask the LLM to call `mcpproxy__getfile` with `path="screenshot.png"` — it returns the PNG as a base64 string that the LLM can describe or pass to a vision model. -#### `mcpproxy-listfiles` parameters +#### `mcpproxy__listfiles` parameters | Parameter | Type | Required | Default | Description | |---|---|---|---|---| @@ -792,7 +816,7 @@ Only files **inside** the base directory are accessible — path-traversal attem Returns an object with `ok`, `base_dir`, `path`, and `entries` (list of `{name, type, size}`). -#### `mcpproxy-getfile` parameters +#### `mcpproxy__getfile` parameters | Parameter | Type | Required | Default | Description | |---|---|---|---|---| @@ -850,9 +874,12 @@ setup_commands: # shell commands run on every server startup # ── Tool declarations (required) ────────────────────────────────────────────── tools: - - name: string # unique MCP tool name + - name: string # tool name as written here; the LLM sees + # "__" (e.g. playwright__browser_navigate) function: string # async function name from code block (code providers only) description: string # shown to the LLM + enabled: true # optional (default true); set false to keep the tool + # in YAML but not advertise / register it documentation: string # optional per-tool notes shown in the web UI input_schema: # JSON Schema type: object diff --git a/builtin_tools.py b/builtin_tools.py index c1b30a8..14d76c9 100644 --- a/builtin_tools.py +++ b/builtin_tools.py @@ -2,8 +2,8 @@ Built-in mcpproxy utility tools — registered automatically at startup, no YAML config file required. - mcpproxy-listfiles List files/directories inside the mcpproxy files dir. - mcpproxy-getfile Read a file from the mcpproxy files dir (text or base64). + mcpproxy__listfiles List files/directories inside the mcpproxy files dir. + mcpproxy__getfile Read a file from the mcpproxy files dir (text or base64). The *base directory* defaults to ``.playwright-mcp`` (relative to the server's working directory) and can be overridden at runtime with the diff --git a/config.py b/config.py index 6d0cdb4..4a57008 100644 --- a/config.py +++ b/config.py @@ -6,7 +6,7 @@ ENV_FILE = Path(os.environ.get("MCP_ENV_FILE", ".env")) SERVER_NAME = os.environ.get("MCP_SERVER_NAME", "local-config-driven-mcp") -# Base directory exposed by the built-in mcpproxy-listfiles / mcpproxy-getfile tools. +# Base directory exposed by the built-in mcpproxy__listfiles / mcpproxy__getfile tools. # Defaults to .playwright-mcp (relative to the server's working directory) so that # screenshots and snapshots produced by the Playwright MCP package provider are # immediately accessible. Override with MCPPROXY_FILES_DIR. diff --git a/frontend/app.py b/frontend/app.py index 75fe595..03eb0e6 100644 --- a/frontend/app.py +++ b/frontend/app.py @@ -140,6 +140,7 @@ def _provider_to_structured(name: str, spec: dict[str, Any]) -> dict[str, Any]: "function": t.get("function", ""), "description": t.get("description", ""), "documentation": t.get("documentation", ""), + "enabled": False if t.get("enabled") is False else True, "parameters": params, "secrets": secrets, }) @@ -207,6 +208,7 @@ def _structured_to_yaml(provider: dict[str, Any]) -> str: tool_entry: dict[str, Any] = { "name": t["name"], "description": t.get("description", ""), + "enabled": False if t.get("enabled") is False else True, "input_schema": {"type": "object", "properties": props, "required": required}, } if ptype == "code": @@ -572,6 +574,18 @@ async def index(): .req-no{background:#45475a;color:#cdd6f4} .badge-warn{background:var(--yellow);color:#1e1e2e;font-size:.62em;padding:2px 6px;border-radius:3px;font-weight:700} .badge-err{background:var(--red);color:#1e1e2e;font-size:.62em;padding:2px 6px;border-radius:3px;font-weight:700} +.badge-disabled{background:#45475a;color:var(--muted);font-size:.62em;padding:2px 6px;border-radius:3px;font-weight:700;text-transform:uppercase;letter-spacing:.4px} +.tool-card.disabled .tool-card-body{opacity:.55;filter:saturate(.6)} +.tool-card.disabled .tool-card-header{background:#1f1f2c} +.fn-pick-row{display:flex;gap:6px;align-items:stretch} +.fn-pick-row .form-select{max-width:220px;flex:0 0 auto} +.fn-pick-row .form-control{flex:1 1 auto;min-width:0} +.fn-status{font-size:.75em;color:var(--muted);margin-top:4px} +.fn-status.error{color:var(--red)} +.fn-status.ok{color:var(--green)} +.fn-status.busy{color:var(--accent)} +.form-check-input{background-color:#1e1e2e;border-color:#45475a} +.form-check-input:checked{background-color:var(--accent);border-color:var(--accent)} @@ -658,7 +672,8 @@ async def index():
📦 Package Command
+ style="font-size:.875em" + onblur="discoverFunctions().catch(() => {})">
Any command that spawns a stdio MCP server: npx, uvx, python -m, or an installed binary. The process is started on demand and kept alive between calls.
@@ -692,9 +707,13 @@ async def index():
- 🔧 Tools - + 🔧 Tools +
+ + +
+
No tools yet — click + Add Tool.
@@ -785,7 +804,7 @@ async def index():
Shell commands run on every server startup (e.g. npx playwright install chrome).
- +
When you click Next the command is introspected automatically; tools it advertises become the dropdown options in the editor. If introspection fails you can still proceed and add tools by hand.
@@ -810,7 +829,7 @@ async def index():
- +
Functions are detected automatically as you type. Each async def fn(context, …) becomes a tool entry.
@@ -847,6 +866,13 @@ async def index(): let wzStep = 'type'; let wzIntrospectedTools = []; // tools returned by introspect let providersMeta = {}; // name → {missing_secrets, validation_errors} +let knownFunctions = []; // names available in the current provider: + // code provider → async def names in the code block + // package provider → tool names from upstream introspection +let knownFnStatus = 'idle'; // 'idle' | 'busy' | 'ok' | 'error' +let knownFnMessage = ''; // human-readable status text +let _analyzeDebounce = null; +let _wzAnalyzeDebounce = null; // ───────────────────────────────────────────────────────────────────────────── // Boot @@ -856,8 +882,18 @@ async def index(): mode: 'python', theme: 'dracula', lineNumbers: true, indentWithTabs: false, indentUnit: 4, tabSize: 4, }); + codeEditor.on('change', () => { + if (!currentProvider || currentProvider.type !== 'code') return; + clearTimeout(_analyzeDebounce); + _analyzeDebounce = setTimeout(() => discoverFunctions().catch(() => {}), 300); + }); secretsModal = new bootstrap.Modal('#secrets-modal'); wizModal = new bootstrap.Modal('#wizard-modal'); + // Wizard: live function detection as the user types into the code textarea + document.getElementById('wz-code-input').addEventListener('input', () => { + clearTimeout(_wzAnalyzeDebounce); + _wzAnalyzeDebounce = setTimeout(() => wzAnalyze().catch(() => {}), 300); + }); loadList(); }); @@ -944,6 +980,9 @@ async def index(): const p = await api('GET', `/api/tools/${name}`); currentName = name; currentProvider = p; + knownFunctions = []; + knownFnStatus = 'idle'; + knownFnMessage = ''; renderProvider(p); document.getElementById('empty-panel').style.display = 'none'; document.getElementById('editor-panel').style.display = 'block'; @@ -953,9 +992,83 @@ async def index(): document.querySelectorAll('.provider-item').forEach(el => { el.classList.toggle('active', el.querySelector('.fw-semibold')?.textContent === name); }); + // Kick off auto-discovery in the background — silent on failure. + discoverFunctions().catch(() => {}); } catch(e) { toast(e.message, false); } } +// Auto-discover the set of legal function/tool names for the current provider. +// Code providers: parse the code for async def names. +// Package providers: spawn the command and ask it for tools/list. +// Failure (or absence of input) is silent — the editor falls back to free-form text. +async function discoverFunctions() { + if (!currentProvider) return; + const isPkg = currentProvider.type === 'package'; + knownFnStatus = 'busy'; + knownFnMessage = isPkg ? 'Introspecting package…' : 'Analyzing code…'; + _renderKnownFnStatus(); + try { + if (isPkg) { + const cmd = (document.getElementById('f-command').value || '').trim(); + if (!cmd) { + knownFunctions = []; knownFnStatus = 'idle'; knownFnMessage = ''; + _renderKnownFnStatus(); _refreshToolDropdowns(); return; + } + const r = await api('POST', '/api/introspect', { + command: cmd, + requirements: currentProvider.requirements || [], + setup_commands: currentProvider.setup_commands || [], + }); + if (!r.ok) throw new Error(r.error || 'introspection failed'); + knownFunctions = (r.tools || []).map(t => t.name).filter(Boolean); + knownFnStatus = 'ok'; + knownFnMessage = `Found ${knownFunctions.length} tool${knownFunctions.length === 1 ? '' : 's'}`; + } else { + const code = codeEditor.getValue(); + const r = await api('POST', '/api/extract-functions', {code}); + if (!r.ok) throw new Error(r.error || 'parse error'); + knownFunctions = (r.functions || []).map(f => f.name).filter(Boolean); + knownFnStatus = 'ok'; + knownFnMessage = `Found ${knownFunctions.length} function${knownFunctions.length === 1 ? '' : 's'}`; + } + } catch(e) { + knownFunctions = []; + knownFnStatus = 'error'; + knownFnMessage = e.message || 'discovery failed'; + } + _renderKnownFnStatus(); + _refreshToolDropdowns(); +} + +function _renderKnownFnStatus() { + const el = document.getElementById('fn-discovery-status'); + if (!el) return; + el.className = `fn-status ${knownFnStatus}`; + el.textContent = knownFnMessage || ''; +} + +// Re-render every tool dropdown without rebuilding the entire form (which would +// reset focus / scroll position). Each dropdown's inner option list is replaced. +function _refreshToolDropdowns() { + if (!currentProvider) return; + const isPkg = currentProvider.type === 'package'; + const field = isPkg ? 'name' : 'function'; + (currentProvider.tools || []).forEach((t, i) => { + const sel = document.getElementById(`fn-pick-${i}`); + if (!sel) return; + sel.innerHTML = _fnPickOptionsHtml(t[field] || ''); + }); +} + +function _fnPickOptionsHtml(currentValue) { + const opts = [``]; + for (const fn of knownFunctions) { + opts.push(``); + } + opts.push(``); + return opts.join(''); +} + function _refreshEditorBars(name) { const meta = providersMeta[name] || {}; const missing = meta.missing_secrets || []; @@ -1074,6 +1187,7 @@ async def index(): } function renderToolCard(t, i, isPkg) { + const enabled = t.enabled !== false; const params = (t.parameters || []).map((p, j) => `
`).join(''); + // Function/name picker: a dropdown of known names + "Other…" alongside the + // text input. Picking a menu option fills the input; the input is the + // source of truth so users can always free-type when "Other…" is chosen + // or the upstream list is unknown. + const nameField = ` +
+ +
+ ${isPkg ? ` + ` : ''} + +
+
`; + const fnField = isPkg ? '' : `
- +
+ + +
`; const docField = ` @@ -1117,9 +1257,16 @@ async def index(): `; return ` -
+
- ${esc(t.name||'(unnamed)')} +
+ + ${esc(t.name||'(unnamed)')} + disabled +
@@ -1128,9 +1275,7 @@ async def index():
- - + ${nameField}
@@ -1150,6 +1295,44 @@ async def index():
`; } +// Dropdown picked — fill the matching text input. An empty value ("— pick +// from menu —") leaves the input alone; "__other__" clears it and focuses it. +function onFnPick(i, field, value) { + ensureProvider(); + if (value === '') return; + if (value === '__other__') { + const input = document.getElementById(`tool-${field}-input-${i}`); + if (input) { input.value = ''; input.focus(); } + updateTool(i, field, ''); + if (field === 'name') document.getElementById(`tool-label-${i}`).textContent = '(unnamed)'; + return; + } + const input = document.getElementById(`tool-${field}-input-${i}`); + if (input) input.value = value; + updateTool(i, field, value); + if (field === 'name') document.getElementById(`tool-label-${i}`).textContent = value; +} + +// Keep the dropdown's selection in sync when the user types in the input. +function _syncFnPick(i, field, value) { + const sel = document.getElementById(`fn-pick-${i}`); + if (!sel) return; + if (knownFunctions.includes(value)) { + sel.value = value; + } else { + sel.value = ''; // "— pick from menu —" + } +} + +function setToolEnabled(i, enabled) { + ensureProvider(); + currentProvider.tools[i].enabled = !!enabled; + const card = document.getElementById(`tool-card-${i}`); + if (card) card.classList.toggle('disabled', !enabled); + const badge = document.getElementById(`tool-disabled-badge-${i}`); + if (badge) badge.style.display = enabled ? 'none' : ''; +} + function toggleToolCard(i) { const body = document.getElementById(`tool-body-${i}`); body.style.display = body.style.display === 'none' ? '' : 'none'; @@ -1196,7 +1379,7 @@ async def index(): const isPkg = currentProvider.type === 'package'; currentProvider.tools.push({ name: '', function: '', description: '', documentation: '', - parameters: [], secrets: [], + enabled: true, parameters: [], secrets: [], }); renderTools(currentProvider.tools, isPkg); } @@ -1385,7 +1568,18 @@ async def index(): const cmd = document.getElementById('wz-pkg-cmd').value.trim(); if (!name) { errEl.textContent = 'Provider name is required.'; return; } if (!cmd) { errEl.textContent = 'Command is required.'; return; } - if (!wzIntrospectedTools.length) { errEl.textContent = 'Click "Introspect Tools" first.'; return; } + // Auto-introspect now (silent fallback on failure — the user can still + // proceed and add tools by hand in the editor). + const nextBtn = document.getElementById('wz-next-btn'); + nextBtn.disabled = true; + const origText = nextBtn.textContent; + nextBtn.textContent = '⏳ Introspecting…'; + try { + await wzIntrospect(); + } finally { + nextBtn.disabled = false; + nextBtn.textContent = origText; + } const requirements = _wzGetListValues('wz-pkg-reqs-container'); const setup_commands = _wzGetListValues('wz-pkg-cmds-container'); const provider = { @@ -1396,6 +1590,7 @@ async def index(): function: '', description: t.description || '', documentation: '', + enabled: true, parameters: _schemaToParams(t.inputSchema || t.input_schema || {}), secrets: [], })), @@ -1424,6 +1619,7 @@ async def index(): name: fn.name, function: fn.name, description: `TODO — describe what ${fn.name} does`, documentation: '', + enabled: true, parameters: fn.params.map(p => ({name:p.name,type:p.type||'string',description:'',required:true,default:null})), secrets: [], })), @@ -1450,34 +1646,39 @@ async def index(): async function wzIntrospect() { 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; } + if (!cmd) { + wzIntrospectedTools = []; + el.innerHTML = ''; + 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 — this may take a moment on first use…'; + el.innerHTML = 'Introspecting — this may take a moment on first use…'; try { 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(', ')}
`; + if (wzIntrospectedTools.length) { + el.innerHTML = `
✓ Found ${wzIntrospectedTools.length} tool(s): ${esc(wzIntrospectedTools.map(t=>t.name).join(', '))}
`; + } else { + el.innerHTML = `
No tools advertised by this command — you can still add tools manually in the editor.
`; + } } catch(e) { wzIntrospectedTools = []; - el.innerHTML = `
✗ ${e.message}
`; - } finally { - btn.disabled = false; btn.textContent = '🔍 Introspect Tools'; + el.innerHTML = `
⚠ Introspection failed (${esc(e.message)}). Continuing without auto-detected tools — add them manually in the editor.
`; } } async function wzAnalyze() { const code = document.getElementById('wz-code-input').value; const el = document.getElementById('wz-analyze-result'); + if (!code.trim()) { el.innerHTML = ''; return; } try { const r = await api('POST', '/api/extract-functions', {code}); - if (!r.ok) { el.innerHTML = `${r.error}`; return; } - if (!r.functions.length) { el.innerHTML = 'No async def fn(context, …) found.'; return; } - el.innerHTML = `✓ Found: ${r.functions.map(f=>f.name).join(', ')}`; - } catch(e) { el.innerHTML = `${e.message}`; } + if (!r.ok) { el.innerHTML = `⚠ ${esc(r.error)}`; return; } + if (!r.functions.length) { el.innerHTML = 'No async def fn(context, …) found yet.'; return; } + el.innerHTML = `✓ Found: ${esc(r.functions.map(f=>f.name).join(', '))}`; + } catch(e) { el.innerHTML = `⚠ ${esc(e.message)}`; } } async function _analyzeFns(code) { diff --git a/server.py b/server.py index 330dbef..9e2c6f0 100755 --- a/server.py +++ b/server.py @@ -22,9 +22,14 @@ 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 + name — unique MCP tool name (advertised as + "__", with the provider's filename + normalized to [a-zA-Z0-9-]) function — async function name from code block (code providers only) description — shown to the LLM + enabled — optional bool, default true; when false the tool is + not registered (kept in YAML so you can re-enable + it without re-typing the schema) input_schema — JSON Schema object secrets.env — maps handler arg names to environment variable names auth — arbitrary dict forwarded to context["auth"] @@ -36,6 +41,7 @@ import builtins import inspect import os +import re import shlex import subprocess import sys @@ -72,11 +78,33 @@ SUBPROCESS_KEYS = ("package",) +ADVERTISED_NAME_SEP = "__" + # --------------------------------------------------------------------------- # Pure helpers # --------------------------------------------------------------------------- +def normalize_provider_name(name: str) -> str: + """Normalize a provider name for use as a tool-name prefix. + + Any character outside [a-zA-Z0-9] is replaced with a single ``-`` so the + result is a stable, MCP-safe identifier (callers can prepend it to a + tool name with ``__`` to namespace it: ``playwright__browser_navigate``). + """ + return re.sub(r"[^a-zA-Z0-9]", "-", name or "") + + +def advertised_tool_name(provider_name: str, tool_name: str) -> str: + """Return the namespaced tool name advertised to MCP clients.""" + return f"{normalize_provider_name(provider_name)}{ADVERTISED_NAME_SEP}{tool_name}" + + +def tool_is_enabled(tool_spec: dict[str, Any]) -> bool: + """Return False only when the spec explicitly sets ``enabled: false``.""" + return tool_spec.get("enabled", True) is not False + + def redact_secrets(value: Any) -> Any: try: if isinstance(value, dict): @@ -198,26 +226,39 @@ def _build_typed_signature( return inspect.Signature(params, return_annotation=Any), annotations -def register_tool(tool_spec: dict[str, Any], handler: Callable[..., Any]) -> None: - """Register a single MCP tool backed by the given async handler function.""" +def register_tool( + tool_spec: dict[str, Any], + handler: Callable[..., Any], + advertised_name: str | None = None, +) -> None: + """Register a single MCP tool backed by the given async handler function. + + ``advertised_name`` is the name shown to MCP clients (defaults to + ``tool_spec["name"]``). Provider-loaded tools pass a namespaced value + such as ``playwright__browser_navigate`` so tools from different + providers cannot collide. ``tool_spec["name"]`` itself is the + upstream / unprefixed name used when proxying to subprocesses. + """ try: + exposed_name = advertised_name or tool_spec["name"] + async def dynamic_tool(ctx: Context, **kwargs: Any) -> Any: try: resolved_kwargs = resolve_env_defaults(tool_spec, kwargs) runtime_context = build_runtime_context(tool_spec, ctx) return await handler(context=runtime_context, **resolved_kwargs) except Exception as exc: - print(f"dynamic_tool error in {tool_spec.get('name')}: {exc}") + print(f"dynamic_tool error in {exposed_name}: {exc}") traceback.print_exc() - return {"ok": False, "error": str(exc), "tool": tool_spec.get("name")} + return {"ok": False, "error": str(exc), "tool": exposed_name} - dynamic_tool.__name__ = tool_spec["name"] + dynamic_tool.__name__ = exposed_name sig, annotations = _build_typed_signature(tool_spec) dynamic_tool.__signature__ = sig # type: ignore[attr-defined] dynamic_tool.__annotations__ = annotations - mcp.tool(name=tool_spec["name"], description=tool_spec.get("description", ""))(dynamic_tool) - print(f"Registered tool: {tool_spec['name']}") + mcp.tool(name=exposed_name, description=tool_spec.get("description", ""))(dynamic_tool) + print(f"Registered tool: {exposed_name}") except Exception as exc: print(f"register_tool error for '{tool_spec.get('name')}': {exc}") traceback.print_exc() @@ -249,8 +290,16 @@ async def process_handler(context: dict[str, Any], **kwargs: Any) -> Any: def register_provider(spec: dict[str, Any]) -> None: - """Register all tools declared in one provider spec.""" + """Register all tools declared in one provider spec. + + The advertised name of each tool is ``__``, where + ```` comes from the YAML filename (normalized via + ``normalize_provider_name``). Tools whose spec carries + ``enabled: false`` are skipped entirely — they remain in the YAML so + they can be flipped back on without re-typing the schema. + """ source_path = spec.get("_config_path", "") + provider_name = Path(source_path).stem if source_path != "" else "" try: command = _get_package_command(spec) @@ -258,8 +307,15 @@ def register_provider(spec: dict[str, Any]) -> None: # ── package provider (npx / uvx / python -m / any binary) ────────── for tool_spec in spec.get("tools", []): tool_name = tool_spec.get("name", "") + if not tool_is_enabled(tool_spec): + print(f"Skipping disabled tool: {advertised_tool_name(provider_name, tool_name)}") + continue handler = _make_process_handler(command, tool_name) - register_tool(tool_spec, handler) + register_tool( + tool_spec, + handler, + advertised_name=advertised_tool_name(provider_name, tool_name), + ) else: # ── code provider ───────────────────────────────────────────────── @@ -271,6 +327,9 @@ def register_provider(spec: dict[str, Any]) -> None: for tool_spec in tools: tool_name = tool_spec.get("name", "") + if not tool_is_enabled(tool_spec): + print(f"Skipping disabled tool: {advertised_tool_name(provider_name, tool_name)}") + continue function_name = tool_spec.get("function") if not function_name: raise ValueError( @@ -282,7 +341,11 @@ def register_provider(spec: dict[str, Any]) -> None: f"Function '{function_name}' (tool '{tool_name}') not found " f"in the code block of {source_path}" ) - register_tool(tool_spec, handler) + register_tool( + tool_spec, + handler, + advertised_name=advertised_tool_name(provider_name, tool_name), + ) except Exception as exc: print(f"register_provider error in {source_path}: {exc}") @@ -322,7 +385,7 @@ def run_provider_setup(spec: dict[str, Any]) -> None: # --------------------------------------------------------------------------- def register_builtin_tools() -> None: - """Register the mcpproxy-listfiles and mcpproxy-getfile utility tools. + """Register the mcpproxy__listfiles and mcpproxy__getfile utility tools. These tools expose read-only access to the files directory (default: ``.playwright-mcp``, override with ``MCPPROXY_FILES_DIR``). They are @@ -335,7 +398,7 @@ def register_builtin_tools() -> None: register_tool( { - "name": "mcpproxy-listfiles", + "name": "mcpproxy__listfiles", "description": ( "List files and directories inside the mcpproxy files directory " "(default: .playwright-mcp, override with MCPPROXY_FILES_DIR). " @@ -363,13 +426,13 @@ def register_builtin_tools() -> None: register_tool( { - "name": "mcpproxy-getfile", + "name": "mcpproxy__getfile", "description": ( "Read the contents of a file from the mcpproxy files directory " "(default: .playwright-mcp). " "Returns UTF-8 text for text files (JSON, HTML, Markdown, …) or " "base64-encoded bytes for binary files (PNG screenshots, …). " - "Use mcpproxy-listfiles first to discover available file paths." + "Use mcpproxy__listfiles first to discover available file paths." ), "input_schema": { "type": "object", @@ -395,7 +458,7 @@ def register_builtin_tools() -> None: get_file, ) - print("Registered built-in tools: mcpproxy-listfiles, mcpproxy-getfile") + print("Registered built-in tools: mcpproxy__listfiles, mcpproxy__getfile") except Exception as exc: print(f"register_builtin_tools error: {exc}") traceback.print_exc() diff --git a/tests/test_builtin_tools.py b/tests/test_builtin_tools.py index 383972f..1a38351 100644 --- a/tests/test_builtin_tools.py +++ b/tests/test_builtin_tools.py @@ -1,4 +1,4 @@ -"""Tests for builtin_tools.py — mcpproxy-listfiles and mcpproxy-getfile. +"""Tests for builtin_tools.py — mcpproxy__listfiles and mcpproxy__getfile. These tests monkeypatch MCPPROXY_FILES_DIR to a fresh temp directory so they never touch the real .playwright-mcp directory. diff --git a/tests/test_frontend.py b/tests/test_frontend.py index c86fa00..537565e 100644 --- a/tests/test_frontend.py +++ b/tests/test_frontend.py @@ -56,7 +56,7 @@ def client(app): "setup_commands": [], "tools": [{ "name": "ping", "function": "ping", "description": "Ping tool", - "documentation": "", "parameters": [], "secrets": [], + "documentation": "", "enabled": True, "parameters": [], "secrets": [], }], } @@ -70,7 +70,7 @@ def client(app): "setup_commands": [], "tools": [{ "name": "playwright_navigate", "function": "", "description": "Navigate", - "documentation": "", "parameters": [ + "documentation": "", "enabled": True, "parameters": [ {"name": "url", "type": "string", "description": "URL", "required": True, "default": None} ], "secrets": [], }], @@ -412,6 +412,32 @@ def test_wizard_has_two_type_cards(self, client): assert "wzSelectType('code')" in text assert "wzSelectType('package')" in text + def test_no_manual_introspect_button(self, client): + """The 🔍 Introspect Tools button is replaced by auto-introspection.""" + text = client.get("/").text + assert "wz-introspect-btn" not in text + assert ">🔍 Introspect Tools<" not in text + + def test_no_manual_analyze_button(self, client): + """The 🔍 Analyze Functions button is replaced by live auto-analysis.""" + assert ">🔍 Analyze Functions<" not in client.get("/").text + + def test_html_has_discover_functions(self, client): + """Auto-discovery wiring is present in the JS.""" + assert "discoverFunctions" in client.get("/").text + + def test_html_has_enable_disable_helper(self, client): + """Per-tool enable/disable hooks are wired up in the JS.""" + text = client.get("/").text + assert "setToolEnabled" in text + assert "knownFunctions" in text + + def test_html_has_function_picker(self, client): + """The function-name dropdown ('Other…') is wired up.""" + text = client.get("/").text + assert "onFnPick" in text + assert "Other…" in text + # --------------------------------------------------------------------------- # Pure function tests @@ -490,6 +516,47 @@ def test_setup_commands_round_trip(self): structured = _provider_to_structured("p", spec) assert structured["setup_commands"] == ["npx playwright install chrome"] + def test_enabled_true_round_trip(self): + yaml_str = _structured_to_yaml(CODE_PROVIDER) + spec = yaml.safe_load(yaml_str) + assert spec["tools"][0]["enabled"] is True + structured = _provider_to_structured("p", spec) + assert structured["tools"][0]["enabled"] is True + + def test_enabled_false_round_trip(self): + provider = { + **CODE_PROVIDER, + "tools": [{**CODE_PROVIDER["tools"][0], "enabled": False}], + } + yaml_str = _structured_to_yaml(provider) + spec = yaml.safe_load(yaml_str) + assert spec["tools"][0]["enabled"] is False + structured = _provider_to_structured("p", spec) + assert structured["tools"][0]["enabled"] is False + + def test_missing_enabled_in_yaml_defaults_true(self): + """A YAML that pre-dates the `enabled` field is read as enabled=True.""" + spec = { + "code": "async def t(context): pass\n", + "tools": [{ + "name": "t", "function": "t", "description": "x", + "input_schema": {"type": "object", "properties": {}, "required": []}, + }], + } + structured = _provider_to_structured("p", spec) + assert structured["tools"][0]["enabled"] is True + + def test_enabled_always_written_explicitly(self): + """Writer always emits enabled: true|false, never omits it.""" + yaml_str = _structured_to_yaml(CODE_PROVIDER) + assert "enabled: true" in yaml_str + provider = { + **CODE_PROVIDER, + "tools": [{**CODE_PROVIDER["tools"][0], "enabled": False}], + } + yaml_str = _structured_to_yaml(provider) + assert "enabled: false" in yaml_str + def test_empty_requirements_omitted_from_yaml(self): yaml_str = _structured_to_yaml(CODE_PROVIDER) # requirements = [] spec = yaml.safe_load(yaml_str) diff --git a/tests/test_mcp_client.sh b/tests/test_mcp_client.sh index 1edc45f..c5f9c2e 100755 --- a/tests/test_mcp_client.sh +++ b/tests/test_mcp_client.sh @@ -686,8 +686,8 @@ def _extract(rpc): return {'content': content} -# 1. Call mcpproxy-listfiles (root directory) -listing = _extract(_mcp_call('tools/call', {'name': 'mcpproxy-listfiles', 'arguments': {}})) +# 1. Call mcpproxy__listfiles (root directory) +listing = _extract(_mcp_call('tools/call', {'name': 'mcpproxy__listfiles', 'arguments': {}})) entries = listing.get('entries', []) base_dir = listing.get('base_dir', '.playwright-mcp') @@ -698,7 +698,7 @@ for entry in entries: continue fname = entry['name'] file_result = _extract( - _mcp_call('tools/call', {'name': 'mcpproxy-getfile', 'arguments': {'path': fname}}) + _mcp_call('tools/call', {'name': 'mcpproxy__getfile', 'arguments': {'path': fname}}) ) files_fetched.append({ 'name': fname, diff --git a/tests/test_server.py b/tests/test_server.py index 1679b10..a8f42ec 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -21,14 +21,18 @@ SUBPROCESS_KEYS, _build_typed_signature, _get_package_command, + advertised_tool_name, build_runtime_context, exec_provider_code, load_provider_specs, + normalize_provider_name, redact_secrets, register_builtin_tools, + register_provider, register_tool, resolve_env_defaults, run_provider_setup, + tool_is_enabled, ) @@ -464,11 +468,11 @@ def fake_decorator(**kwargs): register_builtin_tools() assert len(tool_calls) == 2 - assert "mcpproxy-listfiles" in tool_calls - assert "mcpproxy-getfile" in tool_calls + assert "mcpproxy__listfiles" in tool_calls + assert "mcpproxy__getfile" in tool_calls def test_listfiles_tool_spec_has_no_required_fields(self): - """mcpproxy-listfiles 'path' parameter should be optional.""" + """mcpproxy__listfiles 'path' parameter should be optional.""" captured_specs = [] def fake_decorator(**kwargs): @@ -481,12 +485,199 @@ def fake_decorator(**kwargs): # Find the listfiles call — it was the first one registered names = [s["name"] for s in captured_specs] - assert "mcpproxy-listfiles" in names + assert "mcpproxy__listfiles" in names def test_getfile_tool_spec_requires_path(self): - """mcpproxy-getfile should declare 'path' as a required parameter.""" + """mcpproxy__getfile should declare 'path' as a required parameter.""" from builtin_tools import get_file import inspect sig = inspect.signature(get_file) # path has no default → required assert sig.parameters["path"].default is inspect.Parameter.empty + + +# --------------------------------------------------------------------------- +# normalize_provider_name / advertised_tool_name +# --------------------------------------------------------------------------- + +class TestNormalizeProviderName: + def test_simple_lowercase(self): + assert normalize_provider_name("playwright") == "playwright" + + def test_simple_mixed_case_preserved(self): + assert normalize_provider_name("MyProvider") == "MyProvider" + + def test_digits_preserved(self): + assert normalize_provider_name("v2tool") == "v2tool" + + def test_hyphen_kept_as_hyphen(self): + # Hyphens are outside [a-zA-Z0-9] so they get replaced with hyphens + # (same character — net result unchanged). + assert normalize_provider_name("my-provider") == "my-provider" + + def test_underscore_replaced_with_hyphen(self): + assert normalize_provider_name("my_provider") == "my-provider" + + def test_dot_replaced_with_hyphen(self): + assert normalize_provider_name("my.tool") == "my-tool" + + def test_spaces_replaced_with_hyphen(self): + assert normalize_provider_name("my tool") == "my-tool" + + def test_at_sign_and_slash_replaced(self): + assert normalize_provider_name("@scope/pkg") == "-scope-pkg" + + def test_empty_string(self): + assert normalize_provider_name("") == "" + + def test_none_treated_as_empty(self): + assert normalize_provider_name(None) == "" + + +class TestAdvertisedToolName: + def test_basic_combination(self): + assert advertised_tool_name("playwright", "browser_navigate") == "playwright__browser_navigate" + + def test_provider_normalized(self): + assert advertised_tool_name("my.tool", "do_thing") == "my-tool__do_thing" + + def test_double_underscore_separator(self): + # Must always be exactly two underscores, even if the tool name + # already starts with one. + assert advertised_tool_name("p", "_internal") == "p___internal" + + +# --------------------------------------------------------------------------- +# tool_is_enabled +# --------------------------------------------------------------------------- + +class TestToolIsEnabled: + def test_missing_field_defaults_true(self): + assert tool_is_enabled({"name": "t"}) is True + + def test_explicit_true(self): + assert tool_is_enabled({"name": "t", "enabled": True}) is True + + def test_explicit_false(self): + assert tool_is_enabled({"name": "t", "enabled": False}) is False + + def test_other_truthy_values_treated_as_enabled(self): + # Only an explicit `False` disables; everything else is on. + assert tool_is_enabled({"name": "t", "enabled": 0}) is True + assert tool_is_enabled({"name": "t", "enabled": ""}) is True + assert tool_is_enabled({"name": "t", "enabled": None}) is True + + +# --------------------------------------------------------------------------- +# register_provider — name prefixing and enabled filtering +# --------------------------------------------------------------------------- + +class TestRegisterProviderPrefixing: + def _capture_registered(self, spec): + names: list[str] = [] + + def fake_decorator(**kwargs): + names.append(kwargs.get("name")) + return lambda fn: fn + + with patch("server.mcp") as mock_mcp: + mock_mcp.tool.side_effect = fake_decorator + register_provider(spec) + return names + + def test_code_provider_tools_are_prefixed(self, tmp_path: Path): + spec = { + "_config_path": str(tmp_path / "playwright.yaml"), + "code": "async def navigate(context, url):\n return {'ok': True}\n", + "tools": [{ + "name": "navigate", + "function": "navigate", + "description": "x", + "input_schema": {"type": "object", "properties": {}, "required": []}, + }], + } + names = self._capture_registered(spec) + assert names == ["playwright__navigate"] + + def test_package_provider_tools_are_prefixed(self, tmp_path: Path): + spec = { + "_config_path": str(tmp_path / "playwright.yaml"), + "package": {"command": "npx @playwright/mcp@latest"}, + "tools": [{ + "name": "browser_navigate", + "description": "x", + "input_schema": {"type": "object", "properties": {}, "required": []}, + }], + } + names = self._capture_registered(spec) + assert names == ["playwright__browser_navigate"] + + def test_provider_name_normalized(self, tmp_path: Path): + spec = { + "_config_path": str(tmp_path / "my.tool.yaml"), + "package": {"command": "echo hi"}, + "tools": [{ + "name": "do", + "description": "x", + "input_schema": {"type": "object", "properties": {}, "required": []}, + }], + } + # .stem strips the final .yaml only, leaving "my.tool" — dots → hyphens. + names = self._capture_registered(spec) + assert names == ["my-tool__do"] + + def test_disabled_tool_is_skipped(self, tmp_path: Path): + spec = { + "_config_path": str(tmp_path / "p.yaml"), + "package": {"command": "echo hi"}, + "tools": [ + { + "name": "alive", + "description": "x", + "enabled": True, + "input_schema": {"type": "object", "properties": {}, "required": []}, + }, + { + "name": "dead", + "description": "x", + "enabled": False, + "input_schema": {"type": "object", "properties": {}, "required": []}, + }, + ], + } + names = self._capture_registered(spec) + assert names == ["p__alive"] + + def test_missing_enabled_defaults_to_registered(self, tmp_path: Path): + spec = { + "_config_path": str(tmp_path / "p.yaml"), + "package": {"command": "echo hi"}, + "tools": [{ + "name": "t", + "description": "x", + "input_schema": {"type": "object", "properties": {}, "required": []}, + }], + } + names = self._capture_registered(spec) + assert names == ["p__t"] + + def test_disabled_code_tool_skipped_without_loading_handler(self, tmp_path: Path): + # The code block must NOT define `dead` — registration should still + # succeed because the disabled tool is never looked up. + spec = { + "_config_path": str(tmp_path / "p.yaml"), + "code": "async def alive(context):\n return {'ok': True}\n", + "tools": [ + { + "name": "alive", "function": "alive", "description": "x", + "input_schema": {"type": "object", "properties": {}, "required": []}, + }, + { + "name": "dead", "function": "missing_function", + "description": "x", "enabled": False, + "input_schema": {"type": "object", "properties": {}, "required": []}, + }, + ], + } + names = self._capture_registered(spec) + assert names == ["p__alive"]