diff --git a/libraries/microsoft-agents-a365-tooling-extensions-agentframework/docs/design.md b/libraries/microsoft-agents-a365-tooling-extensions-agentframework/docs/design.md index 7d8af5ee..a9b31e5f 100644 --- a/libraries/microsoft-agents-a365-tooling-extensions-agentframework/docs/design.md +++ b/libraries/microsoft-agents-a365-tooling-extensions-agentframework/docs/design.md @@ -41,7 +41,7 @@ McpToolRegistrationService.add_tool_servers_to_agent() │ ├── Resolve agent identity ├── Exchange token for MCP scope - ├── Create MCPStreamableHTTPTool for each server + ├── Gate (per server, non-blocking): Ready → MCPStreamableHTTPTool; Pending → placeholder tool └── Create ChatAgent with all tools │ ▼ @@ -76,6 +76,66 @@ mcp_tool = MCPStreamableHTTPTool( ) ``` +### Connection-readiness gating (non-blocking, per server) + +MCP server discovery runs every turn. The gateway reports each server's `connectivityStatus` +(`"Ready"` or `"Pending"`) along with a `missingConnectionsUrl` the user can visit to set up the +connection(s) that server needs. Gating is **per server and non-blocking**: + +- A **Ready** server (or a legacy source with no `connectivityStatus`) is wired as a live + `MCPStreamableHTTPTool`, exactly as before. +- A **Pending** server is wired as a single **placeholder tool** named after the server. The agent + is still built and the Ready servers remain fully usable for the turn. Only if the model invokes + the placeholder — because the user's request actually needs that server — does it return a static + message (including `missingConnectionsUrl`) for the model to relay to the user. If the turn never + needs the Pending server, the user is never bothered. A later turn re-runs discovery and wires the + server for real once its connections are in place. + +The placeholder **returns** the message rather than raising it. Agent Framework's tool-call loop +catches exceptions raised inside a `FunctionTool` and reflects them to the model as an opaque error +string (`"Error: Function failed."` by default), and it converts `UserInputRequiredException` into +tool-result content rather than propagating it — so returning the text is the only way to surface +the setup URL through the model. Because surfacing flows through the model, exact verbatim delivery +is best-effort; the placeholder's description instructs the model to relay the message and URL +verbatim, and `max_invocations=1` stops the model from looping on it within a turn. + +> **Note:** A Pending server is represented by one placeholder named after the server (its real +> sub-tools are invisible until it connects). The model routes the user's intent to it by server +> name plus description — best-effort, not guaranteed. + +```python +from microsoft_agents_a365.tooling.extensions.agentframework import ( + McpToolRegistrationService, +) + +service = McpToolRegistrationService() + +# Non-blocking: Pending servers become placeholders, so no special handling is needed in the +# turn handler. The agent is always built and Ready tools always run. +agent = await service.add_tool_servers_to_agent( + chat_client=chat_client, + agent_instructions="You are a helpful assistant.", + initial_tools=[], + auth=auth_context, + auth_handler_name="graph", + turn_context=turn_context, +) +``` + +For developers who instead want **blocking** behavior (abort the whole turn until every server is +connected), the extension still exports `McpConnectionsRequiredError`. Inspect the discovered +servers yourself and raise it before building the agent: + +```python +from microsoft_agents_a365.tooling.extensions.agentframework import McpConnectionsRequiredError +``` + +`McpConnectionsRequiredError` exposes `missing_connections_url`, `connectivity_status`, and +`server_names`, and its message is built from the same `format_mcp_connections_required_message` +helper the placeholder uses. It is owned and exported by this extension +(`microsoft_agents_a365.tooling` core only parses the per-server connection metadata; it never +gates or raises). + ### Chat History API The service provides methods to send chat history to the MCP platform for real-time threat protection analysis. This enables security scanning of conversation content. diff --git a/libraries/microsoft-agents-a365-tooling-extensions-agentframework/microsoft_agents_a365/tooling/extensions/agentframework/__init__.py b/libraries/microsoft-agents-a365-tooling-extensions-agentframework/microsoft_agents_a365/tooling/extensions/agentframework/__init__.py index c4ba4378..f26814e1 100644 --- a/libraries/microsoft-agents-a365-tooling-extensions-agentframework/microsoft_agents_a365/tooling/extensions/agentframework/__init__.py +++ b/libraries/microsoft-agents-a365-tooling-extensions-agentframework/microsoft_agents_a365/tooling/extensions/agentframework/__init__.py @@ -21,8 +21,10 @@ __version__ = "1.0.0" # Import services from the services module +from .exceptions import McpConnectionsRequiredError from .services import McpToolRegistrationService __all__ = [ "McpToolRegistrationService", + "McpConnectionsRequiredError", ] diff --git a/libraries/microsoft-agents-a365-tooling-extensions-agentframework/microsoft_agents_a365/tooling/extensions/agentframework/exceptions.py b/libraries/microsoft-agents-a365-tooling-extensions-agentframework/microsoft_agents_a365/tooling/extensions/agentframework/exceptions.py new file mode 100644 index 00000000..83b9e501 --- /dev/null +++ b/libraries/microsoft-agents-a365-tooling-extensions-agentframework/microsoft_agents_a365/tooling/extensions/agentframework/exceptions.py @@ -0,0 +1,77 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +"""Exceptions and message helpers for the Agent Framework MCP tooling extension.""" + +from typing import List, Optional + + +def format_mcp_connections_required_message( + *, + server_names: List[str], + connectivity_status: Optional[str], + missing_connections_url: Optional[str], +) -> str: + """Build the static, user-facing message shown when an MCP server needs connection setup. + + This single helper is the source of truth for the wording so that the message a Pending + server's placeholder tool returns to the model is identical to the message carried by + ``McpConnectionsRequiredError``. + + Args: + server_names: Names of the MCP server(s) whose downstream connections are not set up. + connectivity_status: The gateway-reported ``connectivityStatus`` (typically ``"Pending"``). + missing_connections_url: URL the user opens to set up the missing connection(s). May be + ``None`` when the gateway did not supply one. + + Returns: + A human-readable message suitable for relaying verbatim to the end user. + """ + servers_text = ", ".join(server_names) if server_names else "(unknown)" + message = ( + f"The tool(s) from MCP server(s) [{servers_text}] can't be used yet because the " + f"required data connection(s) aren't set up (connectivityStatus={connectivity_status})." + ) + if missing_connections_url: + message += f" Set up the missing connection(s) here: {missing_connections_url}" + else: + message += " Ask your administrator to set up the required connection(s)." + return message + + +class McpConnectionsRequiredError(Exception): + """Raised when an MCP server the user invoked is not yet connection-ready. + + The tooling gateway reports a per-server ``connectivityStatus`` of ``"Pending"`` when an MCP + server has downstream connections the user has not yet established. Discovery runs every turn. + + The agentframework extension gates **per server, non-blocking**: Ready servers are wired as + real tools and a Pending server is registered as a placeholder tool. Only when the model + actually invokes that placeholder (because the user's request needs the server) is the static + setup message — including ``missing_connections_url`` — surfaced; the rest of the turn runs + normally with the Ready tools. + + This exception is **not** raised by the non-blocking gate (Agent Framework swallows exceptions + raised inside a tool, and converts ``UserInputRequiredException`` into tool-result content, so + neither reaches the developer's turn handler). It is exported for developers who want to + implement their own *blocking* gating — e.g. inspect the discovered servers and raise this + before building the agent to abort the whole turn — and as the structured carrier of the same + message the placeholder returns. + """ + + def __init__( + self, + missing_connections_url: Optional[str], + connectivity_status: Optional[str], + server_names: List[str], + ) -> None: + self.missing_connections_url = missing_connections_url + self.connectivity_status = connectivity_status + self.server_names = server_names + super().__init__( + format_mcp_connections_required_message( + server_names=server_names, + connectivity_status=connectivity_status, + missing_connections_url=missing_connections_url, + ) + ) diff --git a/libraries/microsoft-agents-a365-tooling-extensions-agentframework/microsoft_agents_a365/tooling/extensions/agentframework/services/mcp_tool_registration_service.py b/libraries/microsoft-agents-a365-tooling-extensions-agentframework/microsoft_agents_a365/tooling/extensions/agentframework/services/mcp_tool_registration_service.py index f9c8d8b9..0b97fa4c 100644 --- a/libraries/microsoft-agents-a365-tooling-extensions-agentframework/microsoft_agents_a365/tooling/extensions/agentframework/services/mcp_tool_registration_service.py +++ b/libraries/microsoft-agents-a365-tooling-extensions-agentframework/microsoft_agents_a365/tooling/extensions/agentframework/services/mcp_tool_registration_service.py @@ -8,7 +8,13 @@ from datetime import datetime, timezone from typing import TYPE_CHECKING, List, Optional, Sequence -from agent_framework import RawAgent, Message, HistoryProvider, MCPStreamableHTTPTool +from agent_framework import ( + RawAgent, + Message, + HistoryProvider, + MCPStreamableHTTPTool, + FunctionTool, +) import httpx if TYPE_CHECKING: @@ -18,7 +24,7 @@ from microsoft_agents_a365.runtime import OperationResult from microsoft_agents_a365.runtime.utility import Utility -from microsoft_agents_a365.tooling.models import ChatHistoryMessage, ToolOptions +from microsoft_agents_a365.tooling.models import ChatHistoryMessage, MCPServerConfig, ToolOptions from microsoft_agents_a365.tooling.services.mcp_tool_server_configuration_service import ( McpToolServerConfigurationService, ) @@ -28,10 +34,21 @@ is_development_environment, ) +from ..exceptions import format_mcp_connections_required_message + # Default timeout for MCP server HTTP requests (in seconds) MCP_HTTP_CLIENT_TIMEOUT_SECONDS = 90.0 +# Sentinel per-server status that means a server's downstream connections are +# already in place. Anything else (typically "Pending") means the server is not +# connection-ready: instead of wiring it as a real tool, the extension registers +# a placeholder tool that returns a static "set up your connections" message when +# the model invokes it. This gates per server without blocking the rest of the +# turn — Ready servers stay usable. None means the source predates the field +# (dev manifest / legacy gateway) and is treated as Ready. +_CONNECTIVITY_READY = "Ready" + class McpToolRegistrationService: """ @@ -114,15 +131,47 @@ async def add_tool_servers_to_agent( ) self._logger.info(f"Loaded {len(server_configs)} MCP server configurations") + for c in server_configs: + _name = c.mcp_server_name or c.mcp_server_unique_name + self._logger.info( + " per-server gateway state: name=%s connectivity_status=%r missing_url=%s", + _name, + c.connectivity_status, + c.missing_connections_url, + ) # Create the agent with all tools (initial + MCP tools) all_tools = list(initial_tools) - # Add servers as MCPStreamableHTTPTool instances + # Connection-readiness gate (non-blocking, per server). Discovery runs + # every turn; the gateway flags each server's downstream connections via + # ``connectivityStatus``. A "Ready" (or legacy ``None``) server is wired + # as a real ``MCPStreamableHTTPTool``. A "Pending" server is instead + # registered as a placeholder tool that, when the model invokes it, + # returns a static "set up your connections" message (including the + # server's ``missing_connections_url``). This keeps the turn running with + # the Ready tools and only prompts the user about connection setup if the + # model actually needs the Pending server. A later turn re-runs discovery + # and wires the server for real once its connections are in place. for config in server_configs: # Use mcp_server_name if available (not None or empty), otherwise fall back to mcp_server_unique_name server_name = config.mcp_server_name or config.mcp_server_unique_name + if ( + config.connectivity_status is not None + and config.connectivity_status != _CONNECTIVITY_READY + ): + placeholder = self._build_pending_placeholder_tool(config) + all_tools.append(placeholder) + self._logger.info( + "MCP server '%s' is %s; registered connection-setup placeholder " + "instead of live tools (setup URL=%s)", + server_name, + config.connectivity_status, + config.missing_connections_url or config.all_connections_url, + ) + continue + try: # Merge base (non-auth) headers with per-server headers from list_tool_servers. # server.headers already contains the correct per-audience Authorization token. @@ -187,6 +236,50 @@ async def add_tool_servers_to_agent( self._logger.error(f"Failed to add tool servers to agent: {ex}") raise + def _build_pending_placeholder_tool(self, config: MCPServerConfig) -> FunctionTool: + """Build the placeholder tool registered in place of a not-yet-connected MCP server. + + The gateway reported this server's ``connectivityStatus`` as something other than + ``"Ready"`` (typically ``"Pending"``), meaning the user has not finished setting up the + downstream data connection(s) the server needs. The server's real sub-tools are not + available until those connections exist, so instead of wiring it as a live + ``MCPStreamableHTTPTool`` we expose a single stand-in tool named after the server. When + the model invokes it — because the user's request needs that server — the tool returns a + static message (including ``missing_connections_url``) that the model relays to the user. + + The message is **returned**, not raised: Agent Framework swallows exceptions raised inside + a tool (and converts ``UserInputRequiredException`` into tool-result content), so returning + the text is the only way to surface the URL through the model. ``max_invocations=1`` keeps + the model from calling the placeholder repeatedly within a turn. + + Args: + config: The discovered configuration for the Pending MCP server. + + Returns: + A ``FunctionTool`` standing in for the not-yet-connected server. + """ + server_name = config.mcp_server_name or config.mcp_server_unique_name + message = format_mcp_connections_required_message( + server_names=[server_name], + connectivity_status=config.connectivity_status, + missing_connections_url=config.missing_connections_url or config.all_connections_url, + ) + + def _connections_required() -> str: + return message + + return FunctionTool( + name=server_name, + description=( + f"Tools provided by the '{server_name}' MCP server. The required data " + "connection(s) for this server are not set up yet, so its tools cannot run. " + "Call this when the user's request needs this server, then relay the returned " + "message — including the setup URL — to the user verbatim." + ), + func=_connections_required, + max_invocations=1, + ) + def _convert_chat_messages_to_history( self, chat_messages: Sequence[Message], diff --git a/libraries/microsoft-agents-a365-tooling/CHANGELOG.md b/libraries/microsoft-agents-a365-tooling/CHANGELOG.md index 73e4267c..59264b69 100644 --- a/libraries/microsoft-agents-a365-tooling/CHANGELOG.md +++ b/libraries/microsoft-agents-a365-tooling/CHANGELOG.md @@ -35,3 +35,5 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Added `ChatHistoryMessage` Pydantic model for representing individual messages in chat history - Added `ChatMessageRequest` Pydantic model for the chat history API request payload - Added `py.typed` marker for PEP 561 compliance, enabling type checker support +- Added `id`, `all_connections_url`, `missing_connections_url`, and `connectivity_status` fields to `MCPServerConfig`, parsed from the per-server gateway payload (`allConnectionsUrl`, `missingConnectionsUrl`, `connectivityStatus`). The core service parses this connection metadata onto each server but does not gate on it — connection-readiness gating is owned by the framework-specific tooling extensions +- `_parse_gateway_response()` and `_load_servers_from_gateway()` return `List[MCPServerConfig]`; the wrapped `{"mcpServers": [...]}` and legacy raw-array gateway response shapes are both supported, and response-level (aggregate) connection fields are ignored diff --git a/libraries/microsoft-agents-a365-tooling/docs/design.md b/libraries/microsoft-agents-a365-tooling/docs/design.md index bf83f8b1..3d9209d1 100644 --- a/libraries/microsoft-agents-a365-tooling/docs/design.md +++ b/libraries/microsoft-agents-a365-tooling/docs/design.md @@ -108,6 +108,16 @@ User-Agent: Agent365SDK/0.1.0 (...) The gateway returns the same JSON structure, but `mcpServerUniqueName` contains the full endpoint URL. +### Connection metadata (per-server) + +The core service parses each server's connection metadata — `connectivityStatus`, +`allConnectionsUrl`, and `missingConnectionsUrl` — onto the returned `MCPServerConfig` +objects, but it does **not** gate on them. Connection-readiness gating is the +responsibility of the framework-specific tooling extensions (e.g. +`microsoft-agents-a365-tooling-extensions-agentframework`), which decide how to surface a +`Pending` server and raise/handle the appropriate error. See the relevant extension's +design document for details. + ### MCPServerConfig ([models/mcp_server_config.py](../microsoft_agents_a365/tooling/models/mcp_server_config.py)) Data class representing an MCP server configuration: diff --git a/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/models/mcp_server_config.py b/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/models/mcp_server_config.py index dd57ff98..a7ed2631 100644 --- a/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/models/mcp_server_config.py +++ b/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/models/mcp_server_config.py @@ -37,6 +37,19 @@ class MCPServerConfig: #: Publisher identifier for the MCP server. publisher: Optional[str] = None + #: Unique identifier (GUID) of the MCP server from the gateway, if provided. + id: Optional[str] = None + + #: Per-server URL to view/manage all connections for this server's connector. + all_connections_url: Optional[str] = None + + #: Per-server URL to set up the connections this server is missing. + missing_connections_url: Optional[str] = None + + #: Per-server connectivity status reported by the gateway ("Ready" or "Pending"). + #: None when the source predates the field (dev manifest / legacy raw-array gateway). + connectivity_status: Optional[str] = None + def __post_init__(self): """Validate the configuration after initialization.""" if not self.mcp_server_name: diff --git a/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/services/mcp_tool_server_configuration_service.py b/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/services/mcp_tool_server_configuration_service.py index 5ee987f9..1a0d018f 100644 --- a/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/services/mcp_tool_server_configuration_service.py +++ b/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/services/mcp_tool_server_configuration_service.py @@ -158,6 +158,13 @@ async def list_tool_servers( servers = await self._load_servers_from_gateway( agentic_app_id, auth_token, options, turn_context ) + # Connection-readiness gating is the responsibility of the + # framework-specific tooling extensions (e.g. + # ``microsoft-agents-a365-tooling-extensions-agentframework``). + # Core simply parses and returns the per-server connection + # metadata (``connectivity_status``, ``all_connections_url``, + # ``missing_connections_url``) on each ``MCPServerConfig`` and + # never gates or raises on it. if ( authorization is not None and auth_handler_name is not None @@ -500,7 +507,8 @@ async def _load_servers_from_gateway( ``activity.id``. A new UUID is generated when not provided. Returns: - List[MCPServerConfig]: List of MCP server configurations from tooling gateway. + List[MCPServerConfig]: Parsed MCP server configurations, each carrying its + per-server connection metadata. Raises: Exception: If there's an error communicating with the tooling gateway. @@ -515,11 +523,11 @@ async def _load_servers_from_gateway( async with aiohttp.ClientSession(timeout=timeout) as session: async with session.get(config_endpoint, headers=headers) as response: if response.status == 200: - mcp_servers = await self._parse_gateway_response(response) + servers = await self._parse_gateway_response(response) self._logger.info( - f"Retrieved {len(mcp_servers)} MCP tool servers from tooling gateway" + f"Retrieved {len(servers)} MCP tool servers from tooling gateway" ) - return mcp_servers + return servers else: raise Exception(f"HTTP {response.status}: {await response.text()}") @@ -643,24 +651,30 @@ async def _parse_gateway_response( Parses the response from the tooling gateway. Supports two response shapes: - - Wrapped: ``{"mcpServers": [...]}`` + - Wrapped: ``{"mcpServers": [...], "connectivityStatus": ..., ...}`` - Raw array: ``[...]`` (legacy V1 gateway format) + Per-server connection metadata (``connectivityStatus``, ``allConnectionsUrl``, + ``missingConnectionsUrl``) is parsed onto each ``MCPServerConfig`` by + ``_parse_server_config``. Response-level (aggregate) fields are ignored — gating + is the responsibility of the framework-specific tooling extensions. + Args: response: HTTP response from the gateway. Returns: - List of parsed MCP server configurations. + List[MCPServerConfig]: parsed MCP server configurations. """ config_data = await response.json(content_type=None) server_elements: Optional[List[object]] = None + if isinstance(config_data, list): - # Raw array format (legacy V1 gateway returns bare array) + # Raw array format (legacy V1 gateway returns bare array). self._logger.debug("Gateway returned raw array response") server_elements = config_data elif isinstance(config_data, dict) and isinstance(config_data.get("mcpServers"), list): - # Wrapped format: {"mcpServers": [...]} + # Wrapped format: {"mcpServers": [...], ...} self._logger.debug("Gateway returned wrapped mcpServers response") server_elements = config_data["mcpServers"] else: @@ -725,6 +739,20 @@ def _parse_server_config(self, server_element: Dict[str, object]) -> Optional[MC publisher_raw = server_element.get("publisher") publisher = str(publisher_raw) if publisher_raw is not None else None + id_raw = server_element.get("id") + server_id = str(id_raw) if id_raw is not None else None + + all_conn_raw = server_element.get("allConnectionsUrl") + all_connections_url = str(all_conn_raw) if all_conn_raw is not None else None + + missing_conn_raw = server_element.get("missingConnectionsUrl") + missing_connections_url = ( + str(missing_conn_raw) if missing_conn_raw is not None else None + ) + + status_raw = server_element.get("connectivityStatus") + connectivity_status = str(status_raw) if status_raw is not None else None + return MCPServerConfig( mcp_server_name=mcp_server_name, mcp_server_unique_name=mcp_server_unique_name, @@ -732,6 +760,10 @@ def _parse_server_config(self, server_element: Dict[str, object]) -> Optional[MC audience=audience, scope=scope, publisher=publisher, + id=server_id, + all_connections_url=all_connections_url, + missing_connections_url=missing_connections_url, + connectivity_status=connectivity_status, ) except Exception as exc: diff --git a/tests/tooling/extensions/agentframework/services/test_mcp_tool_registration_service.py b/tests/tooling/extensions/agentframework/services/test_mcp_tool_registration_service.py index d67b0577..99bc34c0 100644 --- a/tests/tooling/extensions/agentframework/services/test_mcp_tool_registration_service.py +++ b/tests/tooling/extensions/agentframework/services/test_mcp_tool_registration_service.py @@ -64,6 +64,9 @@ def mock_mcp_server_config(self): config.url = "https://test-mcp-server.example.com/api" config.headers = None # per-audience headers attached by list_tool_servers config.audience = "a1b2c3d4-e5f6-7890-abcd-ef1234567890" # V2: GUID audience + config.connectivity_status = None # legacy / Ready — exercise the real-tool branch + config.missing_connections_url = None + config.all_connections_url = None return config @pytest.fixture @@ -493,6 +496,8 @@ async def test_full_client_lifecycle_single_server( mock_server_config.mcp_server_unique_name = "test-server-unique" mock_server_config.url = "https://test.example.com/api" mock_server_config.headers = None # per-audience headers attached by list_tool_servers + mock_server_config.connectivity_status = None # legacy / Ready + mock_server_config.missing_connections_url = None mock_http_client_instance = MagicMock() @@ -566,18 +571,24 @@ async def test_full_client_lifecycle_multiple_servers( mock_server_config1.mcp_server_unique_name = "server-1-unique" mock_server_config1.url = "https://server1.example.com/api" mock_server_config1.headers = None # per-audience headers attached by list_tool_servers + mock_server_config1.connectivity_status = None + mock_server_config1.missing_connections_url = None mock_server_config2 = Mock() mock_server_config2.mcp_server_name = "server-2" mock_server_config2.mcp_server_unique_name = "server-2-unique" mock_server_config2.url = "https://server2.example.com/api" mock_server_config2.headers = None + mock_server_config2.connectivity_status = None + mock_server_config2.missing_connections_url = None mock_server_config3 = Mock() mock_server_config3.mcp_server_name = "server-3" mock_server_config3.mcp_server_unique_name = "server-3-unique" mock_server_config3.url = "https://server3.example.com/api" mock_server_config3.headers = None + mock_server_config3.connectivity_status = None + mock_server_config3.missing_connections_url = None # Create unique mock clients for each server mock_clients = [MagicMock() for _ in range(3)] @@ -677,6 +688,8 @@ async def test_cleanup_called_twice_after_creating_clients( mock_server_config.mcp_server_unique_name = "test-server-unique" mock_server_config.url = "https://test.example.com/api" mock_server_config.headers = None # per-audience headers attached by list_tool_servers + mock_server_config.connectivity_status = None # legacy / Ready + mock_server_config.missing_connections_url = None mock_http_client_instance = MagicMock() @@ -729,6 +742,296 @@ async def test_cleanup_called_twice_after_creating_clients( assert len(service._http_clients) == 0 +class TestPendingServerGate: + """Tests for the non-blocking, per-server connection gate in ``add_tool_servers_to_agent``. + + When the gateway flags an MCP server's downstream connections as ``Pending``, the service + registers a placeholder ``FunctionTool`` (named after the server) in place of a live + ``MCPStreamableHTTPTool``. The agent is still built and Ready servers stay usable; only if the + model invokes the placeholder does it return a static setup message (including + ``missing_connections_url``). The turn is never aborted. + """ + + @pytest.fixture + def service(self): + return McpToolRegistrationService() + + @pytest.fixture + def mock_chat_client(self): + return Mock() + + @pytest.fixture + def mock_turn_context(self): + ctx = Mock() + ctx.activity = Mock() + ctx.activity.conversation = Mock() + ctx.activity.conversation.id = "conv-1" + ctx.activity.id = "msg-1" + return ctx + + @pytest.fixture + def mock_auth(self): + auth = Mock() + auth.exchange_token = AsyncMock() + auth.exchange_token.return_value = Mock(token="discovery-token") + return auth + + @staticmethod + def _config(name, status, missing_url=None, all_url=None): + c = Mock() + c.mcp_server_name = name + c.mcp_server_unique_name = name + c.url = f"https://gw.example/{name}" + c.headers = None + c.connectivity_status = status + c.missing_connections_url = missing_url + c.all_connections_url = all_url + return c + + @pytest.mark.asyncio + @pytest.mark.unit + async def test_pending_server_registers_placeholder_and_builds_agent( + self, service, mock_chat_client, mock_turn_context, mock_auth + ): + """A Pending server is wired as a placeholder tool; the agent is still built.""" + from agent_framework import FunctionTool + + pending = self._config( + "mcp_Salesforce", "Pending", missing_url="https://make.example/missing/salesforce" + ) + captured_tools = [] + with ( + patch.object( + service._mcp_server_configuration_service, + "list_tool_servers", + new_callable=AsyncMock, + return_value=[pending], + ), + patch( + "microsoft_agents_a365.tooling.extensions.agentframework.services.mcp_tool_registration_service.httpx.AsyncClient" + ) as mock_httpx_client, + patch( + "microsoft_agents_a365.tooling.extensions.agentframework.services.mcp_tool_registration_service.MCPStreamableHTTPTool" + ) as mock_mcp_tool, + patch( + "microsoft_agents_a365.tooling.extensions.agentframework.services.mcp_tool_registration_service.RawAgent" + ) as mock_raw_agent, + patch( + "microsoft_agents_a365.tooling.extensions.agentframework.services.mcp_tool_registration_service.Utility.resolve_agent_identity", + return_value="test-agent-id", + ), + ): + mock_raw_agent.side_effect = lambda **kw: captured_tools.extend(kw["tools"]) or Mock() + await service.add_tool_servers_to_agent( + chat_client=mock_chat_client, + agent_instructions="Test", + initial_tools=[], + auth=mock_auth, + auth_handler_name="h", + turn_context=mock_turn_context, + auth_token="discovery-token", + ) + # No live MCP tool / httpx client for the Pending server; agent still built. + mock_httpx_client.assert_not_called() + mock_mcp_tool.assert_not_called() + mock_raw_agent.assert_called_once() + # Exactly one placeholder FunctionTool, named after the server. + placeholders = [t for t in captured_tools if isinstance(t, FunctionTool)] + assert len(placeholders) == 1 + placeholder = placeholders[0] + assert placeholder.name == "mcp_Salesforce" + # The placeholder is registered, not connected, so it isn't tracked for cleanup. + assert service._connected_servers == [] + # Invoking the placeholder returns the static message with the missing-connections URL. + message = placeholder.func() + assert "https://make.example/missing/salesforce" in message + assert "mcp_Salesforce" in message + # Capped so the model can't loop on it within a turn. + assert placeholder.max_invocations == 1 + + @pytest.mark.asyncio + @pytest.mark.unit + async def test_placeholder_falls_back_to_all_connections_url( + self, service, mock_chat_client, mock_turn_context, mock_auth + ): + """When the gateway omits missing_connections_url, the placeholder uses all_connections_url.""" + from agent_framework import FunctionTool + + pending = self._config( + "mcp_Zendesk", "Pending", missing_url=None, all_url="https://make.example/all/zendesk" + ) + captured_tools = [] + with ( + patch.object( + service._mcp_server_configuration_service, + "list_tool_servers", + new_callable=AsyncMock, + return_value=[pending], + ), + patch( + "microsoft_agents_a365.tooling.extensions.agentframework.services.mcp_tool_registration_service.RawAgent" + ) as mock_raw_agent, + patch( + "microsoft_agents_a365.tooling.extensions.agentframework.services.mcp_tool_registration_service.Utility.resolve_agent_identity", + return_value="test-agent-id", + ), + ): + mock_raw_agent.side_effect = lambda **kw: captured_tools.extend(kw["tools"]) or Mock() + await service.add_tool_servers_to_agent( + chat_client=mock_chat_client, + agent_instructions="Test", + initial_tools=[], + auth=mock_auth, + auth_handler_name="h", + turn_context=mock_turn_context, + auth_token="discovery-token", + ) + placeholder = next(t for t in captured_tools if isinstance(t, FunctionTool)) + assert "https://make.example/all/zendesk" in placeholder.func() + + @pytest.mark.asyncio + @pytest.mark.unit + async def test_mixed_ready_and_pending_wires_both( + self, service, mock_chat_client, mock_turn_context, mock_auth + ): + """Ready servers become real tools; Pending servers become placeholders — turn proceeds.""" + from agent_framework import FunctionTool + + ready = self._config("mcp_Mail", "Ready") + pending = self._config( + "mcp_Salesforce", "Pending", missing_url="https://make.example/missing/sf" + ) + captured_tools = [] + with ( + patch.object( + service._mcp_server_configuration_service, + "list_tool_servers", + new_callable=AsyncMock, + return_value=[ready, pending], + ), + patch( + "microsoft_agents_a365.tooling.extensions.agentframework.services.mcp_tool_registration_service.httpx.AsyncClient" + ), + patch( + "microsoft_agents_a365.tooling.extensions.agentframework.services.mcp_tool_registration_service.MCPStreamableHTTPTool" + ) as mock_mcp_tool, + patch( + "microsoft_agents_a365.tooling.extensions.agentframework.services.mcp_tool_registration_service.RawAgent" + ) as mock_raw_agent, + patch( + "microsoft_agents_a365.tooling.extensions.agentframework.services.mcp_tool_registration_service.Utility.resolve_agent_identity", + return_value="test-agent-id", + ), + ): + mock_raw_agent.side_effect = lambda **kw: captured_tools.extend(kw["tools"]) or Mock() + await service.add_tool_servers_to_agent( + chat_client=mock_chat_client, + agent_instructions="Test", + initial_tools=[], + auth=mock_auth, + auth_handler_name="h", + turn_context=mock_turn_context, + auth_token="discovery-token", + ) + # Ready server wired once as a live MCP tool; agent built once. + assert mock_mcp_tool.call_count == 1 + mock_raw_agent.assert_called_once() + # One placeholder for the Pending server, and the live tool for the Ready one. + placeholders = [t for t in captured_tools if isinstance(t, FunctionTool)] + assert [p.name for p in placeholders] == ["mcp_Salesforce"] + assert len(captured_tools) == 2 + + @pytest.mark.asyncio + @pytest.mark.unit + async def test_all_ready_builds_agent_with_real_tools( + self, service, mock_chat_client, mock_turn_context, mock_auth + ): + """All-Ready (and legacy None) servers → real MCPStreamableHTTPTool; no placeholders.""" + from agent_framework import FunctionTool + + ready = self._config("mcp_Mail", "Ready") + legacy = self._config("mcp_Files", None) # legacy / no status + captured_tools = [] + with ( + patch.object( + service._mcp_server_configuration_service, + "list_tool_servers", + new_callable=AsyncMock, + return_value=[ready, legacy], + ), + patch( + "microsoft_agents_a365.tooling.extensions.agentframework.services.mcp_tool_registration_service.httpx.AsyncClient" + ), + patch( + "microsoft_agents_a365.tooling.extensions.agentframework.services.mcp_tool_registration_service.MCPStreamableHTTPTool" + ) as mock_mcp_tool, + patch( + "microsoft_agents_a365.tooling.extensions.agentframework.services.mcp_tool_registration_service.RawAgent" + ) as mock_raw_agent, + patch( + "microsoft_agents_a365.tooling.extensions.agentframework.services.mcp_tool_registration_service.Utility.resolve_agent_identity", + return_value="test-agent-id", + ), + ): + mock_raw_agent.side_effect = lambda **kw: captured_tools.extend(kw["tools"]) or Mock() + await service.add_tool_servers_to_agent( + chat_client=mock_chat_client, + agent_instructions="Test", + initial_tools=[], + auth=mock_auth, + auth_handler_name="h", + turn_context=mock_turn_context, + auth_token="discovery-token", + ) + # Both servers wired as real MCP tools; agent constructed; no placeholders. + assert mock_mcp_tool.call_count == 2 + mock_raw_agent.assert_called_once() + assert [t for t in captured_tools if isinstance(t, FunctionTool)] == [] + assert len(captured_tools) == 2 + + +class TestExtensionExceptionExport: + """The connection-gating exception/formatter are owned by this extension.""" + + def test_exception_importable_from_package_root(self): + from microsoft_agents_a365.tooling.extensions.agentframework import ( + McpConnectionsRequiredError, + ) + + assert issubclass(McpConnectionsRequiredError, Exception) + + def test_exception_exposes_payload(self): + from microsoft_agents_a365.tooling.extensions.agentframework import ( + McpConnectionsRequiredError, + ) + + err = McpConnectionsRequiredError( + missing_connections_url="https://make.example/missing", + connectivity_status="Pending", + server_names=["mcp_Salesforce", "mcp_Zendesk"], + ) + assert err.missing_connections_url == "https://make.example/missing" + assert err.connectivity_status == "Pending" + assert err.server_names == ["mcp_Salesforce", "mcp_Zendesk"] + message = str(err) + assert "mcp_Salesforce" in message + assert "https://make.example/missing" in message + + def test_formatter_handles_missing_url(self): + from microsoft_agents_a365.tooling.extensions.agentframework.exceptions import ( + format_mcp_connections_required_message, + ) + + message = format_mcp_connections_required_message( + server_names=["mcp_Salesforce"], + connectivity_status="Pending", + missing_connections_url=None, + ) + assert "mcp_Salesforce" in message + assert "http" not in message # no URL when none is supplied + assert "administrator" in message + + class TestMcpHttpClientTimeoutConstant: """Tests for the MCP_HTTP_CLIENT_TIMEOUT_SECONDS constant.""" diff --git a/tests/tooling/test_mcp_server_configuration.py b/tests/tooling/test_mcp_server_configuration.py index 1c365799..60cad5dc 100644 --- a/tests/tooling/test_mcp_server_configuration.py +++ b/tests/tooling/test_mcp_server_configuration.py @@ -48,6 +48,32 @@ def test_mcp_server_config_validation(self): with pytest.raises(ValueError, match="mcp_server_unique_name cannot be empty"): MCPServerConfig(mcp_server_name="test", mcp_server_unique_name="") + def test_mcp_server_config_connection_fields_default_none(self): + """Connection fields default to None for backward compatibility.""" + config = MCPServerConfig( + mcp_server_name="TestServer", + mcp_server_unique_name="test_server", + ) + assert config.id is None + assert config.all_connections_url is None + assert config.missing_connections_url is None + assert config.connectivity_status is None + + def test_mcp_server_config_connection_fields_set(self): + """Connection fields are stored when provided.""" + config = MCPServerConfig( + mcp_server_name="TestServer", + mcp_server_unique_name="test_server", + id="3fa85f64-5717-4562-b3fc-2c963f66afa6", + all_connections_url="https://make.example/all", + missing_connections_url="https://make.example/missing", + connectivity_status="Pending", + ) + assert config.id == "3fa85f64-5717-4562-b3fc-2c963f66afa6" + assert config.all_connections_url == "https://make.example/all" + assert config.missing_connections_url == "https://make.example/missing" + assert config.connectivity_status == "Pending" + class TestMcpToolServerConfigurationService: """Tests for McpToolServerConfigurationService.""" @@ -160,6 +186,42 @@ def test_parse_gateway_server_config_without_custom_url(self, mock_build_url, se assert config.url == "https://default.server/agents/servers/GatewayServer" mock_build_url.assert_called_once_with("GatewayServer") + def test_parse_server_config_populates_connection_fields(self, service): + """Per-server connection fields are parsed from a V2 gateway element.""" + server_element = { + "mcpServerName": "mcp_Salesforce", + "mcpServerUniqueName": "mcp_Salesforce", + "id": "3fa85f64-5717-4562-b3fc-2c963f66afa6", + "url": "https://gw.example/agents/v2/servers/mcp_Salesforce", + "allConnectionsUrl": "https://make.example/all", + "missingConnectionsUrl": "https://make.example/missing", + "connectivityStatus": "Pending", + } + + config = service._parse_server_config(server_element) + + assert config is not None + assert config.id == "3fa85f64-5717-4562-b3fc-2c963f66afa6" + assert config.all_connections_url == "https://make.example/all" + assert config.missing_connections_url == "https://make.example/missing" + assert config.connectivity_status == "Pending" + + def test_parse_server_config_connection_fields_absent(self, service): + """Manifest elements without connection fields yield None.""" + server_element = { + "mcpServerName": "DevServer", + "mcpServerUniqueName": "dev_server", + "url": "https://dev.server/mcp", + } + + config = service._parse_server_config(server_element) + + assert config is not None + assert config.id is None + assert config.all_connections_url is None + assert config.missing_connections_url is None + assert config.connectivity_status is None + @patch.dict(os.environ, {"ENVIRONMENT": "Development"}) def test_is_development_scenario(self, service): """Test development scenario detection.""" @@ -322,6 +384,57 @@ async def test_legacy_prod_path_ok_for_v1_only_servers(self, service): assert len(servers) == 1 assert servers[0].mcp_server_name == "V1Server" + @pytest.mark.asyncio + async def test_parse_gateway_response_wrapped_returns_servers(self, service): + """Wrapped responses return a List[MCPServerConfig] with per-server metadata.""" + payload = { + "mcpServers": [ + { + "mcpServerName": "mcp_Salesforce", + "mcpServerUniqueName": "mcp_Salesforce", + "url": "https://gw.example/agents/v2/servers/mcp_Salesforce", + "connectivityStatus": "Pending", + "allConnectionsUrl": "https://make.example/all/salesforce", + "missingConnectionsUrl": "https://make.example/missing/salesforce", + } + ], + "allConnectionsUrl": "https://make.example/all", + "missingConnectionsUrl": "https://make.example/missing", + "connectivityStatus": "Pending", + } + mock_response = MagicMock() + mock_response.json = AsyncMock(return_value=payload) + + servers = await service._parse_gateway_response(mock_response) + + assert isinstance(servers, list) + assert len(servers) == 1 + assert servers[0].mcp_server_name == "mcp_Salesforce" + assert servers[0].connectivity_status == "Pending" + assert servers[0].all_connections_url == "https://make.example/all/salesforce" + assert servers[0].missing_connections_url == "https://make.example/missing/salesforce" + + @pytest.mark.asyncio + async def test_parse_gateway_response_raw_array_returns_servers(self, service): + """Legacy raw-array responses return a list; per-server metadata is None.""" + payload = [ + { + "mcpServerName": "V1Server", + "mcpServerUniqueName": "v1_server", + "url": "https://v1.example.com/mcp", + } + ] + mock_response = MagicMock() + mock_response.json = AsyncMock(return_value=payload) + + servers = await service._parse_gateway_response(mock_response) + + assert isinstance(servers, list) + assert len(servers) == 1 + assert servers[0].connectivity_status is None + assert servers[0].all_connections_url is None + assert servers[0].missing_connections_url is None + class TestResolveTokenScopeForServer: """Tests for resolve_token_scope_for_server() utility function.""" @@ -878,3 +991,147 @@ def test_skips_empty_blueprint_id(self, service, create_test_jwt): result = service._resolve_agent_id_for_header(token, mock_context) assert result == "token-appid" + + +class TestConnectionGating: + """Core no longer gates on connectivityStatus — it parses the per-server + connection metadata onto each MCPServerConfig and returns the servers + unchanged. Gating is the responsibility of the framework extensions.""" + + @pytest.fixture + def service(self): + return McpToolServerConfigurationService() + + @staticmethod + def _gateway_response(payload): + """Build a patched aiohttp.ClientSession context manager returning payload.""" + mock_response = MagicMock() + mock_response.status = 200 + mock_response.json = AsyncMock(return_value=payload) + mock_response_cm = MagicMock() + mock_response_cm.__aenter__ = AsyncMock(return_value=mock_response) + mock_response_cm.__aexit__ = AsyncMock(return_value=None) + 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) + return patch("aiohttp.ClientSession", return_value=mock_session_cm) + + @patch.dict(os.environ, {"ENVIRONMENT": "Production"}) + @pytest.mark.asyncio + async def test_pending_servers_returned_without_raising(self, service): + """Pending servers flow through ``list_tool_servers`` unchanged. + + Connection gating has been removed from the core service. It is now + the responsibility of framework-specific extensions (see e.g. + ``microsoft-agents-a365-tooling-extensions-agentframework``), which register + a non-blocking placeholder tool for each Pending server. Core simply returns + the configs with their per-server connection metadata intact. + """ + payload = { + "mcpServers": [ + { + "mcpServerName": "mcp_Salesforce", + "mcpServerUniqueName": "mcp_Salesforce", + "url": "https://gw.example/mcp_Salesforce", + "connectivityStatus": "Pending", + "missingConnectionsUrl": "https://make.example/missing/salesforce", + } + ], + "allConnectionsUrl": "https://make.example/all", + "missingConnectionsUrl": "https://make.example/missing", + "connectivityStatus": "Pending", + } + with self._gateway_response(payload): + 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 == "mcp_Salesforce" + assert servers[0].connectivity_status == "Pending" + assert servers[0].missing_connections_url == "https://make.example/missing/salesforce" + + @patch.dict(os.environ, {"ENVIRONMENT": "Production"}) + @pytest.mark.asyncio + async def test_gate_passes_when_ready(self, service): + payload = { + "mcpServers": [ + { + "mcpServerName": "mcp_Salesforce", + "mcpServerUniqueName": "mcp_Salesforce", + "url": "https://gw.example/mcp_Salesforce", + "connectivityStatus": "Ready", + } + ], + "allConnectionsUrl": "https://make.example/all", + "missingConnectionsUrl": "https://make.example/missing", + "connectivityStatus": "Ready", + } + with self._gateway_response(payload): + 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 == "mcp_Salesforce" + + @patch.dict(os.environ, {"ENVIRONMENT": "Production"}) + @pytest.mark.asyncio + async def test_gate_passes_for_legacy_raw_array(self, service): + payload = [ + { + "mcpServerName": "V1Server", + "mcpServerUniqueName": "v1_server", + "url": "https://v1.example.com/mcp", + } + ] + with self._gateway_response(payload): + 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 == "V1Server" + + @patch.dict(os.environ, {"ENVIRONMENT": "Production"}) + @pytest.mark.asyncio + async def test_aggregate_pending_returns_servers_without_raising(self, service): + """Even when the response-level aggregate is Pending, ``list_tool_servers`` + returns the servers without raising — core does not act on the aggregate + (or per-server) connection metadata. + """ + payload = { + "mcpServers": [ + { + "mcpServerName": "mcp_Salesforce", + "mcpServerUniqueName": "mcp_Salesforce", + "url": "https://gw.example/mcp_Salesforce", + "connectivityStatus": "Ready", + } + ], + "allConnectionsUrl": "https://make.example/all", + "missingConnectionsUrl": "https://make.example/missing", + "connectivityStatus": "Pending", + } + with self._gateway_response(payload): + 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 == "mcp_Salesforce" + + @patch.object(McpToolServerConfigurationService, "_load_servers_from_manifest") + @patch.dict(os.environ, {"ENVIRONMENT": "Development"}) + @pytest.mark.asyncio + async def test_gate_not_applied_in_dev_mode(self, mock_load_manifest, service): + mock_load_manifest.return_value = [ + MCPServerConfig( + mcp_server_name="DevServer", + mcp_server_unique_name="dev_server", + url="https://dev.server/mcp", + ) + ] + 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 == "DevServer"