diff --git a/libraries/microsoft-agents-a365-tooling-extensions-semantickernel/microsoft_agents_a365/tooling/extensions/semantickernel/services/mcp_tool_registration_service.py b/libraries/microsoft-agents-a365-tooling-extensions-semantickernel/microsoft_agents_a365/tooling/extensions/semantickernel/services/mcp_tool_registration_service.py index b427408..613a029 100644 --- a/libraries/microsoft-agents-a365-tooling-extensions-semantickernel/microsoft_agents_a365/tooling/extensions/semantickernel/services/mcp_tool_registration_service.py +++ b/libraries/microsoft-agents-a365-tooling-extensions-semantickernel/microsoft_agents_a365/tooling/extensions/semantickernel/services/mcp_tool_registration_service.py @@ -14,8 +14,9 @@ import os import re import uuid +from dataclasses import replace from datetime import datetime, timezone -from typing import Any, List, Optional, Sequence +from typing import List, Optional, Sequence # Third-party imports from semantic_kernel import kernel as sk @@ -175,7 +176,7 @@ async def add_tool_servers_to_agent( # Private Methods - Input Validation & Processing # ============================================================================ - def _validate_inputs(self, kernel: Any, agentic_app_id: str, auth_token: str) -> None: + def _validate_inputs(self, kernel: sk.Kernel, agentic_app_id: str, auth_token: str) -> None: """Validate all required inputs.""" if kernel is None: raise ValueError("kernel cannot be None") @@ -267,7 +268,8 @@ async def send_chat_history( # Re-raise validation errors raise except Exception as ex: - self._logger.error(f"Failed to send chat history: {ex}") + self._logger.error("Failed to send chat history") + self._logger.debug(f"Exception details: {ex}") return OperationResult.failed(OperationError(ex)) async def send_chat_history_messages( @@ -320,11 +322,11 @@ async def send_chat_history_messages( self._logger.info(f"Sending {len(messages)} Semantic Kernel messages as chat history") - # Set default options + # Set default options (create new instance to avoid mutating caller's object) if options is None: options = ToolOptions(orchestrator_name=self._orchestrator_name) elif options.orchestrator_name is None: - options.orchestrator_name = self._orchestrator_name + options = replace(options, orchestrator_name=self._orchestrator_name) try: # Convert Semantic Kernel messages to ChatHistoryMessage format @@ -351,7 +353,8 @@ async def send_chat_history_messages( # Re-raise validation errors from the core service raise except Exception as ex: - self._logger.error(f"Failed to send chat history messages: {ex}") + self._logger.error("Failed to send chat history messages") + self._logger.debug(f"Exception details: {ex}") return OperationResult.failed(OperationError(ex)) # ============================================================================ diff --git a/tests/tooling/extensions/semantickernel/services/conftest.py b/tests/tooling/extensions/semantickernel/services/conftest.py index c7bf8a7..8dc0faa 100644 --- a/tests/tooling/extensions/semantickernel/services/conftest.py +++ b/tests/tooling/extensions/semantickernel/services/conftest.py @@ -184,12 +184,6 @@ def sample_system_message(): ) -@pytest.fixture -def sample_tool_message(): - """Create a sample tool message.""" - return MockChatMessageContent(role=MockAuthorRole.TOOL, content="Tool result here") - - @pytest.fixture def sample_sk_messages(): """Create a list of sample Semantic Kernel messages.""" diff --git a/tests/tooling/extensions/semantickernel/services/test_send_chat_history.py b/tests/tooling/extensions/semantickernel/services/test_send_chat_history.py index 3c50914..a4fe09b 100644 --- a/tests/tooling/extensions/semantickernel/services/test_send_chat_history.py +++ b/tests/tooling/extensions/semantickernel/services/test_send_chat_history.py @@ -104,6 +104,29 @@ async def test_send_chat_history_messages_whitespace_only_messages_filtered( assert len(chat_history_messages) == 1 assert chat_history_messages[0].content == "Valid message" + # UV-07 + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_all_messages_filtered_still_calls_core_service( + self, service, mock_turn_context, mock_config_service + ): + """Test that when all messages are filtered, core service is still called with empty list.""" + messages = [ + MockChatMessageContent(role=MockAuthorRole.USER, content=""), # Empty - filtered + MockChatMessageContent(role=MockAuthorRole.USER, content=" "), # Whitespace - filtered + MockChatMessageContent(role=MockAuthorRole.USER, content=None), # None - filtered + ] + + mock_config_service.send_chat_history.return_value = OperationResult.success() + + result = await service.send_chat_history_messages(mock_turn_context, messages) + + assert result.succeeded is True + mock_config_service.send_chat_history.assert_called_once() + call_args = mock_config_service.send_chat_history.call_args + # Verify empty list was passed to core service + assert call_args.kwargs["chat_history_messages"] == [] + # ============================================================================= # ROLE MAPPING TESTS (RM-01) @@ -621,6 +644,28 @@ async def test_send_chat_history_limit_zero_sends_all( # limit=0 should be treated as "no limit" per the condition `limit > 0` assert len(chat_history_messages) == 5 + # LF-05 + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_negative_limit_sends_all( + self, service, mock_turn_context, mock_config_service + ): + """Test that negative limit is treated as no limit (sends all messages).""" + messages = [ + MockChatMessageContent(role=MockAuthorRole.USER, content=f"Message {i}") + for i in range(5) + ] + chat_history = MockChatHistory(messages=messages) + + mock_config_service.send_chat_history.return_value = OperationResult.success() + + await service.send_chat_history(mock_turn_context, chat_history, limit=-1) + + call_args = mock_config_service.send_chat_history.call_args + chat_history_messages = call_args.kwargs["chat_history_messages"] + # Negative limit should be treated as "no limit" per the condition `limit > 0` + assert len(chat_history_messages) == 5 + # ============================================================================= # ERROR HANDLING TESTS (EH-01 to EH-06)