Skip to content

Commit 2a453eb

Browse files
committed
Resolving comments
1 parent 6d484c8 commit 2a453eb

5 files changed

Lines changed: 46 additions & 131 deletions

File tree

libraries/microsoft-agents-a365-tooling-extensions-semantickernel/microsoft_agents_a365/tooling/extensions/semantickernel/services/mcp_tool_registration_service.py

Lines changed: 5 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
from microsoft_agents_a365.tooling.models.mcp_server_config import MCPServerConfig
2424
from microsoft_agents_a365.tooling.utils.constants import Constants
2525
from microsoft_agents_a365.tooling.utils.utility import (
26-
get_tools_mode,
2726
get_mcp_platform_authentication_scope,
2827
)
2928

@@ -112,25 +111,12 @@ async def add_tool_servers_to_agent(
112111
)
113112
self._logger.info(f"🔧 Adding MCP tools from {len(servers)} servers")
114113

115-
# Get tools mode
116-
tools_mode = get_tools_mode()
117-
118-
# Process each server (matching C# foreach pattern)
114+
# Process each server
119115
for server in servers:
120116
try:
121-
if tools_mode == "HardCodedTools":
122-
await self._add_hardcoded_tools_for_server(kernel, server)
123-
continue
124-
125-
headers = {}
126-
127-
if tools_mode == "MockMCPServer":
128-
if mock_auth_header := os.getenv("MOCK_MCP_AUTHORIZATION"):
129-
headers[Constants.Headers.AUTHORIZATION] = mock_auth_header
130-
else:
131-
headers = {
132-
Constants.Headers.AUTHORIZATION: f"{Constants.Headers.BEARER_PREFIX} {auth_token}",
133-
}
117+
headers = {
118+
Constants.Headers.AUTHORIZATION: f"{Constants.Headers.BEARER_PREFIX} {auth_token}",
119+
}
134120

