diff --git a/docs/superpowers/specs/2026-08-03-mcp-status-panel-design.md b/docs/superpowers/specs/2026-08-03-mcp-status-panel-design.md new file mode 100644 index 0000000..70440b0 --- /dev/null +++ b/docs/superpowers/specs/2026-08-03-mcp-status-panel-design.md @@ -0,0 +1,198 @@ +# MCP Status Panel — Design + +Date: 2026-08-03 +Status: Approved + +## Problem + +`app/mcp/` already exposes the compliance knowledge base over MCP +(`search_regulations`, Streamable HTTP at `/mcp/`, JWT-guarded, Host +allow-listed). It is completely invisible from the product: an operator +looking at the System Status page cannot tell whether the MCP endpoint is +enabled, what URL a client should point at, which tools are advertised, or +whether anything has ever called it. + +This design adds that visibility, and only that. + +## Goals + +- Show MCP endpoint configuration (public URL, auth on/off, Host allow-list). +- Show the advertised tool list, read from the live MCP registry rather than + hard-coded. +- Show per-tool call counters: total calls, errors, average duration, last + call time. +- Give the operator a one-click "copy client config" JSON they can paste into + Claude Desktop / Cursor. + +## Non-Goals + +- No persistence. Counters are in-memory and reset on restart. Token + consumption caused by MCP calls is already persisted by the existing + `ModelUsageTracker` (MCP calls route through `AgentConversationService.ask()` + like every other caller), so nothing billable is lost. +- No historical trends or time-series charts. +- No active-client / session list. That would require hooking the MCP SDK's + internal `StreamableHTTPSessionManager`, which is private API and breaks on + SDK upgrades. +- No per-client attribution. +- No new frontend test framework — the project has none, and this change does + not justify introducing one. + +## Architecture + +Module boundary is unchanged: everything new on the backend lands inside the +existing `app/mcp/` module, plus one thin HTTP adapter route. + +``` +frontend/src/pages/Status/StatusPage.tsx + │ GET /api/v1/status/mcp + ▼ +backend/app/api/routes/status.py ← HTTP adapter only + │ get_mcp_status(public_url) + ▼ +backend/app/mcp/server.py ← assembles the status payload + ├── settings (endpoint / auth / allowed hosts) + ├── mcp.list_tools() ← live tool registry + └── app/mcp/stats.py ← in-memory counters +``` + +The status route must not reach into the MCP server's internals. It passes in +the resolved public URL (the one thing only the HTTP layer knows) and receives +a finished dict. This keeps the MCP protocol details in one module. + +## Components + +### `app/mcp/stats.py` (new) + +Mirrors `app/shared/model_usage_tracker.py` in shape and in defensive posture. + +- `MCPToolStats` dataclass: `calls`, `errors`, `last_called_at`, + `total_duration_ms`; `avg_duration_ms` computed as a property. +- `MCPStatsTracker`: one `threading.Lock` guarding a + `dict[str, MCPToolStats]`. `record(tool, duration_ms, success)` and + `snapshot()`. +- `get_mcp_stats_tracker()`, `@lru_cache` singleton. + +The lock is not optional. The mcp SDK (2.0.0) dispatches synchronous tool +functions via `anyio.to_thread.run_sync`, so `search_regulations` genuinely +runs on multiple worker threads concurrently — unlike the async REST routes, +which serialize on the event loop. + +`record()` swallows and logs its own exceptions, matching `ModelUsageTracker`: +a defect in observability code must never fail a real MCP tool call. + +### `app/mcp/server.py` (modified) + +- `search_regulations` gets a `try/except/finally` wrapper that measures + elapsed time with `time.perf_counter()` and records success or failure. The + exception is re-raised after recording — the MCP SDK still needs to turn it + into a protocol-level error. +- New `async def get_mcp_status(public_url: str) -> dict` merges three + sources: settings, `await mcp.list_tools()`, and the stats snapshot. Tools + are matched to their stats by name; a tool that has never been called + reports zeros. + +### `app/api/routes/status.py` (modified) + +`GET /status/mcp` resolves the public URL, then delegates: + +```python +public_url = settings.mcp_public_url or f"{str(request.base_url).rstrip('/')}/mcp/" +return await get_mcp_status(public_url) +``` + +### `app/config/settings.py` + `.env.example` (modified) + +New optional `mcp_public_url: str = ""`. + +This override is required, not cosmetic. The Vite dev proxy sets +`changeOrigin: true` (`frontend/vite.config.ts`), which rewrites the `Host` +header to the proxy target, so `request.base_url` on the backend reads +`http://127.0.0.1:8000/` in development regardless of how the operator +actually reached the page. Deployments behind a reverse proxy that does not +forward the original Host have the same problem. When unset, the derived +value is correct for the common same-origin case. + +### Frontend + +- `api/index.ts`: `MCPToolEntry` and `MCPStatusResponse` types, alongside the + existing `ModelUsageEntry` / `SystemHealth` types. +- `api/status.ts`: `getMCPStatus()`, using the typed `fetchAPI` client. +- `StatusPage.tsx`: new "MCP Server" card in the left column, directly below + the existing "AI Models" card. It joins the existing + `Promise.allSettled([...])` batch, so it refreshes with the page's existing + Refresh button and needs no independent polling. `handleExport()` includes + the MCP payload. + - Header row: title + "copy client config" button. + - Endpoint row: `StatusIcon` + monospace URL + auth badge + Host allow-list. + - One row per tool: name, calls, errors, average duration, last call time. + - Reuses the existing `card`, `card-header`, `service-row` classes and the + `StatusIcon` component. No new CSS. +- `locales/zh.ts` / `locales/en.ts`: new keys under the existing `status` + section. + +### Copy client config + +Produces the Streamable HTTP form both Claude Desktop and Cursor accept: + +```json +{ + "mcpServers": { + "ai-regulations": { + "url": "http://6.86.80.9:8000/mcp/", + "headers": { "Authorization": "Bearer " } + } + } +} +``` + +The real JWT from `localStorage` is embedded, because a config with a +placeholder does not work when pasted and defeats the button's purpose. This +is the operator's own token, already present in their own browser; the button +moves it from one local store to another local store on the same machine. The +`headers` key is omitted entirely when `auth_required` is false. + +## Data Flow + +1. StatusPage mounts (or Refresh is pressed) → `getMCPStatus()` in the + existing parallel batch. +2. Route resolves the public URL and calls `get_mcp_status()`. +3. `get_mcp_status()` reads settings, awaits `mcp.list_tools()`, snapshots + stats, joins tools to stats by name. +4. Card renders. Counters advance only when a real MCP client calls a tool. + +## Error Handling + +- `GET /status/mcp` fails or times out → `Promise.allSettled` leaves the state + `null` → card renders a muted "unavailable" body. This is the same pattern + the existing model-usage card already uses; one failing status endpoint must + never blank the whole page. +- `mcp.list_tools()` reads an in-memory registry populated at import time and + has no failure mode worth special-casing; an unexpected exception surfaces + as a 500 on this one endpoint and is contained by the point above. +- `MCPStatsTracker.record()` never raises (logged and swallowed). +- `navigator.clipboard.writeText` rejects on insecure origins and when + permission is denied. The button reports failure in its own label rather + than throwing — a silent no-op would leave the operator believing they + copied something. + +## Testing + +`backend/tests/mcp/test_mcp_stats.py`: + +- Concurrent `record()` from multiple threads yields an exact total (proves + the lock). +- `avg_duration_ms` is correct across several calls, and is `None` with zero + calls (no division by zero). +- Successes and failures land in `calls` vs `errors` correctly. +- `record()` on malformed input logs instead of raising. + +`backend/tests/mcp/test_mcp_status.py`: + +- `get_mcp_status()` returns the advertised tool with zeroed stats before any + call, and reflects recorded stats after. +- `auth_required` follows `settings.auth_enabled`. +- The passed-in `public_url` appears unmodified in the payload. + +Frontend: no new tests; verified via `npm --prefix frontend run lint` and +`npm --prefix frontend run build`.