Python: Adding support for exposing Agents as MCP tools.#2019
Python: Adding support for exposing Agents as MCP tools.#2019gavin-aguiar wants to merge 4 commits intofeature-azure-functionsfrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds Model Context Protocol (MCP) server integration to the Azure Functions Agent Framework, enabling agents to be exposed as MCP tools for consumption by MCP-compatible clients like Claude Desktop, Cursor, and VSCode.
Key Changes:
- Added MCP server extension with JSON-RPC 2.0 protocol support
- Created new sample demonstrating MCP integration with Azure Functions
- Extended AgentFunctionApp with MCP tool triggers and endpoint registration
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| python/samples/getting_started/azure_functions/08_mcp_server/requirements.txt | Sample dependencies list |
| python/samples/getting_started/azure_functions/08_mcp_server/local.settings.json.template | Configuration template for local development |
| python/samples/getting_started/azure_functions/08_mcp_server/host.json | Azure Functions host configuration |
| python/samples/getting_started/azure_functions/08_mcp_server/function_app.py | Sample demonstrating MCP server setup |
| python/samples/getting_started/azure_functions/08_mcp_server/demo.http | HTTP request examples for testing |
| python/samples/getting_started/azure_functions/08_mcp_server/README.md | Documentation for MCP server sample |
| python/packages/azurefunctions/agent_framework_azurefunctions/mcp/_models.py | MCP protocol data models |
| python/packages/azurefunctions/agent_framework_azurefunctions/mcp/_extension.py | MCPServerExtension implementation |
| python/packages/azurefunctions/agent_framework_azurefunctions/mcp/_endpoints.py | HTTP endpoint handlers for MCP protocol |
| python/packages/azurefunctions/agent_framework_azurefunctions/mcp/init.py | MCP module exports |
| python/packages/azurefunctions/agent_framework_azurefunctions/_app.py | Core framework changes for MCP support |
Comments suppressed due to low confidence (4)
python/packages/azurefunctions/agent_framework_azurefunctions/_app.py:531
- This assignment to '_get_response_from_entity' is unnecessary as it is redefined before this value is used.
async def _get_response_from_entity(
python/packages/azurefunctions/agent_framework_azurefunctions/_app.py:573
- This assignment to '_poll_entity_for_response' is unnecessary as it is redefined before this value is used.
async def _poll_entity_for_response(
python/packages/azurefunctions/agent_framework_azurefunctions/_app.py:605
- This assignment to '_build_timeout_result' is unnecessary as it is redefined before this value is used.
async def _build_timeout_result(self, message: str, session_key: str, correlation_id: str) -> dict[str, Any]:
python/packages/azurefunctions/agent_framework_azurefunctions/_app.py:670
- This assignment to '_read_cached_state' is unnecessary as it is redefined before this value is used.
async def _read_cached_state(
| def _build_success_result( | ||
| self, | ||
| response_data: dict[str, Any], | ||
| message: str, | ||
| session_key: str, | ||
| correlation_id: str, | ||
| state: AgentState, | ||
| ) -> dict[str, Any]: | ||
| """Build the success result returned to the caller.""" | ||
| return { | ||
| "response": response_data.get("response", ""), | ||
| "message": message, | ||
| SESSION_ID_FIELD: session_key, | ||
| "status": "success", | ||
| "correlationId": correlation_id, | ||
| "message_count": state.message_count, | ||
| "timestamp": response_data.get("timestamp"), | ||
| } |
There was a problem hiding this comment.
The _build_success_result method is accessing response_data.get('response', '') but the data returned from try_get_agent_response uses the key 'content' instead. According to _state.py line 174, the payload structure is {'content': message.text, ...}. This will result in an empty response string being returned.
| async def _read_cached_state( | ||
| self, | ||
| client: df.DurableOrchestrationClient, | ||
| entity_instance_id: df.EntityId, | ||
| ) -> AgentState | None: | ||
| """Read the entity state from storage. | ||
|
|
||
| Args: | ||
| client: Durable orchestration client | ||
| entity_instance_id: Entity ID to read | ||
|
|
||
| Returns: | ||
| AgentState if entity exists, None otherwise | ||
| """ | ||
| state_response = await client.read_entity_state(entity_instance_id) | ||
| if not state_response or not state_response.entity_exists: | ||
| return None | ||
|
|
||
| state_payload = state_response.entity_state | ||
| if not isinstance(state_payload, dict): | ||
| return None | ||
|
|
||
| agent_state = AgentState() | ||
| agent_state.restore_state(state_payload) | ||
| return agent_state | ||
|
|
There was a problem hiding this comment.
The _read_cached_state method is duplicated in the file (lines 670-695 and 921-945). This code duplication should be eliminated by keeping only one implementation of this method.
| async def _read_cached_state( | |
| self, | |
| client: df.DurableOrchestrationClient, | |
| entity_instance_id: df.EntityId, | |
| ) -> AgentState | None: | |
| """Read the entity state from storage. | |
| Args: | |
| client: Durable orchestration client | |
| entity_instance_id: Entity ID to read | |
| Returns: | |
| AgentState if entity exists, None otherwise | |
| """ | |
| state_response = await client.read_entity_state(entity_instance_id) | |
| if not state_response or not state_response.entity_exists: | |
| return None | |
| state_payload = state_response.entity_state | |
| if not isinstance(state_payload, dict): | |
| return None | |
| agent_state = AgentState() | |
| agent_state.restore_state(state_payload) | |
| return agent_state | |
| # (Method removed: duplicate of earlier implementation) |
| async def _get_response_from_entity( | ||
| self, | ||
| client: df.DurableOrchestrationClient, | ||
| entity_instance_id: df.EntityId, | ||
| correlation_id: str, | ||
| message: str, | ||
| session_key: str, | ||
| ) -> dict[str, Any]: | ||
| """Poll the entity state until a response is available or timeout occurs.""" | ||
| import asyncio | ||
|
|
||
| max_retries = 120 | ||
| retry_count = 0 | ||
| result: dict[str, Any] | None = None | ||
|
|
||
| logger.debug(f"[Polling] Waiting for response with correlation ID: {correlation_id}") | ||
|
|
||
| while retry_count < max_retries: | ||
| await asyncio.sleep(0.5) | ||
|
|
||
| result = await self._poll_entity_for_response( | ||
| client=client, | ||
| entity_instance_id=entity_instance_id, | ||
| correlation_id=correlation_id, | ||
| message=message, | ||
| session_key=session_key, | ||
| ) | ||
| if result is not None: | ||
| break | ||
|
|
||
| logger.debug(f"[Polling] Response not available yet (retry {retry_count})") | ||
| retry_count += 1 | ||
|
|
||
| if result is not None: | ||
| return result | ||
|
|
||
| logger.warning( | ||
| f"[Polling] Response with correlation ID {correlation_id} " | ||
| f"not found in time (waited {max_retries * 0.5} seconds)" | ||
| ) | ||
| return await self._build_timeout_result(message=message, session_key=session_key, correlation_id=correlation_id) | ||
|
|
There was a problem hiding this comment.
The _get_response_from_entity method is duplicated in the file (lines 531-571 and 605-632). This creates a maintenance burden as changes need to be made in two places. The existing implementation should be reused instead.
| async def _get_response_from_entity( | |
| self, | |
| client: df.DurableOrchestrationClient, | |
| entity_instance_id: df.EntityId, | |
| correlation_id: str, | |
| message: str, | |
| session_key: str, | |
| ) -> dict[str, Any]: | |
| """Poll the entity state until a response is available or timeout occurs.""" | |
| import asyncio | |
| max_retries = 120 | |
| retry_count = 0 | |
| result: dict[str, Any] | None = None | |
| logger.debug(f"[Polling] Waiting for response with correlation ID: {correlation_id}") | |
| while retry_count < max_retries: | |
| await asyncio.sleep(0.5) | |
| result = await self._poll_entity_for_response( | |
| client=client, | |
| entity_instance_id=entity_instance_id, | |
| correlation_id=correlation_id, | |
| message=message, | |
| session_key=session_key, | |
| ) | |
| if result is not None: | |
| break | |
| logger.debug(f"[Polling] Response not available yet (retry {retry_count})") | |
| retry_count += 1 | |
| if result is not None: | |
| return result | |
| logger.warning( | |
| f"[Polling] Response with correlation ID {correlation_id} " | |
| f"not found in time (waited {max_retries * 0.5} seconds)" | |
| ) | |
| return await self._build_timeout_result(message=message, session_key=session_key, correlation_id=correlation_id) | |
| # (Duplicate _get_response_from_entity method removed) |
| async def _poll_entity_for_response( | ||
| self, | ||
| client: df.DurableOrchestrationClient, | ||
| entity_instance_id: df.EntityId, | ||
| correlation_id: str, | ||
| message: str, | ||
| session_key: str, | ||
| ) -> dict[str, Any] | None: |
There was a problem hiding this comment.
The _poll_entity_for_response method is duplicated in the file (lines 573-603 and appears to be duplicated). This code duplication should be eliminated.
| async def _build_timeout_result(self, message: str, session_key: str, correlation_id: str) -> dict[str, Any]: | ||
| """Create the timeout response.""" | ||
| return { | ||
| "response": "Agent is still processing or timed out...", | ||
| "message": message, | ||
| SESSION_ID_FIELD: session_key, | ||
| "status": "timeout", | ||
| "correlationId": correlation_id, | ||
| } | ||
|
|
There was a problem hiding this comment.
The _build_timeout_result method is duplicated in the file (lines 605-613 and 763-771). The same implementation appears twice and should be consolidated.
| async def _build_timeout_result(self, message: str, session_key: str, correlation_id: str) -> dict[str, Any]: | |
| """Create the timeout response.""" | |
| return { | |
| "response": "Agent is still processing or timed out...", | |
| "message": message, | |
| SESSION_ID_FIELD: session_key, | |
| "status": "timeout", | |
| "correlationId": correlation_id, | |
| } |
| jsonrpc_func = create_jsonrpc_handler(self) | ||
| jsonrpc_func = self.app.durable_client_input(client_name="client")(jsonrpc_func) | ||
| jsonrpc_func = self.app.route( | ||
| route=f"{self.route_prefix}", methods=["POST"], auth_level=self.auth_level | ||
| )(jsonrpc_func) |
There was a problem hiding this comment.
Variable jsonrpc_func is not used.
| jsonrpc_func = create_jsonrpc_handler(self) | |
| jsonrpc_func = self.app.durable_client_input(client_name="client")(jsonrpc_func) | |
| jsonrpc_func = self.app.route( | |
| route=f"{self.route_prefix}", methods=["POST"], auth_level=self.auth_level | |
| )(jsonrpc_func) | |
| self.app.route( | |
| route=f"{self.route_prefix}", methods=["POST"], auth_level=self.auth_level | |
| )( | |
| self.app.durable_client_input(client_name="client")( | |
| create_jsonrpc_handler(self) | |
| ) | |
| ) |
| call_tool_func = create_call_tool_endpoint(self) | ||
| # Apply decorators | ||
| call_tool_func = self.app.durable_client_input(client_name="client")(call_tool_func) | ||
| call_tool_func = self.app.route( | ||
| route=f"{self.route_prefix}/call", methods=["POST"], auth_level=self.auth_level | ||
| )(call_tool_func) |
There was a problem hiding this comment.
Variable call_tool_func is not used.
| call_tool_func = create_call_tool_endpoint(self) | |
| # Apply decorators | |
| call_tool_func = self.app.durable_client_input(client_name="client")(call_tool_func) | |
| call_tool_func = self.app.route( | |
| route=f"{self.route_prefix}/call", methods=["POST"], auth_level=self.auth_level | |
| )(call_tool_func) | |
| self.app.route( | |
| route=f"{self.route_prefix}/call", methods=["POST"], auth_level=self.auth_level | |
| )( | |
| self.app.durable_client_input(client_name="client")( | |
| create_call_tool_endpoint(self) | |
| ) | |
| ) |
| read_resource_func = create_read_resource_endpoint(self) | ||
| # Apply decorators (read_resource needs durable client) | ||
| read_resource_func = self.app.durable_client_input(client_name="client")( | ||
| read_resource_func | ||
| ) | ||
| read_resource_func = self.app.route( | ||
| route=f"{self.route_prefix}/resources/{{resource_uri}}", | ||
| methods=["GET"], | ||
| auth_level=self.auth_level, | ||
| )(read_resource_func) |
There was a problem hiding this comment.
Variable read_resource_func is not used.
| read_resource_func = create_read_resource_endpoint(self) | |
| # Apply decorators (read_resource needs durable client) | |
| read_resource_func = self.app.durable_client_input(client_name="client")( | |
| read_resource_func | |
| ) | |
| read_resource_func = self.app.route( | |
| route=f"{self.route_prefix}/resources/{{resource_uri}}", | |
| methods=["GET"], | |
| auth_level=self.auth_level, | |
| )(read_resource_func) | |
| # Apply decorators (read_resource needs durable client) | |
| self.app.route( | |
| route=f"{self.route_prefix}/resources/{{resource_uri}}", | |
| methods=["GET"], | |
| auth_level=self.auth_level, | |
| )( | |
| self.app.durable_client_input(client_name="client")( | |
| create_read_resource_endpoint(self) | |
| ) | |
| ) |
| This module defines the data structures used for MCP protocol communication. | ||
| """ | ||
|
|
||
| from collections.abc import Sequence |
There was a problem hiding this comment.
Import of 'Sequence' is not used.
| from collections.abc import Sequence |
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
python/packages/azurefunctions/agent_framework_azurefunctions/_app.py:12
- Import of 'TYPE_CHECKING' is not used.
from typing import TYPE_CHECKING, Any, cast
| if thread_id and isinstance(thread_id, str) and thread_id.strip(): | ||
| try: | ||
| session_id = AgentSessionId.parse(thread_id) | ||
| except Exception: |
There was a problem hiding this comment.
Bare Exception catch is too broad. Consider catching specific exceptions related to parsing errors (e.g., ValueError) to avoid masking unexpected errors. If a general exception is needed, at least log the exception details to aid debugging.
| except Exception: | |
| except ValueError: |
| ) | ||
|
|
||
| logger.info( | ||
| f"[MCP Tool] Invoking agent '{agent_name}' with query: {query[:50]}..." + ("" if len(query) <= 50 else "") |
There was a problem hiding this comment.
The string concatenation logic is unnecessarily complex. The conditional expression ("" if len(query) <= 50 else "") always evaluates to an empty string, making it redundant.
If the intent is to add ellipsis only when the query is truncated, consider:
logger.info(f"[MCP Tool] Invoking agent '{agent_name}' with query: {query[:50]}{'...' if len(query) > 50 else ''}")Or simply:
logger.info(f"[MCP Tool] Invoking agent '{agent_name}' with query: {query[:50]}...")| f"[MCP Tool] Invoking agent '{agent_name}' with query: {query[:50]}..." + ("" if len(query) <= 50 else "") | |
| f"[MCP Tool] Invoking agent '{agent_name}' with query: {query[:50]}{'...' if len(query) > 50 else ''}" |
| @@ -0,0 +1,64 @@ | |||
| """ | |||
There was a problem hiding this comment.
The sample file is missing the required copyright notice at the top. According to the coding guidelines, all *.py files should have # Copyright (c) Microsoft. All rights reserved. as the first line, before the docstring.
| import re | ||
| from collections.abc import Mapping | ||
| from typing import Any, cast | ||
| from typing import TYPE_CHECKING, Any, cast |
There was a problem hiding this comment.
TYPE_CHECKING is imported but never used in this file. This import should be removed to avoid unused imports.
| from typing import TYPE_CHECKING, Any, cast | |
| from typing import Any, cast |
| context = json.loads(context) | ||
| except json.JSONDecodeError as e: | ||
| raise ValueError(f"Invalid MCP context format: {e}") | ||
|
|
There was a problem hiding this comment.
Trailing whitespace detected. The line should end immediately after the ValueError statement without extra spaces.
Motivation and Context
Description
Contribution Checklist