135121
plugin = MCPStreamableHttpPlugin(
136122
name=server.mcp_server_name,
@@ -151,7 +137,7 @@ async def add_tool_servers_to_agent(
151137
self._connected_plugins.append(plugin)
152138

153139
self._logger.info(
154-
f"✅ Connected and added MCP plugin ({tools_mode}) for: {server.mcp_server_name}"
140+
f"✅ Connected and added MCP plugin for: {server.mcp_server_name}"
155141
)
156142

157143
except Exception as e:
@@ -172,29 +158,6 @@ def _validate_inputs(self, kernel: Any, agentic_app_id: str, auth_token: str) ->
172158
if not auth_token or not auth_token.strip():
173159
raise ValueError("auth_token cannot be null or empty")
174160

175-
async def _add_hardcoded_tools_for_server(self, kernel: Any, server: MCPServerConfig) -> None:
176-
"""Add hardcoded tools for a specific server (equivalent to C# hardcoded tool logic)."""
177-
server_name = server.mcp_server_name
178-
179-
if server_name.lower() == "mcp_mailtools":
180-
# TODO: Implement hardcoded mail tools
181-
# kernel.plugins.add(KernelPluginFactory.create_from_type(HardCodedMailTools, server.mcp_server_name, self._service_provider))
182-
self._logger.info(f"Adding hardcoded mail tools for {server_name}")
183-
elif server_name.lower() == "mcp_sharepointtools":
184-
# TODO: Implement hardcoded SharePoint tools
185-
# kernel.plugins.add(KernelPluginFactory.create_from_type(HardCodedSharePointTools, server.mcp_server_name, self._service_provider))
186-
self._logger.info(f"Adding hardcoded SharePoint tools for {server_name}")
187-
elif server_name.lower() == "onedrivemcpserver":
188-
# TODO: Implement hardcoded OneDrive tools
189-
# kernel.plugins.add(KernelPluginFactory.create_from_type(HardCodedOneDriveTools, server.mcp_server_name, self._service_provider))
190-
self._logger.info(f"Adding hardcoded OneDrive tools for {server_name}")
191-
elif server_name.lower() == "wordmcpserver":
192-
# TODO: Implement hardcoded Word tools
193-
# kernel.plugins.add(KernelPluginFactory.create_from_type(HardCodedWordTools, server.mcp_server_name, self._service_provider))
194-
self._logger.info(f"Adding hardcoded Word tools for {server_name}")
195-
else:
196-
self._logger.warning(f"No hardcoded tools available for server: {server_name}")
197-
198161
# ============================================================================
199162
# Private Methods - Kernel Function Creation
200163
# ============================================================================

libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/utils/__init__.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,13 @@
1010
get_tooling_gateway_for_digital_worker,
1111
get_mcp_base_url,
1212
build_mcp_server_url,
13-
get_ppapi_token_scope,
13+
get_mcp_platform_authentication_scope,
1414
)
1515

1616
__all__ = [
1717
"Constants",
1818
"get_tooling_gateway_for_digital_worker",
1919
"get_mcp_base_url",
2020
"build_mcp_server_url",
21-
"get_ppapi_token_scope",
21+
"get_mcp_platform_authentication_scope",
2222
]

libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/utils/utility.py

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
"""
66

77
import os
8-
from enum import Enum
98

109

1110
# Constants for base URLs
@@ -15,13 +14,6 @@
1514
PROD_MCP_PLATFORM_AUTHENTICATION_SCOPE = "ea9ffc3e-8a23-4a7d-836d-234d7c7565c1/.default"
1615

1716

18-
class ToolsMode(Enum):
19-
"""Enum for different tools modes."""
20-
21-
MCP_PLATFORM = 1
22-
MOCK_MCP_SERVER = 2
23-
24-
2517
def get_tooling_gateway_for_digital_worker(agentic_app_id: str) -> str:
2618
"""
2719
Gets the tooling gateway URL for the specified digital worker.
@@ -43,13 +35,6 @@ def get_mcp_base_url() -> str:
4335
Returns:
4436
str: The base URL for MCP servers.
4537
"""
46-
environment = _get_current_environment().lower()
47-
48-
if environment == "development":
49-
tools_mode = get_tools_mode()
50-
if tools_mode == ToolsMode.MOCK_MCP_SERVER:
51-
return os.getenv("MOCK_MCP_SERVER_URL", "http://localhost:5309/mcp-mock/agents/servers")
52-
5338
return f"{_get_mcp_platform_base_url()}/agents/servers"
5439

5540

@@ -91,21 +76,6 @@ def _get_mcp_platform_base_url() -> str:
9176
return MCP_PLATFORM_PROD_BASE_URL
9277

9378

94-
def get_tools_mode() -> ToolsMode:
95-
"""
96-
Gets the tools mode for the application.
97-
98-
Returns:
99-
ToolsMode: The tools mode enum value.
100-
"""
101-
tools_mode = os.getenv("TOOLS_MODE", "MCPPlatform").lower()
102-
103-
if tools_mode == "mockmcpserver":
104-
return ToolsMode.MOCK_MCP_SERVER
105-
else:
106-
return ToolsMode.MCP_PLATFORM
107-
108-
10979
def get_mcp_platform_authentication_scope():
11080
"""
11181
Gets the MCP platform authentication scope based on the current environment.

tests/microsoft-agents-a365-tooling-unittest/utils/test_utility.py

Lines changed: 24 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,10 @@
1313
build_mcp_server_url,
1414
_get_current_environment,
1515
_get_mcp_platform_base_url,
16-
get_ppapi_token_scope,
16+
get_mcp_platform_authentication_scope,
1717
MCP_PLATFORM_PROD_BASE_URL,
1818
PPAPI_TOKEN_SCOPE,
19-
PPAPI_TEST_TOKEN_SCOPE,
19+
PROD_MCP_PLATFORM_AUTHENTICATION_SCOPE,
2020
)
2121

