diff --git a/api/controllers/mcp/mcp.py b/api/controllers/mcp/mcp.py index 6295bf00ebc..3d1aac65547 100644 --- a/api/controllers/mcp/mcp.py +++ b/api/controllers/mcp/mcp.py @@ -1,4 +1,4 @@ -from typing import Any +from typing import Any, Literal, get_args from flask import Response, request from flask_restx import Resource @@ -11,12 +11,20 @@ from controllers.mcp import mcp_ns from core.mcp import types as mcp_types from core.mcp.server.streamable_http import handle_mcp_request, negotiate_protocol_version from extensions.ext_database import db +from fields.base import ResponseModel from graphon.variables.input_entities import VariableEntity, VariableEntityType from libs import helper from models.enums import EndUserType from models.model import App, AppMCPServer, AppMode, EndUser from services.app.mcp_server_service import AppMCPServerStatus +_MCP_METHODS = frozenset( + method + for message_type in (mcp_types.ClientRequest, mcp_types.ClientNotification) + for model in get_args(message_type.model_fields["root"].annotation) + for method in get_args(model.model_fields["method"].annotation) +) + class MCPRequestError(Exception): """Custom exception for MCP request processing errors""" @@ -34,7 +42,13 @@ class MCPRequestPayload(BaseModel): id: int | str | None = Field(default=None, description="Request ID for tracking responses") -class MCPJSONRPCResponse(RootModel[mcp_types.JSONRPCResponse | mcp_types.JSONRPCError]): +class MCPJSONRPCErrorResponse(ResponseModel): + jsonrpc: Literal["2.0"] + id: int | str | None + error: mcp_types.ErrorData + + +class MCPJSONRPCResponse(RootModel[mcp_types.JSONRPCResponse | MCPJSONRPCErrorResponse]): pass @@ -50,7 +64,7 @@ class MCPAppApi(Resource): @mcp_ns.doc(params={"server_code": "Unique identifier for the MCP server"}) @mcp_ns.response(200, "MCP JSON-RPC response", mcp_ns.models[MCPJSONRPCResponse.__name__]) @mcp_ns.response(202, "MCP notification accepted") - @mcp_ns.response(400, "Invalid MCP request or parameters") + @mcp_ns.response(400, "Invalid MCP request or parameters", mcp_ns.models[MCPJSONRPCErrorResponse.__name__]) @mcp_ns.response(404, "Server or app not found") def post(self, server_code: str): """Handle MCP requests for a specific server. @@ -70,7 +84,21 @@ class MCPAppApi(Resource): # response-contract:ignore MCP route returns Flask Response from JSON-RPC handler args = MCPRequestPayload.model_validate(mcp_ns.payload or {}) request_id: int | str | None = args.id - mcp_request = self._parse_mcp_request(args.model_dump(exclude_none=True)) + try: + mcp_request = self._parse_mcp_request(args.model_dump(exclude_none=True)) + except MCPRequestError as e: + if "id" not in args.model_fields_set: + return Response("", status=202) + response = helper.compact_generate_response( + { + "jsonrpc": "2.0", + "id": request_id, + "error": {"code": e.error_code, "message": e.message}, + } + ) + # A legacy HTTP error lets modern clients fall back to initialize. + response.status_code = 400 + return response # Resolve the negotiated protocol version from the MCP-Protocol-Version header. is_initialize = isinstance(mcp_request.root, mcp_types.InitializeRequest) @@ -229,13 +257,15 @@ class MCPAppApi(Resource): def _parse_mcp_request(self, args: dict[str, Any]) -> mcp_types.ClientRequest | mcp_types.ClientNotification: """Parse and validate MCP request""" + if args.get("method") not in _MCP_METHODS: + raise MCPRequestError(mcp_types.METHOD_NOT_FOUND, "Method not found") try: return mcp_types.ClientRequest.model_validate(args) except ValidationError: try: return mcp_types.ClientNotification.model_validate(args) except ValidationError as e: - raise MCPRequestError(mcp_types.INVALID_PARAMS, f"Invalid MCP request: {str(e)}") + raise MCPRequestError(mcp_types.INVALID_PARAMS, "Invalid MCP request parameters") from e def _retrieve_end_user(self, tenant_id: str, mcp_server_id: str, session: Session) -> EndUser | None: """Get end user in the caller session""" diff --git a/api/tests/unit_tests/controllers/mcp/test_mcp.py b/api/tests/unit_tests/controllers/mcp/test_mcp.py index 0dd960de400..127552cdd5d 100644 --- a/api/tests/unit_tests/controllers/mcp/test_mcp.py +++ b/api/tests/unit_tests/controllers/mcp/test_mcp.py @@ -41,11 +41,11 @@ def app() -> Iterator[Flask]: @pytest.fixture(autouse=True) -def mock_mcp_ns(): +def mock_mcp_ns(monkeypatch: pytest.MonkeyPatch): fake_ns = types.SimpleNamespace() fake_ns.payload = None fake_ns.models = {} - module.mcp_ns = fake_ns + monkeypatch.setattr(module, "mcp_ns", fake_ns) @pytest.fixture @@ -208,14 +208,15 @@ class TestMCPAppApi: post_fn = unwrap(api.post) - with pytest.raises(module.MCPRequestError): - post_fn("server-1") + response = post_fn("server-1") + assert response.status_code == 202 + assert response.data == b"" def test_inactive_server(self): fake_payload( { "jsonrpc": "2.0", - "method": "test", + "method": "ping", "id": 1, "params": {}, } @@ -241,7 +242,7 @@ class TestMCPAppApi: with pytest.raises(ValidationError): post_fn("server-1") - def test_missing_request_id(self): + def test_unknown_method_without_request_id_is_ignored(self): fake_payload( { "jsonrpc": "2.0", @@ -258,8 +259,9 @@ class TestMCPAppApi: post_fn = unwrap(api.post) - with pytest.raises(module.MCPRequestError): - post_fn("server-1") + response = post_fn("server-1") + assert response.status_code == 202 + assert response.data == b"" def test_server_not_found(self): """Test when MCP server doesn't exist""" @@ -469,9 +471,13 @@ class TestMCPAppApi: post_fn = unwrap(api.post) - with pytest.raises(module.MCPRequestError) as exc_info: - post_fn("server-1") - assert "Invalid MCP request" in str(exc_info.value) + response = post_fn("server-1") + assert response.status_code == 400 + assert response.get_json() == { + "jsonrpc": "2.0", + "id": 1, + "error": {"code": module.mcp_types.METHOD_NOT_FOUND, "message": "Method not found"}, + } def test_validate_server_status_active(self): """Test successful server status validation""" diff --git a/api/tests/unit_tests/controllers/mcp/test_mcp_request_errors.py b/api/tests/unit_tests/controllers/mcp/test_mcp_request_errors.py new file mode 100644 index 00000000000..4f1789490aa --- /dev/null +++ b/api/tests/unit_tests/controllers/mcp/test_mcp_request_errors.py @@ -0,0 +1,188 @@ +"""MCP request parsing errors exercised through the real HTTP blueprint.""" + +from uuid import uuid4 + +import pytest +from flask import Flask +from flask.testing import FlaskClient +from sqlalchemy import select +from sqlalchemy.engine import Engine +from sqlalchemy.orm import Session + +import controllers.mcp as mcp_controller +import controllers.mcp.mcp as module +from models.engine import db +from models.enums import EndUserType +from models.model import App, AppMCPServer, AppMode, AppModelConfig, EndUser +from services.app.mcp_server_service import AppMCPServerStatus + +_URL = "/mcp/server/test-server/mcp" +_MODERN_HEADERS = { + "Accept": "application/json, text/event-stream", + "MCP-Protocol-Version": "2026-07-28", + "Mcp-Method": "server/discover", +} + + +@pytest.fixture +def app(sqlite_engine: Engine, monkeypatch: pytest.MonkeyPatch) -> Flask: + app = Flask(__name__) + app.config["SQLALCHEMY_DATABASE_URI"] = str(sqlite_engine.url) + db.init_app(app) + app.register_blueprint(mcp_controller.bp) + monkeypatch.setattr(module, "mcp_ns", mcp_controller.mcp_ns) + return app + + +@pytest.fixture +def client(app: Flask) -> FlaskClient: + return app.test_client() + + +@pytest.fixture +def published_server(sqlite_session: Session) -> AppMCPServer: + app_id = str(uuid4()) + tenant_id = str(uuid4()) + model_config = AppModelConfig(app_id=app_id, user_input_form="[]") + app = App( + id=app_id, + tenant_id=tenant_id, + name="test_app", + mode=AppMode.CHAT, + enable_site=False, + enable_api=True, + app_model_config_id=model_config.id, + ) + server = AppMCPServer( + tenant_id=tenant_id, + app_id=app_id, + name="test_app", + description="Test MCP app", + server_code="test-server", + status=AppMCPServerStatus.ACTIVE, + parameters="{}", + ) + sqlite_session.add_all([app, model_config, server]) + sqlite_session.commit() + return server + + +@pytest.mark.parametrize("request_id", [1, 0, "discover-1", None]) +def test_server_discover_returns_method_not_found(client: FlaskClient, request_id: int | str | None) -> None: + response = client.post( + _URL, + json={ + "jsonrpc": "2.0", + "id": request_id, + "method": "server/discover", + "params": { + "_meta": { + "io.modelcontextprotocol/protocolVersion": "2026-07-28", + "io.modelcontextprotocol/clientInfo": {"name": "modern-client", "version": "1.0"}, + "io.modelcontextprotocol/clientCapabilities": { + "extensions": {"io.modelcontextprotocol/ui": {"mimeTypes": ["text/html;profile=mcp-app"]}} + }, + } + }, + }, + headers=_MODERN_HEADERS, + ) + + assert response.status_code == 400 + body = response.get_json() + assert body == { + "jsonrpc": "2.0", + "id": request_id, + "error": {"code": -32601, "message": "Method not found"}, + } + assert type(body["id"]) is type(request_id) + + +def test_invalid_tool_params_return_safe_jsonrpc_error(client: FlaskClient) -> None: + response = client.post( + _URL, + json={ + "jsonrpc": "2.0", + "id": "tool-call-1", + "method": "tools/call", + "params": {"arguments": {"secret": "private-input-do-not-echo"}}, + }, + ) + + assert response.status_code == 400 + body = response.get_json() + assert body["jsonrpc"] == "2.0" + assert body["id"] == "tool-call-1" + assert body["error"]["code"] == -32602 + assert "private-input-do-not-echo" not in response.get_data(as_text=True) + assert "validation error" not in body["error"]["message"].lower() + + +@pytest.mark.parametrize("method", ["server/discover", "notifications/unknown", "tools/call"]) +def test_unparseable_notification_has_no_jsonrpc_response(client: FlaskClient, method: str) -> None: + response = client.post( + _URL, + json={"jsonrpc": "2.0", "method": method, "params": {}}, + headers={**_MODERN_HEADERS, "Mcp-Method": method}, + ) + + assert response.status_code == 202 + assert response.data == b"" + + +def test_legacy_initialize_and_list_tools_remain_compatible( + client: FlaskClient, published_server: AppMCPServer, sqlite_session: Session +) -> None: + response = client.post( + _URL, + json={ + "jsonrpc": "2.0", + "id": 0, + "method": "initialize", + "params": { + "protocolVersion": "2024-11-05", + "capabilities": {}, + "clientInfo": {"name": "legacy-client", "version": "1.0"}, + }, + }, + ) + + assert response.status_code == 200 + initialized = response.get_json() + assert initialized["id"] == 0 + assert initialized["result"]["protocolVersion"] == "2024-11-05" + end_user = sqlite_session.scalar(select(EndUser).where(EndUser.session_id == published_server.id)) + assert end_user is not None + assert end_user.tenant_id == published_server.tenant_id + assert end_user.app_id == published_server.app_id + assert end_user.type == EndUserType.MCP + assert end_user.name == "legacy-client@1.0" + + headers = {"MCP-Protocol-Version": "2024-11-05"} + notification = client.post( + _URL, + json={"jsonrpc": "2.0", "method": "notifications/initialized"}, + headers=headers, + ) + assert notification.status_code == 202 + assert notification.data == b"" + + listed = client.post( + _URL, + json={"jsonrpc": "2.0", "id": "list-1", "method": "tools/list"}, + headers=headers, + ) + assert listed.status_code == 200 + body = listed.get_json() + assert body["id"] == "list-1" + assert body["result"]["tools"] == [ + { + "name": "test_app", + "description": "Test MCP app", + "inputSchema": { + "type": "object", + "properties": {"query": {"type": "string", "description": "User Input/Question content"}}, + "required": ["query"], + }, + } + ]