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 19859eca..5a0187b0 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 @@ -47,9 +47,11 @@ def errors(self) -> List[OperationError]: Get the list of errors that occurred during the operation. Returns: - List[OperationError]: List of operation errors. + 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 self._errors + return list(self._errors) @staticmethod def success() -> "OperationResult": @@ -88,4 +90,4 @@ def __str__(self) -> str: return "Succeeded" else: error_messages = ", ".join(str(error.message) for error in self._errors) - return f"Failed : {error_messages}" + return f"Failed: {error_messages}" if error_messages else "Failed" 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 bd2fcf9c..56aeaa89 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 @@ -38,8 +38,8 @@ def __post_init__(self): raise ValueError("role cannot be empty") if not self.content: raise ValueError("content cannot be empty") - if not self.timestamp: - raise ValueError("timestamp cannot be empty") + if self.timestamp is None: + raise ValueError("timestamp cannot be None") def to_dict(self): """ 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 bcaeb12e..ace0b466 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 @@ -31,11 +31,16 @@ from microsoft_agents.hosting.core import TurnContext # Local imports -from ..models import ChatHistoryMessage, MCPServerConfig, ToolOptions +from ..models import ChatHistoryMessage, ChatMessageRequest, MCPServerConfig, ToolOptions from ..utils import Constants -from ..utils.utility import get_tooling_gateway_for_digital_worker, build_mcp_server_url +from ..utils.utility import ( + get_tooling_gateway_for_digital_worker, + build_mcp_server_url, + get_chat_history_endpoint, +) # Runtime Imports +from microsoft_agents_a365.runtime import OperationError, OperationResult from microsoft_agents_a365.runtime.utility import Utility as RuntimeUtility @@ -520,12 +525,6 @@ async def send_chat_history( Raises: ValueError: If required parameters are invalid or empty. """ - # Import here to avoid circular dependency - from microsoft_agents_a365.runtime import OperationError, OperationResult - - from ..models import ChatMessageRequest - from ..utils.utility import get_chat_history_endpoint - # Validate input parameters if not turn_context: raise ValueError("turn_context cannot be empty or None") @@ -560,7 +559,7 @@ async def send_chat_history( # Get the endpoint URL endpoint = get_chat_history_endpoint() - self._logger.info(f"Sending chat history to endpoint: {endpoint}") + self._logger.debug(f"Sending chat history to endpoint: {endpoint}") # Create the request payload request = ChatMessageRequest( diff --git a/tests/runtime/test_operation_result.py b/tests/runtime/test_operation_result.py index 77c8a3dc..61edbed9 100644 --- a/tests/runtime/test_operation_result.py +++ b/tests/runtime/test_operation_result.py @@ -85,7 +85,7 @@ def test_operation_result_failed_string_representation_no_errors(self): result = OperationResult.failed() # Assert - assert str(result) == "Failed : " + assert str(result) == "Failed" def test_operation_result_failed_string_representation_with_errors(self): """Test that failed OperationResult with errors has correct string representation.""" diff --git a/tests/tooling/models/test_chat_history_message.py b/tests/tooling/models/test_chat_history_message.py index bbd00cb2..570e6141 100644 --- a/tests/tooling/models/test_chat_history_message.py +++ b/tests/tooling/models/test_chat_history_message.py @@ -70,7 +70,7 @@ def test_chat_history_message_requires_non_empty_content(self): def test_chat_history_message_requires_timestamp(self): """Test that ChatHistoryMessage requires a timestamp.""" # Act & Assert - with pytest.raises(ValueError, match="timestamp cannot be empty"): + with pytest.raises(ValueError, match="timestamp cannot be None"): ChatHistoryMessage("msg-001", "user", "Test content", None) def test_chat_history_message_supports_system_role(self): diff --git a/tests/tooling/services/test_send_chat_history.py b/tests/tooling/services/test_send_chat_history.py index 90a65843..1783c34c 100644 --- a/tests/tooling/services/test_send_chat_history.py +++ b/tests/tooling/services/test_send_chat_history.py @@ -124,23 +124,24 @@ async def test_send_chat_history_with_options( # Assert assert result.succeeded is True - def test_send_chat_history_validates_turn_context(self, service, chat_history_messages): + @pytest.mark.asyncio + async def test_send_chat_history_validates_turn_context(self, service, chat_history_messages): """Test that send_chat_history validates turn_context parameter.""" # Act & Assert with pytest.raises(ValueError, match="turn_context cannot be empty or None"): - import asyncio - - asyncio.run(service.send_chat_history(None, chat_history_messages)) + await service.send_chat_history(None, chat_history_messages) - def test_send_chat_history_validates_chat_history_messages(self, service, mock_turn_context): + @pytest.mark.asyncio + async def test_send_chat_history_validates_chat_history_messages( + self, service, mock_turn_context + ): """Test that send_chat_history validates chat_history_messages parameter.""" # Act & Assert with pytest.raises(ValueError, match="chat_history_messages cannot be empty or None"): - import asyncio + await service.send_chat_history(mock_turn_context, None) - asyncio.run(service.send_chat_history(mock_turn_context, None)) - - def test_send_chat_history_validates_activity(self, service, chat_history_messages): + @pytest.mark.asyncio + async def test_send_chat_history_validates_activity(self, service, chat_history_messages): """Test that send_chat_history validates turn_context.activity.""" # Arrange mock_context = Mock() @@ -148,11 +149,12 @@ def test_send_chat_history_validates_activity(self, service, chat_history_messag # Act & Assert with pytest.raises(ValueError, match="turn_context.activity cannot be None"): - import asyncio - - asyncio.run(service.send_chat_history(mock_context, chat_history_messages)) + await service.send_chat_history(mock_context, chat_history_messages) - def test_send_chat_history_validates_conversation_id(self, service, chat_history_messages): + @pytest.mark.asyncio + async def test_send_chat_history_validates_conversation_id( + self, service, chat_history_messages + ): """Test that send_chat_history validates conversation_id from activity.""" # Arrange mock_context = Mock() @@ -167,11 +169,10 @@ def test_send_chat_history_validates_conversation_id(self, service, chat_history ValueError, match="conversation_id cannot be empty or None.*turn_context.activity.conversation.id", ): - import asyncio + await service.send_chat_history(mock_context, chat_history_messages) - asyncio.run(service.send_chat_history(mock_context, chat_history_messages)) - - def test_send_chat_history_validates_message_id(self, service, chat_history_messages): + @pytest.mark.asyncio + async def test_send_chat_history_validates_message_id(self, service, chat_history_messages): """Test that send_chat_history validates message_id from activity.""" # Arrange mock_context = Mock() @@ -187,11 +188,10 @@ def test_send_chat_history_validates_message_id(self, service, chat_history_mess with pytest.raises( ValueError, match="message_id cannot be empty or None.*turn_context.activity.id" ): - import asyncio + await service.send_chat_history(mock_context, chat_history_messages) - asyncio.run(service.send_chat_history(mock_context, chat_history_messages)) - - def test_send_chat_history_validates_user_message(self, service, chat_history_messages): + @pytest.mark.asyncio + async def test_send_chat_history_validates_user_message(self, service, chat_history_messages): """Test that send_chat_history validates user_message from activity.""" # Arrange mock_context = Mock() @@ -207,9 +207,7 @@ def test_send_chat_history_validates_user_message(self, service, chat_history_me with pytest.raises( ValueError, match="user_message cannot be empty or None.*turn_context.activity.text" ): - import asyncio - - asyncio.run(service.send_chat_history(mock_context, chat_history_messages)) + await service.send_chat_history(mock_context, chat_history_messages) @pytest.mark.asyncio async def test_send_chat_history_handles_client_error(