From 47e5a275ef20d23f4f74038b6be2b59a113ed590 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 30 Jun 2026 18:49:19 +0800 Subject: [PATCH] feat(mcp): add pluggable service registry in common/mcp_service Adds common/mcp_service.py with the McpService dataclass (moved from main.py), BUILTIN_SERVICES list, _filter_by_env, and discover_and_filter. MCP_SERVICES env var acts as a whitelist (unset=all, empty=zero). Adds tests/unit/test_mcp_service.py with 12 cases covering: load order, broken-import handling (warn vs raise by env), missing/wrong-typed SERVICE, duplicate name and duplicate mount_path detection, env filter (unset/empty/subset/order), and unknown-name error. Co-Authored-By: Claude Fable 5 --- common/mcp_service.py | 131 +++++++++++++++++++++++ tests/unit/test_mcp_service.py | 184 +++++++++++++++++++++++++++++++++ 2 files changed, 315 insertions(+) create mode 100644 common/mcp_service.py create mode 100644 tests/unit/test_mcp_service.py diff --git a/common/mcp_service.py b/common/mcp_service.py new file mode 100644 index 0000000..4ce54c1 --- /dev/null +++ b/common/mcp_service.py @@ -0,0 +1,131 @@ +# coding=utf-8 +""" +@Time :2026/6/30 +@Author :tao.chen + +The MCP service registry: a single place that knows which FastAPI sub-apps +to mount under the root app and how to gate them by the MCP_SERVICES env +var. Each service package declares itself via a `service.py` exporting a +module-level `SERVICE: McpService` instance. +""" +from __future__ import annotations + +import importlib +import os +from dataclasses import dataclass +from typing import TYPE_CHECKING + +from common.logging import logger + +if TYPE_CHECKING: + from fastapi import FastAPI + + +# The codebase standardizes on loguru (see common.logging), but pytest's +# `caplog` fixture captures stdlib `logging` records, not loguru. We mirror +# the single skip-warning through a stdlib logger named `common.logging` +# (matching how `caplog.at_level(..., logger="common.logging")` is invoked +# in the tests) so the warning is assertable. A NullHandler keeps the +# stdlib logger silent in production; `caplog` attaches its own handler. +import logging as _logging +_stdlib_logger = _logging.getLogger("common.logging") +if not _stdlib_logger.handlers: + _stdlib_logger.addHandler(_logging.NullHandler()) +_stdlib_logger.propagate = False + + +@dataclass(frozen=True) +class McpService: + """One MCP-exposed FastAPI sub-app to be mounted under the root.""" + + name: str # short id used in startup logs and the MCP_SERVICES env whitelist + app: "FastAPI" # the sub-app whose routes fastapi-mcp exposes as tools + mount_path: str # HTTP path under the root app, e.g. "/spark-executor-mcp" + + +# Every known MCP service's import path. Add a new service by appending one +# line here and creating /service.py that exports SERVICE. +BUILTIN_SERVICES: list[str] = [ + "spark_executor.service", + "files_mcp.service", +] + + +def _filter_by_env(services: list[McpService]) -> list[McpService]: + """Whitelist by `MCP_SERVICES` env. Unset = no filter (backward compat). + Empty string = zero services (explicit operator choice, distinct from unset).""" + if "MCP_SERVICES" not in os.environ: + return services + raw = os.environ["MCP_SERVICES"] + if not raw: + return [] + allowed = {n.strip() for n in raw.split(",") if n.strip()} + return [s for s in services if s.name in allowed] + + +def discover_and_filter() -> list[McpService]: + """Import each BUILTIN_SERVICES module, read its SERVICE attribute, + then apply the env whitelist. + + Raises: + RuntimeError: see `Error Handling` in the spec. + """ + services: list[McpService] = [] + raw = os.environ.get("MCP_SERVICES") + requested = {n.strip() for n in raw.split(",") if n.strip()} if raw else set() + origins: dict[str, str] = {} # service.name -> import_path (for diagnostics) + + for import_path in BUILTIN_SERVICES: + try: + module = importlib.import_module(import_path) + except ImportError as e: + pkg_name = import_path.rsplit(".", 1)[0] + if pkg_name in requested or import_path in requested: + raise RuntimeError( + f"MCP service {import_path!r} is requested via MCP_SERVICES " + f"but cannot be imported: {e}" + ) from e + logger.warning(f"Skipping missing MCP service {import_path!r}: {e}") + _stdlib_logger.warning(f"Skipping missing MCP service {import_path!r}: {e}") + continue + svc = getattr(module, "SERVICE", None) + if svc is None: + raise RuntimeError( + f"{import_path!r} does not export a SERVICE attribute" + ) + if not isinstance(svc, McpService): + raise RuntimeError( + f"{import_path!r}.SERVICE is not a McpService " + f"(got {type(svc).__name__})" + ) + services.append(svc) + origins[svc.name] = import_path + + # Duplicate detection - surface typos at startup. + seen_names: dict[str, str] = {} + seen_mounts: dict[str, str] = {} + for s in services: + if s.name in seen_names: + raise RuntimeError( + f"duplicate MCP service name {s.name!r}: " + f"{seen_names[s.name]!r} and {origins[s.name]!r}" + ) + seen_names[s.name] = origins[s.name] + if s.mount_path in seen_mounts: + raise RuntimeError( + f"duplicate MCP mount_path {s.mount_path!r}: " + f"{seen_mounts[s.mount_path]!r} and {origins[s.name]!r}" + ) + seen_mounts[s.mount_path] = origins[s.name] + + filtered = _filter_by_env(services) + + found_names = {s.name for s in services} + missing = requested - found_names + if missing: + raise RuntimeError( + f"MCP_SERVICES env references unknown service names: " + f"{sorted(missing)}. Available: {sorted(found_names)}" + ) + + return filtered diff --git a/tests/unit/test_mcp_service.py b/tests/unit/test_mcp_service.py new file mode 100644 index 0000000..771719f --- /dev/null +++ b/tests/unit/test_mcp_service.py @@ -0,0 +1,184 @@ +# coding=utf-8 +""" +@Time :2026/6/30 +@Author :tao.chen +""" +import sys +import types + +import pytest +from fastapi import FastAPI + +from common import mcp_service +from common.mcp_service import McpService, _filter_by_env, discover_and_filter + + +# --- helpers ---------------------------------------------------------------- + + +def _fake_app(title: str = "Fake") -> FastAPI: + return FastAPI(title=title) + + +def _make_module(monkeypatch, name: str, *, service: object = None) -> None: + """Inject a fake module into sys.modules with the given SERVICE attribute. + + For "broken import" tests, the test author should set BUILTIN_SERVICES + to a name that is NOT injected into sys.modules, simulating ImportError. + """ + mod = types.ModuleType(name) + if service is not None: + mod.SERVICE = service + monkeypatch.setitem(sys.modules, name, mod) + + +@pytest.fixture +def reset_builtin(monkeypatch): + """Restore BUILTIN_SERVICES to a known good state after each test.""" + monkeypatch.setattr( + mcp_service, "BUILTIN_SERVICES", + ["fake_spark_executor", "fake_files"], + ) + + +@pytest.fixture +def reset_env(monkeypatch): + """Make sure MCP_SERVICES is unset at the start of each test.""" + monkeypatch.delenv("MCP_SERVICES", raising=False) + + +# --- TestDiscover ---------------------------------------------------------- + + +class TestDiscover: + def test_loads_builtin_services_in_order( + self, monkeypatch, reset_builtin, reset_env + ): + a = McpService(name="fake_spark_executor", app=_fake_app(), mount_path="/a-mcp") + b = McpService(name="fake_files", app=_fake_app(), mount_path="/b-mcp") + _make_module(monkeypatch, "fake_spark_executor", service=a) + _make_module(monkeypatch, "fake_files", service=b) + result = discover_and_filter() + assert [s.name for s in result] == ["fake_spark_executor", "fake_files"] + + def test_skips_broken_service_if_not_requested( + self, monkeypatch, reset_builtin, reset_env, caplog + ): + # "fake_spark_executor" is not importable; env doesn't mention it. + # Only "fake_files" is real. + b = McpService(name="fake_files", app=_fake_app(), mount_path="/b-mcp") + _make_module(monkeypatch, "fake_files", service=b) + with caplog.at_level("WARNING", logger="common.logging"): + result = discover_and_filter() + assert [s.name for s in result] == ["fake_files"] + assert any("fake_spark_executor" in r.message for r in caplog.records) + + def test_raises_on_broken_service_if_requested( + self, monkeypatch, reset_builtin, reset_env + ): + # "fake_spark_executor" is not importable AND operator asked for it. + monkeypatch.setenv("MCP_SERVICES", "fake_spark_executor") + b = McpService(name="fake_files", app=_fake_app(), mount_path="/b-mcp") + _make_module(monkeypatch, "fake_files", service=b) + with pytest.raises(RuntimeError, match="requested via MCP_SERVICES"): + discover_and_filter() + + def test_raises_when_service_module_lacks_SERVICE( + self, monkeypatch, reset_builtin, reset_env + ): + # Module exists but has no SERVICE attribute. + _make_module(monkeypatch, "fake_spark_executor", service=None) + b = McpService(name="fake_files", app=_fake_app(), mount_path="/b-mcp") + _make_module(monkeypatch, "fake_files", service=b) + with pytest.raises(RuntimeError, match="does not export a SERVICE"): + discover_and_filter() + + def test_raises_when_SERVICE_is_wrong_type( + self, monkeypatch, reset_builtin, reset_env + ): + # SERVICE attribute exists but is not a McpService. + _make_module(monkeypatch, "fake_spark_executor", service="not a McpService") + b = McpService(name="fake_files", app=_fake_app(), mount_path="/b-mcp") + _make_module(monkeypatch, "fake_files", service=b) + with pytest.raises(RuntimeError, match="is not a McpService"): + discover_and_filter() + + def test_raises_on_duplicate_service_names( + self, monkeypatch, reset_builtin, reset_env + ): + a = McpService(name="dup", app=_fake_app(), mount_path="/a-mcp") + b = McpService(name="dup", app=_fake_app(), mount_path="/b-mcp") + _make_module(monkeypatch, "fake_spark_executor", service=a) + _make_module(monkeypatch, "fake_files", service=b) + with pytest.raises(RuntimeError, match="duplicate MCP service name"): + discover_and_filter() + + def test_raises_on_duplicate_mount_paths( + self, monkeypatch, reset_builtin, reset_env + ): + a = McpService(name="alpha", app=_fake_app(), mount_path="/same") + b = McpService(name="beta", app=_fake_app(), mount_path="/same") + _make_module(monkeypatch, "fake_spark_executor", service=a) + _make_module(monkeypatch, "fake_files", service=b) + with pytest.raises(RuntimeError, match="duplicate MCP mount_path"): + discover_and_filter() + + +# --- TestFilter (unit-tests the private _filter_by_env directly) ---------- + + +class TestFilter: + def test_unset_env_returns_all(self, monkeypatch): + monkeypatch.delenv("MCP_SERVICES", raising=False) + svcs = [ + McpService(name="a", app=_fake_app(), mount_path="/a"), + McpService(name="b", app=_fake_app(), mount_path="/b"), + ] + assert _filter_by_env(svcs) == svcs + + def test_empty_env_returns_zero(self, monkeypatch): + monkeypatch.setenv("MCP_SERVICES", "") + svcs = [ + McpService(name="a", app=_fake_app(), mount_path="/a"), + McpService(name="b", app=_fake_app(), mount_path="/b"), + ] + assert _filter_by_env(svcs) == [] + + def test_whitelist_filters_by_name(self, monkeypatch): + monkeypatch.setenv("MCP_SERVICES", "a") + svcs = [ + McpService(name="a", app=_fake_app(), mount_path="/a"), + McpService(name="b", app=_fake_app(), mount_path="/b"), + ] + result = _filter_by_env(svcs) + assert [s.name for s in result] == ["a"] + + def test_whitelist_preserves_bulitin_order(self, monkeypatch): + # env order: "b,a"; result order must follow svcs order, not env order. + monkeypatch.setenv("MCP_SERVICES", "b,a") + svcs = [ + McpService(name="a", app=_fake_app(), mount_path="/a"), + McpService(name="b", app=_fake_app(), mount_path="/b"), + ] + result = _filter_by_env(svcs) + assert [s.name for s in result] == ["a", "b"] + + +# --- TestEnvErrors --------------------------------------------------------- + + +class TestEnvErrors: + def test_unknown_name_in_env_raises_with_available_list( + self, monkeypatch, reset_builtin, reset_env + ): + a = McpService(name="fake_spark_executor", app=_fake_app(), mount_path="/a-mcp") + b = McpService(name="fake_files", app=_fake_app(), mount_path="/b-mcp") + _make_module(monkeypatch, "fake_spark_executor", service=a) + _make_module(monkeypatch, "fake_files", service=b) + monkeypatch.setenv("MCP_SERVICES", "bogus") + with pytest.raises(RuntimeError) as exc_info: + discover_and_filter() + msg = str(exc_info.value) + assert "bogus" in msg + assert "fake_files" in msg + assert "fake_spark_executor" in msg