2222

@@ -31,6 +31,7 @@ def setup_method(self):
3131
for key in [
3232
"ENVIRONMENT",
3333
"MCP_PLATFORM_ENDPOINT",
34+
"MCP_PLATFORM_AUTHENTICATION_SCOPE",
3435
]
3536
}
3637

@@ -52,7 +53,7 @@ def test_get_tooling_gateway_for_digital_worker(self):
5253
result = get_tooling_gateway_for_digital_worker(agent_user_id)
5354

5455
# Assert
55-
expected = f"{MCP_PLATFORM_PROD_BASE_URL}/agentGateway/agentApplicationInstances/{agent_user_id}/mcpServers"
56+
expected = f"{MCP_PLATFORM_PROD_BASE_URL}/agents/{agent_user_id}/mcpServers"
5657
assert result == expected
5758

5859
@patch.dict(os.environ, {"MCP_PLATFORM_ENDPOINT": "https://custom.endpoint.com"}, clear=False)
@@ -65,7 +66,7 @@ def test_get_tooling_gateway_with_custom_endpoint(self):
6566
result = get_tooling_gateway_for_digital_worker(agent_user_id)
6667

6768
# Assert
68-
expected = "https://custom.endpoint.com/agentGateway/agentApplicationInstances/test-agent-456/mcpServers"
69+
expected = "https://custom.endpoint.com/agents/test-agent-456/mcpServers"
6970
assert result == expected
7071

7172
def test_get_mcp_base_url_production(self):
@@ -77,7 +78,7 @@ def test_get_mcp_base_url_production(self):
7778
result = get_mcp_base_url()
7879

7980
# Assert
80-
expected = f"{MCP_PLATFORM_PROD_BASE_URL}/mcp/environments"
81+
expected = f"{MCP_PLATFORM_PROD_BASE_URL}/agents/servers"
8182
assert result == expected
8283

8384
@patch.dict(os.environ, {"MCP_PLATFORM_ENDPOINT": "https://custom.endpoint.com"}, clear=False)
@@ -87,38 +88,19 @@ def test_get_mcp_base_url_with_custom_endpoint(self):
8788
result = get_mcp_base_url()
8889

8990
# Assert
90-
expected = "https://custom.endpoint.com/mcp/environments"
91+
expected = "https://custom.endpoint.com/agents/servers"
9192
assert result == expected
9293

9394
def test_build_mcp_server_url_production(self):
9495
"""Test build_mcp_server_url in production environment."""
9596
# Arrange
96-
environment_id = "prod-env-123"
9797
server_name = "mail_server"
9898

9999
# Act
100-
result = build_mcp_server_url(environment_id, server_name)
100+
result = build_mcp_server_url(server_name)
101101

102102
# Assert
103-
expected = (
104-
f"{MCP_PLATFORM_PROD_BASE_URL}/mcp/environments/{environment_id}/servers/{server_name}"
105-
)
106-
assert result == expected
107-
108-
@patch.dict(os.environ, {"ENVIRONMENT": "Development"}, clear=False)
109-
def test_build_mcp_server_url_development_platform_mode(self):
110-
"""Test build_mcp_server_url in development with platform mode."""
111-
# Arrange
112-
environment_id = "dev-env-456"
113-
server_name = "platform_server"
114-
115-
# Act
116-
result = build_mcp_server_url(environment_id, server_name)
117-
118-
# Assert
119-
expected = (
120-
f"{MCP_PLATFORM_PROD_BASE_URL}/mcp/environments/{environment_id}/servers/{server_name}"
121-
)
103+
expected = f"{MCP_PLATFORM_PROD_BASE_URL}/agents/servers/{server_name}"
122104
assert result == expected
123105

124106
def test_get_current_environment_default(self):
@@ -161,36 +143,34 @@ def test_get_mcp_platform_base_url_custom(self):
161143
# Assert
162144
assert result == "https://test.platform.com"
163145

