diff --git a/libraries/microsoft-agents-a365-runtime/microsoft_agents_a365/runtime/operation_result.py b/libraries/microsoft-agents-a365-runtime/microsoft_agents_a365/runtime/operation_result.py index 5a0187b0..e51e8c12 100644 --- a/libraries/microsoft-agents-a365-runtime/microsoft_agents_a365/runtime/operation_result.py +++ b/libraries/microsoft-agents-a365-runtime/microsoft_agents_a365/runtime/operation_result.py @@ -46,10 +46,13 @@ def errors(self) -> List[OperationError]: """ Get the list of errors that occurred during the operation. + Note: + This property returns a defensive copy of the internal error list + to prevent external modifications, which is especially important for + protecting the singleton instance returned by success(). + Returns: List[OperationError]: A copy of the list of operation errors. - The returned list is a defensive copy to protect the singleton - instance returned by success() from accidental modification. """ return list(self._errors) @@ -61,8 +64,6 @@ def success() -> "OperationResult": Returns: OperationResult: An OperationResult indicating a successful operation. """ - if OperationResult._success_instance is None: - OperationResult._success_instance = OperationResult(succeeded=True) return OperationResult._success_instance @staticmethod @@ -91,3 +92,7 @@ def __str__(self) -> str: else: error_messages = ", ".join(str(error.message) for error in self._errors) return f"Failed: {error_messages}" if error_messages else "Failed" + + +# Module-level eager initialization (thread-safe by Python's import lock) +OperationResult._success_instance = OperationResult(succeeded=True) diff --git a/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/models/chat_history_message.py b/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/models/chat_history_message.py index 06f819c3..05201861 100644 --- a/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/models/chat_history_message.py +++ b/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/models/chat_history_message.py @@ -42,11 +42,11 @@ def __post_init__(self): or if timestamp is None. """ if not self.id or not self.id.strip(): - raise ValueError("id cannot be empty") + raise ValueError("id cannot be empty or whitespace-only") if not self.role or not self.role.strip(): - raise ValueError("role cannot be empty") + raise ValueError("role cannot be empty or whitespace-only") if not self.content or not self.content.strip(): - raise ValueError("content cannot be empty") + raise ValueError("content cannot be empty or whitespace-only") if self.timestamp is None: raise ValueError("timestamp cannot be None") 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 4e13ecd1..056c0a05 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 @@ -45,6 +45,17 @@ from microsoft_agents_a365.runtime.utility import Utility as RuntimeUtility +# ============================================================================== +# CONSTANTS +# ============================================================================== + +# HTTP timeout in seconds for request operations +DEFAULT_REQUEST_TIMEOUT_SECONDS = 30 + +# HTTP status code for successful response +HTTP_STATUS_OK = 200 + + # ============================================================================== # MAIN SERVICE CLASS # ============================================================================== @@ -532,6 +543,20 @@ async def send_chat_history( ValueError: If turn_context is None, chat_history_messages is None or empty, turn_context.activity is None, or any of the required fields (conversation.id, activity.id, activity.text) are missing or empty. + + Example: + >>> from datetime import datetime, timezone + >>> from microsoft_agents_a365.tooling.models import ChatHistoryMessage + >>> + >>> history = [ + ... ChatHistoryMessage("msg-1", "user", "Hello", datetime.now(timezone.utc)), + ... ChatHistoryMessage("msg-2", "assistant", "Hi!", datetime.now(timezone.utc)) + ... ] + >>> + >>> service = McpToolServerConfigurationService() + >>> result = await service.send_chat_history(turn_context, history) + >>> if result.succeeded: + ... print("Chat history sent successfully") """ # Validate input parameters if turn_context is None: @@ -543,13 +568,15 @@ async def send_chat_history( if not turn_context.activity: raise ValueError("turn_context.activity cannot be None") - conversation_id = ( + conversation_id: Optional[str] = ( turn_context.activity.conversation.id if turn_context.activity.conversation else None ) - message_id = turn_context.activity.id - user_message = turn_context.activity.text + message_id: Optional[str] = turn_context.activity.id + user_message: Optional[str] = turn_context.activity.text - if conversation_id is None or (isinstance(conversation_id, str) and not conversation_id.strip()): + if conversation_id is None or ( + isinstance(conversation_id, str) and not conversation_id.strip() + ): raise ValueError( "conversation_id cannot be empty or None (from turn_context.activity.conversation.id)" ) @@ -592,16 +619,17 @@ async def send_chat_history( json_data = json.dumps(request.to_dict()) # Send POST request with timeout to prevent indefinite hangs - timeout = aiohttp.ClientTimeout(total=30) # 30 second timeout + timeout = aiohttp.ClientTimeout(total=DEFAULT_REQUEST_TIMEOUT_SECONDS) async with aiohttp.ClientSession(timeout=timeout) as session: async with session.post(endpoint, headers=headers, data=json_data) as response: - if response.status == 200: + if response.status == HTTP_STATUS_OK: self._logger.info("Successfully sent chat history to MCP platform") return OperationResult.success() else: error_text = await response.text() self._logger.error( - f"HTTP error sending chat history: HTTP {response.status}" + f"HTTP error sending chat history: HTTP {response.status}. " + f"Response: {error_text[:500]}" ) # Use ClientResponseError for consistent error handling http_error = aiohttp.ClientResponseError( @@ -613,14 +641,16 @@ async def send_chat_history( ) return OperationResult.failed(OperationError(http_error)) - except aiohttp.ClientError as http_ex: - self._logger.error(f"HTTP error sending chat history to '{endpoint}': {str(http_ex)}") - return OperationResult.failed(OperationError(http_ex)) except asyncio.TimeoutError as timeout_ex: + # Catch TimeoutError before ClientError since aiohttp.ServerTimeoutError + # inherits from both asyncio.TimeoutError and aiohttp.ClientError self._logger.error( f"Request timeout sending chat history to '{endpoint}': {str(timeout_ex)}" ) return OperationResult.failed(OperationError(timeout_ex)) + except aiohttp.ClientError as http_ex: + self._logger.error(f"HTTP error sending chat history to '{endpoint}': {str(http_ex)}") + return OperationResult.failed(OperationError(http_ex)) except Exception as ex: self._logger.error(f"Failed to send chat history to '{endpoint}': {str(ex)}") return OperationResult.failed(OperationError(ex)) diff --git a/tests/tooling/services/test_send_chat_history.py b/tests/tooling/services/test_send_chat_history.py index 741fb2b2..2b4416a8 100644 --- a/tests/tooling/services/test_send_chat_history.py +++ b/tests/tooling/services/test_send_chat_history.py @@ -7,6 +7,7 @@ from unittest.mock import AsyncMock, MagicMock, Mock, patch import pytest +from microsoft_agents.hosting.core import TurnContext from microsoft_agents_a365.tooling.models import ChatHistoryMessage from microsoft_agents_a365.tooling.services import McpToolServerConfigurationService @@ -16,8 +17,8 @@ class TestSendChatHistory: @pytest.fixture def mock_turn_context(self): - """Create a mock TurnContext.""" - mock_context = Mock() + """Create a mock TurnContext with spec for stricter interface validation.""" + mock_context = Mock(spec=TurnContext) mock_activity = Mock() mock_conversation = Mock()