diff --git a/docs/openapi/monitoring-api.json b/docs/openapi/monitoring-api.json index 41903c1..02f8524 100644 --- a/docs/openapi/monitoring-api.json +++ b/docs/openapi/monitoring-api.json @@ -6412,7 +6412,9 @@ "enum": [ "DASHBOARD", "CLI", - "TERRAFORM" + "TERRAFORM", + "MCP", + "API" ] } }, @@ -23307,11 +23309,13 @@ }, "managedBy": { "type": "string", - "description": "Who manages this monitor: DASHBOARD or CLI", + "description": "Source that created/owns this monitor: DASHBOARD, CLI, TERRAFORM, MCP, or API. Use the value matching your surface so audit logs, drift detection, and analytics attribute correctly.", "enum": [ "DASHBOARD", "CLI", - "TERRAFORM" + "TERRAFORM", + "MCP", + "API" ] }, "environmentId": { @@ -27194,11 +27198,13 @@ }, "managedBy": { "type": "string", - "description": "Management source: DASHBOARD or CLI", + "description": "Source that created/owns this monitor: DASHBOARD, CLI, TERRAFORM, MCP, or API", "enum": [ "DASHBOARD", "CLI", - "TERRAFORM" + "TERRAFORM", + "MCP", + "API" ] }, "createdAt": { @@ -33345,12 +33351,14 @@ }, "managedBy": { "type": "string", - "description": "New management source; null preserves current", + "description": "New ownership source: DASHBOARD, CLI, TERRAFORM, MCP, or API; null preserves current value", "nullable": true, "enum": [ "DASHBOARD", "CLI", - "TERRAFORM" + "TERRAFORM", + "MCP", + "API" ] }, "environmentId": { diff --git a/src/devhelm_mcp/client.py b/src/devhelm_mcp/client.py index a7677c5..cded4cc 100644 --- a/src/devhelm_mcp/client.py +++ b/src/devhelm_mcp/client.py @@ -5,7 +5,7 @@ import os from importlib.metadata import PackageNotFoundError from importlib.metadata import version as _pkg_version -from typing import Any +from typing import Any, NoReturn from devhelm import ( Devhelm, @@ -15,6 +15,7 @@ DevhelmTransportError, DevhelmValidationError, ) +from fastmcp.exceptions import ToolError from pydantic import BaseModel API_BASE_URL = os.getenv("DEVHELM_API_URL", "https://api.devhelm.io") @@ -201,6 +202,26 @@ def format_error(err: DevhelmError) -> str: return f"Error: {err}" +def raise_tool_error(err: DevhelmError) -> NoReturn: + """Convert an SDK error into a FastMCP ``ToolError`` so ``isError=true``. + + Per the MCP spec, upstream API failures must surface as + ``CallToolResult.isError = true`` so the LLM can distinguish a tool that + *ran but failed* from one that *succeeded with an error message in the + response* — those have the same shape on the wire otherwise. + + The previous behavior returned ``format_error(err)`` as a regular tool + return value (``isError = false``), which caused agents to confidently + report success after a 4xx/5xx (silent-corruption bug from the round-3 + DevEx audit). FastMCP catches the ``ToolError`` raised here and + serializes it into ``CallToolResult(isError=True, content=[...])``, + preserving the human-readable formatted message for the LLM. + + See https://modelcontextprotocol.io/specification/server/tools#error-handling. + """ + raise ToolError(format_error(err)) from err + + JsonValue = dict[str, "JsonValue"] | list["JsonValue"] | str | int | float | bool | None diff --git a/src/devhelm_mcp/server.py b/src/devhelm_mcp/server.py index 2e1a34a..0a4f28d 100644 --- a/src/devhelm_mcp/server.py +++ b/src/devhelm_mcp/server.py @@ -10,18 +10,21 @@ from __future__ import annotations import argparse +import asyncio import os import sys from importlib.metadata import PackageNotFoundError from importlib.metadata import version as _pkg_version -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Any from fastmcp import FastMCP from starlette.requests import Request from starlette.responses import JSONResponse +from starlette.types import Receive, Scope, Send if TYPE_CHECKING: from starlette.applications import Starlette + from starlette.types import ASGIApp from devhelm_mcp.tools import ( alert_channels, @@ -41,8 +44,33 @@ webhooks, ) + +def _package_version() -> str: + """Best-effort lookup of the installed package version. + + Reported via the MCP ``initialize`` ``serverInfo.version`` field and on + every API call as ``X-DevHelm-Surface-Version``. Falls back to + ``"unknown"`` for source-tree installs (no installed dist-info). + """ + try: + return _pkg_version("devhelm-mcp-server") + except PackageNotFoundError: + return "unknown" + + +__version__ = _package_version() + + +# Pinning the FastMCP ``name``/``version`` here pegs the values reported in +# the MCP ``initialize`` handshake's ``serverInfo`` to *our* package, not to +# the FastMCP framework version (which the constructor would otherwise use +# as a default for ``version``). Before this fix every MCP client surfaced +# the FastMCP runtime version (e.g. "3.2.4") as the DevHelm server version, +# which made it impossible to correlate user reports with a specific MCP +# server release. END / DevEx P2.Bug8. mcp = FastMCP( - "DevHelm", + "devhelm-mcp-server", + version=__version__, instructions=( "DevHelm MCP server for monitoring infrastructure. " "Use these tools to manage uptime monitors, incidents, alert channels, " @@ -74,6 +102,122 @@ mod.register(mcp) +def _strip_internal_schema_fields() -> None: + """Hide server-controlled / sensitive fields from the LLM-facing schema. + + Two targets: + + 1. ``api_token`` — every tool accepts it as a kwarg for back-compat with + path-style ``/{api_key}/mcp`` clients, but after the 0.7.0 Bearer / + env-var fix the LLM should never need to set it. Surfacing it in + ``inputSchema.properties`` invites the model to populate the field + from chat context, which leaks the user's API token into telemetry + and tool-call traces. Removing the property entirely keeps the + Python signature wired (so direct callers / tests still work) while + preventing the LLM from seeing it. DevEx P2.Bug7. + + 2. ``managedBy`` on ``create_monitor`` — the MCP server forces this to + ``"MCP"`` server-side (see ``tools/monitors.py``). Hiding the field + from the schema stops the LLM from trying to set it (which would + either be ignored or, worse, fail validation on a stale SDK enum). + The Pydantic model for the body still excludes the field from + serialization, so this strip is purely about LLM ergonomics. + DevEx P0.Bug5 + P1.Bug4 + P1.Bug5. + + ``run_middleware=False`` returns the source-of-truth ``Tool`` objects + from the providers; the dereference middleware copies them on every + ``tools/list`` and would lose any edits we made to the copies. + """ + tools = asyncio.run(mcp.list_tools(run_middleware=False)) + for tool in tools: + params = tool.parameters + if not isinstance(params, dict): + continue + properties = params.get("properties") + if isinstance(properties, dict): + properties.pop("api_token", None) + required = params.get("required") + if isinstance(required, list) and "api_token" in required: + required.remove("api_token") + + if tool.name == "create_monitor": + _strip_managed_by_from_create_monitor(params) + + +def _strip_managed_by_from_create_monitor(params: dict[str, Any]) -> None: + """Remove ``managedBy`` from the ``create_monitor`` body schema. + + FastMCP emits the body either inline (``properties.body.properties``) or + behind a ``$ref`` into ``$defs`` — depends on whether the Pydantic model + has nested ``$ref``-able children. For ``CreateMonitorRequest`` it's the + second shape because of the discriminated ``config`` union, so the fix + has to drop ``managedBy`` from the def *before* the dereference + middleware inlines it for the wire response. + """ + properties = params.get("properties") + if isinstance(properties, dict): + body = properties.get("body") + if isinstance(body, dict): + _strip_field_from_object_schema(body, "managedBy") + ref = body.get("$ref") + if isinstance(ref, str): + defs = params.get("$defs") + if isinstance(defs, dict): + def_name = ref.rsplit("/", 1)[-1] + target = defs.get(def_name) + if isinstance(target, dict): + _strip_field_from_object_schema(target, "managedBy") + + +def _strip_field_from_object_schema(schema: dict[str, Any], field: str) -> None: + """Remove ``field`` from a JSON Schema object's ``properties`` and ``required``.""" + schema_props = schema.get("properties") + if isinstance(schema_props, dict): + schema_props.pop(field, None) + schema_required = schema.get("required") + if isinstance(schema_required, list) and field in schema_required: + schema_required.remove(field) + + +_strip_internal_schema_fields() + + +class _NormalizeMcpPath: + """Rewrite ``/mcp`` → ``/mcp/`` (and ``/{key}/mcp`` → ``/{key}/mcp/``). + + Starlette's ``Mount("/mcp", inner_app)`` only forwards requests under + ``/mcp/`` to the inner app — the bare ``/mcp`` URL gets a 307 redirect + to ``/mcp/``. Naive HTTP clients (some MCP wrappers, raw curl scripts, + LangChain's older HTTP transport) drop the request body and the + ``Authorization`` header on a 307, then fail with ``-32600 invalid + request`` or 401. + + This middleware sits *outside* the Starlette router and rewrites the + ASGI scope's ``path`` / ``raw_path`` before routing happens, so the + request is delivered straight to the inner MCP handler with no + redirect. The rewrite is idempotent — paths that already end in + ``/`` are left alone — and only fires for the ``/mcp`` family, so + ``/health`` and other routes are untouched. + + DevEx P1.Bug6. + """ + + def __init__(self, app: ASGIApp) -> None: + self.app = app + + async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: + if scope["type"] == "http": + path = scope.get("path", "") + if path == "/mcp" or ( + path.endswith("/mcp") + and len(path) > len("/mcp") + and path.count("/") >= 2 + ): + new_path = path + "/" + scope = {**scope, "path": new_path, "raw_path": new_path.encode()} + await self.app(scope, receive, send) + + def _get_app() -> Starlette: """Build the ASGI app with path-based auth routing.""" from starlette.applications import Starlette @@ -117,16 +261,7 @@ async def health_handler(request: Request) -> JSONResponse: # ``StreamableHTTPSessionManager`` — without it the inner app raises # "task group was not initialized" on the first POST. See # https://gofastmcp.com/deployment/asgi. - # - # Trailing-slash semantics: Starlette's ``Mount("/mcp", inner_app)`` - # only forwards paths under ``/mcp/`` to the inner app — the bare - # ``/mcp`` URL gets a 307 redirect to ``/mcp/``. With - # ``proxy_headers=True`` on Uvicorn (set in ``main()``), the redirect's - # ``Location`` honors ``X-Forwarded-Proto: https`` from the upstream - # proxy (Cloudflare → Traefik → here), so clients still end up on - # ``https://mcp.devhelm.io/mcp/`` instead of being downgraded to HTTP. - # See END-1186 for why this matters. - app = Starlette( + starlette_app = Starlette( routes=[ Route("/health", health_handler, methods=["GET"]), Mount("/mcp", app=mcp_app), @@ -135,20 +270,17 @@ async def health_handler(request: Request) -> JSONResponse: middleware=middleware, lifespan=mcp_app.lifespan, ) - return app + # Wrap with the path normalizer so POST /mcp (no trailing slash) reaches + # the inner app directly instead of bouncing through a 307. The + # middleware lives outside Starlette's router because the redirect is + # emitted by ``Mount`` before the middleware chain executes. + starlette_app.add_middleware(_NormalizeMcpPath) + return starlette_app app = _get_app() -def _package_version() -> str: - """Best-effort lookup of the installed package version for ``--version``.""" - try: - return _pkg_version("devhelm-mcp-server") - except PackageNotFoundError: - return "unknown" - - def _build_arg_parser() -> argparse.ArgumentParser: """Build the CLI parser for ``devhelm-mcp-server``. @@ -215,7 +347,7 @@ def _build_arg_parser() -> argparse.ArgumentParser: parser.add_argument( "--version", action="version", - version=f"devhelm-mcp-server {_package_version()}", + version=f"devhelm-mcp-server {__version__}", ) return parser @@ -247,6 +379,30 @@ def _resolve_port(arg: int | None) -> int: raise SystemExit(f"Invalid port {raw!r}: {exc}") from exc +def _run_stdio() -> None: + """Start the stdio transport without FastMCP's ASCII banner. + + FastMCP's default ``run()`` prints a multi-line ASCII banner on stderr + that includes a third-party promo URL. On stdio it's the very first + thing local MCP clients (Cursor, Claude Desktop, Windsurf) see when + they tail the server log, and it's been a recurring source of + "is the server working?" support tickets — DevEx P2.Bug9. + + Setting ``show_banner=False`` is supported on FastMCP 2.x+ and is the + cleanest way to suppress it without intercepting stderr globally + (which would also swallow real warnings / tracebacks the user needs + to see). + """ + extra: dict[str, Any] = {} + try: + mcp.run(show_banner=False, **extra) + except TypeError: + # Older FastMCP releases don't accept ``show_banner``; fall back + # to the default behavior so the server still starts. The banner + # is cosmetic — never break startup over it. + mcp.run() + + def main(argv: list[str] | None = None) -> None: """Entry point for the ``devhelm-mcp-server`` console script. @@ -280,7 +436,7 @@ def main(argv: list[str] | None = None) -> None: transport = _resolve_transport(args.transport) if transport == "stdio": - mcp.run() + _run_stdio() return import uvicorn @@ -297,6 +453,10 @@ def main(argv: list[str] | None = None) -> None: # discovered this by seeing every MCP client (Cursor, Claude Desktop, # raw curl) bounce through ``Location: http://mcp.devhelm.io/mcp/`` and # never reach the JSON-RPC handler. END-1186. + # + # The trailing-slash redirect itself is now obviated by the + # ``_NormalizeMcpPath`` middleware in ``_get_app``, but proxy_headers + # remains useful for any future redirect (auth challenge, etc.). uvicorn.run( "devhelm_mcp.server:app", host=host, diff --git a/src/devhelm_mcp/tools/alert_channels.py b/src/devhelm_mcp/tools/alert_channels.py index becf41d..057da43 100644 --- a/src/devhelm_mcp/tools/alert_channels.py +++ b/src/devhelm_mcp/tools/alert_channels.py @@ -9,8 +9,8 @@ from devhelm_mcp.client import ( ToolResult, as_payload, - format_error, get_client, + raise_tool_error, serialize, ) @@ -22,7 +22,7 @@ def list_alert_channels(api_token: str | None = None) -> ToolResult: try: return serialize(get_client(api_token).alert_channels.list()) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def get_alert_channel(channel_id: str, api_token: str | None = None) -> ToolResult: @@ -30,7 +30,7 @@ def get_alert_channel(channel_id: str, api_token: str | None = None) -> ToolResu try: return serialize(get_client(api_token).alert_channels.get(channel_id)) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def create_alert_channel( @@ -48,7 +48,7 @@ def create_alert_channel( get_client(api_token).alert_channels.create(as_payload(body)) ) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def update_alert_channel( @@ -64,7 +64,7 @@ def update_alert_channel( ) ) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def delete_alert_channel(channel_id: str, api_token: str | None = None) -> str: @@ -73,7 +73,7 @@ def delete_alert_channel(channel_id: str, api_token: str | None = None) -> str: get_client(api_token).alert_channels.delete(channel_id) return "Alert channel deleted successfully." except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def test_alert_channel(channel_id: str, api_token: str | None = None) -> ToolResult: @@ -81,4 +81,4 @@ def test_alert_channel(channel_id: str, api_token: str | None = None) -> ToolRes try: return serialize(get_client(api_token).alert_channels.test(channel_id)) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) diff --git a/src/devhelm_mcp/tools/api_keys.py b/src/devhelm_mcp/tools/api_keys.py index aea3547..fb826ba 100644 --- a/src/devhelm_mcp/tools/api_keys.py +++ b/src/devhelm_mcp/tools/api_keys.py @@ -9,8 +9,8 @@ from devhelm_mcp.client import ( ToolResult, as_payload, - format_error, get_client, + raise_tool_error, serialize, ) @@ -22,7 +22,7 @@ def list_api_keys(api_token: str | None = None) -> ToolResult: try: return serialize(get_client(api_token).api_keys.list()) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def create_api_key( @@ -35,7 +35,7 @@ def create_api_key( try: return serialize(get_client(api_token).api_keys.create(as_payload(body))) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def revoke_api_key(key_id: str, api_token: str | None = None) -> str: @@ -44,7 +44,7 @@ def revoke_api_key(key_id: str, api_token: str | None = None) -> str: get_client(api_token).api_keys.revoke(key_id) return "API key revoked successfully." except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def delete_api_key(key_id: str, api_token: str | None = None) -> str: @@ -53,4 +53,4 @@ def delete_api_key(key_id: str, api_token: str | None = None) -> str: get_client(api_token).api_keys.delete(key_id) return "API key deleted successfully." except DevhelmError as e: - return format_error(e) + raise_tool_error(e) diff --git a/src/devhelm_mcp/tools/dependencies.py b/src/devhelm_mcp/tools/dependencies.py index ef9d710..57ef233 100644 --- a/src/devhelm_mcp/tools/dependencies.py +++ b/src/devhelm_mcp/tools/dependencies.py @@ -5,7 +5,7 @@ from devhelm import DevhelmError from fastmcp import FastMCP -from devhelm_mcp.client import ToolResult, format_error, get_client, serialize +from devhelm_mcp.client import ToolResult, get_client, raise_tool_error, serialize def register(mcp: FastMCP) -> None: @@ -15,7 +15,7 @@ def list_dependencies(api_token: str | None = None) -> ToolResult: try: return serialize(get_client(api_token).dependencies.list()) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def get_dependency(dependency_id: str, api_token: str | None = None) -> ToolResult: @@ -23,7 +23,7 @@ def get_dependency(dependency_id: str, api_token: str | None = None) -> ToolResu try: return serialize(get_client(api_token).dependencies.get(dependency_id)) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def track_dependency(slug: str, api_token: str | None = None) -> ToolResult: @@ -31,7 +31,7 @@ def track_dependency(slug: str, api_token: str | None = None) -> ToolResult: try: return serialize(get_client(api_token).dependencies.track(slug)) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def delete_dependency(dependency_id: str, api_token: str | None = None) -> str: @@ -40,4 +40,4 @@ def delete_dependency(dependency_id: str, api_token: str | None = None) -> str: get_client(api_token).dependencies.delete(dependency_id) return "Dependency removed successfully." except DevhelmError as e: - return format_error(e) + raise_tool_error(e) diff --git a/src/devhelm_mcp/tools/deploy_lock.py b/src/devhelm_mcp/tools/deploy_lock.py index 035f3a4..7709a42 100644 --- a/src/devhelm_mcp/tools/deploy_lock.py +++ b/src/devhelm_mcp/tools/deploy_lock.py @@ -9,8 +9,8 @@ from devhelm_mcp.client import ( ToolResult, as_payload, - format_error, get_client, + raise_tool_error, serialize, ) @@ -31,7 +31,7 @@ def acquire_deploy_lock( get_client(api_token).deploy_lock.acquire(as_payload(body)) ) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def get_current_deploy_lock(api_token: str | None = None) -> ToolResult | None: @@ -40,7 +40,7 @@ def get_current_deploy_lock(api_token: str | None = None) -> ToolResult | None: result = get_client(api_token).deploy_lock.current() return serialize(result) if result else None except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def release_deploy_lock(lock_id: str, api_token: str | None = None) -> str: @@ -49,7 +49,7 @@ def release_deploy_lock(lock_id: str, api_token: str | None = None) -> str: get_client(api_token).deploy_lock.release(lock_id) return "Deploy lock released." except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def force_release_deploy_lock(api_token: str | None = None) -> str: @@ -58,4 +58,4 @@ def force_release_deploy_lock(api_token: str | None = None) -> str: get_client(api_token).deploy_lock.force_release() return "Deploy lock force-released." except DevhelmError as e: - return format_error(e) + raise_tool_error(e) diff --git a/src/devhelm_mcp/tools/environments.py b/src/devhelm_mcp/tools/environments.py index 9c8952c..2cd9609 100644 --- a/src/devhelm_mcp/tools/environments.py +++ b/src/devhelm_mcp/tools/environments.py @@ -9,8 +9,8 @@ from devhelm_mcp.client import ( ToolResult, as_payload, - format_error, get_client, + raise_tool_error, serialize, ) @@ -22,7 +22,7 @@ def list_environments(api_token: str | None = None) -> ToolResult: try: return serialize(get_client(api_token).environments.list()) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def get_environment(slug: str, api_token: str | None = None) -> ToolResult: @@ -30,7 +30,7 @@ def get_environment(slug: str, api_token: str | None = None) -> ToolResult: try: return serialize(get_client(api_token).environments.get(slug)) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def create_environment( @@ -46,7 +46,7 @@ def create_environment( get_client(api_token).environments.create(as_payload(body)) ) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def update_environment( @@ -60,7 +60,7 @@ def update_environment( get_client(api_token).environments.update(slug, as_payload(body)) ) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def delete_environment(slug: str, api_token: str | None = None) -> str: @@ -69,4 +69,4 @@ def delete_environment(slug: str, api_token: str | None = None) -> str: get_client(api_token).environments.delete(slug) return "Environment deleted successfully." except DevhelmError as e: - return format_error(e) + raise_tool_error(e) diff --git a/src/devhelm_mcp/tools/forensics.py b/src/devhelm_mcp/tools/forensics.py index 0af2b4c..7be9694 100644 --- a/src/devhelm_mcp/tools/forensics.py +++ b/src/devhelm_mcp/tools/forensics.py @@ -15,8 +15,8 @@ from devhelm_mcp.client import ( ToolResult, - format_error, get_client, + raise_tool_error, serialize, ) @@ -40,7 +40,7 @@ def get_incident_timeline( get_client(api_token).forensics.incident_timeline(incident_id) ) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def get_check_trace(check_id: str, api_token: str | None = None) -> ToolResult: @@ -54,7 +54,7 @@ def get_check_trace(check_id: str, api_token: str | None = None) -> ToolResult: try: return serialize(get_client(api_token).forensics.check_trace(check_id)) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def get_policy_snapshot(hash_hex: str, api_token: str | None = None) -> ToolResult: @@ -68,7 +68,7 @@ def get_policy_snapshot(hash_hex: str, api_token: str | None = None) -> ToolResu try: return serialize(get_client(api_token).forensics.policy_snapshot(hash_hex)) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def list_monitor_rule_evaluations( @@ -113,7 +113,7 @@ def list_monitor_rule_evaluations( } ) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def list_monitor_transitions( @@ -150,4 +150,4 @@ def list_monitor_transitions( } ) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) diff --git a/src/devhelm_mcp/tools/incidents.py b/src/devhelm_mcp/tools/incidents.py index d0d931c..cbbe07f 100644 --- a/src/devhelm_mcp/tools/incidents.py +++ b/src/devhelm_mcp/tools/incidents.py @@ -9,8 +9,8 @@ from devhelm_mcp.client import ( ToolResult, as_payload, - format_error, get_client, + raise_tool_error, serialize, ) @@ -22,7 +22,7 @@ def list_incidents(api_token: str | None = None) -> ToolResult: try: return serialize(get_client(api_token).incidents.list()) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def get_incident(incident_id: str, api_token: str | None = None) -> ToolResult: @@ -30,7 +30,7 @@ def get_incident(incident_id: str, api_token: str | None = None) -> ToolResult: try: return serialize(get_client(api_token).incidents.get(incident_id)) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def create_incident( @@ -45,7 +45,7 @@ def create_incident( try: return serialize(get_client(api_token).incidents.create(as_payload(body))) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def resolve_incident( @@ -61,4 +61,4 @@ def resolve_incident( get_client(api_token).incidents.resolve(incident_id, payload) ) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) diff --git a/src/devhelm_mcp/tools/monitors.py b/src/devhelm_mcp/tools/monitors.py index 0b1a682..257cae0 100644 --- a/src/devhelm_mcp/tools/monitors.py +++ b/src/devhelm_mcp/tools/monitors.py @@ -2,18 +2,52 @@ from __future__ import annotations +from typing import Any + from devhelm import DevhelmError from devhelm.types import CreateMonitorRequest, UpdateMonitorRequest from fastmcp import FastMCP +from pydantic import Field from devhelm_mcp.client import ( ToolResult, as_payload, - format_error, get_client, + raise_tool_error, serialize, ) +# Wire-format value of ``ManagedBy.MCP``. Hard-coded as a string (rather than +# pulled from ``devhelm.types.ManagedBy``) because the SDK enum may lag behind +# the API enum during the spec-sync release window — the API is the source of +# truth for the value, and pinning the literal here keeps the MCP server +# attribution working even when the SDK rebuild hasn't shipped yet. +_MCP_MANAGED_BY = "MCP" + + +class _McpCreateMonitorRequest(CreateMonitorRequest): + """``CreateMonitorRequest`` with ``managed_by`` hidden from MCP callers. + + The MCP server *always* sets ``managedBy="MCP"`` on the API call so the + dashboard can attribute the monitor to its real origin (an AI agent), + rather than letting the LLM thread an arbitrary value through. This + subclass: + + 1. Re-declares ``managed_by`` as optional (``default=None``) so a body + that omits it passes Pydantic validation — the parent class makes the + field required, which would force the LLM to set it. + 2. Marks the field with ``exclude=True`` so any value the LLM does + smuggle in via a permissive client never reaches ``model_dump()``. + The server-side ``managedBy`` injection in :func:`create_monitor` is + the only writer that survives the boundary. + + The field is also stripped from the JSON Schema FastMCP advertises (see + ``server.py`` post-registration step), so well-behaved LLMs never see + ``managedBy`` as a callable parameter at all. + """ + + managed_by: Any = Field(default=None, alias="managedBy", exclude=True) + def register(mcp: FastMCP) -> None: @mcp.tool() @@ -22,7 +56,7 @@ def list_monitors(api_token: str | None = None) -> ToolResult: try: return serialize(get_client(api_token).monitors.list()) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def get_monitor(monitor_id: str, api_token: str | None = None) -> ToolResult: @@ -30,21 +64,32 @@ def get_monitor(monitor_id: str, api_token: str | None = None) -> ToolResult: try: return serialize(get_client(api_token).monitors.get(monitor_id)) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def create_monitor( - body: CreateMonitorRequest, api_token: str | None = None + body: _McpCreateMonitorRequest, api_token: str | None = None ) -> ToolResult: """Create a new uptime monitor. Required fields: name, type (HTTP/DNS/TCP/ICMP/MCP/HEARTBEAT), config (type-specific), frequencySeconds (30-86400). + + ``managedBy`` is set automatically to ``MCP`` server-side; callers + cannot override it. Use the SDK or CLI directly if you need a + different attribution. """ try: - return serialize(get_client(api_token).monitors.create(as_payload(body))) + payload = as_payload(body) + # Belt-and-suspenders: even if a permissive client manages to + # smuggle ``managedBy`` past the schema strip, drop it before + # the SDK call so server-side attribution is *guaranteed*. + payload.pop("managedBy", None) + payload.pop("managed_by", None) + payload["managedBy"] = _MCP_MANAGED_BY + return serialize(get_client(api_token).monitors.create(payload)) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def update_monitor( @@ -58,7 +103,7 @@ def update_monitor( get_client(api_token).monitors.update(monitor_id, as_payload(body)) ) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def delete_monitor(monitor_id: str, api_token: str | None = None) -> str: @@ -67,7 +112,7 @@ def delete_monitor(monitor_id: str, api_token: str | None = None) -> str: get_client(api_token).monitors.delete(monitor_id) return "Monitor deleted successfully." except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def pause_monitor(monitor_id: str, api_token: str | None = None) -> ToolResult: @@ -75,7 +120,7 @@ def pause_monitor(monitor_id: str, api_token: str | None = None) -> ToolResult: try: return serialize(get_client(api_token).monitors.pause(monitor_id)) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def resume_monitor(monitor_id: str, api_token: str | None = None) -> ToolResult: @@ -83,7 +128,7 @@ def resume_monitor(monitor_id: str, api_token: str | None = None) -> ToolResult: try: return serialize(get_client(api_token).monitors.resume(monitor_id)) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def test_monitor(monitor_id: str, api_token: str | None = None) -> ToolResult: @@ -91,7 +136,7 @@ def test_monitor(monitor_id: str, api_token: str | None = None) -> ToolResult: try: return serialize(get_client(api_token).monitors.test(monitor_id)) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def list_monitor_results( @@ -107,7 +152,7 @@ def list_monitor_results( ) return serialize({"data": page.data, "next_cursor": page.next_cursor}) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def list_monitor_versions( @@ -123,4 +168,4 @@ def list_monitor_versions( ) return serialize({"data": result.data, "hasNext": result.has_next}) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) diff --git a/src/devhelm_mcp/tools/notification_policies.py b/src/devhelm_mcp/tools/notification_policies.py index b4764cd..5bda54b 100644 --- a/src/devhelm_mcp/tools/notification_policies.py +++ b/src/devhelm_mcp/tools/notification_policies.py @@ -12,8 +12,8 @@ from devhelm_mcp.client import ( ToolResult, as_payload, - format_error, get_client, + raise_tool_error, serialize, ) @@ -25,7 +25,7 @@ def list_notification_policies(api_token: str | None = None) -> ToolResult: try: return serialize(get_client(api_token).notification_policies.list()) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def get_notification_policy( @@ -35,7 +35,7 @@ def get_notification_policy( try: return serialize(get_client(api_token).notification_policies.get(policy_id)) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def create_notification_policy( @@ -53,7 +53,7 @@ def create_notification_policy( get_client(api_token).notification_policies.create(as_payload(body)) ) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def update_notification_policy( @@ -69,7 +69,7 @@ def update_notification_policy( ) ) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def delete_notification_policy(policy_id: str, api_token: str | None = None) -> str: @@ -78,7 +78,7 @@ def delete_notification_policy(policy_id: str, api_token: str | None = None) -> get_client(api_token).notification_policies.delete(policy_id) return "Notification policy deleted successfully." except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def test_notification_policy(policy_id: str, api_token: str | None = None) -> str: @@ -87,4 +87,4 @@ def test_notification_policy(policy_id: str, api_token: str | None = None) -> st get_client(api_token).notification_policies.test(policy_id) return "Test dispatch sent successfully." except DevhelmError as e: - return format_error(e) + raise_tool_error(e) diff --git a/src/devhelm_mcp/tools/resource_groups.py b/src/devhelm_mcp/tools/resource_groups.py index a301648..a49f849 100644 --- a/src/devhelm_mcp/tools/resource_groups.py +++ b/src/devhelm_mcp/tools/resource_groups.py @@ -13,8 +13,8 @@ from devhelm_mcp.client import ( ToolResult, as_payload, - format_error, get_client, + raise_tool_error, serialize, ) @@ -26,7 +26,7 @@ def list_resource_groups(api_token: str | None = None) -> ToolResult: try: return serialize(get_client(api_token).resource_groups.list()) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def get_resource_group(group_id: str, api_token: str | None = None) -> ToolResult: @@ -34,7 +34,7 @@ def get_resource_group(group_id: str, api_token: str | None = None) -> ToolResul try: return serialize(get_client(api_token).resource_groups.get(group_id)) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def create_resource_group( @@ -50,7 +50,7 @@ def create_resource_group( get_client(api_token).resource_groups.create(as_payload(body)) ) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def update_resource_group( @@ -64,7 +64,7 @@ def update_resource_group( get_client(api_token).resource_groups.update(group_id, as_payload(body)) ) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def delete_resource_group(group_id: str, api_token: str | None = None) -> str: @@ -73,7 +73,7 @@ def delete_resource_group(group_id: str, api_token: str | None = None) -> str: get_client(api_token).resource_groups.delete(group_id) return "Resource group deleted successfully." except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def add_resource_group_member( @@ -92,7 +92,7 @@ def add_resource_group_member( ) ) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def remove_resource_group_member( @@ -105,4 +105,4 @@ def remove_resource_group_member( get_client(api_token).resource_groups.remove_member(group_id, member_id) return "Member removed from resource group." except DevhelmError as e: - return format_error(e) + raise_tool_error(e) diff --git a/src/devhelm_mcp/tools/secrets.py b/src/devhelm_mcp/tools/secrets.py index edeea17..5a2363c 100644 --- a/src/devhelm_mcp/tools/secrets.py +++ b/src/devhelm_mcp/tools/secrets.py @@ -9,8 +9,8 @@ from devhelm_mcp.client import ( ToolResult, as_payload, - format_error, get_client, + raise_tool_error, serialize, ) @@ -22,7 +22,7 @@ def list_secrets(api_token: str | None = None) -> ToolResult: try: return serialize(get_client(api_token).secrets.list()) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def create_secret( @@ -36,7 +36,7 @@ def create_secret( try: return serialize(get_client(api_token).secrets.create(as_payload(body))) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def update_secret( @@ -50,7 +50,7 @@ def update_secret( get_client(api_token).secrets.update(key, as_payload(body)) ) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def delete_secret(key: str, api_token: str | None = None) -> str: @@ -59,4 +59,4 @@ def delete_secret(key: str, api_token: str | None = None) -> str: get_client(api_token).secrets.delete(key) return "Secret deleted successfully." except DevhelmError as e: - return format_error(e) + raise_tool_error(e) diff --git a/src/devhelm_mcp/tools/status.py b/src/devhelm_mcp/tools/status.py index 8253ff4..d1164b8 100644 --- a/src/devhelm_mcp/tools/status.py +++ b/src/devhelm_mcp/tools/status.py @@ -5,7 +5,7 @@ from devhelm import DevhelmError from fastmcp import FastMCP -from devhelm_mcp.client import ToolResult, format_error, get_client, serialize +from devhelm_mcp.client import ToolResult, get_client, raise_tool_error, serialize def register(mcp: FastMCP) -> None: @@ -16,4 +16,4 @@ def get_status_overview(api_token: str | None = None) -> ToolResult: try: return serialize(get_client(api_token).status.overview()) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) diff --git a/src/devhelm_mcp/tools/status_pages.py b/src/devhelm_mcp/tools/status_pages.py index b64500d..13af01f 100644 --- a/src/devhelm_mcp/tools/status_pages.py +++ b/src/devhelm_mcp/tools/status_pages.py @@ -24,8 +24,8 @@ from devhelm_mcp.client import ( ToolResult, as_payload, - format_error, get_client, + raise_tool_error, serialize, ) @@ -44,7 +44,7 @@ def list_status_pages(api_token: str | None = None) -> ToolResult: try: return serialize(_sp(api_token).list()) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def get_status_page(page_id: str, api_token: str | None = None) -> ToolResult: @@ -52,7 +52,7 @@ def get_status_page(page_id: str, api_token: str | None = None) -> ToolResult: try: return serialize(_sp(api_token).get(page_id)) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def create_status_page( @@ -67,7 +67,7 @@ def create_status_page( try: return serialize(_sp(api_token).create(as_payload(body))) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def update_status_page( @@ -79,7 +79,7 @@ def update_status_page( try: return serialize(_sp(api_token).update(page_id, as_payload(body))) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def delete_status_page(page_id: str, api_token: str | None = None) -> str: @@ -88,7 +88,7 @@ def delete_status_page(page_id: str, api_token: str | None = None) -> str: _sp(api_token).delete(page_id) return "Status page deleted successfully." except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def reorder_status_page_layout( @@ -113,7 +113,7 @@ def reorder_status_page_layout( _sp(api_token).reorder_layout(page_id, as_payload(body)) return "Status page layout reordered successfully." except DevhelmError as e: - return format_error(e) + raise_tool_error(e) # ── Components ──────────────────────────────────────────────────────── @@ -125,7 +125,7 @@ def list_status_page_components( try: return serialize(_sp(api_token).components.list(page_id)) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def create_status_page_component( @@ -143,7 +143,7 @@ def create_status_page_component( _sp(api_token).components.create(page_id, as_payload(body)) ) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def update_status_page_component( @@ -160,7 +160,7 @@ def update_status_page_component( ) ) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def delete_status_page_component( @@ -173,7 +173,7 @@ def delete_status_page_component( _sp(api_token).components.delete(page_id, component_id) return "Component deleted successfully." except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def reorder_status_page_components( @@ -191,7 +191,7 @@ def reorder_status_page_components( _sp(api_token).components.reorder(page_id, as_payload(body)) return "Components reordered successfully." except DevhelmError as e: - return format_error(e) + raise_tool_error(e) # ── Component Groups ────────────────────────────────────────────────── @@ -203,7 +203,7 @@ def list_status_page_groups( try: return serialize(_sp(api_token).groups.list(page_id)) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def create_status_page_group( @@ -218,7 +218,7 @@ def create_status_page_group( try: return serialize(_sp(api_token).groups.create(page_id, as_payload(body))) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def update_status_page_group( @@ -233,7 +233,7 @@ def update_status_page_group( _sp(api_token).groups.update(page_id, group_id, as_payload(body)) ) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def delete_status_page_group( @@ -244,7 +244,7 @@ def delete_status_page_group( _sp(api_token).groups.delete(page_id, group_id) return "Component group deleted successfully." except DevhelmError as e: - return format_error(e) + raise_tool_error(e) # ── Incidents ───────────────────────────────────────────────────────── @@ -260,7 +260,7 @@ def list_status_page_incidents( result = _sp(api_token).incidents.list(page_id, page=page, size=size) return serialize({"data": result.data, "hasNext": result.has_next}) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def get_status_page_incident( @@ -272,7 +272,7 @@ def get_status_page_incident( try: return serialize(_sp(api_token).incidents.get(page_id, incident_id)) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def create_status_page_incident( @@ -289,7 +289,7 @@ def create_status_page_incident( try: return serialize(_sp(api_token).incidents.create(page_id, as_payload(body))) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def update_status_page_incident( @@ -304,7 +304,7 @@ def update_status_page_incident( _sp(api_token).incidents.update(page_id, incident_id, as_payload(body)) ) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def post_status_page_incident_update( @@ -326,7 +326,7 @@ def post_status_page_incident_update( ) ) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def publish_status_page_incident( @@ -342,7 +342,7 @@ def publish_status_page_incident( try: return serialize(_sp(api_token).incidents.publish(page_id, incident_id)) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def dismiss_status_page_incident( @@ -355,7 +355,7 @@ def dismiss_status_page_incident( _sp(api_token).incidents.dismiss(page_id, incident_id) return "Draft incident dismissed successfully." except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def delete_status_page_incident( @@ -368,7 +368,7 @@ def delete_status_page_incident( _sp(api_token).incidents.delete(page_id, incident_id) return "Status page incident deleted successfully." except DevhelmError as e: - return format_error(e) + raise_tool_error(e) # ── Subscribers ─────────────────────────────────────────────────────── @@ -384,7 +384,7 @@ def list_status_page_subscribers( result = _sp(api_token).subscribers.list(page_id, page=page, size=size) return serialize({"data": result.data, "hasNext": result.has_next}) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def add_status_page_subscriber( @@ -399,7 +399,7 @@ def add_status_page_subscriber( try: return serialize(_sp(api_token).subscribers.add(page_id, as_payload(body))) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def remove_status_page_subscriber( @@ -412,7 +412,7 @@ def remove_status_page_subscriber( _sp(api_token).subscribers.remove(page_id, subscriber_id) return "Subscriber removed successfully." except DevhelmError as e: - return format_error(e) + raise_tool_error(e) # ── Custom Domains ──────────────────────────────────────────────────── @@ -424,7 +424,7 @@ def list_status_page_domains( try: return serialize(_sp(api_token).domains.list(page_id)) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def add_status_page_domain( @@ -441,7 +441,7 @@ def add_status_page_domain( try: return serialize(_sp(api_token).domains.add(page_id, as_payload(body))) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def verify_status_page_domain( @@ -456,7 +456,7 @@ def verify_status_page_domain( try: return serialize(_sp(api_token).domains.verify(page_id, domain_id)) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def remove_status_page_domain( @@ -467,4 +467,4 @@ def remove_status_page_domain( _sp(api_token).domains.remove(page_id, domain_id) return "Custom domain removed successfully." except DevhelmError as e: - return format_error(e) + raise_tool_error(e) diff --git a/src/devhelm_mcp/tools/tags.py b/src/devhelm_mcp/tools/tags.py index f623e8b..ba80793 100644 --- a/src/devhelm_mcp/tools/tags.py +++ b/src/devhelm_mcp/tools/tags.py @@ -9,8 +9,8 @@ from devhelm_mcp.client import ( ToolResult, as_payload, - format_error, get_client, + raise_tool_error, serialize, ) @@ -22,7 +22,7 @@ def list_tags(api_token: str | None = None) -> ToolResult: try: return serialize(get_client(api_token).tags.list()) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def get_tag(tag_id: str, api_token: str | None = None) -> ToolResult: @@ -30,7 +30,7 @@ def get_tag(tag_id: str, api_token: str | None = None) -> ToolResult: try: return serialize(get_client(api_token).tags.get(tag_id)) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def create_tag(body: CreateTagRequest, api_token: str | None = None) -> ToolResult: @@ -41,7 +41,7 @@ def create_tag(body: CreateTagRequest, api_token: str | None = None) -> ToolResu try: return serialize(get_client(api_token).tags.create(as_payload(body))) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def update_tag( @@ -53,7 +53,7 @@ def update_tag( get_client(api_token).tags.update(tag_id, as_payload(body)) ) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def delete_tag(tag_id: str, api_token: str | None = None) -> str: @@ -62,4 +62,4 @@ def delete_tag(tag_id: str, api_token: str | None = None) -> str: get_client(api_token).tags.delete(tag_id) return "Tag deleted successfully." except DevhelmError as e: - return format_error(e) + raise_tool_error(e) diff --git a/src/devhelm_mcp/tools/webhooks.py b/src/devhelm_mcp/tools/webhooks.py index 3eb2e50..389e203 100644 --- a/src/devhelm_mcp/tools/webhooks.py +++ b/src/devhelm_mcp/tools/webhooks.py @@ -9,8 +9,8 @@ from devhelm_mcp.client import ( ToolResult, as_payload, - format_error, get_client, + raise_tool_error, serialize, ) @@ -22,7 +22,7 @@ def list_webhooks(api_token: str | None = None) -> ToolResult: try: return serialize(get_client(api_token).webhooks.list()) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def get_webhook(webhook_id: str, api_token: str | None = None) -> ToolResult: @@ -30,7 +30,7 @@ def get_webhook(webhook_id: str, api_token: str | None = None) -> ToolResult: try: return serialize(get_client(api_token).webhooks.get(webhook_id)) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def create_webhook( @@ -44,7 +44,7 @@ def create_webhook( try: return serialize(get_client(api_token).webhooks.create(as_payload(body))) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def update_webhook( @@ -58,7 +58,7 @@ def update_webhook( get_client(api_token).webhooks.update(webhook_id, as_payload(body)) ) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def delete_webhook(webhook_id: str, api_token: str | None = None) -> str: @@ -67,7 +67,7 @@ def delete_webhook(webhook_id: str, api_token: str | None = None) -> str: get_client(api_token).webhooks.delete(webhook_id) return "Webhook deleted successfully." except DevhelmError as e: - return format_error(e) + raise_tool_error(e) @mcp.tool() def test_webhook(webhook_id: str, api_token: str | None = None) -> ToolResult: @@ -75,4 +75,4 @@ def test_webhook(webhook_id: str, api_token: str | None = None) -> ToolResult: try: return serialize(get_client(api_token).webhooks.test(webhook_id)) except DevhelmError as e: - return format_error(e) + raise_tool_error(e) diff --git a/tests/test_cli.py b/tests/test_cli.py index 019f971..8318fc3 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -144,7 +144,11 @@ def test_no_args_runs_stdio_transport( patch("uvicorn.run") as uvi_run, ): main([]) - run.assert_called_once_with() + # Banner suppression on stdio is mandatory now (DevEx P2.Bug9): + # FastMCP's ASCII banner shows up as the first line in MCP + # client logs and includes a third-party promo URL, which + # confuses Cursor / Claude Desktop users tailing the log. + run.assert_called_once_with(show_banner=False) uvi_run.assert_not_called() def test_transport_http_invokes_uvicorn_with_resolved_host_port( diff --git a/tests/test_devex_round3_fixes.py b/tests/test_devex_round3_fixes.py new file mode 100644 index 0000000..b8a9eb1 --- /dev/null +++ b/tests/test_devex_round3_fixes.py @@ -0,0 +1,400 @@ +"""Round-3 DevEx fixes — pinned regression tests. + +Each ``TestX`` class below pins one fix from the round-3 audit. Keeping the +coverage in a dedicated file (rather than scattered across ``test_tools``, +``test_client``, etc.) makes it obvious which behavior survives re-shuffles +of the SDK or FastMCP framework. + +Bug index (mirrors PR #18 description): + P0.Bug5 + P1.Bug4 + P1.Bug5 — ``managedBy="MCP"`` is server-set + P1.Bug3 — upstream API errors surface as ``isError=True`` + P2.Bug7 — ``api_token`` is hidden from ``inputSchema`` + P2.Bug8 — ``serverInfo.version`` reports the package + P2.Bug9 — FastMCP banner suppressed on stdio + P1.Bug6 — ``/mcp`` and ``/mcp/`` both return 200, no 307 +""" + +from __future__ import annotations + +import asyncio +from typing import Any +from unittest.mock import MagicMock, patch + +import pytest +from devhelm import DevhelmApiError, DevhelmTransportError +from fastmcp import Client +from starlette.testclient import TestClient + +from devhelm_mcp.server import _package_version, app, mcp + +RegisteredTools = dict[str, Any] + + +@pytest.fixture(scope="module") +def registered_tools() -> RegisteredTools: + tools = asyncio.run(mcp.list_tools()) + return {t.name: t for t in tools} + + +# --------------------------------------------------------------------------- # +# P0.Bug5 + P1.Bug4 + P1.Bug5 — managedBy is forced server-side to "MCP" +# --------------------------------------------------------------------------- # + + +class TestCreateMonitorAlwaysSetsManagedByMcp: + """``create_monitor`` is the AI agent's single entry into the monitor + table — every row it creates must be attributed to ``managedBy="MCP"`` + so the dashboard can filter / count agent-created monitors honestly. + + The fix is two-layer: + 1. Hide ``managedBy`` from the JSON Schema FastMCP advertises so the + LLM never sees the field as a callable parameter. + 2. ``pop()`` the field from the payload server-side and inject ``"MCP"`` + before forwarding to the SDK, so a permissive client that smuggles + the field through still gets overridden. + """ + + def _create_monitor_body_schema( + self, registered_tools: RegisteredTools + ) -> dict[str, Any]: + schema = registered_tools["create_monitor"].parameters + body = schema["properties"]["body"] + if "$ref" in body: + ref = body["$ref"].rsplit("/", 1)[-1] + return schema["$defs"][ref] # type: ignore[no-any-return] + return body # type: ignore[no-any-return] + + def test_managed_by_not_in_body_schema( + self, registered_tools: RegisteredTools + ) -> None: + body = self._create_monitor_body_schema(registered_tools) + assert "managedBy" not in body.get("properties", {}), ( + "managedBy must be hidden from the create_monitor input schema" + ) + assert "managedBy" not in body.get("required", []), ( + "managedBy must not be required (it's server-set)" + ) + + def test_payload_carries_managed_by_mcp_when_omitted(self) -> None: + captured: dict[str, Any] = {} + + def fake_create(payload: dict[str, Any]) -> dict[str, Any]: + captured["payload"] = payload + return {"id": "mon_x", **payload} + + mock_client = MagicMock() + mock_client.monitors.create.side_effect = fake_create + + with patch("devhelm_mcp.tools.monitors.get_client", return_value=mock_client): + asyncio.run( + mcp.call_tool( + "create_monitor", + { + "body": { + "name": "test", + "type": "HTTP", + "config": { + "url": "https://example.com", + "method": "GET", + }, + }, + }, + ) + ) + + assert captured["payload"]["managedBy"] == "MCP" + + def test_payload_overrides_managed_by_when_caller_smuggles_one(self) -> None: + # A permissive HTTP client could still POST a body with ``managedBy`` + # set even though the schema doesn't expose it. Belt-and-suspenders: + # the server-side strip + inject must overwrite *any* value before + # the SDK call, so attribution is guaranteed. + captured: dict[str, Any] = {} + + def fake_create(payload: dict[str, Any]) -> dict[str, Any]: + captured["payload"] = payload + return {"id": "mon_x", **payload} + + mock_client = MagicMock() + mock_client.monitors.create.side_effect = fake_create + + with patch("devhelm_mcp.tools.monitors.get_client", return_value=mock_client): + asyncio.run( + mcp.call_tool( + "create_monitor", + { + "body": { + "name": "test", + "type": "HTTP", + "config": { + "url": "https://example.com", + "method": "GET", + }, + "managedBy": "DASHBOARD", + }, + }, + ) + ) + + assert captured["payload"]["managedBy"] == "MCP", ( + "managedBy must be overridden, not preserved" + ) + + +# --------------------------------------------------------------------------- # +# P1.Bug3 — upstream API errors surface with isError=True +# --------------------------------------------------------------------------- # + + +class TestUpstreamErrorsReportIsError: + """Per the MCP spec, tools that fail must set ``isError=true`` on the + ``CallToolResult``. Returning the formatted error as a regular tool + return value (the previous behavior) made every API failure look like + a successful tool call to the LLM, which then confidently reported + "monitor created" after a 4xx. + + This test exercises the full FastMCP middleware chain via ``Client`` so + we catch any future middleware that might swallow the ``ToolError``. + """ + + def _run_tool_and_get_result( + self, tool_name: str, arguments: dict[str, Any] + ) -> Any: + async def go() -> Any: + async with Client(mcp) as client: + return await client.call_tool( + tool_name, arguments, raise_on_error=False + ) + + return asyncio.run(go()) + + def test_api_error_returns_is_error_true(self) -> None: + mock_client = MagicMock() + mock_client.monitors.list.side_effect = DevhelmApiError( + "Monitor not found", + status=404, + code="NOT_FOUND", + request_id="req_abc123", + ) + + with patch("devhelm_mcp.tools.monitors.get_client", return_value=mock_client): + result = self._run_tool_and_get_result("list_monitors", {}) + + assert result.is_error is True + text = result.content[0].text + assert "ApiError (404 NOT_FOUND): Monitor not found" in text + # Request id must survive the wrap so users can quote it for support. + assert "request_id=req_abc123" in text + + def test_transport_error_returns_is_error_true(self) -> None: + mock_client = MagicMock() + mock_client.monitors.list.side_effect = DevhelmTransportError( + "connection refused" + ) + + with patch("devhelm_mcp.tools.monitors.get_client", return_value=mock_client): + result = self._run_tool_and_get_result("list_monitors", {}) + + assert result.is_error is True + assert "TransportError: connection refused" in result.content[0].text + + def test_delete_tool_also_sets_is_error_on_failure(self) -> None: + # ``delete_monitor`` returns a plain string on success (rather than a + # serialized dict) — make sure the ``isError`` wrap survives that + # narrower return-type contract too. + mock_client = MagicMock() + mock_client.monitors.delete.side_effect = DevhelmApiError( + "Insufficient permissions", + status=403, + code="FORBIDDEN", + ) + + with patch("devhelm_mcp.tools.monitors.get_client", return_value=mock_client): + result = self._run_tool_and_get_result( + "delete_monitor", {"monitor_id": "mon_x"} + ) + + assert result.is_error is True + assert "ApiError (403 FORBIDDEN)" in result.content[0].text + + +# --------------------------------------------------------------------------- # +# P2.Bug7 — api_token must not appear in any tool's inputSchema +# --------------------------------------------------------------------------- # + + +class TestApiTokenHiddenFromInputSchema: + """``api_token`` is still accepted as a Python kwarg for back-compat with + path-style ``/{api_key}/mcp`` clients (and direct test invocation), but + it must NEVER appear in the JSON Schema FastMCP advertises. Surfacing it + invited the LLM to populate the field from chat context, leaking the + user's token into prompt traces / model telemetry. + """ + + def test_no_tool_exposes_api_token_in_properties( + self, registered_tools: RegisteredTools + ) -> None: + offenders = [ + name + for name, tool in registered_tools.items() + if "api_token" in tool.parameters.get("properties", {}) + ] + assert not offenders, f"Tools still surfacing api_token to the LLM: {offenders}" + + def test_no_tool_requires_api_token( + self, registered_tools: RegisteredTools + ) -> None: + offenders = [ + name + for name, tool in registered_tools.items() + if "api_token" in tool.parameters.get("required", []) + ] + assert not offenders, ( + f"Tools still requiring api_token: {offenders} — must resolve " + "from header / env" + ) + + def test_api_token_still_accepted_as_python_kwarg(self) -> None: + # Path-style ``/{api_key}/mcp`` clients (and direct test calls) need + # to continue passing ``api_token`` through. The schema strip must + # not break the function signature. + captured: dict[str, Any] = {} + + def fake_list() -> list[dict[str, Any]]: + return [] + + mock_client = MagicMock() + mock_client.monitors.list.side_effect = fake_list + + def fake_get_client(api_token: str | None = None) -> MagicMock: + captured["api_token"] = api_token + return mock_client + + with patch( + "devhelm_mcp.tools.monitors.get_client", side_effect=fake_get_client + ): + asyncio.run(mcp.call_tool("list_monitors", {"api_token": "explicit_token"})) + + assert captured["api_token"] == "explicit_token" + + +# --------------------------------------------------------------------------- # +# P2.Bug8 — serverInfo.version reports the package, not the FastMCP framework +# --------------------------------------------------------------------------- # + + +class TestServerInfoVersion: + def test_fastmcp_version_pinned_to_package_version(self) -> None: + # The FastMCP server's ``version`` field flows directly into the + # MCP ``initialize`` handshake's ``serverInfo.version``. Before the + # round-3 fix it defaulted to the FastMCP framework version (e.g. + # ``"3.2.4"``), which made it impossible to correlate user reports + # with a specific MCP server release. + version = _package_version() + assert isinstance(version, str) + assert mcp.version == version + + def test_fastmcp_name_pinned_to_package_name(self) -> None: + # The MCP wire identity must match the package name so client + # configs / docs that point at "devhelm-mcp-server" actually find it. + assert mcp.name == "devhelm-mcp-server" + + def test_initialize_response_includes_server_version(self) -> None: + # End-to-end: drive the real Starlette app and assert + # ``serverInfo.version`` matches the package version. Anything less + # would let a future refactor that bypasses ``mcp.version`` slip + # through. + with TestClient(app) as client: + resp = client.post( + "/mcp/", + json={ + "jsonrpc": "2.0", + "id": 1, + "method": "initialize", + "params": { + "protocolVersion": "2024-11-05", + "capabilities": {}, + "clientInfo": {"name": "test", "version": "0.0.0"}, + }, + }, + headers={"Accept": "application/json, text/event-stream"}, + ) + assert resp.status_code == 200, resp.text + body_text = resp.text + # The handshake may come back as either JSON or an SSE event; both + # carry the same JSON-RPC body. + assert _package_version() in body_text + + +# --------------------------------------------------------------------------- # +# P1.Bug6 — POST /mcp and POST /mcp/ both reach the JSON-RPC handler +# --------------------------------------------------------------------------- # + + +class TestTrailingSlashNormalization: + """Naive HTTP clients drop the body and ``Authorization`` header on a + 307. Production observed every MCP client (Cursor, Claude Desktop, raw + curl) bouncing through ``/mcp`` → ``/mcp/`` and failing the JSON-RPC + handshake because the body was lost in flight. + + The middleware fix rewrites the ASGI scope before routing so both URLs + deliver to the inner streamable-HTTP handler with no redirect. + """ + + def _initialize_payload(self) -> dict[str, Any]: + return { + "jsonrpc": "2.0", + "id": 1, + "method": "initialize", + "params": { + "protocolVersion": "2024-11-05", + "capabilities": {}, + "clientInfo": {"name": "trailing-slash-test", "version": "0.0.0"}, + }, + } + + def test_post_mcp_no_trailing_slash_returns_200_not_307(self) -> None: + with TestClient(app) as client: + resp = client.post( + "/mcp", + json=self._initialize_payload(), + headers={"Accept": "application/json, text/event-stream"}, + follow_redirects=False, + ) + assert resp.status_code == 200, ( + f"Expected 200 (no redirect), got {resp.status_code}: {resp.text[:200]}" + ) + assert resp.headers.get("mcp-session-id"), ( + "initialize must hand out a session id" + ) + + def test_post_mcp_trailing_slash_returns_200(self) -> None: + with TestClient(app) as client: + resp = client.post( + "/mcp/", + json=self._initialize_payload(), + headers={"Accept": "application/json, text/event-stream"}, + follow_redirects=False, + ) + assert resp.status_code == 200, resp.text + assert resp.headers.get("mcp-session-id") + + def test_path_style_mcp_no_trailing_slash_returns_200(self) -> None: + # ``/{api_key}/mcp`` is the URL-only auth variant; same redirect + # bug applies, same fix has to cover it. + with TestClient(app) as client: + resp = client.post( + "/dh_test_token/mcp", + json=self._initialize_payload(), + headers={"Accept": "application/json, text/event-stream"}, + follow_redirects=False, + ) + assert resp.status_code == 200, resp.text + + def test_health_endpoint_unaffected(self) -> None: + # The path normalizer must only fire on the MCP family — health and + # any future top-level routes must pass through untouched. + with TestClient(app) as client: + resp = client.get("/health") + assert resp.status_code == 200 + assert resp.json()["status"] == "healthy" diff --git a/tests/test_tools.py b/tests/test_tools.py index cd395c8..4e53a02 100644 --- a/tests/test_tools.py +++ b/tests/test_tools.py @@ -266,19 +266,24 @@ def _required(self, registered_tools: RegisteredTools, name: str) -> list[str]: result: list[str] = tool.parameters.get("required", []) return result - def test_all_tools_accept_optional_api_token( + def test_api_token_hidden_from_input_schema( self, registered_tools: RegisteredTools ) -> None: - # ``api_token`` is now resolved from the request context (the - # ``Authorization: Bearer …`` header on the hosted ``/mcp`` endpoint, - # or ``DEVHELM_API_TOKEN`` for stdio), so it must NOT appear in the - # tool's ``required`` list — that was the bug behind the round-2 - # DevEx ``-32602: api_token Field required`` error. The parameter - # is still surfaced in ``properties`` so path-style callers can - # override it on a per-call basis. + # Round-3 DevEx (P2.Bug7): ``api_token`` MUST NOT appear in the + # JSON Schema FastMCP advertises. Surfacing it tempted LLMs to + # populate it from chat context, which leaked the user's API token + # into prompt-trace telemetry. After 0.7.0 the token is resolved + # exclusively from (1) the explicit Python kwarg for + # back-compat with path-style ``/{api_key}/mcp`` clients, + # (2) the ``Authorization: Bearer …`` header, or + # (3) the ``DEVHELM_API_TOKEN`` env var — the LLM never needs to + # see the field at all. for name in STATUS_PAGE_TOOLS: params = self._params(registered_tools, name) - assert "api_token" in params, f"{name} missing api_token parameter" + assert "api_token" not in params, ( + f"{name} should hide api_token from the input schema " + f"(resolved from header / env)" + ) required = self._required(registered_tools, name) assert "api_token" not in required, ( f"{name} should not require api_token (resolved from request)" diff --git a/uv.lock b/uv.lock index 3b9645d..02c3209 100644 --- a/uv.lock +++ b/uv.lock @@ -388,20 +388,20 @@ wheels = [ [[package]] name = "devhelm" -version = "0.6.0" +version = "0.6.1" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "httpx" }, { name = "pydantic", extra = ["email"] }, ] -sdist = { url = "https://files.pythonhosted.org/packages/ec/76/bb541d95824fd39ccdcaf7e57afe445cde2b6750636e4853592f90295b44/devhelm-0.6.0.tar.gz", hash = "sha256:fd99b1ce05553d4e4fcb6dd6bddaa11ce570f1dedf52b5e43f360e8ac6edec29", size = 233505, upload-time = "2026-05-01T20:03:21.833Z" } +sdist = { url = "https://files.pythonhosted.org/packages/68/97/8b65fac3e60668fda443cac606f5f87b2c2cd3df72795d8a0d819f68d18b/devhelm-0.6.1.tar.gz", hash = "sha256:a5cfd34fb71512a17823df5082cf2ba5ddf96431a0e80b8cdb7be2e47be5e9d9", size = 233744, upload-time = "2026-05-05T17:56:43.576Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/b4/c1/ea62aaf8304231026fd804d93892c42d2fa532676c1039c0a4431183fcb1/devhelm-0.6.0-py3-none-any.whl", hash = "sha256:0a832fbf50ec7c8847b1c4824fc55e1ef7ea550d9aacadf4b5f91872ea07f0dc", size = 73176, upload-time = "2026-05-01T20:03:20.686Z" }, + { url = "https://files.pythonhosted.org/packages/15/ad/dc8105f5b165f15a51603dd353af613ded1be342d3774d686f3a292ba4fe/devhelm-0.6.1-py3-none-any.whl", hash = "sha256:97fdd1165ca8c2491c10c6e67b6165b3ab743f7fb78b1c2cb3470b24a5a584c6", size = 73318, upload-time = "2026-05-05T17:56:42.031Z" }, ] [[package]] name = "devhelm-mcp-server" -version = "0.6.0" +version = "0.7.0" source = { editable = "." } dependencies = [ { name = "devhelm" },