diff --git a/CLAUDE.md b/CLAUDE.md index b273e8dd..be898b02 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -173,6 +173,7 @@ Place it before imports with one blank line after. - Use explicit `None` checks: `if x is not None:` not `if x:` - Local imports should be moved to top of file - Return defensive copies of mutable data to protect singletons +- **Async method naming**: Do NOT use `_async` suffix on async methods. The `_async` suffix is only appropriate when providing both sync and async versions of the same method. Since this SDK is async-only, use plain method names (e.g., `send_chat_history_messages` not `send_chat_history_messages_async`) ### Type Hints - NEVER Use `Any` 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 569f5860..7d8af5ee 100644 --- a/libraries/microsoft-agents-a365-tooling-extensions-agentframework/docs/design.md +++ b/libraries/microsoft-agents-a365-tooling-extensions-agentframework/docs/design.md @@ -76,6 +76,87 @@ mcp_tool = MCPStreamableHTTPTool( ) ``` +### 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. + +#### send_chat_history_messages + +The primary method for sending chat history. Converts Agent Framework `ChatMessage` objects to the `ChatHistoryMessage` format expected by the MCP platform. + +```python +from agent_framework import ChatMessage, Role + +service = McpToolRegistrationService() + +# Create messages +messages = [ + ChatMessage(role=Role.USER, text="Hello, how are you?"), + ChatMessage(role=Role.ASSISTANT, text="I'm doing well, thank you!"), +] + +# Send to MCP platform for threat protection +result = await service.send_chat_history_messages(messages, turn_context) + +if result.succeeded: + print("Chat history sent successfully") +else: + print(f"Failed: {result.errors}") +``` + +#### send_chat_history_from_store + +A convenience method that extracts messages from a `ChatMessageStoreProtocol` and delegates to `send_chat_history_messages`. + +```python +# Using a ChatMessageStore directly +result = await service.send_chat_history_from_store( + thread.chat_message_store, + turn_context +) +``` + +#### Chat History API Parameters + +| Method | Parameter | Type | Description | +|--------|-----------|------|-------------| +| `send_chat_history_messages` | `chat_messages` | `Sequence[ChatMessage]` | Messages to send | +| | `turn_context` | `TurnContext` | Conversation context | +| | `tool_options` | `ToolOptions \| None` | Optional configuration | +| `send_chat_history_from_store` | `chat_message_store` | `ChatMessageStoreProtocol` | Message store | +| | `turn_context` | `TurnContext` | Conversation context | +| | `tool_options` | `ToolOptions \| None` | Optional configuration | + +#### Chat History Integration Flow + +``` +Agent Framework ChatMessage objects + │ + ▼ +McpToolRegistrationService.send_chat_history_messages() + │ + ├── Convert ChatMessage → ChatHistoryMessage + │ ├── Extract role via .value property + │ ├── Generate UUID if message_id is None + │ ├── Filter out empty/whitespace content + │ └── Filter out None roles + │ + ▼ +McpToolServerConfigurationService.send_chat_history() + │ + ▼ +MCP Platform Real-Time Threat Protection Endpoint +``` + +#### Message Filtering Behavior + +The conversion process filters out invalid messages: +- Messages with `None` role are skipped (logged at WARNING level) +- Messages with empty or whitespace-only content are skipped +- If all messages are filtered out, the method returns success without calling the backend + +This ensures only valid, meaningful messages are sent for threat analysis. + ## File Structure ``` 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 11cb8f27..c4b1ec88 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 @@ -1,22 +1,24 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. -from typing import Optional, List, Any, Union import logging +import uuid +from datetime import datetime, timezone +from typing import Any, List, Optional, Sequence, Union -from agent_framework import ChatAgent, MCPStreamableHTTPTool +from agent_framework import ChatAgent, ChatMessage, ChatMessageStoreProtocol, MCPStreamableHTTPTool from agent_framework.azure import AzureOpenAIChatClient from agent_framework.openai import OpenAIChatClient from microsoft_agents.hosting.core import Authorization, TurnContext +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.services.mcp_tool_server_configuration_service import ( McpToolServerConfigurationService, ) -from microsoft_agents_a365.tooling.models import ToolOptions from microsoft_agents_a365.tooling.utils.constants import Constants - from microsoft_agents_a365.tooling.utils.utility import ( get_mcp_platform_authentication_scope, ) @@ -148,6 +150,186 @@ async def add_tool_servers_to_agent( self._logger.error(f"Failed to add tool servers to agent: {ex}") raise + def _convert_chat_messages_to_history( + self, + chat_messages: Sequence[ChatMessage], + ) -> List[ChatHistoryMessage]: + """ + Convert Agent Framework ChatMessage objects to ChatHistoryMessage format. + + This internal helper method transforms Agent Framework's native ChatMessage + objects into the ChatHistoryMessage format expected by the MCP platform's + real-time threat protection endpoint. + + Args: + chat_messages: Sequence of ChatMessage objects to convert. + + Returns: + List of ChatHistoryMessage objects ready for the MCP platform. + + Note: + - If message_id is None, a new UUID is generated + - Role is extracted via the .value property of the Role object + - Timestamp is set to current UTC time (ChatMessage has no timestamp) + - Messages with empty or whitespace-only content are filtered out and + logged at WARNING level. This is because ChatHistoryMessage requires + non-empty content for validation. The filtered messages will not be + sent to the MCP platform. + """ + history_messages: List[ChatHistoryMessage] = [] + current_time = datetime.now(timezone.utc) + + for msg in chat_messages: + message_id = msg.message_id if msg.message_id is not None else str(uuid.uuid4()) + if msg.role is None: + self._logger.warning( + "Skipping message %s with missing role during conversion", message_id + ) + continue + # Defensive handling: use .value if role is an enum, otherwise convert to string + role = msg.role.value if hasattr(msg.role, "value") else str(msg.role) + content = msg.text if msg.text is not None else "" + + # Skip messages with empty content as ChatHistoryMessage validates non-empty content + if not content.strip(): + self._logger.warning( + "Skipping message %s with empty content during conversion", message_id + ) + continue + + history_message = ChatHistoryMessage( + id=message_id, + role=role, + content=content, + timestamp=current_time, + ) + history_messages.append(history_message) + + self._logger.debug( + "Converted message %s with role '%s' to ChatHistoryMessage", message_id, role + ) + + return history_messages + + async def send_chat_history_messages( + self, + chat_messages: Sequence[ChatMessage], + turn_context: TurnContext, + tool_options: Optional[ToolOptions] = None, + ) -> OperationResult: + """ + Send chat history messages to the MCP platform for real-time threat protection. + + This is the primary implementation method that handles message conversion + and delegation to the core tooling service. + + Args: + chat_messages: Sequence of Agent Framework ChatMessage objects to send. + turn_context: TurnContext from the Agents SDK containing conversation info. + tool_options: Optional configuration for the request. Defaults to + AgentFramework-specific options if not provided. + + Returns: + OperationResult indicating success or failure of the operation. + + Raises: + ValueError: If chat_messages or turn_context is None. + + Example: + >>> service = McpToolRegistrationService() + >>> messages = [ChatMessage(role=Role.USER, text="Hello")] + >>> result = await service.send_chat_history_messages(messages, turn_context) + >>> if result.succeeded: + ... print("Chat history sent successfully") + """ + # Input validation + if chat_messages is None: + raise ValueError("chat_messages cannot be None") + + if turn_context is None: + raise ValueError("turn_context cannot be None") + + # Handle empty messages - return success with warning + if len(chat_messages) == 0: + self._logger.warning("Empty message list provided to send_chat_history_messages") + return OperationResult.success() + + self._logger.info(f"Send chat history initiated with {len(chat_messages)} messages") + + # Use default options if not provided + if tool_options is None: + tool_options = ToolOptions(orchestrator_name=self._orchestrator_name) + + # Convert messages to ChatHistoryMessage format + history_messages = self._convert_chat_messages_to_history(chat_messages) + + # Check if all messages were filtered out during conversion + if len(history_messages) == 0: + self._logger.warning("All messages were filtered out during conversion (empty content)") + return OperationResult.success() + + # Delegate to core service + result = await self._mcp_server_configuration_service.send_chat_history( + turn_context=turn_context, + chat_history_messages=history_messages, + options=tool_options, + ) + + if result.succeeded: + self._logger.info( + f"Chat history sent successfully with {len(history_messages)} messages" + ) + else: + self._logger.error(f"Failed to send chat history: {result}") + + return result + + async def send_chat_history_from_store( + self, + chat_message_store: ChatMessageStoreProtocol, + turn_context: TurnContext, + tool_options: Optional[ToolOptions] = None, + ) -> OperationResult: + """ + Send chat history from a ChatMessageStore to the MCP platform. + + This is a convenience method that extracts messages from the store + and delegates to send_chat_history_messages(). + + Args: + chat_message_store: ChatMessageStore containing the conversation history. + turn_context: TurnContext from the Agents SDK containing conversation info. + tool_options: Optional configuration for the request. + + Returns: + OperationResult indicating success or failure of the operation. + + Raises: + ValueError: If chat_message_store or turn_context is None. + + Example: + >>> service = McpToolRegistrationService() + >>> result = await service.send_chat_history_from_store( + ... thread.chat_message_store, turn_context + ... ) + """ + # Input validation + if chat_message_store is None: + raise ValueError("chat_message_store cannot be None") + + if turn_context is None: + raise ValueError("turn_context cannot be None") + + # Extract messages from the store + messages = await chat_message_store.list_messages() + + # Delegate to the primary implementation + return await self.send_chat_history_messages( + chat_messages=messages, + turn_context=turn_context, + tool_options=tool_options, + ) + async def cleanup(self): """Clean up any resources used by the service.""" try: diff --git a/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/__init__.py b/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/__init__.py index 567db1bb..edee7a4a 100644 --- a/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/__init__.py +++ b/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/__init__.py @@ -27,3 +27,6 @@ "get_mcp_base_url", "build_mcp_server_url", ] + +# Enable namespace package extension for tooling-extensions-* packages +__path__ = __import__("pkgutil").extend_path(__path__, __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 05e1453b..71d2ee5d 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 @@ -599,8 +599,13 @@ async def send_chat_history( # Validate input parameters if turn_context is None: raise ValueError("turn_context cannot be None") - if chat_history_messages is None or len(chat_history_messages) == 0: - raise ValueError("chat_history_messages cannot be None or empty") + if chat_history_messages is None: + raise ValueError("chat_history_messages cannot be None") + + # Handle empty messages - return success with warning (consistent with extension behavior) + if len(chat_history_messages) == 0: + self._logger.warning("Empty message list provided to send_chat_history") + return OperationResult.success() # Extract required information from turn context if not turn_context.activity: diff --git a/tests/tooling/extensions/agentframework/__init__.py b/tests/tooling/extensions/agentframework/__init__.py new file mode 100644 index 00000000..eaac7401 --- /dev/null +++ b/tests/tooling/extensions/agentframework/__init__.py @@ -0,0 +1,4 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +"""Test package for Agent Framework tooling extensions.""" diff --git a/tests/tooling/extensions/agentframework/services/__init__.py b/tests/tooling/extensions/agentframework/services/__init__.py new file mode 100644 index 00000000..7d0206f4 --- /dev/null +++ b/tests/tooling/extensions/agentframework/services/__init__.py @@ -0,0 +1,4 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +"""Test package for Agent Framework tooling extension services.""" diff --git a/tests/tooling/extensions/agentframework/services/test_send_chat_history.py b/tests/tooling/extensions/agentframework/services/test_send_chat_history.py new file mode 100644 index 00000000..f3a780a3 --- /dev/null +++ b/tests/tooling/extensions/agentframework/services/test_send_chat_history.py @@ -0,0 +1,559 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +"""Unit tests for send_chat_history_from_store methods in McpToolRegistrationService.""" + +import uuid +from datetime import UTC, datetime +from unittest.mock import AsyncMock, Mock, patch + +import pytest +from microsoft_agents.hosting.core import TurnContext +from microsoft_agents_a365.runtime import OperationError, OperationResult +from microsoft_agents_a365.tooling.extensions.agentframework.services import ( + McpToolRegistrationService, +) +from microsoft_agents_a365.tooling.models import ToolOptions + + +class TestSendChatHistoryAsync: + """Tests for send_chat_history_messages and send_chat_history_from_store methods.""" + + @pytest.fixture + def mock_turn_context(self): + """Create a mock TurnContext with valid activity data.""" + mock_context = Mock(spec=TurnContext) + mock_activity = Mock() + mock_conversation = Mock() + + mock_conversation.id = "conv-test-123" + mock_activity.conversation = mock_conversation + mock_activity.id = "msg-test-456" + mock_activity.text = "Test user message" + + mock_context.activity = mock_activity + return mock_context + + @pytest.fixture + def mock_role(self): + """Create a mock Role object with .value property.""" + role = Mock() + role.value = "user" + return role + + @pytest.fixture + def mock_assistant_role(self): + """Create a mock Role object for assistant.""" + role = Mock() + role.value = "assistant" + return role + + @pytest.fixture + def sample_chat_messages(self, mock_role, mock_assistant_role): + """Create sample Agent Framework ChatMessage-like objects.""" + msg1 = Mock() + msg1.message_id = "msg-1" + msg1.role = mock_role + msg1.text = "Hello" + + msg2 = Mock() + msg2.message_id = "msg-2" + msg2.role = mock_assistant_role + msg2.text = "Hi there!" + + return [msg1, msg2] + + @pytest.fixture + def mock_chat_message_store(self, sample_chat_messages): + """Create a mock ChatMessageStoreProtocol.""" + store = AsyncMock() + store.list_messages = AsyncMock(return_value=sample_chat_messages) + return store + + @pytest.fixture + def service(self): + """Create McpToolRegistrationService instance with mocked core service.""" + svc = McpToolRegistrationService() + svc._mcp_server_configuration_service = Mock() + svc._mcp_server_configuration_service.send_chat_history = AsyncMock( + return_value=OperationResult.success() + ) + return svc + + # ==================== Validation Tests (8 tests) ==================== + + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_messages_validates_messages_none( + self, service, mock_turn_context + ): + """Test that send_chat_history_messages raises ValueError for None messages.""" + with pytest.raises(ValueError, match="chat_messages cannot be None"): + await service.send_chat_history_messages(None, mock_turn_context) + + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_messages_validates_turn_context_none( + self, service, sample_chat_messages + ): + """Test that send_chat_history_messages raises ValueError for None turn_context.""" + with pytest.raises(ValueError, match="turn_context cannot be None"): + await service.send_chat_history_messages(sample_chat_messages, None) + + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_from_store_validates_store_none( + self, service, mock_turn_context + ): + """Test that send_chat_history_from_store raises ValueError for None store.""" + with pytest.raises(ValueError, match="chat_message_store cannot be None"): + await service.send_chat_history_from_store(None, mock_turn_context) + + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_from_store_validates_turn_context_none( + self, service, mock_chat_message_store + ): + """Test that send_chat_history_from_store raises ValueError for None turn_context.""" + with pytest.raises(ValueError, match="turn_context cannot be None"): + await service.send_chat_history_from_store(mock_chat_message_store, None) + + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_messages_empty_messages_returns_success( + self, service, mock_turn_context + ): + """Test that empty message list returns success with warning log.""" + # Act + result = await service.send_chat_history_messages([], mock_turn_context) + + # Assert + assert result.succeeded is True + # Core service should not be called for empty messages + service._mcp_server_configuration_service.send_chat_history.assert_not_called() + + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_messages_generates_uuid_for_missing_id( + self, service, mock_turn_context, mock_role + ): + """Test that UUID is generated when message_id is None.""" + # Arrange + msg = Mock() + msg.message_id = None # No message ID + msg.role = mock_role + msg.text = "Hello" + + # Act + await service.send_chat_history_messages([msg], mock_turn_context) + + # Assert + call_args = service._mcp_server_configuration_service.send_chat_history.call_args + history_messages = call_args.kwargs["chat_history_messages"] + + assert len(history_messages) == 1 + # Verify a UUID was generated (not None and valid UUID format) + assert history_messages[0].id is not None + # Use uuid.UUID() to validate format - raises ValueError if invalid + uuid.UUID(history_messages[0].id) + + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_messages_generates_timestamp( + self, service, mock_turn_context, sample_chat_messages + ): + """Test that current UTC timestamp is generated for messages.""" + # Act + before_time = datetime.now(UTC) + await service.send_chat_history_messages(sample_chat_messages, mock_turn_context) + after_time = datetime.now(UTC) + + # Assert + call_args = service._mcp_server_configuration_service.send_chat_history.call_args + history_messages = call_args.kwargs["chat_history_messages"] + + for msg in history_messages: + assert msg.timestamp is not None + assert before_time <= msg.timestamp <= after_time + + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_messages_handles_missing_text( + self, service, mock_turn_context, mock_role + ): + """Test that messages with None text are skipped (empty content not allowed).""" + # Arrange + msg_with_text = Mock() + msg_with_text.message_id = "msg-1" + msg_with_text.role = mock_role + msg_with_text.text = "Hello" + + msg_without_text = Mock() + msg_without_text.message_id = "msg-2" + msg_without_text.role = mock_role + msg_without_text.text = None # No text + + # Act + await service.send_chat_history_messages( + [msg_with_text, msg_without_text], mock_turn_context + ) + + # Assert + call_args = service._mcp_server_configuration_service.send_chat_history.call_args + history_messages = call_args.kwargs["chat_history_messages"] + + # Only the message with text should be included + assert len(history_messages) == 1 + assert history_messages[0].content == "Hello" + + # ==================== Success and Delegation Tests (5 tests) ==================== + + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_messages_success( + self, service, mock_turn_context, sample_chat_messages + ): + """Test successful send_chat_history_messages call.""" + # Act + result = await service.send_chat_history_messages(sample_chat_messages, mock_turn_context) + + # Assert + assert result.succeeded is True + assert len(result.errors) == 0 + service._mcp_server_configuration_service.send_chat_history.assert_called_once() + + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_from_store_with_store_success( + self, service, mock_turn_context, mock_chat_message_store + ): + """Test successful send_chat_history_from_store call with ChatMessageStore.""" + # Act + result = await service.send_chat_history_from_store( + mock_chat_message_store, mock_turn_context + ) + + # Assert + assert result.succeeded is True + mock_chat_message_store.list_messages.assert_called_once() + service._mcp_server_configuration_service.send_chat_history.assert_called_once() + + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_from_store_delegates_to_messages_async( + self, service, mock_turn_context, mock_chat_message_store, sample_chat_messages + ): + """Test that send_chat_history_from_store delegates to send_chat_history_messages.""" + # Arrange + with patch.object( + service, "send_chat_history_messages", new_callable=AsyncMock + ) as mock_messages_method: + mock_messages_method.return_value = OperationResult.success() + + # Act + result = await service.send_chat_history_from_store( + mock_chat_message_store, mock_turn_context + ) + + # Assert + assert result.succeeded is True + mock_chat_message_store.list_messages.assert_called_once() + mock_messages_method.assert_called_once_with( + chat_messages=sample_chat_messages, + turn_context=mock_turn_context, + tool_options=None, + ) + + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_messages_with_tool_options( + self, service, mock_turn_context, sample_chat_messages + ): + """Test that ToolOptions are passed correctly to core service.""" + # Arrange + options = ToolOptions(orchestrator_name="TestOrchestrator") + + # Act + await service.send_chat_history_messages( + sample_chat_messages, mock_turn_context, tool_options=options + ) + + # Assert + call_args = service._mcp_server_configuration_service.send_chat_history.call_args + assert call_args.kwargs["options"] == options + + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_messages_converts_messages_correctly( + self, service, mock_turn_context, sample_chat_messages + ): + """Test that ChatMessage objects are correctly converted to ChatHistoryMessage.""" + # Act + await service.send_chat_history_messages(sample_chat_messages, mock_turn_context) + + # Assert + call_args = service._mcp_server_configuration_service.send_chat_history.call_args + history_messages = call_args.kwargs["chat_history_messages"] + + assert len(history_messages) == 2 + + # Verify first message conversion + assert history_messages[0].id == "msg-1" + assert history_messages[0].role == "user" + assert history_messages[0].content == "Hello" + assert history_messages[0].timestamp is not None + + # Verify second message conversion + assert history_messages[1].id == "msg-2" + assert history_messages[1].role == "assistant" + assert history_messages[1].content == "Hi there!" + assert history_messages[1].timestamp is not None + + # ==================== Error Handling Tests (4 tests) ==================== + + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_messages_handles_http_error( + self, service, mock_turn_context, sample_chat_messages + ): + """Test send_chat_history_messages handles HTTP errors from core service.""" + # Arrange + error = OperationError(Exception("500, Internal Server Error")) + service._mcp_server_configuration_service.send_chat_history = AsyncMock( + return_value=OperationResult.failed(error) + ) + + # Act + result = await service.send_chat_history_messages(sample_chat_messages, mock_turn_context) + + # Assert + assert result.succeeded is False + assert len(result.errors) == 1 + assert "500" in str(result.errors[0].message) + + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_messages_handles_timeout( + self, service, mock_turn_context, sample_chat_messages + ): + """Test send_chat_history_messages handles timeout errors.""" + # Arrange + error = OperationError(Exception("Request timed out")) + service._mcp_server_configuration_service.send_chat_history = AsyncMock( + return_value=OperationResult.failed(error) + ) + + # Act + result = await service.send_chat_history_messages(sample_chat_messages, mock_turn_context) + + # Assert + assert result.succeeded is False + assert len(result.errors) == 1 + assert "timed out" in str(result.errors[0].message) + + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_messages_handles_connection_error( + self, service, mock_turn_context, sample_chat_messages + ): + """Test send_chat_history_messages handles connection errors.""" + # Arrange + error = OperationError(Exception("Connection failed")) + service._mcp_server_configuration_service.send_chat_history = AsyncMock( + return_value=OperationResult.failed(error) + ) + + # Act + result = await service.send_chat_history_messages(sample_chat_messages, mock_turn_context) + + # Assert + assert result.succeeded is False + assert len(result.errors) == 1 + assert "Connection failed" in str(result.errors[0].message) + + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_messages_role_value_conversion( + self, service, mock_turn_context + ): + """Test that Role.value is used for string conversion.""" + # Arrange - Create messages with different role values + system_role = Mock() + system_role.value = "system" + + user_role = Mock() + user_role.value = "user" + + assistant_role = Mock() + assistant_role.value = "assistant" + + msg1 = Mock() + msg1.message_id = "msg-1" + msg1.role = system_role + msg1.text = "System prompt" + + msg2 = Mock() + msg2.message_id = "msg-2" + msg2.role = user_role + msg2.text = "User message" + + msg3 = Mock() + msg3.message_id = "msg-3" + msg3.role = assistant_role + msg3.text = "Assistant response" + + # Act + await service.send_chat_history_messages([msg1, msg2, msg3], mock_turn_context) + + # Assert + call_args = service._mcp_server_configuration_service.send_chat_history.call_args + history_messages = call_args.kwargs["chat_history_messages"] + + assert len(history_messages) == 3 + assert history_messages[0].role == "system" + assert history_messages[1].role == "user" + assert history_messages[2].role == "assistant" + + # ==================== Additional Coverage Tests (CRM-001, 004, 005, 006, 011) ==================== + + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_from_store_propagates_store_exception( + self, service, mock_turn_context + ): + """Test that exceptions from chat_message_store.list_messages() propagate (CRM-001).""" + # Arrange + mock_store = AsyncMock() + mock_store.list_messages = AsyncMock(side_effect=RuntimeError("Store connection failed")) + + # Act & Assert + with pytest.raises(RuntimeError, match="Store connection failed"): + await service.send_chat_history_from_store(mock_store, mock_turn_context) + + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_messages_skips_whitespace_only_content( + self, service, mock_turn_context, mock_role + ): + """Test that messages with whitespace-only content are filtered out (CRM-004).""" + # Arrange + msg_with_text = Mock() + msg_with_text.message_id = "msg-1" + msg_with_text.role = mock_role + msg_with_text.text = "Valid content" + + msg_whitespace_only = Mock() + msg_whitespace_only.message_id = "msg-2" + msg_whitespace_only.role = mock_role + msg_whitespace_only.text = " \t\n " # Whitespace only + + # Act + await service.send_chat_history_messages( + [msg_with_text, msg_whitespace_only], mock_turn_context + ) + + # Assert + call_args = service._mcp_server_configuration_service.send_chat_history.call_args + history_messages = call_args.kwargs["chat_history_messages"] + + # Only the message with actual content should be included + assert len(history_messages) == 1 + assert history_messages[0].content == "Valid content" + + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_messages_skips_messages_with_none_role( + self, service, mock_turn_context, mock_role + ): + """Test that messages with None role are filtered out (CRM-005).""" + # Arrange + msg_with_role = Mock() + msg_with_role.message_id = "msg-1" + msg_with_role.role = mock_role + msg_with_role.text = "Valid message" + + msg_without_role = Mock() + msg_without_role.message_id = "msg-2" + msg_without_role.role = None # No role + msg_without_role.text = "This should be skipped" + + # Act + await service.send_chat_history_messages( + [msg_with_role, msg_without_role], mock_turn_context + ) + + # Assert + call_args = service._mcp_server_configuration_service.send_chat_history.call_args + history_messages = call_args.kwargs["chat_history_messages"] + + # Only the message with a role should be included + assert len(history_messages) == 1 + assert history_messages[0].content == "Valid message" + + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_messages_all_filtered_returns_success( + self, service, mock_turn_context, mock_role + ): + """Test that all messages filtered out returns success without calling core (CRM-006).""" + # Arrange - all messages have empty content + msg1 = Mock() + msg1.message_id = "msg-1" + msg1.role = mock_role + msg1.text = "" # Empty + + msg2 = Mock() + msg2.message_id = "msg-2" + msg2.role = mock_role + msg2.text = " " # Whitespace only + + msg3 = Mock() + msg3.message_id = "msg-3" + msg3.role = None # None role + msg3.text = "Valid text but no role" + + # Act + result = await service.send_chat_history_messages([msg1, msg2, msg3], mock_turn_context) + + # Assert + assert result.succeeded is True + # Core service should not be called when all messages are filtered out + service._mcp_server_configuration_service.send_chat_history.assert_not_called() + + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_messages_creates_default_tool_options( + self, service, mock_turn_context, sample_chat_messages + ): + """Test that default ToolOptions with AgentFramework orchestrator is created (CRM-011).""" + # Act - call without providing tool_options + await service.send_chat_history_messages(sample_chat_messages, mock_turn_context) + + # Assert + call_args = service._mcp_server_configuration_service.send_chat_history.call_args + options = call_args.kwargs["options"] + + assert options is not None + assert options.orchestrator_name == "AgentFramework" + + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_messages_handles_role_without_value_attribute( + self, service, mock_turn_context + ): + """Test defensive handling when role doesn't have .value attribute (CRM-003).""" + # Arrange - role is a plain string, not an enum + msg = Mock() + msg.message_id = "msg-1" + msg.role = "user" # String, not an enum with .value + msg.text = "Hello" + + # Act + await service.send_chat_history_messages([msg], mock_turn_context) + + # Assert + call_args = service._mcp_server_configuration_service.send_chat_history.call_args + history_messages = call_args.kwargs["chat_history_messages"] + + assert len(history_messages) == 1 + assert history_messages[0].role == "user" diff --git a/tests/tooling/services/test_send_chat_history.py b/tests/tooling/services/test_send_chat_history.py index 259b2a6e..60b56c80 100644 --- a/tests/tooling/services/test_send_chat_history.py +++ b/tests/tooling/services/test_send_chat_history.py @@ -142,17 +142,18 @@ async def test_send_chat_history_validates_chat_history_messages( ): """Test that send_chat_history validates chat_history_messages parameter.""" # Act & Assert - with pytest.raises(ValueError, match="chat_history_messages cannot be None or empty"): + with pytest.raises(ValueError, match="chat_history_messages cannot be None"): await service.send_chat_history(mock_turn_context, None) @pytest.mark.asyncio - async def test_send_chat_history_validates_empty_chat_history_list( - self, service, mock_turn_context - ): - """Test that send_chat_history validates empty chat_history list.""" - # Act & Assert - with pytest.raises(ValueError, match="chat_history_messages cannot be None or empty"): - await service.send_chat_history(mock_turn_context, []) + async def test_send_chat_history_empty_list_returns_success(self, service, mock_turn_context): + """Test that send_chat_history returns success for empty list (CRM-008).""" + # Act + result = await service.send_chat_history(mock_turn_context, []) + + # Assert - empty list should return success, not raise exception + assert result.succeeded is True + assert len(result.errors) == 0 @pytest.mark.asyncio async def test_send_chat_history_validates_activity(self, service, chat_history_messages):