From 04a199a3b200b7e18560ba48b01419f43e561fe4 Mon Sep 17 00:00:00 2001 From: RockChinQ Date: Fri, 3 Jul 2026 00:17:29 +0800 Subject: [PATCH] fix: preserve MCP server ids with slashes Fixes #2301 --- .../http/controller/groups/resources/mcp.py | 18 ++--- src/langbot/pkg/api/http/service/mcp.py | 11 +++ .../api/service/test_mcp_service.py | 21 ++++- tests/unit_tests/api/test_mcp_controller.py | 76 +++++++++++++++++++ .../home/mcp/components/mcp-form/MCPForm.tsx | 21 +++-- 5 files changed, 130 insertions(+), 17 deletions(-) create mode 100644 tests/unit_tests/api/test_mcp_controller.py diff --git a/src/langbot/pkg/api/http/controller/groups/resources/mcp.py b/src/langbot/pkg/api/http/controller/groups/resources/mcp.py index 4ee3f3e84..57fc06255 100644 --- a/src/langbot/pkg/api/http/controller/groups/resources/mcp.py +++ b/src/langbot/pkg/api/http/controller/groups/resources/mcp.py @@ -29,11 +29,11 @@ class MCPRouterGroup(group.RouterGroup): traceback.print_exc() return self.http_status(500, -1, f'Failed to create MCP server: {str(e)}') - @self.route('/servers/', methods=['GET', 'PUT', 'DELETE'], auth_type=group.AuthType.USER_TOKEN) + @self.route( + '/servers/', methods=['GET', 'PUT', 'DELETE'], auth_type=group.AuthType.USER_TOKEN + ) async def _(server_name: str) -> str: """获取、更新或删除MCP服务器配置""" - from urllib.parse import unquote - server_name = unquote(server_name) server_data = await self.ap.mcp_service.get_mcp_server_by_name(server_name) @@ -58,17 +58,15 @@ class MCPRouterGroup(group.RouterGroup): except Exception as e: return self.http_status(500, -1, f'Failed to delete MCP server: {str(e)}') - @self.route('/servers//test', methods=['POST'], auth_type=group.AuthType.USER_TOKEN) + @self.route('/servers//test', methods=['POST'], auth_type=group.AuthType.USER_TOKEN) async def _(server_name: str) -> str: """测试MCP服务器连接""" - from urllib.parse import unquote - server_name = unquote(server_name) server_data = await quart.request.json task_id = await self.ap.mcp_service.test_mcp_server(server_name=server_name, server_data=server_data) return self.success(data={'task_id': task_id}) - @self.route('/servers//resources', methods=['GET'], auth_type=group.AuthType.USER_TOKEN) + @self.route('/servers//resources', methods=['GET'], auth_type=group.AuthType.USER_TOKEN) async def _(server_name: str) -> str: """Get resources from an MCP server""" server_name = unquote(server_name) @@ -86,7 +84,9 @@ class MCPRouterGroup(group.RouterGroup): except Exception as e: return self.http_status(500, -1, f'Failed to get resources: {str(e)}') - @self.route('/servers//resource-templates', methods=['GET'], auth_type=group.AuthType.USER_TOKEN) + @self.route( + '/servers//resource-templates', methods=['GET'], auth_type=group.AuthType.USER_TOKEN + ) async def _(server_name: str) -> str: """Get resource templates from an MCP server""" server_name = unquote(server_name) @@ -96,7 +96,7 @@ class MCPRouterGroup(group.RouterGroup): except Exception as e: return self.http_status(500, -1, f'Failed to get resource templates: {str(e)}') - @self.route('/servers//resources/read', methods=['POST'], auth_type=group.AuthType.USER_TOKEN) + @self.route('/servers//resources/read', methods=['POST'], auth_type=group.AuthType.USER_TOKEN) async def _(server_name: str) -> str: """Read a resource from an MCP server""" server_name = unquote(server_name) diff --git a/src/langbot/pkg/api/http/service/mcp.py b/src/langbot/pkg/api/http/service/mcp.py index 22e281b6f..157294d48 100644 --- a/src/langbot/pkg/api/http/service/mcp.py +++ b/src/langbot/pkg/api/http/service/mcp.py @@ -48,6 +48,17 @@ class MCPService: if total_extensions >= max_extensions: raise ValueError(f'Maximum number of extensions ({max_extensions}) reached') + server_name = str(server_data.get('name') or '').strip() + if not server_name: + raise ValueError('MCP server name is required') + server_data['name'] = server_name + + existing_result = await self.ap.persistence_mgr.execute_async( + sqlalchemy.select(persistence_mcp.MCPServer).where(persistence_mcp.MCPServer.name == server_name) + ) + if existing_result.first() is not None: + raise ValueError(f'MCP server already exists: {server_name}') + server_data['uuid'] = str(uuid.uuid4()) await self.ap.persistence_mgr.execute_async(sqlalchemy.insert(persistence_mcp.MCPServer).values(server_data)) diff --git a/tests/unit_tests/api/service/test_mcp_service.py b/tests/unit_tests/api/service/test_mcp_service.py index 8e3e6cd98..ea08897a1 100644 --- a/tests/unit_tests/api/service/test_mcp_service.py +++ b/tests/unit_tests/api/service/test_mcp_service.py @@ -280,6 +280,25 @@ class TestMCPServiceCreateMCPServer: assert server_uuid is not None assert len(server_uuid) == 36 # UUID format + async def test_create_mcp_server_duplicate_name_raises(self): + """Rejects duplicate MCP server names.""" + # Setup + ap = SimpleNamespace() + ap.persistence_mgr = SimpleNamespace() + ap.instance_config = SimpleNamespace() + ap.instance_config.data = {'system': {'limitation': {'max_extensions': -1}}} + ap.tool_mgr = None + + existing_server = _create_mock_mcp_server(name='Existing Server') + ap.persistence_mgr.execute_async = AsyncMock(return_value=_create_mock_result(first_item=existing_server)) + ap.persistence_mgr.serialize_model = Mock(return_value={}) + + service = MCPService(ap) + + # Execute & Verify + with pytest.raises(ValueError, match='MCP server already exists: Existing Server'): + await service.create_mcp_server({'name': 'Existing Server'}) + async def test_create_mcp_server_loads_server(self): """Loads server into tool_mgr when enabled.""" # Setup @@ -301,7 +320,7 @@ class TestMCPServiceCreateMCPServer: nonlocal call_count call_count += 1 if call_count == 1: - return _create_mock_result([]) # Empty list for limit check + return _create_mock_result([]) # Empty result for duplicate-name check elif call_count == 2: return Mock() # Insert return _create_mock_result(first_item=server_entity) # Select created diff --git a/tests/unit_tests/api/test_mcp_controller.py b/tests/unit_tests/api/test_mcp_controller.py new file mode 100644 index 000000000..47f2247f2 --- /dev/null +++ b/tests/unit_tests/api/test_mcp_controller.py @@ -0,0 +1,76 @@ +from __future__ import annotations + +import sys +import types +from importlib import import_module +from types import SimpleNamespace +from unittest.mock import AsyncMock + +import pytest +import quart + +core_app_module = types.ModuleType('langbot.pkg.core.app') +core_app_module.Application = object +sys.modules.setdefault('langbot.pkg.core.app', core_app_module) + + +pytestmark = pytest.mark.asyncio + + +async def _create_test_client(mcp_service: SimpleNamespace): + app = quart.Quart(__name__) + user_service = SimpleNamespace( + verify_jwt_token=AsyncMock(return_value='test@example.com'), + get_user_by_email=AsyncMock(return_value=SimpleNamespace(user='test@example.com')), + ) + ap = SimpleNamespace(mcp_service=mcp_service, user_service=user_service) + MCPRouterGroup = import_module('langbot.pkg.api.http.controller.groups.resources.mcp').MCPRouterGroup + group = MCPRouterGroup(ap, app) + await group.initialize() + return app.test_client() + + +async def test_mcp_server_route_accepts_encoded_slash_name(): + mcp_service = SimpleNamespace( + get_mcp_server_by_name=AsyncMock( + return_value={ + 'uuid': 'test-uuid', + 'name': 'pab1it0/prometheus', + 'enable': True, + 'mode': 'stdio', + 'extra_args': {}, + } + ) + ) + client = await _create_test_client(mcp_service) + + response = await client.get( + '/api/v1/mcp/servers/pab1it0%2Fprometheus', + headers={'Authorization': 'Bearer test-token'}, + ) + + assert response.status_code == 200 + mcp_service.get_mcp_server_by_name.assert_awaited_once_with('pab1it0/prometheus') + payload = await response.get_json() + assert payload['data']['server']['name'] == 'pab1it0/prometheus' + + +async def test_mcp_resource_route_accepts_encoded_slash_name(): + mcp_service = SimpleNamespace( + get_mcp_server_by_name=AsyncMock(), + get_mcp_server_resources=AsyncMock(return_value=[]), + get_mcp_server_resource_templates=AsyncMock(return_value=[]), + get_runtime_info=AsyncMock(return_value={'resource_capabilities': {'subscribe': False}}), + ) + client = await _create_test_client(mcp_service) + + response = await client.get( + '/api/v1/mcp/servers/pab1it0%2Fprometheus/resources', + headers={'Authorization': 'Bearer test-token'}, + ) + + assert response.status_code == 200 + mcp_service.get_mcp_server_by_name.assert_not_awaited() + mcp_service.get_mcp_server_resources.assert_awaited_once_with('pab1it0/prometheus') + payload = await response.get_json() + assert payload['data']['resource_capabilities'] == {'subscribe': False} diff --git a/web/src/app/home/mcp/components/mcp-form/MCPForm.tsx b/web/src/app/home/mcp/components/mcp-form/MCPForm.tsx index a5a057d61..4b6188652 100644 --- a/web/src/app/home/mcp/components/mcp-form/MCPForm.tsx +++ b/web/src/app/home/mcp/components/mcp-form/MCPForm.tsx @@ -750,6 +750,8 @@ const MCPForm = forwardRef(function MCPForm( } try { let serverConfig: MCPServer; + const serverName = + isEditMode && initServerName ? initServerName : value.name; if (value.mode === 'remote') { const headers: Record = {}; @@ -758,7 +760,7 @@ const MCPForm = forwardRef(function MCPForm( }); serverConfig = { - name: value.name, + name: serverName, mode: 'remote', enable: true, extra_args: { @@ -774,7 +776,7 @@ const MCPForm = forwardRef(function MCPForm( }); serverConfig = { - name: value.name, + name: serverName, mode: 'stdio', enable: true, extra_args: { @@ -818,6 +820,8 @@ const MCPForm = forwardRef(function MCPForm( // `uvx` with no package (exit 2 / "Connection closed", no detail). // The form values are kept in sync on every edit and on load, so they // are always current. + const serverName = + isEditMode && initServerName ? initServerName : form.getValues('name'); const formExtraArgs = form.getValues('extra_args') ?? []; const formStdioArgs = form.getValues('args') ?? []; let extraArgsData: MCPServerExtraArgsRemote | MCPServerExtraArgsStdio; @@ -841,7 +845,7 @@ const MCPForm = forwardRef(function MCPForm( } const { task_id } = await httpClient.testMCPServer('_', { - name: form.getValues('name'), + name: serverName, mode, enable: true, extra_args: extraArgsData, @@ -873,7 +877,7 @@ const MCPForm = forwardRef(function MCPForm( }); } else { if (isEditMode) { - await loadServerForEdit(form.getValues('name')); + await loadServerForEdit(serverName); } else { // Create mode has no persisted server to reload tools from. // The backend stashes the discovered runtime info (status + @@ -1163,11 +1167,14 @@ const MCPForm = forwardRef(function MCPForm( ); + const persistedServerName = + isEditMode && initServerName ? initServerName : form.getValues('name'); + const runtimePanel = ( ); @@ -1211,7 +1218,7 @@ const MCPForm = forwardRef(function MCPForm( @@ -1223,7 +1230,7 @@ const MCPForm = forwardRef(function MCPForm(