Code review fixes/pr 132

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`)
 
 ## CI/CD
 

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
 
 ```

libraries/microsoft-agents-a365-tooling-extensions-agentframework/microsoft_agents_a365/tooling/extensions/agentframework/services/mcp_tool_registration_service.py

@@ -190,16 +190,17 @@ def _convert_chat_messages_to_history(
             message_id = msg.message_id if msg.message_id is not None else str(uuid.uuid4())
             if msg.role is None:
                 self._logger.warning(
-                    f"Skipping message {message_id} with missing role during conversion"
+                    "Skipping message %s with missing role during conversion", message_id
                 )
                 continue
-            role = msg.role.value
+            # 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 or not content.strip():
+            if not content.strip():
                 self._logger.warning(
-                    f"Skipping message {message_id} with empty content during conversion"
+                    "Skipping message %s with empty content during conversion", message_id
                 )
                 continue
 
@@ -212,12 +213,12 @@ def _convert_chat_messages_to_history(
             history_messages.append(history_message)
 
             self._logger.debug(
-                f"Converted message {message_id} with role '{role}' to ChatHistoryMessage"
+                "Converted message %s with role '%s' to ChatHistoryMessage", message_id, role
             )
 
         return history_messages
 
-    async def send_chat_history_messages_async(
+    async def send_chat_history_messages(
         self,
         chat_messages: Sequence[ChatMessage],
         turn_context: TurnContext,
@@ -244,7 +245,7 @@ async def send_chat_history_messages_async(
         Example:
             >>> service = McpToolRegistrationService()
             >>> messages = [ChatMessage(role=Role.USER, text="Hello")]
-            >>> result = await service.send_chat_history_messages_async(messages, turn_context)
+            >>> result = await service.send_chat_history_messages(messages, turn_context)
             >>> if result.succeeded:
             ...     print("Chat history sent successfully")
         """
@@ -257,7 +258,7 @@ async def send_chat_history_messages_async(
 
         # Handle empty messages - return success with warning
         if len(chat_messages) == 0:
-            self._logger.warning("Empty message list provided to send_chat_history_messages_async")
+            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")
@@ -290,7 +291,7 @@ async def send_chat_history_messages_async(
 
         return result
 
-    async def send_chat_history_async(
+    async def send_chat_history_from_store(
         self,
         chat_message_store: ChatMessageStoreProtocol,
         turn_context: TurnContext,
@@ -300,7 +301,7 @@ async def send_chat_history_async(
         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_async().
+        and delegates to send_chat_history_messages().
 
         Args:
             chat_message_store: ChatMessageStore containing the conversation history.
@@ -315,7 +316,7 @@ async def send_chat_history_async(
 
         Example:
             >>> service = McpToolRegistrationService()
-            >>> result = await service.send_chat_history_async(
+            >>> result = await service.send_chat_history_from_store(
             ...     thread.chat_message_store, turn_context
             ... )
         """
@@ -330,7 +331,7 @@ async def send_chat_history_async(
         messages = await chat_message_store.list_messages()
 
         # Delegate to the primary implementation
-        return await self.send_chat_history_messages_async(
+        return await self.send_chat_history_messages(
             chat_messages=messages,
             turn_context=turn_context,
             tool_options=tool_options,

libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/services/mcp_tool_server_configuration_service.py

@@ -561,8 +561,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: