From b30de79d476cb06acf1ad261af0930cc274b8bb2 Mon Sep 17 00:00:00 2001 From: Johan Broberg Date: Sat, 17 Jan 2026 20:19:03 -0800 Subject: [PATCH 1/9] fix(runtime): implement thread-safe singleton with eager initialization Convert OperationResult.success() singleton from lazy to eager initialization at module level. Python's import lock ensures thread-safe initialization, eliminating the race condition in the previous check-and-create pattern. Addresses CRM-001 from code review. Co-Authored-By: Claude Opus 4.5 --- .../microsoft_agents_a365/runtime/operation_result.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) 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..10f7802f 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 @@ -61,8 +61,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 +89,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) From fc23083fa883f508993e083545510a6a94c2ba51 Mon Sep 17 00:00:00 2001 From: Johan Broberg Date: Sat, 17 Jan 2026 20:19:33 -0800 Subject: [PATCH 2/9] fix(tooling): include error response body in HTTP error log message Add truncated error_text (max 500 chars) to the log message when HTTP errors occur in send_chat_history. This improves debugging by showing the actual error response from the MCP platform. Addresses CRM-002 from code review. Co-Authored-By: Claude Opus 4.5 --- .../services/mcp_tool_server_configuration_service.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) 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..966a796e 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 @@ -549,7 +549,9 @@ async def send_chat_history( message_id = turn_context.activity.id user_message = 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)" ) @@ -601,7 +603,8 @@ async def send_chat_history( 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( From 00a79854d8168e66990041e6e97032fe42e41b2d Mon Sep 17 00:00:00 2001 From: Johan Broberg Date: Sat, 17 Jan 2026 20:20:03 -0800 Subject: [PATCH 3/9] fix(tooling): reorder exception handlers for proper timeout handling Move asyncio.TimeoutError handler before aiohttp.ClientError to ensure timeouts are caught correctly. Since aiohttp.ServerTimeoutError inherits from both exceptions, the previous order could misclassify timeouts. Addresses CRM-003 from code review. Co-Authored-By: Claude Opus 4.5 --- .../services/mcp_tool_server_configuration_service.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) 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 966a796e..95ce3c5a 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 @@ -616,14 +616,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)) From aa1d37724a499336f38fb85f5eac7a422e2d2ce3 Mon Sep 17 00:00:00 2001 From: Johan Broberg Date: Sat, 17 Jan 2026 20:20:30 -0800 Subject: [PATCH 4/9] refactor(tooling): add type hints for local variables in send_chat_history Add Optional[str] type annotations for conversation_id, message_id, and user_message variables to improve code clarity and IDE support. Addresses CRM-004 from code review. Co-Authored-By: Claude Opus 4.5 --- .../services/mcp_tool_server_configuration_service.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 95ce3c5a..9d1a7b8a 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 @@ -543,11 +543,11 @@ 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() From e074405a9566d2f51652fbe5b1084d734b5f378b Mon Sep 17 00:00:00 2001 From: Johan Broberg Date: Sat, 17 Jan 2026 20:21:05 -0800 Subject: [PATCH 5/9] docs(tooling): improve validation error messages in ChatHistoryMessage Update error messages to say "cannot be empty or whitespace-only" instead of just "cannot be empty" for clearer feedback when whitespace validation fails. Addresses CRM-005 from code review. Co-Authored-By: Claude Opus 4.5 --- .../tooling/models/chat_history_message.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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") From fe611bb5b39b7a0a2e49d433d17b054760e5f23b Mon Sep 17 00:00:00 2001 From: Johan Broberg Date: Sat, 17 Jan 2026 20:21:48 -0800 Subject: [PATCH 6/9] refactor(tooling): extract timeout and HTTP status code to constants Add DEFAULT_REQUEST_TIMEOUT_SECONDS and HTTP_STATUS_OK module-level constants to replace magic values. Improves maintainability and makes configuration easier to modify. Addresses CRM-006 from code review. Co-Authored-By: Claude Opus 4.5 --- .../mcp_tool_server_configuration_service.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) 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 9d1a7b8a..16597a28 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 # ============================================================================== @@ -594,10 +605,10 @@ 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: From 830392116af67d130274d63cd8759c8b20aa075c Mon Sep 17 00:00:00 2001 From: Johan Broberg Date: Sat, 17 Jan 2026 20:22:16 -0800 Subject: [PATCH 7/9] docs(runtime): enhance defensive copy docstring in OperationResult.errors Move the defensive copy rationale into a prominent Note section in the docstring to make it more visible to developers. This clarifies why the property returns a copy rather than the internal list. Addresses CRM-007 from code review. Co-Authored-By: Claude Opus 4.5 --- .../microsoft_agents_a365/runtime/operation_result.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) 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 10f7802f..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) From 7bc2c2afc2ca47949a075484a59074cf077762fd Mon Sep 17 00:00:00 2001 From: Johan Broberg Date: Sat, 17 Jan 2026 20:23:02 -0800 Subject: [PATCH 8/9] test(tooling): use Mock(spec=TurnContext) for stricter interface validation Import TurnContext and use it as a spec for the mock fixture. This ensures the mock matches the actual TurnContext interface, catching potential issues if the API changes. Addresses CRM-008 from code review. Co-Authored-By: Claude Opus 4.5 --- tests/tooling/services/test_send_chat_history.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) 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() From 5893d75a2f0a2c4f2ca33e6c1b5c6a3be33b171b Mon Sep 17 00:00:00 2001 From: Johan Broberg Date: Sat, 17 Jan 2026 20:23:34 -0800 Subject: [PATCH 9/9] docs(tooling): add usage example to send_chat_history docstring Add an Example section showing how to create ChatHistoryMessage objects and call send_chat_history with proper error handling. This helps developers understand the intended usage pattern. Addresses CRM-009 from code review. Co-Authored-By: Claude Opus 4.5 --- .../mcp_tool_server_configuration_service.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) 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 16597a28..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 @@ -543,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: