-
Notifications
You must be signed in to change notification settings - Fork 15
Add support for custom MCP server URLs #96
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 8 commits
Commits
Show all changes
27 commits
Select commit
Hold shift + click to select a range
d3fbcec
Initial plan
Copilot 7468ea0
Add support for custom MCP server URLs
Copilot dba9456
Add comprehensive tests for custom MCP server URL support
Copilot fe26a98
Remove unnecessary hasattr check in exception handler
Copilot 31f547a
Simplify custom URL checks by removing redundant strip calls
Copilot 967d25a
Improve exception handling and clarify test comments
Copilot 7ce0f25
Refactor to consistently store URLs in the url field
Copilot ccaa294
Use 'url' field consistently in both manifest and gateway responses
Copilot ec89302
Refactor gateway parsing to match manifest logic and improve variable…
Copilot 5f056f1
Fix formatting: remove extra blank line in test
Copilot cb3f7b8
Rename custom_url to endpoint for better clarity
Copilot 6ce6e4f
Remove fallback logic and use config.url directly in all tool registr…
Copilot 463d9bf
Remove redundant null check for server_url in Agent Framework service
Copilot ad365b0
Use 'or' operator for cleaner fallback logic in server_name assignment
Copilot 32039fc
Add clarifying comment for server_name fallback logic
Copilot cce7219
Fix undefined variable reference in log statement
Copilot 4a2bd66
Move server_name assignment outside try block to ensure it's always a…
Copilot 1cf6acc
Clarify comment for server_name fallback logic
Copilot b5eeb7e
Add server_name fallback logic in OpenAI MCP tool registration service
Copilot 6eb7901
Improve variable naming consistency and add server_name fallback in S…
Copilot 0195f73
Fix inconsistent server_name usage in Semantic Kernel service
Copilot 5cec8d5
Add server_name fallback logic in configuration service URL construction
Copilot 76790f1
Run ruff formatter to fix formatting issues
Copilot b7a9044
Update tests/tooling/test_mcp_server_configuration.py
pontemonti 025fb53
Update tests/tooling/test_mcp_server_configuration.py
pontemonti 2621e12
Update tests/tooling/test_mcp_server_configuration.py
pontemonti 6c86601
Merge branch 'main' into copilot/add-custom-mcp-server-support
pontemonti File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| # Copyright (c) Microsoft Corporation. | ||
| # Licensed under the MIT License. | ||
|
|
||
| """Tests for tooling components.""" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,233 @@ | ||
| # Copyright (c) Microsoft Corporation. | ||
| # Licensed under the MIT License. | ||
|
|
||
| """Unit tests for MCP Server Configuration Service.""" | ||
|
|
||
| import json | ||
| import os | ||
| from pathlib import Path | ||
|
pontemonti marked this conversation as resolved.
Outdated
|
||
| from typing import Dict, Any | ||
| from unittest.mock import Mock, patch, AsyncMock, MagicMock | ||
|
pontemonti marked this conversation as resolved.
Outdated
|
||
| import pytest | ||
| import aiohttp | ||
|
pontemonti marked this conversation as resolved.
Outdated
|
||
|
|
||
| from microsoft_agents_a365.tooling.services.mcp_tool_server_configuration_service import ( | ||
| McpToolServerConfigurationService, | ||
| ) | ||
| from microsoft_agents_a365.tooling.models import MCPServerConfig | ||
|
|
||
|
|
||
| class TestMCPServerConfig: | ||
| """Tests for MCPServerConfig model.""" | ||
|
|
||
| def test_mcp_server_config_with_custom_url(self): | ||
| """Test that MCPServerConfig can be created with a custom URL.""" | ||
| config = MCPServerConfig( | ||
| mcp_server_name="TestServer", | ||
| mcp_server_unique_name="test_server", | ||
| url="https://custom.mcp.server/endpoint", | ||
| ) | ||
|
|
||
| assert config.mcp_server_name == "TestServer" | ||
| assert config.mcp_server_unique_name == "test_server" | ||
| assert config.url == "https://custom.mcp.server/endpoint" | ||
|
|
||
| def test_mcp_server_config_without_custom_url(self): | ||
| """Test that MCPServerConfig works without a custom URL.""" | ||
| config = MCPServerConfig( | ||
| mcp_server_name="TestServer", | ||
| mcp_server_unique_name="test_server", | ||
| ) | ||
|
|
||
| assert config.mcp_server_name == "TestServer" | ||
| assert config.mcp_server_unique_name == "test_server" | ||
| assert config.url is None | ||
|
|
||
| def test_mcp_server_config_validation(self): | ||
| """Test that MCPServerConfig validates required fields.""" | ||
| with pytest.raises(ValueError, match="mcp_server_name cannot be empty"): | ||
| MCPServerConfig(mcp_server_name="", mcp_server_unique_name="test") | ||
|
|
||
| with pytest.raises(ValueError, match="mcp_server_unique_name cannot be empty"): | ||
| MCPServerConfig(mcp_server_name="test", mcp_server_unique_name="") | ||
|
|
||
|
|
||
| class TestMcpToolServerConfigurationService: | ||
| """Tests for McpToolServerConfigurationService.""" | ||
|
|
||
| @pytest.fixture | ||
| def service(self): | ||
| """Create a service instance for testing.""" | ||
| return McpToolServerConfigurationService() | ||
|
|
||
| @pytest.fixture | ||
| def mock_manifest_data(self) -> Dict[str, Any]: | ||
| """Create mock manifest data.""" | ||
| return { | ||
| "mcpServers": [ | ||
| { | ||
| "mcpServerName": "TestServer1", | ||
| "mcpServerUniqueName": "test_server_1", | ||
| }, | ||
| { | ||
| "mcpServerName": "TestServer2", | ||
| "mcpServerUniqueName": "test_server_2", | ||
| "url": "https://custom.server.com/mcp", | ||
| }, | ||
| ] | ||
| } | ||
|
|
||
| def test_extract_server_url_from_manifest(self, service): | ||
| """Test extracting custom URL from manifest element.""" | ||
| # Test with url field | ||
| element = {"url": "https://custom.url.com"} | ||
| url = service._extract_server_url(element) | ||
| assert url == "https://custom.url.com" | ||
|
|
||
| # Test with no URL | ||
| element = {} | ||
| url = service._extract_server_url(element) | ||
| assert url is None | ||
|
|
||
| def test_parse_manifest_server_config_with_custom_url(self, service): | ||
| """Test parsing manifest config with custom URL.""" | ||
| server_element = { | ||
| "mcpServerName": "CustomServer", | ||
| "mcpServerUniqueName": "custom_server", | ||
| "url": "https://my.custom.server/mcp", | ||
| } | ||
|
|
||
| config = service._parse_manifest_server_config(server_element) | ||
|
|
||
| assert config is not None | ||
| assert config.mcp_server_name == "CustomServer" | ||
| assert config.mcp_server_unique_name == "custom_server" | ||
| assert config.url == "https://my.custom.server/mcp" | ||
|
|
||
| @patch("microsoft_agents_a365.tooling.services.mcp_tool_server_configuration_service.build_mcp_server_url") | ||
| def test_parse_manifest_server_config_without_custom_url(self, mock_build_url, service): | ||
| """Test parsing manifest config without custom URL constructs URL.""" | ||
| mock_build_url.return_value = "https://default.server/agents/servers/test_server" | ||
|
|
||
| server_element = { | ||
| "mcpServerName": "DefaultServer", | ||
| "mcpServerUniqueName": "test_server", | ||
| } | ||
|
|
||
| config = service._parse_manifest_server_config(server_element) | ||
|
|
||
| assert config is not None | ||
| assert config.mcp_server_name == "DefaultServer" | ||
| assert config.mcp_server_unique_name == "test_server" | ||
| # Without a custom URL, build_mcp_server_url constructs the full URL and stores it in the url field | ||
| assert config.url == "https://default.server/agents/servers/test_server" | ||
| mock_build_url.assert_called_once_with("test_server") | ||
|
|
||
| def test_parse_gateway_server_config_with_custom_url(self, service): | ||
| """Test parsing gateway config with custom URL.""" | ||
| server_element = { | ||
| "mcpServerName": "GatewayServer", | ||
| "mcpServerUniqueName": "gateway_server_endpoint", | ||
| "url": "https://gateway.custom.url/mcp", | ||
| } | ||
|
|
||
| config = service._parse_gateway_server_config(server_element) | ||
|
|
||
| assert config is not None | ||
| assert config.mcp_server_name == "GatewayServer" | ||
| assert config.mcp_server_unique_name == "gateway_server_endpoint" | ||
| assert config.url == "https://gateway.custom.url/mcp" | ||
|
|
||
| def test_parse_gateway_server_config_without_custom_url(self, service): | ||
| """Test parsing gateway config without custom URL.""" | ||
| server_element = { | ||
| "mcpServerName": "GatewayServer", | ||
| "mcpServerUniqueName": "https://gateway.default/endpoint", | ||
| } | ||
|
|
||
| config = service._parse_gateway_server_config(server_element) | ||
|
|
||
| assert config is not None | ||
| assert config.mcp_server_name == "GatewayServer" | ||
| assert config.mcp_server_unique_name == "https://gateway.default/endpoint" | ||
| # Without a custom URL, the endpoint is used as the url | ||
| assert config.url == "https://gateway.default/endpoint" | ||
|
|
||
| @patch.dict(os.environ, {"ENVIRONMENT": "Development"}) | ||
| def test_is_development_scenario(self, service): | ||
| """Test development scenario detection.""" | ||
| assert service._is_development_scenario() is True | ||
|
|
||
| @patch.dict(os.environ, {"ENVIRONMENT": "Production"}) | ||
| def test_is_production_scenario(self, service): | ||
| """Test production scenario detection.""" | ||
| assert service._is_development_scenario() is False | ||
|
|
||
| @patch.object(McpToolServerConfigurationService, "_load_servers_from_manifest") | ||
| @patch.dict(os.environ, {"ENVIRONMENT": "Development"}) | ||
| @pytest.mark.asyncio | ||
| async def test_list_tool_servers_development(self, mock_load_manifest, service): | ||
| """Test listing servers in development mode.""" | ||
| mock_servers = [ | ||
| MCPServerConfig( | ||
| mcp_server_name="DevServer", | ||
| mcp_server_unique_name="dev_server", | ||
| url="https://dev.server/mcp", | ||
| ) | ||
| ] | ||
| mock_load_manifest.return_value = mock_servers | ||
|
|
||
| servers = await service.list_tool_servers( | ||
| agentic_app_id="test-app-id", auth_token="test-token" | ||
| ) | ||
|
|
||
| assert servers == mock_servers | ||
| mock_load_manifest.assert_called_once() | ||
|
|
||
| @patch("microsoft_agents_a365.tooling.services.mcp_tool_server_configuration_service.get_tooling_gateway_for_digital_worker") | ||
| @patch.dict(os.environ, {"ENVIRONMENT": "Production"}) | ||
| @pytest.mark.asyncio | ||
| async def test_list_tool_servers_production_with_custom_url(self, mock_gateway_url, service): | ||
| """Test listing servers in production mode with custom URL.""" | ||
| mock_gateway_url.return_value = "https://gateway.test/agents/test-app-id/mcpServers" | ||
|
|
||
| # Mock aiohttp response | ||
| mock_response_data = { | ||
| "mcpServers": [ | ||
| { | ||
| "mcpServerName": "ProdServer", | ||
| "mcpServerUniqueName": "prod_server", | ||
| "url": "https://prod.custom.url/mcp", | ||
| } | ||
| ] | ||
| } | ||
|
|
||
| with patch("aiohttp.ClientSession") as mock_session_class: | ||
| # Create proper async context managers | ||
| mock_response = MagicMock() | ||
| mock_response.status = 200 | ||
| mock_response.text = AsyncMock(return_value=json.dumps(mock_response_data)) | ||
|
|
||
| # Create async context manager for response | ||
| mock_response_cm = MagicMock() | ||
| mock_response_cm.__aenter__ = AsyncMock(return_value=mock_response) | ||
| mock_response_cm.__aexit__ = AsyncMock(return_value=None) | ||
|
|
||
| # Create async context manager for session | ||
| mock_session = MagicMock() | ||
| mock_session.get = MagicMock(return_value=mock_response_cm) | ||
|
|
||
| mock_session_cm = MagicMock() | ||
| mock_session_cm.__aenter__ = AsyncMock(return_value=mock_session) | ||
| mock_session_cm.__aexit__ = AsyncMock(return_value=None) | ||
|
|
||
| mock_session_class.return_value = mock_session_cm | ||
|
|
||
| servers = await service.list_tool_servers( | ||
| agentic_app_id="test-app-id", auth_token="test-token" | ||
| ) | ||
|
|
||
| assert len(servers) == 1 | ||
| assert servers[0].mcp_server_name == "ProdServer" | ||
| assert servers[0].mcp_server_unique_name == "prod_server" | ||
| assert servers[0].url == "https://prod.custom.url/mcp" | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.