164-
def test_get_ppapi_token_scope_production(self):
165-
"""Test get_ppapi_token_scope returns production scope."""
166-
# Arrange - Set environment to production explicitly
167-
os.environ.pop("ENVIRONMENT", None)
168-
# The _get_current_environment defaults to "Development", so we need to set it to something else
169-
os.environ["ENVIRONMENT"] = "Production"
146+
def test_get_mcp_platform_authentication_scope_production(self):
147+
"""Test get_mcp_platform_authentication_scope returns production scope."""
148+
# Arrange - Clear environment variable to use default
149+
os.environ.pop("MCP_PLATFORM_AUTHENTICATION_SCOPE", None)
170150

171151
# Act
172-
result = get_ppapi_token_scope()
152+
result = get_mcp_platform_authentication_scope()
173153

174154
# Assert
175-
expected = [PPAPI_TOKEN_SCOPE + "/.default"]
155+
expected = [PROD_MCP_PLATFORM_AUTHENTICATION_SCOPE]
176156
assert result == expected
177157

178-
@patch.dict(os.environ, {"ENVIRONMENT": "Development"}, clear=False)
179-
def test_get_ppapi_token_scope_development(self):
180-
"""Test get_ppapi_token_scope returns test scope in development."""
158+
@patch.dict(os.environ, {"MCP_PLATFORM_AUTHENTICATION_SCOPE": "custom-scope/.default"}, clear=False)
159+
def test_get_mcp_platform_authentication_scope_custom(self):
160+
"""Test get_mcp_platform_authentication_scope returns custom scope from environment."""
181161
# Act
182-
result = get_ppapi_token_scope()
162+
result = get_mcp_platform_authentication_scope()
183163

184164
# Assert
185-
expected = [PPAPI_TEST_TOKEN_SCOPE + "/.default"]
165+
expected = ["custom-scope/.default"]
186166
assert result == expected
187167

188168
def test_constants_values(self):
189169
"""Test that constants have expected values."""
190170
# Assert
191171
assert MCP_PLATFORM_PROD_BASE_URL == "https://agent365.svc.cloud.microsoft"
192172
assert PPAPI_TOKEN_SCOPE == "https://api.powerplatform.com"
193-
assert PPAPI_TEST_TOKEN_SCOPE == "https://api.test.powerplatform.com"
173+
assert PROD_MCP_PLATFORM_AUTHENTICATION_SCOPE == "ea9ffc3e-8a23-4a7d-836d-234d7c7565c1/.default"
194174

195175
def test_get_tooling_gateway_empty_agent_id(self):
196176
"""Test get_tooling_gateway_for_digital_worker with empty agent ID."""
@@ -201,20 +181,17 @@ def test_get_tooling_gateway_empty_agent_id(self):
201181
result = get_tooling_gateway_for_digital_worker(agent_user_id)
202182

203183
# Assert - Function should still work but produce invalid URL
204-
expected = (
205-
f"{MCP_PLATFORM_PROD_BASE_URL}/agentGateway/agentApplicationInstances//mcpServers"
206-
)
184+
expected = f"{MCP_PLATFORM_PROD_BASE_URL}/agents//mcpServers"
207185
assert result == expected
208186

209187
def test_build_mcp_server_url_empty_params(self):
210188
"""Test build_mcp_server_url with empty parameters."""
211189
# Arrange
212-
environment_id = ""
213190
server_name = ""
214191

215192
# Act
216-
result = build_mcp_server_url(environment_id, server_name)
193+
result = build_mcp_server_url(server_name)
217194

218195
# Assert
219-
expected = f"{MCP_PLATFORM_PROD_BASE_URL}/mcp/environments//servers/"
196+
expected = f"{MCP_PLATFORM_PROD_BASE_URL}/agents/servers/"
220197
assert result == expected

0 commit comments

Comments
 (0)