From ae0b8cb4f6cc1c3cdc8aebfcd46c5346ff472461 Mon Sep 17 00:00:00 2001 From: Johan Broberg Date: Wed, 21 Jan 2026 15:28:52 -0800 Subject: [PATCH 01/11] Add PRD for Open AI chat history API --- docs/prd/openai-send-chat-history-api.md | 974 +++++++++++++++++++++++ 1 file changed, 974 insertions(+) create mode 100644 docs/prd/openai-send-chat-history-api.md diff --git a/docs/prd/openai-send-chat-history-api.md b/docs/prd/openai-send-chat-history-api.md new file mode 100644 index 00000000..8d9eb490 --- /dev/null +++ b/docs/prd/openai-send-chat-history-api.md @@ -0,0 +1,974 @@ +# Product Requirements Document (PRD) + +## OpenAI `send_chat_history_async` API for Agent365-python SDK + +| **Document Information** | | +|--------------------------|----------------------------------------------| +| **Version** | 1.0 | +| **Status** | Draft | +| **Author** | Agent365 Python SDK Team | +| **Created** | January 21, 2026 | +| **Last Updated** | January 21, 2026 | +| **Target Package** | `microsoft-agents-a365-tooling-extensions-openai` | + +--- + +## Table of Contents + +1. [Overview and Business Justification](#1-overview-and-business-justification) +2. [Objectives](#2-objectives) +3. [User Stories](#3-user-stories) +4. [Functional Requirements](#4-functional-requirements) +5. [Technical Requirements](#5-technical-requirements) +6. [Package Impact Analysis](#6-package-impact-analysis) +7. [API Design](#7-api-design) +8. [Observability Requirements](#8-observability-requirements) +9. [Testing Strategy](#9-testing-strategy) +10. [Acceptance Criteria](#10-acceptance-criteria) +11. [Non-Functional Requirements](#11-non-functional-requirements) +12. [Dependencies](#12-dependencies) +13. [Risks and Mitigations](#13-risks-and-mitigations) +14. [Open Questions](#14-open-questions) + +--- + +## 1. Overview and Business Justification + +### 1.1 Problem Statement + +The Agent365-python SDK currently provides a core `send_chat_history` method in `McpToolServerConfigurationService` that sends conversation history to the MCP platform for real-time threat protection. However, this method requires developers using the OpenAI Agents SDK to manually convert OpenAI-native types (such as `Session`, `TResponseInputItem`, `UserMessage`, `AssistantMessage`, etc.) to the SDK's `ChatHistoryMessage` format. + +This manual conversion creates: +- **Developer friction**: Extra boilerplate code to transform OpenAI types +- **Inconsistency risk**: Different developers may implement conversion logic differently +- **Error-prone integrations**: Missing ID or timestamp handling may vary +- **Feature parity gap**: The .NET SDK already provides framework-specific `SendChatHistoryAsync` methods for Agent Framework (PR #171) and Semantic Kernel (PR #173) + +### 1.2 Proposed Solution + +Implement an OpenAI-specific `send_chat_history_async` API in the `microsoft-agents-a365-tooling-extensions-openai` package that: +- Accepts OpenAI SDK native types directly (Session protocol items, message types) +- Handles all conversion logic internally +- Provides a seamless developer experience for OpenAI Agents SDK users +- Maintains feature parity with the .NET SDK + +### 1.3 Business Value + +| Value Driver | Description | +|--------------|-------------| +| **Developer Experience** | Reduces integration effort from ~20 lines of conversion code to a single method call | +| **Adoption** | Lowers barrier to entry for OpenAI SDK developers adopting Agent365 | +| **Consistency** | Ensures standardized handling of missing IDs, timestamps, and role mapping | +| **Feature Parity** | Aligns Python SDK capabilities with .NET SDK | +| **Enterprise Readiness** | Enables real-time threat protection for OpenAI-based agents with minimal effort | + +### 1.4 Success Metrics + +| Metric | Target | +|--------|--------| +| API adoption rate | 80% of OpenAI extension users use `send_chat_history_async` within 3 months | +| Code reduction | Average 15+ lines of boilerplate eliminated per integration | +| Test coverage | ≥95% line coverage, ≥90% branch coverage | +| Documentation completeness | 100% of public APIs documented with examples | + +--- + +## 2. Objectives + +### 2.1 Primary Objectives + +1. **O1**: Provide OpenAI-native API for sending chat history to the MCP platform +2. **O2**: Support OpenAI Session protocol items (`TResponseInputItem` types) +3. **O3**: Support direct list of OpenAI message types +4. **O4**: Maintain backward compatibility with existing `McpToolServerConfigurationService` +5. **O5**: Achieve feature parity with .NET SDK implementation + +### 2.2 Secondary Objectives + +1. **O6**: Provide extensible design for future OpenAI SDK version support +2. **O7**: Enable observability integration for tracing chat history operations +3. **O8**: Support both synchronous and asynchronous usage patterns + +### 2.3 Out of Scope + +- Modifications to the core `McpToolServerConfigurationService` +- Support for other orchestrator SDKs (covered by separate extensions) +- Persistent storage of chat history (handled by MCP platform) +- Chat history retrieval APIs (read operations) + +--- + +## 3. User Stories + +### 3.1 Primary User Stories + +| ID | User Story | Priority | Acceptance Criteria | +|----|------------|----------|---------------------| +| **US-01** | As an OpenAI agent developer, I want to send my agent's Session history to the MCP platform so that my conversations are protected by real-time threat detection | P0 | Session items are converted and sent successfully | +| **US-02** | As an OpenAI agent developer, I want to send a list of messages to the MCP platform without manual conversion so that I can focus on agent logic | P0 | List of OpenAI messages converts and sends correctly | +| **US-03** | As an OpenAI agent developer, I want missing message IDs to be auto-generated so that I don't need to track IDs manually | P0 | UUIDs generated for messages without IDs | +| **US-04** | As an OpenAI agent developer, I want missing timestamps to use current UTC time so that all messages have valid timestamps | P0 | Current UTC timestamp used when not provided | +| **US-05** | As an OpenAI agent developer, I want to receive clear success/failure results so that I can handle errors appropriately | P0 | `OperationResult` returned with error details on failure | +| **US-05a** | As an OpenAI agent developer, I want to send my Session's conversation history directly so that I don't need to extract messages manually | P0 | `send_session_history_async` extracts and sends Session items | + +### 3.2 Secondary User Stories + +| ID | User Story | Priority | Acceptance Criteria | +|----|------------|----------|---------------------| +| **US-06** | As an OpenAI agent developer, I want to pass custom `ToolOptions` so that I can customize orchestrator identification | P1 | ToolOptions parameter accepted and applied | +| **US-07** | As an OpenAI agent developer, I want detailed logging of conversion operations so that I can debug issues | P1 | Debug-level logs for conversion operations | +| **US-08** | As an OpenAI agent developer, I want support for all standard OpenAI message roles so that system, user, and assistant messages are handled | P1 | All roles mapped correctly | + +--- + +## 4. Functional Requirements + +### 4.1 Core Functional Requirements + +| ID | Requirement | Priority | Verification Method | +|----|-------------|----------|---------------------| +| **FR-01** | The API SHALL accept OpenAI `TResponseInputItem` types and convert them to `ChatHistoryMessage` | P0 | Unit tests | +| **FR-02** | The API SHALL support `UserMessage`, `AssistantMessage`, and `SystemMessage` OpenAI types | P0 | Unit tests | +| **FR-03** | The API SHALL generate a UUID for messages without an ID | P0 | Unit tests | +| **FR-04** | The API SHALL use `datetime.now(timezone.utc)` for messages without a timestamp | P0 | Unit tests | +| **FR-05** | The API SHALL delegate to `McpToolServerConfigurationService.send_chat_history` | P0 | Integration tests | +| **FR-06** | The API SHALL return `OperationResult` indicating success or failure | P0 | Unit tests | +| **FR-07** | The API SHALL validate that `turn_context` is not None | P0 | Unit tests | +| **FR-08** | The API SHALL allow empty message lists (no-op, return success) | P0 | Unit tests | +| **FR-09** | The API SHALL map OpenAI roles to `ChatHistoryMessage` roles ("user", "assistant", "system") | P0 | Unit tests | +| **FR-10** | The API SHALL extract text content from OpenAI message content arrays | P0 | Unit tests | +| **FR-11** | The API SHALL provide a `send_session_history_async` method that accepts an OpenAI Session | P0 | Unit tests | +| **FR-12** | The `send_session_history_async` method SHALL call `session.get_items()` to retrieve messages | P0 | Unit tests | +| **FR-13** | The `send_session_history_async` method SHALL support an optional `limit` parameter for `get_items()` | P1 | Unit tests | +| **FR-14** | The API SHALL include all item types from the session without filtering | P0 | Unit tests | + +### 4.2 Method Signatures + +The implementation SHALL provide the following method overloads: + +```python +# Primary method - accepts list of OpenAI message types +async def send_chat_history_async( + self, + turn_context: TurnContext, + messages: List[TResponseInputItem], + options: Optional[ToolOptions] = None, +) -> OperationResult: + """ + Sends OpenAI chat history to the MCP platform for threat protection. + + Args: + turn_context: TurnContext from the Agents SDK containing conversation info. + messages: List of OpenAI TResponseInputItem messages to send. + options: Optional ToolOptions for customization. + + Returns: + OperationResult indicating success or failure. + """ + +# Secondary method - extracts messages from OpenAI Session +async def send_session_history_async( + self, + turn_context: TurnContext, + session: Session, + limit: Optional[int] = None, + options: Optional[ToolOptions] = None, +) -> OperationResult: + """ + Extracts chat history from an OpenAI Session and sends it to the MCP platform. + + Args: + turn_context: TurnContext from the Agents SDK containing conversation info. + session: OpenAI Session instance to extract messages from. + limit: Optional maximum number of items to retrieve from session. + If None, retrieves all items. + options: Optional ToolOptions for customization. + + Returns: + OperationResult indicating success or failure. + """ +``` + +### 4.3 Role Mapping + +| OpenAI Type | ChatHistoryMessage Role | +|-------------|------------------------| +| `UserMessage` | `"user"` | +| `AssistantMessage` | `"assistant"` | +| `SystemMessage` | `"system"` | +| `ResponseOutputMessage` with role="assistant" | `"assistant"` | +| Other/Unknown | `"user"` (default fallback with warning log) | + +### 4.4 Content Extraction + +The API SHALL extract text content following this priority: +1. If message has `.content` as string → use directly +2. If message has `.content` as list → concatenate all text parts +3. If message has `.text` attribute → use directly +4. If content is empty/None → use empty string with warning log + +--- + +## 5. Technical Requirements + +### 5.1 Language and Runtime + +| Requirement | Specification | +|-------------|---------------| +| Python version | ≥3.10 | +| Async support | Full `async/await` pattern | +| Type hints | Complete type annotations (PEP 484) | +| Pydantic version | ≥2.0 (for model validation) | + +### 5.2 OpenAI SDK Compatibility + +| Requirement | Specification | +|-------------|---------------| +| OpenAI Agents SDK | Compatible with `openai-agents` package | +| Message types | Support `TResponseInputItem` union type | +| Session protocol | Support items from `Session.get_items()` | + +### 5.3 Error Handling + +| Error Condition | Expected Behavior | +|-----------------|-------------------| +| `turn_context` is None | Raise `ValueError` with descriptive message | +| `messages` is None | Raise `ValueError` with descriptive message | +| `messages` is empty list | Return `OperationResult.success()` (no-op) | +| `turn_context.activity` is None | Raise `ValueError` with descriptive message | +| Missing conversation ID | Raise `ValueError` with field path | +| Missing message ID | Raise `ValueError` with field path | +| Missing user message text | Raise `ValueError` with field path | +| HTTP error from MCP platform | Return `OperationResult.failed()` with error | +| Network timeout | Return `OperationResult.failed()` with error | +| Conversion error | Return `OperationResult.failed()` with error | + +### 5.4 Thread Safety + +- The service class SHALL be thread-safe for concurrent calls +- State SHALL NOT be shared between method invocations +- Logger instance SHALL be initialized per instance + +--- + +## 6. Package Impact Analysis + +### 6.1 Modified Package + +**Package**: `microsoft-agents-a365-tooling-extensions-openai` + +| File | Change Type | Description | +|------|-------------|-------------| +| `mcp_tool_registration_service.py` | Modified | Add `send_chat_history_async` method | +| `__init__.py` | Modified | Export new types if needed | + +### 6.2 New Files + +| File | Purpose | +|------|---------| +| `tests/tooling/extensions/openai/test_send_chat_history_async.py` | Unit tests | +| `tests/tooling/extensions/openai/test_message_conversion.py` | Conversion logic tests | + +### 6.3 Dependency Changes + +**New Dependencies (in `pyproject.toml`)**: + +```toml +[project.dependencies] +# Existing dependencies... +openai-agents = ">=0.1.0" # For type definitions +``` + +### 6.4 Package Exports + +Update `__init__.py` to export: +- `McpToolRegistrationService` (already exported, method added) + +--- + +## 7. API Design + +### 7.1 Class Diagram + +``` +┌─────────────────────────────────────────────────────────────────────┐ +│ McpToolRegistrationService │ +├─────────────────────────────────────────────────────────────────────┤ +│ - _orchestrator_name: str = "OpenAI" │ +│ - _logger: logging.Logger │ +│ - config_service: McpToolServerConfigurationService │ +├─────────────────────────────────────────────────────────────────────┤ +│ + __init__(logger: Optional[Logger]) │ +│ + add_tool_servers_to_agent(...) -> Agent │ +│ + send_chat_history_async( │ +│ turn_context: TurnContext, │ +│ messages: List[TResponseInputItem], │ +│ options: Optional[ToolOptions] │ +│ ) -> OperationResult │ +│ + send_session_history_async( │ +│ turn_context: TurnContext, │ +│ session: Session, │ +│ limit: Optional[int], │ +│ options: Optional[ToolOptions] │ +│ ) -> OperationResult │ +│ - _convert_openai_messages_to_chat_history( │ +│ messages: List[TResponseInputItem] │ +│ ) -> List[ChatHistoryMessage] │ +│ - _convert_single_message( │ +│ message: TResponseInputItem │ +│ ) -> Optional[ChatHistoryMessage] │ +│ - _extract_role(message: TResponseInputItem) -> str │ +│ - _extract_content(message: TResponseInputItem) -> str │ +│ - _extract_id(message: TResponseInputItem) -> str │ +│ - _extract_timestamp(message: TResponseInputItem) -> datetime │ +└─────────────────────────────────────────────────────────────────────┘ + │ + │ delegates to + ▼ +┌─────────────────────────────────────────────────────────────────────┐ +│ McpToolServerConfigurationService │ +├─────────────────────────────────────────────────────────────────────┤ +│ + send_chat_history( │ +│ turn_context: TurnContext, │ +│ chat_history_messages: List[ChatHistoryMessage], │ +│ options: Optional[ToolOptions] │ +│ ) -> OperationResult │ +└─────────────────────────────────────────────────────────────────────┘ +``` + +### 7.2 Sequence Diagram + +``` +Developer McpToolRegistrationService McpToolServerConfigurationService MCP Platform + │ │ │ │ + │ send_chat_history_async│ │ │ + │──────────────────────>│ │ │ + │ │ │ │ + │ │ validate inputs │ │ + │ │──────────────┐ │ │ + │ │ │ │ │ + │ │<─────────────┘ │ │ + │ │ │ │ + │ │ _convert_openai_messages_to_chat_history │ + │ │──────────────┐ │ │ + │ │ │ for each message: │ + │ │ │ - extract role │ + │ │ │ - extract content │ + │ │ │ - extract/generate ID │ + │ │ │ - extract/generate timestamp │ + │ │<─────────────┘ │ │ + │ │ │ │ + │ │ send_chat_history │ │ + │ │─────────────────────────────>│ │ + │ │ │ │ + │ │ │ POST /chat-message │ + │ │ │──────────────────────────────>│ + │ │ │ │ + │ │ │ HTTP 200 / Error │ + │ │ │<──────────────────────────────│ + │ │ │ │ + │ │ OperationResult │ │ + │ │<─────────────────────────────│ │ + │ │ │ │ + │ OperationResult │ │ │ + │<──────────────────────│ │ │ + │ │ │ │ +``` + +### 7.3 Data Models + +#### 7.3.1 Existing Models (No Changes) + +```python +# From microsoft_agents_a365.tooling.models + +class ChatHistoryMessage(BaseModel): + """Represents a single message in the chat history.""" + + id: Optional[str] = Field(default=None) + role: Literal["user", "assistant", "system"] + content: str + timestamp: Optional[datetime] = Field(default=None) + + +class ToolOptions: + """Configuration options for tooling operations.""" + + orchestrator_name: Optional[str] +``` + +#### 7.3.2 OpenAI Types (External, for reference) + +```python +# From openai-agents SDK (external types for reference) + +TResponseInputItem = Union[ + EasyInputMessage, + ResponseOutputMessage, + ResponseFileSearchToolCall, + ResponseFunctionWebSearch, + ResponseComputerToolCall, + ResponseFunctionToolCall, + ResponseFunctionToolCallOutput, + ResponseReasoningItem, + ItemReference, + # ... other types +] + +class UserMessage: + role: Literal["user"] + content: Union[str, List[ContentPart]] + +class AssistantMessage: + role: Literal["assistant"] + content: Union[str, List[ContentPart]] + +class SystemMessage: + role: Literal["system"] + content: Union[str, List[ContentPart]] +``` + +### 7.4 Method Implementation Pseudocode + +```python +async def send_chat_history_async( + self, + turn_context: TurnContext, + messages: List[TResponseInputItem], + options: Optional[ToolOptions] = None, +) -> OperationResult: + """ + Sends OpenAI chat history to the MCP platform for threat protection. + """ + # Validate inputs + if turn_context is None: + raise ValueError("turn_context cannot be None") + if messages is None: + raise ValueError("messages cannot be None") + + # Handle empty list as no-op + if len(messages) == 0: + self._logger.info("Empty message list provided, returning success") + return OperationResult.success() + + # Set default options + if options is None: + options = ToolOptions(orchestrator_name=self._orchestrator_name) + elif options.orchestrator_name is None: + options.orchestrator_name = self._orchestrator_name + + try: + # Convert OpenAI messages to ChatHistoryMessage format + chat_history_messages = self._convert_openai_messages_to_chat_history(messages) + + # Delegate to core service + return await self.config_service.send_chat_history( + turn_context=turn_context, + chat_history_messages=chat_history_messages, + options=options, + ) + except ValueError: + # Re-raise validation errors + raise + except Exception as ex: + self._logger.error(f"Failed to send chat history: {ex}") + return OperationResult.failed(OperationError(ex)) + + +async def send_session_history_async( + self, + turn_context: TurnContext, + session: Session, + limit: Optional[int] = None, + options: Optional[ToolOptions] = None, +) -> OperationResult: + """ + Extracts chat history from an OpenAI Session and sends it to the MCP platform. + """ + # Validate inputs + if turn_context is None: + raise ValueError("turn_context cannot be None") + if session is None: + raise ValueError("session cannot be None") + + try: + # Extract messages from session + messages = await session.get_items(limit=limit) + + # Delegate to the list-based method + return await self.send_chat_history_async( + turn_context=turn_context, + messages=messages, + options=options, + ) + except ValueError: + # Re-raise validation errors + raise + except Exception as ex: + self._logger.error(f"Failed to send session history: {ex}") + return OperationResult.failed(OperationError(ex)) +``` + +--- + +## 8. Observability Requirements + +### 8.1 Logging Requirements + +| Level | When | Message Template | +|-------|------|------------------| +| INFO | Method entry | `"Sending {count} OpenAI messages as chat history"` | +| INFO | Success | `"Successfully sent chat history with {count} messages"` | +| DEBUG | Per-message conversion | `"Converting message: role={role}, has_id={has_id}, has_timestamp={has_ts}"` | +| DEBUG | ID generation | `"Generated UUID {id} for message without ID"` | +| DEBUG | Timestamp generation | `"Using current UTC time for message without timestamp"` | +| WARNING | Unknown role | `"Unknown message type {type}, defaulting to 'user' role"` | +| WARNING | Empty content | `"Message has empty content, using empty string"` | +| ERROR | Conversion failure | `"Failed to convert message: {error}"` | +| ERROR | Send failure | `"Failed to send chat history: {error}"` | + +### 8.2 Metrics (Future Enhancement) + +| Metric Name | Type | Description | +|-------------|------|-------------| +| `a365.tooling.openai.send_chat_history.count` | Counter | Total number of send operations | +| `a365.tooling.openai.send_chat_history.success` | Counter | Successful send operations | +| `a365.tooling.openai.send_chat_history.failure` | Counter | Failed send operations | +| `a365.tooling.openai.send_chat_history.messages` | Histogram | Messages per batch | +| `a365.tooling.openai.send_chat_history.duration_ms` | Histogram | Operation duration | + +### 8.3 Tracing Integration + +The method SHOULD integrate with the existing observability framework: + +```python +# Future enhancement - integrate with ExecuteToolScope +from microsoft_agents_a365.observability.core import ExecuteToolScope, ToolCallDetails + +with ExecuteToolScope.start( + tool_call_details=ToolCallDetails( + tool_name="send_chat_history", + tool_arguments={"message_count": len(messages)}, + ) +) as scope: + result = await self._send_internal(...) + scope.record_response(str(result)) +``` + +--- + +## 9. Testing Strategy + +### 9.1 Test Categories + +| Category | Coverage Target | Focus | +|----------|-----------------|-------| +| Unit Tests | ≥95% lines | Method logic, conversion, validation | +| Integration Tests | Key flows | End-to-end with mocked HTTP | +| Edge Case Tests | 100% identified cases | Null handling, empty content, unknown types | + +### 9.2 Unit Test Cases + +#### 9.2.1 Input Validation Tests + +| Test ID | Test Name | Description | +|---------|-----------|-------------| +| UV-01 | `test_send_chat_history_async_validates_turn_context_none` | Verify ValueError when turn_context is None | +| UV-02 | `test_send_chat_history_async_validates_messages_none` | Verify ValueError when messages is None | +| UV-03 | `test_send_chat_history_async_empty_list_returns_success` | Verify empty list returns success (no-op) | +| UV-04 | `test_send_chat_history_async_validates_activity_none` | Verify ValueError when activity is None | +| UV-05 | `test_send_chat_history_async_validates_conversation_id` | Verify ValueError when conversation.id missing | +| UV-06 | `test_send_chat_history_async_validates_message_id` | Verify ValueError when activity.id missing | +| UV-07 | `test_send_chat_history_async_validates_user_message` | Verify ValueError when activity.text missing | +| UV-08 | `test_send_session_history_async_validates_turn_context_none` | Verify ValueError when turn_context is None | +| UV-09 | `test_send_session_history_async_validates_session_none` | Verify ValueError when session is None | + +#### 9.2.2 Conversion Tests + +| Test ID | Test Name | Description | +|---------|-----------|-------------| +| CV-01 | `test_convert_user_message_to_chat_history` | UserMessage converts with role="user" | +| CV-02 | `test_convert_assistant_message_to_chat_history` | AssistantMessage converts with role="assistant" | +| CV-03 | `test_convert_system_message_to_chat_history` | SystemMessage converts with role="system" | +| CV-04 | `test_convert_message_with_string_content` | String content extracted directly | +| CV-05 | `test_convert_message_with_list_content` | List content concatenated | +| CV-06 | `test_convert_message_generates_uuid_when_id_missing` | UUID generated for missing ID | +| CV-07 | `test_convert_message_uses_utc_when_timestamp_missing` | UTC timestamp used when missing | +| CV-08 | `test_convert_message_preserves_existing_id` | Existing ID preserved | +| CV-09 | `test_convert_message_preserves_existing_timestamp` | Existing timestamp preserved | +| CV-10 | `test_convert_unknown_message_type_defaults_to_user` | Unknown type defaults to user role | +| CV-11 | `test_convert_empty_content_uses_empty_string` | Empty content handled gracefully | +| CV-12 | `test_convert_multiple_messages` | Multiple messages converted correctly | + +#### 9.2.3 Success Path Tests + +| Test ID | Test Name | Description | +|---------|-----------|-------------| +| SP-01 | `test_send_chat_history_async_success` | Successful send returns succeeded=True | +| SP-02 | `test_send_chat_history_async_with_options` | Custom ToolOptions applied | +| SP-03 | `test_send_chat_history_async_default_orchestrator_name` | Default orchestrator name set | +| SP-04 | `test_send_chat_history_async_delegates_to_config_service` | Delegation verified | +| SP-05 | `test_send_session_history_async_success` | Session messages extracted and sent | +| SP-06 | `test_send_session_history_async_with_limit` | Limit parameter passed to get_items | +| SP-07 | `test_send_session_history_async_delegates_to_send_chat_history` | Delegation verified | + +#### 9.2.4 Error Handling Tests + +| Test ID | Test Name | Description | +|---------|-----------|-------------| +| EH-01 | `test_send_chat_history_async_http_error` | HTTP error returns failed result | +| EH-02 | `test_send_chat_history_async_timeout_error` | Timeout returns failed result | +| EH-03 | `test_send_chat_history_async_client_error` | Network error returns failed result | +| EH-04 | `test_send_chat_history_async_conversion_error` | Conversion error returns failed result | +| EH-05 | `test_send_session_history_async_get_items_error` | Session get_items error returns failed result | + +### 9.3 Integration Test Cases + +| Test ID | Test Name | Description | +|---------|-----------|-------------| +| IT-01 | `test_send_chat_history_async_end_to_end_success` | Full flow with mocked HTTP | +| IT-02 | `test_send_chat_history_async_end_to_end_server_error` | Full flow with HTTP 500 | +| IT-03 | `test_send_chat_history_async_payload_format` | Verify JSON payload structure | + +### 9.4 Test File Structure + +``` +tests/ +└── tooling/ + └── extensions/ + └── openai/ + ├── __init__.py + ├── test_send_chat_history_async.py # Main API tests + ├── test_message_conversion.py # Conversion logic tests + └── conftest.py # Shared fixtures +``` + +### 9.5 Sample Test Code + +```python +# tests/tooling/extensions/openai/test_send_chat_history_async.py + +import pytest +from datetime import datetime, timezone +from unittest.mock import AsyncMock, Mock, patch +import uuid + +from microsoft_agents.hosting.core import TurnContext +from microsoft_agents_a365.tooling.extensions.openai import McpToolRegistrationService +from microsoft_agents_a365.runtime import OperationResult + + +class TestSendChatHistoryAsync: + """Tests for send_chat_history_async method.""" + + @pytest.fixture + def service(self): + """Create McpToolRegistrationService instance.""" + return McpToolRegistrationService() + + @pytest.fixture + def mock_turn_context(self): + """Create a mock TurnContext.""" + mock_context = Mock(spec=TurnContext) + mock_activity = Mock() + mock_conversation = Mock() + mock_conversation.id = "conv-123" + mock_activity.conversation = mock_conversation + mock_activity.id = "msg-456" + mock_activity.text = "Hello, how are you?" + mock_context.activity = mock_activity + return mock_context + + @pytest.fixture + def sample_openai_messages(self): + """Create sample OpenAI messages.""" + # Mock OpenAI message types + user_msg = Mock() + user_msg.role = "user" + user_msg.content = "Hello" + + assistant_msg = Mock() + assistant_msg.role = "assistant" + assistant_msg.content = "Hi there!" + + return [user_msg, assistant_msg] + + @pytest.mark.asyncio + async def test_send_chat_history_async_validates_turn_context_none( + self, service, sample_openai_messages + ): + """Test that send_chat_history_async validates turn_context.""" + with pytest.raises(ValueError, match="turn_context cannot be None"): + await service.send_chat_history_async(None, sample_openai_messages) + + @pytest.mark.asyncio + async def test_send_chat_history_async_success( + self, service, mock_turn_context, sample_openai_messages + ): + """Test successful send_chat_history_async call.""" + with patch.object( + service.config_service, + 'send_chat_history', + new_callable=AsyncMock + ) as mock_send: + mock_send.return_value = OperationResult.success() + + result = await service.send_chat_history_async( + mock_turn_context, + sample_openai_messages + ) + + assert result.succeeded is True + mock_send.assert_called_once() +``` + +--- + +## 10. Acceptance Criteria + +### 10.1 Functional Acceptance Criteria + +| ID | Criterion | Verification | +|----|-----------|--------------| +| AC-01 | `send_chat_history_async` method exists in `McpToolRegistrationService` | Code review | +| AC-01a | `send_session_history_async` method exists in `McpToolRegistrationService` | Code review | +| AC-02 | Method accepts `List[TResponseInputItem]` parameter | Type checker passes | +| AC-03 | Method returns `OperationResult` | Unit tests pass | +| AC-04 | Missing message IDs are generated as UUIDs | Unit tests pass | +| AC-05 | Missing timestamps use current UTC time | Unit tests pass | +| AC-06 | All OpenAI role types map correctly | Unit tests pass | +| AC-07 | Validation errors raise `ValueError` with descriptive messages | Unit tests pass | +| AC-07a | Empty message list returns `OperationResult.success()` | Unit tests pass | +| AC-08 | HTTP errors return `OperationResult.failed()` | Unit tests pass | +| AC-08a | `send_session_history_async` calls `session.get_items()` correctly | Unit tests pass | + +### 10.2 Non-Functional Acceptance Criteria + +| ID | Criterion | Verification | +|----|-----------|--------------| +| AC-09 | Unit test coverage ≥95% | Coverage report | +| AC-10 | All public methods have docstrings | Code review | +| AC-11 | Type hints on all parameters and returns | Type checker passes | +| AC-12 | No new linting errors | `ruff check` passes | +| AC-13 | Code follows existing patterns | Code review | + +### 10.3 Documentation Acceptance Criteria + +| ID | Criterion | Verification | +|----|-----------|--------------| +| AC-14 | Method has complete docstring with Args/Returns/Raises | Code review | +| AC-15 | README.md updated with usage example | Documentation review | +| AC-16 | CHANGELOG.md updated | Documentation review | + +--- + +## 11. Non-Functional Requirements + +### 11.1 Performance + +| Requirement | Target | +|-------------|--------| +| Conversion overhead | <10ms for 100 messages | +| Memory overhead | <1MB for 1000 messages | +| No blocking calls | All I/O is async | +| Batch size | No limit - endpoint assumed to handle unlimited size | +| Performance benchmarks | Required in CI for regression detection | + +### 11.2 Reliability + +| Requirement | Target | +|-------------|--------| +| Graceful degradation | Return `OperationResult.failed()` on errors | +| No data loss | Messages not modified in place | +| Idempotent IDs | Same message without ID gets different UUID each call | + +### 11.3 Maintainability + +| Requirement | Target | +|-------------|--------| +| Cyclomatic complexity | <10 per method | +| Method length | <50 lines | +| Single responsibility | One public method, helper methods for conversion | + +### 11.4 Security + +| Requirement | Description | +|-------------|-------------| +| No credential logging | Auth tokens never logged | +| Input sanitization | Content not modified, passed through | +| Secure defaults | HTTPS enforced by underlying service | + +--- + +## 12. Dependencies + +### 12.1 Internal Dependencies + +| Package | Version | Purpose | +|---------|---------|---------| +| `microsoft-agents-a365-tooling` | ≥1.0.0 | Core `McpToolServerConfigurationService` | +| `microsoft-agents-a365-runtime` | ≥1.0.0 | `OperationResult`, `OperationError` | + +### 12.2 External Dependencies + +| Package | Version | Purpose | +|---------|---------|---------| +| `openai-agents` | ≥0.1.0 | OpenAI message types (`TResponseInputItem`) | +| `microsoft-agents-hosting-core` | ≥0.1.0 | `TurnContext`, `Authorization` | +| `pydantic` | ≥2.0.0 | Model validation | + +### 12.3 Development Dependencies + +| Package | Version | Purpose | +|---------|---------|---------| +| `pytest` | ≥7.0.0 | Test framework | +| `pytest-asyncio` | ≥0.21.0 | Async test support | +| `pytest-cov` | ≥4.0.0 | Coverage reporting | + +--- + +## 13. Risks and Mitigations + +### 13.1 Technical Risks + +| Risk | Likelihood | Impact | Mitigation | +|------|------------|--------|------------| +| OpenAI SDK type changes | Medium | High | Use duck typing, version pin, adapter pattern | +| Performance regression | Low | Medium | Benchmark tests, lazy evaluation | +| Thread safety issues | Low | High | Stateless design, no shared state | + +### 13.2 Schedule Risks + +| Risk | Likelihood | Impact | Mitigation | +|------|------------|--------|------------| +| Scope creep | Medium | Medium | Clear scope definition, change control | +| Testing delays | Medium | Medium | Parallel test development | +| Review delays | Low | Medium | Early reviewer engagement | + +### 13.3 Integration Risks + +| Risk | Likelihood | Impact | Mitigation | +|------|------------|--------|------------| +| Breaking existing API | Low | High | No changes to existing methods | +| Dependency conflicts | Low | Medium | Version ranges, compatibility testing | + +--- + +## 14. Open Questions + +### 14.1 Design Questions + +| ID | Question | Status | Decision | +|----|----------|--------|----------| +| OQ-01 | Should we support OpenAI Session protocol directly in addition to message lists? | **Resolved** | **Yes** - Provide a separate API that extracts messages from Session | +| OQ-02 | Should we add a synchronous wrapper `send_chat_history` for convenience? | **Deferred** | Skip for now - evaluate in future iteration | +| OQ-03 | Should empty message lists be allowed (no-op) vs. raising ValueError? | **Resolved** | **Empty list allowed** - Return success with no-op | +| OQ-04 | Should we support custom ID generators (injectable)? | **Resolved** | **No** - Use UUID by default for simplicity | + +### 14.2 Implementation Questions + +| ID | Question | Status | Decision | +|----|----------|--------|----------| +| OQ-05 | Which specific OpenAI message types need support beyond User/Assistant/System? | Open | Need to enumerate `TResponseInputItem` union | +| OQ-06 | Should tool call/result messages be included in chat history? | **Resolved** | **Yes** - Include all items retrieved, no filtering required | +| OQ-07 | What is the maximum batch size for messages? | **Resolved** | **No limit assumed** - Assume endpoint handles unlimited size, no batching required | + +### 14.3 Testing Questions + +| ID | Question | Status | Decision | +|----|----------|--------|----------| +| OQ-08 | Should we add performance benchmarks to CI? | **Resolved** | **Yes** - Add performance benchmarks for regression detection | +| OQ-09 | Should we mock OpenAI types or use real SDK types in tests? | **Resolved** | **Yes** - Mock for unit tests, real SDK types for integration tests | + +--- + +## Appendix A: .NET SDK Reference + +### A.1 Agent Framework Implementation (PR #171) + +The .NET implementation provides four method overloads: + +```csharp +// Method 1: IEnumerable +Task SendChatHistoryAsync( + TurnContext turnContext, + IEnumerable chatMessages, + ToolOptions? options = null, + CancellationToken cancellationToken = default); + +// Method 2: ChatMessageStore +Task SendChatHistoryAsync( + TurnContext turnContext, + ChatMessageStore chatMessageStore, + ToolOptions? options = null, + CancellationToken cancellationToken = default); +``` + +Key implementation details: +- Generates `Guid.NewGuid()` for missing IDs +- Uses `DateTime.UtcNow` for missing timestamps +- Converts to `ChatHistoryMessage[]` array for the core API + +### A.2 Semantic Kernel Implementation (PR #173) + +```csharp +// Method 1: ChatHistory +Task SendChatHistoryAsync( + TurnContext turnContext, + ChatHistory chatHistory, + ToolOptions? options = null, + CancellationToken cancellationToken = default); + +// Method 2: IEnumerable +Task SendChatHistoryAsync( + TurnContext turnContext, + IEnumerable chatHistory, + ToolOptions? options = null, + CancellationToken cancellationToken = default); +``` + +--- + +## Appendix B: OpenAI SDK Type Reference + +### B.1 TResponseInputItem Union Type + +The `TResponseInputItem` is a union type in the OpenAI Agents SDK that includes: + +```python +TResponseInputItem = Union[ + EasyInputMessage, # Simple input message + ResponseOutputMessage, # Response from agent + ResponseFileSearchToolCall, # File search tool call + ResponseFunctionWebSearch, # Web search function + ResponseComputerToolCall, # Computer use tool call + ResponseFunctionToolCall, # Function tool call + ResponseFunctionToolCallOutput, # Tool call output + ResponseReasoningItem, # Reasoning step + ItemReference, # Reference to another item + # ... potentially more types +] +``` + +### B.2 Message Content Types + +```python +# Content can be string or list of parts +ContentPart = Union[ + TextContentPart, + ImageContentPart, + AudioContentPart, +] + +class TextContentPart: + type: Literal["text"] + text: str +``` + +--- + +## Document History + +| Version | Date | Author | Changes | +|---------|------|--------|---------| +| 1.0 | 2026-01-21 | Agent365 Python SDK Team | Initial draft | From a68adc12a435b44f89e8092ddb1ce7e3f10a0345 Mon Sep 17 00:00:00 2001 From: Johan Broberg Date: Wed, 21 Jan 2026 16:08:20 -0800 Subject: [PATCH 02/11] Rename `send_chat_history_async` to `send_chat_history_messages_async` and update PRD to reflect new method naming conventions and functionality. --- docs/prd/openai-send-chat-history-api.md | 211 ++++++++++++----------- 1 file changed, 109 insertions(+), 102 deletions(-) diff --git a/docs/prd/openai-send-chat-history-api.md b/docs/prd/openai-send-chat-history-api.md index 8d9eb490..7e7c42b7 100644 --- a/docs/prd/openai-send-chat-history-api.md +++ b/docs/prd/openai-send-chat-history-api.md @@ -44,9 +44,15 @@ This manual conversion creates: - **Error-prone integrations**: Missing ID or timestamp handling may vary - **Feature parity gap**: The .NET SDK already provides framework-specific `SendChatHistoryAsync` methods for Agent Framework (PR #171) and Semantic Kernel (PR #173) +### 1.2.1 Method Naming Convention + +This PRD defines two methods with the following naming convention: +- **`send_chat_history_messages_async`**: Accepts a list of OpenAI `TResponseInputItem` messages directly +- **`send_chat_history_async`**: Extracts messages from an OpenAI `Session` object (the more common use case) + ### 1.2 Proposed Solution -Implement an OpenAI-specific `send_chat_history_async` API in the `microsoft-agents-a365-tooling-extensions-openai` package that: +Implement OpenAI-specific chat history APIs in the `microsoft-agents-a365-tooling-extensions-openai` package that: - Accepts OpenAI SDK native types directly (Session protocol items, message types) - Handles all conversion logic internally - Provides a seamless developer experience for OpenAI Agents SDK users @@ -66,7 +72,7 @@ Implement an OpenAI-specific `send_chat_history_async` API in the `microsoft-age | Metric | Target | |--------|--------| -| API adoption rate | 80% of OpenAI extension users use `send_chat_history_async` within 3 months | +| API adoption rate | 80% of OpenAI extension users use `send_chat_history_async` or `send_chat_history_messages_async` within 3 months | | Code reduction | Average 15+ lines of boilerplate eliminated per integration | | Test coverage | ≥95% line coverage, ≥90% branch coverage | | Documentation completeness | 100% of public APIs documented with examples | @@ -109,7 +115,7 @@ Implement an OpenAI-specific `send_chat_history_async` API in the `microsoft-age | **US-03** | As an OpenAI agent developer, I want missing message IDs to be auto-generated so that I don't need to track IDs manually | P0 | UUIDs generated for messages without IDs | | **US-04** | As an OpenAI agent developer, I want missing timestamps to use current UTC time so that all messages have valid timestamps | P0 | Current UTC timestamp used when not provided | | **US-05** | As an OpenAI agent developer, I want to receive clear success/failure results so that I can handle errors appropriately | P0 | `OperationResult` returned with error details on failure | -| **US-05a** | As an OpenAI agent developer, I want to send my Session's conversation history directly so that I don't need to extract messages manually | P0 | `send_session_history_async` extracts and sends Session items | +| **US-05a** | As an OpenAI agent developer, I want to send my Session's conversation history directly so that I don't need to extract messages manually | P0 | `send_chat_history_async` extracts and sends Session items | ### 3.2 Secondary User Stories @@ -137,9 +143,9 @@ Implement an OpenAI-specific `send_chat_history_async` API in the `microsoft-age | **FR-08** | The API SHALL allow empty message lists (no-op, return success) | P0 | Unit tests | | **FR-09** | The API SHALL map OpenAI roles to `ChatHistoryMessage` roles ("user", "assistant", "system") | P0 | Unit tests | | **FR-10** | The API SHALL extract text content from OpenAI message content arrays | P0 | Unit tests | -| **FR-11** | The API SHALL provide a `send_session_history_async` method that accepts an OpenAI Session | P0 | Unit tests | -| **FR-12** | The `send_session_history_async` method SHALL call `session.get_items()` to retrieve messages | P0 | Unit tests | -| **FR-13** | The `send_session_history_async` method SHALL support an optional `limit` parameter for `get_items()` | P1 | Unit tests | +| **FR-11** | The API SHALL provide a `send_chat_history_async` method that accepts an OpenAI Session | P0 | Unit tests | +| **FR-12** | The `send_chat_history_async` method SHALL call `session.get_items()` to retrieve messages | P0 | Unit tests | +| **FR-13** | The `send_chat_history_async` method SHALL support an optional `limit` parameter for `get_items()` | P1 | Unit tests | | **FR-14** | The API SHALL include all item types from the session without filtering | P0 | Unit tests | ### 4.2 Method Signatures @@ -147,43 +153,43 @@ Implement an OpenAI-specific `send_chat_history_async` API in the `microsoft-age The implementation SHALL provide the following method overloads: ```python -# Primary method - accepts list of OpenAI message types +# Primary method - extracts messages from OpenAI Session (most common use case) async def send_chat_history_async( self, turn_context: TurnContext, - messages: List[TResponseInputItem], + session: Session, + limit: Optional[int] = None, options: Optional[ToolOptions] = None, ) -> OperationResult: """ - Sends OpenAI chat history to the MCP platform for threat protection. - + Extracts chat history from an OpenAI Session and sends it to the MCP platform. + Args: turn_context: TurnContext from the Agents SDK containing conversation info. - messages: List of OpenAI TResponseInputItem messages to send. + session: OpenAI Session instance to extract messages from. + limit: Optional maximum number of items to retrieve from session. + If None, retrieves all items. options: Optional ToolOptions for customization. - + Returns: OperationResult indicating success or failure. """ -# Secondary method - extracts messages from OpenAI Session -async def send_session_history_async( +# Secondary method - accepts list of OpenAI message types directly +async def send_chat_history_messages_async( self, turn_context: TurnContext, - session: Session, - limit: Optional[int] = None, + messages: List[TResponseInputItem], options: Optional[ToolOptions] = None, ) -> OperationResult: """ - Extracts chat history from an OpenAI Session and sends it to the MCP platform. - + Sends OpenAI chat history to the MCP platform for threat protection. + Args: turn_context: TurnContext from the Agents SDK containing conversation info. - session: OpenAI Session instance to extract messages from. - limit: Optional maximum number of items to retrieve from session. - If None, retrieves all items. + messages: List of OpenAI TResponseInputItem messages to send. options: Optional ToolOptions for customization. - + Returns: OperationResult indicating success or failure. """ @@ -302,13 +308,13 @@ Update `__init__.py` to export: │ + add_tool_servers_to_agent(...) -> Agent │ │ + send_chat_history_async( │ │ turn_context: TurnContext, │ -│ messages: List[TResponseInputItem], │ +│ session: Session, │ +│ limit: Optional[int], │ │ options: Optional[ToolOptions] │ │ ) -> OperationResult │ -│ + send_session_history_async( │ +│ + send_chat_history_messages_async( │ │ turn_context: TurnContext, │ -│ session: Session, │ -│ limit: Optional[int], │ +│ messages: List[TResponseInputItem], │ │ options: Optional[ToolOptions] │ │ ) -> OperationResult │ │ - _convert_openai_messages_to_chat_history( │ @@ -341,7 +347,8 @@ Update `__init__.py` to export: ``` Developer McpToolRegistrationService McpToolServerConfigurationService MCP Platform │ │ │ │ - │ send_chat_history_async│ │ │ + │ send_chat_history_ │ │ │ + │ messages_async │ │ │ │──────────────────────>│ │ │ │ │ │ │ │ │ validate inputs │ │ @@ -434,37 +441,27 @@ class SystemMessage: async def send_chat_history_async( self, turn_context: TurnContext, - messages: List[TResponseInputItem], + session: Session, + limit: Optional[int] = None, options: Optional[ToolOptions] = None, ) -> OperationResult: """ - Sends OpenAI chat history to the MCP platform for threat protection. + Extracts chat history from an OpenAI Session and sends it to the MCP platform. """ # Validate inputs if turn_context is None: raise ValueError("turn_context cannot be None") - if messages is None: - raise ValueError("messages cannot be None") - - # Handle empty list as no-op - if len(messages) == 0: - self._logger.info("Empty message list provided, returning success") - return OperationResult.success() - - # Set default options - if options is None: - options = ToolOptions(orchestrator_name=self._orchestrator_name) - elif options.orchestrator_name is None: - options.orchestrator_name = self._orchestrator_name - + if session is None: + raise ValueError("session cannot be None") + try: - # Convert OpenAI messages to ChatHistoryMessage format - chat_history_messages = self._convert_openai_messages_to_chat_history(messages) - - # Delegate to core service - return await self.config_service.send_chat_history( + # Extract messages from session + messages = await session.get_items(limit=limit) + + # Delegate to the list-based method + return await self.send_chat_history_messages_async( turn_context=turn_context, - chat_history_messages=chat_history_messages, + messages=messages, options=options, ) except ValueError: @@ -475,37 +472,47 @@ async def send_chat_history_async( return OperationResult.failed(OperationError(ex)) -async def send_session_history_async( +async def send_chat_history_messages_async( self, turn_context: TurnContext, - session: Session, - limit: Optional[int] = None, + messages: List[TResponseInputItem], options: Optional[ToolOptions] = None, ) -> OperationResult: """ - Extracts chat history from an OpenAI Session and sends it to the MCP platform. + Sends OpenAI chat history to the MCP platform for threat protection. """ # Validate inputs if turn_context is None: raise ValueError("turn_context cannot be None") - if session is None: - raise ValueError("session cannot be None") - + if messages is None: + raise ValueError("messages cannot be None") + + # Handle empty list as no-op + if len(messages) == 0: + self._logger.info("Empty message list provided, returning success") + return OperationResult.success() + + # Set default options + if options is None: + options = ToolOptions(orchestrator_name=self._orchestrator_name) + elif options.orchestrator_name is None: + options.orchestrator_name = self._orchestrator_name + try: - # Extract messages from session - messages = await session.get_items(limit=limit) - - # Delegate to the list-based method - return await self.send_chat_history_async( + # Convert OpenAI messages to ChatHistoryMessage format + chat_history_messages = self._convert_openai_messages_to_chat_history(messages) + + # Delegate to core service + return await self.config_service.send_chat_history( turn_context=turn_context, - messages=messages, + chat_history_messages=chat_history_messages, options=options, ) except ValueError: # Re-raise validation errors raise except Exception as ex: - self._logger.error(f"Failed to send session history: {ex}") + self._logger.error(f"Failed to send chat history messages: {ex}") return OperationResult.failed(OperationError(ex)) ``` @@ -573,15 +580,15 @@ with ExecuteToolScope.start( | Test ID | Test Name | Description | |---------|-----------|-------------| -| UV-01 | `test_send_chat_history_async_validates_turn_context_none` | Verify ValueError when turn_context is None | -| UV-02 | `test_send_chat_history_async_validates_messages_none` | Verify ValueError when messages is None | -| UV-03 | `test_send_chat_history_async_empty_list_returns_success` | Verify empty list returns success (no-op) | -| UV-04 | `test_send_chat_history_async_validates_activity_none` | Verify ValueError when activity is None | -| UV-05 | `test_send_chat_history_async_validates_conversation_id` | Verify ValueError when conversation.id missing | -| UV-06 | `test_send_chat_history_async_validates_message_id` | Verify ValueError when activity.id missing | -| UV-07 | `test_send_chat_history_async_validates_user_message` | Verify ValueError when activity.text missing | -| UV-08 | `test_send_session_history_async_validates_turn_context_none` | Verify ValueError when turn_context is None | -| UV-09 | `test_send_session_history_async_validates_session_none` | Verify ValueError when session is None | +| UV-01 | `test_send_chat_history_messages_async_validates_turn_context_none` | Verify ValueError when turn_context is None | +| UV-02 | `test_send_chat_history_messages_async_validates_messages_none` | Verify ValueError when messages is None | +| UV-03 | `test_send_chat_history_messages_async_empty_list_returns_success` | Verify empty list returns success (no-op) | +| UV-04 | `test_send_chat_history_messages_async_validates_activity_none` | Verify ValueError when activity is None | +| UV-05 | `test_send_chat_history_messages_async_validates_conversation_id` | Verify ValueError when conversation.id missing | +| UV-06 | `test_send_chat_history_messages_async_validates_message_id` | Verify ValueError when activity.id missing | +| UV-07 | `test_send_chat_history_messages_async_validates_user_message` | Verify ValueError when activity.text missing | +| UV-08 | `test_send_chat_history_async_validates_turn_context_none` | Verify ValueError when turn_context is None | +| UV-09 | `test_send_chat_history_async_validates_session_none` | Verify ValueError when session is None | #### 9.2.2 Conversion Tests @@ -604,23 +611,23 @@ with ExecuteToolScope.start( | Test ID | Test Name | Description | |---------|-----------|-------------| -| SP-01 | `test_send_chat_history_async_success` | Successful send returns succeeded=True | -| SP-02 | `test_send_chat_history_async_with_options` | Custom ToolOptions applied | -| SP-03 | `test_send_chat_history_async_default_orchestrator_name` | Default orchestrator name set | -| SP-04 | `test_send_chat_history_async_delegates_to_config_service` | Delegation verified | -| SP-05 | `test_send_session_history_async_success` | Session messages extracted and sent | -| SP-06 | `test_send_session_history_async_with_limit` | Limit parameter passed to get_items | -| SP-07 | `test_send_session_history_async_delegates_to_send_chat_history` | Delegation verified | +| SP-01 | `test_send_chat_history_messages_async_success` | Successful send returns succeeded=True | +| SP-02 | `test_send_chat_history_messages_async_with_options` | Custom ToolOptions applied | +| SP-03 | `test_send_chat_history_messages_async_default_orchestrator_name` | Default orchestrator name set | +| SP-04 | `test_send_chat_history_messages_async_delegates_to_config_service` | Delegation verified | +| SP-05 | `test_send_chat_history_async_success` | Session messages extracted and sent | +| SP-06 | `test_send_chat_history_async_with_limit` | Limit parameter passed to get_items | +| SP-07 | `test_send_chat_history_async_delegates_to_send_chat_history_messages` | Delegation verified | #### 9.2.4 Error Handling Tests | Test ID | Test Name | Description | |---------|-----------|-------------| -| EH-01 | `test_send_chat_history_async_http_error` | HTTP error returns failed result | -| EH-02 | `test_send_chat_history_async_timeout_error` | Timeout returns failed result | -| EH-03 | `test_send_chat_history_async_client_error` | Network error returns failed result | -| EH-04 | `test_send_chat_history_async_conversion_error` | Conversion error returns failed result | -| EH-05 | `test_send_session_history_async_get_items_error` | Session get_items error returns failed result | +| EH-01 | `test_send_chat_history_messages_async_http_error` | HTTP error returns failed result | +| EH-02 | `test_send_chat_history_messages_async_timeout_error` | Timeout returns failed result | +| EH-03 | `test_send_chat_history_messages_async_client_error` | Network error returns failed result | +| EH-04 | `test_send_chat_history_messages_async_conversion_error` | Conversion error returns failed result | +| EH-05 | `test_send_chat_history_async_get_items_error` | Session get_items error returns failed result | ### 9.3 Integration Test Cases @@ -658,8 +665,8 @@ from microsoft_agents_a365.tooling.extensions.openai import McpToolRegistrationS from microsoft_agents_a365.runtime import OperationResult -class TestSendChatHistoryAsync: - """Tests for send_chat_history_async method.""" +class TestSendChatHistoryMessagesAsync: + """Tests for send_chat_history_messages_async method.""" @pytest.fixture def service(self): @@ -686,38 +693,38 @@ class TestSendChatHistoryAsync: user_msg = Mock() user_msg.role = "user" user_msg.content = "Hello" - + assistant_msg = Mock() assistant_msg.role = "assistant" assistant_msg.content = "Hi there!" - + return [user_msg, assistant_msg] @pytest.mark.asyncio - async def test_send_chat_history_async_validates_turn_context_none( + async def test_send_chat_history_messages_async_validates_turn_context_none( self, service, sample_openai_messages ): - """Test that send_chat_history_async validates turn_context.""" + """Test that send_chat_history_messages_async validates turn_context.""" with pytest.raises(ValueError, match="turn_context cannot be None"): - await service.send_chat_history_async(None, sample_openai_messages) + await service.send_chat_history_messages_async(None, sample_openai_messages) @pytest.mark.asyncio - async def test_send_chat_history_async_success( + async def test_send_chat_history_messages_async_success( self, service, mock_turn_context, sample_openai_messages ): - """Test successful send_chat_history_async call.""" + """Test successful send_chat_history_messages_async call.""" with patch.object( - service.config_service, - 'send_chat_history', + service.config_service, + 'send_chat_history', new_callable=AsyncMock ) as mock_send: mock_send.return_value = OperationResult.success() - - result = await service.send_chat_history_async( - mock_turn_context, + + result = await service.send_chat_history_messages_async( + mock_turn_context, sample_openai_messages ) - + assert result.succeeded is True mock_send.assert_called_once() ``` @@ -731,7 +738,7 @@ class TestSendChatHistoryAsync: | ID | Criterion | Verification | |----|-----------|--------------| | AC-01 | `send_chat_history_async` method exists in `McpToolRegistrationService` | Code review | -| AC-01a | `send_session_history_async` method exists in `McpToolRegistrationService` | Code review | +| AC-01a | `send_chat_history_messages_async` method exists in `McpToolRegistrationService` | Code review | | AC-02 | Method accepts `List[TResponseInputItem]` parameter | Type checker passes | | AC-03 | Method returns `OperationResult` | Unit tests pass | | AC-04 | Missing message IDs are generated as UUIDs | Unit tests pass | @@ -740,7 +747,7 @@ class TestSendChatHistoryAsync: | AC-07 | Validation errors raise `ValueError` with descriptive messages | Unit tests pass | | AC-07a | Empty message list returns `OperationResult.success()` | Unit tests pass | | AC-08 | HTTP errors return `OperationResult.failed()` | Unit tests pass | -| AC-08a | `send_session_history_async` calls `session.get_items()` correctly | Unit tests pass | +| AC-08a | `send_chat_history_async` calls `session.get_items()` correctly | Unit tests pass | ### 10.2 Non-Functional Acceptance Criteria @@ -860,7 +867,7 @@ class TestSendChatHistoryAsync: | ID | Question | Status | Decision | |----|----------|--------|----------| -| OQ-01 | Should we support OpenAI Session protocol directly in addition to message lists? | **Resolved** | **Yes** - Provide a separate API that extracts messages from Session | +| OQ-01 | Should we support OpenAI Session protocol directly in addition to message lists? | **Resolved** | **Yes** - `send_chat_history_async` extracts messages from Session, `send_chat_history_messages_async` accepts message list directly | | OQ-02 | Should we add a synchronous wrapper `send_chat_history` for convenience? | **Deferred** | Skip for now - evaluate in future iteration | | OQ-03 | Should empty message lists be allowed (no-op) vs. raising ValueError? | **Resolved** | **Empty list allowed** - Return success with no-op | | OQ-04 | Should we support custom ID generators (injectable)? | **Resolved** | **No** - Use UUID by default for simplicity | From acd40c3653f68c921bc1197be6f9487a90ca1506 Mon Sep 17 00:00:00 2001 From: Johan Broberg Date: Wed, 21 Jan 2026 16:17:24 -0800 Subject: [PATCH 03/11] Add tasks for OpenAI `send_chat_history_messages_async` API implementation --- .../prd/openai-send-chat-history-api-tasks.md | 695 ++++++++++++++++++ 1 file changed, 695 insertions(+) create mode 100644 docs/prd/openai-send-chat-history-api-tasks.md diff --git a/docs/prd/openai-send-chat-history-api-tasks.md b/docs/prd/openai-send-chat-history-api-tasks.md new file mode 100644 index 00000000..1bc6fb97 --- /dev/null +++ b/docs/prd/openai-send-chat-history-api-tasks.md @@ -0,0 +1,695 @@ +# Task Breakdown: OpenAI `send_chat_history_messages_async` API + +| **Document Information** | | +|--------------------------|----------------------------------------------| +| **PRD Reference** | [openai-send-chat-history-api.md](openai-send-chat-history-api.md) | +| **Created** | January 21, 2026 | +| **Target Package** | `microsoft-agents-a365-tooling-extensions-openai` | +| **Estimated Total Effort** | 40-56 hours | + +--- + +## Table of Contents + +1. [Executive Summary](#1-executive-summary) +2. [Architecture Impact Analysis](#2-architecture-impact-analysis) +3. [Task Breakdown by Phase](#3-task-breakdown-by-phase) +4. [Task Dependencies](#4-task-dependencies) +5. [Testing Strategy Overview](#5-testing-strategy-overview) +6. [Risks and Considerations](#6-risks-and-considerations) +7. [Appendix: Task Checklist](#appendix-task-checklist) + +--- + +## 1. Executive Summary + +### 1.1 Overview + +This task breakdown covers the implementation of two new async APIs for the OpenAI orchestrator extension: + +1. **`send_chat_history_async(turn_context, session, limit, options)`** - Extracts messages from an OpenAI `Session` object and delegates to `send_chat_history_messages_async`. This is the primary/most common use case. + +2. **`send_chat_history_messages_async(turn_context, messages, options)`** - Accepts a list of OpenAI `TResponseInputItem` messages and converts them to `ChatHistoryMessage` format before sending to the MCP platform. + +### 1.2 Key Implementation Points + +| Aspect | Details | +|--------|---------| +| **Target File** | `libraries/microsoft-agents-a365-tooling-extensions-openai/microsoft_agents_a365/tooling/extensions/openai/mcp_tool_registration_service.py` | +| **Delegation Target** | `McpToolServerConfigurationService.send_chat_history()` in `microsoft-agents-a365-tooling` | +| **Return Type** | `OperationResult` (success/failure pattern) | +| **Error Handling** | `ValueError` for validation, `OperationResult.failed()` for runtime errors | + +### 1.3 Scope Summary + +- **In Scope**: Two new public methods, message conversion logic, comprehensive unit/integration tests +- **Out of Scope**: Changes to core `McpToolServerConfigurationService`, synchronous wrappers, other orchestrator extensions + +### 1.4 Effort Estimation + +| Phase | Estimated Hours | +|-------|-----------------| +| Phase 1: Foundation & Infrastructure | 6-8 hours | +| Phase 2: Core Implementation | 12-16 hours | +| Phase 3: Testing | 14-20 hours | +| Phase 4: Documentation & Finalization | 8-12 hours | +| **Total** | **40-56 hours** | + +--- + +## 2. Architecture Impact Analysis + +### 2.1 Component Diagram + +``` +┌─────────────────────────────────────────────────────────────────────────────┐ +│ Developer Application │ +└─────────────────────────────────────────────────────────────────────────────┘ + │ + │ calls + ▼ +┌─────────────────────────────────────────────────────────────────────────────┐ +│ microsoft-agents-a365-tooling-extensions-openai │ +│ ┌───────────────────────────────────────────────────────────────────────┐ │ +│ │ McpToolRegistrationService │ │ +│ │ ┌─────────────────────────────────────────────────────────────────┐ │ │ +│ │ │ NEW: send_chat_history_async() │ │ │ +│ │ │ NEW: send_chat_history_messages_async() │ │ │ +│ │ │ NEW: _convert_openai_messages_to_chat_history() │ │ │ +│ │ │ NEW: _convert_single_message() │ │ │ +│ │ │ NEW: _extract_role() / _extract_content() / _extract_id() │ │ │ +│ │ │ EXISTING: add_tool_servers_to_agent() │ │ │ +│ │ └─────────────────────────────────────────────────────────────────┘ │ │ +│ └───────────────────────────────────────────────────────────────────────┘ │ +└─────────────────────────────────────────────────────────────────────────────┘ + │ + │ delegates to + ▼ +┌─────────────────────────────────────────────────────────────────────────────┐ +│ microsoft-agents-a365-tooling (core package) │ +│ ┌───────────────────────────────────────────────────────────────────────┐ │ +│ │ McpToolServerConfigurationService │ │ +│ │ ┌─────────────────────────────────────────────────────────────────┐ │ │ +│ │ │ EXISTING: send_chat_history() │ │ │ +│ │ └─────────────────────────────────────────────────────────────────┘ │ │ +│ └───────────────────────────────────────────────────────────────────────┘ │ +└─────────────────────────────────────────────────────────────────────────────┘ + │ + │ HTTP POST + ▼ +┌─────────────────────────────────────────────────────────────────────────────┐ +│ MCP Platform │ +│ /real-time-threat-protection/chat-message │ +└─────────────────────────────────────────────────────────────────────────────┘ +``` + +### 2.2 Files Impacted + +| File | Impact Type | Changes | +|------|-------------|---------| +| `mcp_tool_registration_service.py` | **Modified** | Add 2 public methods, 5+ private helper methods | +| `__init__.py` (openai extension) | **Modified** | Ensure exports are correct | +| `pyproject.toml` (openai extension) | **Modified** | Add `openai-agents` dependency if not present | +| `tests/tooling/extensions/openai/test_send_chat_history_async.py` | **New** | Main API unit tests | +| `tests/tooling/extensions/openai/test_message_conversion.py` | **New** | Conversion logic tests | +| `tests/tooling/extensions/openai/conftest.py` | **New** | Shared pytest fixtures | +| `README.md` (openai extension) | **Modified** | Add usage documentation | + +### 2.3 Dependency Analysis + +| Dependency | Type | Purpose | Risk | +|------------|------|---------|------| +| `openai-agents` | External (new) | OpenAI SDK types (`TResponseInputItem`, `Session`) | Medium - API stability | +| `microsoft-agents-a365-tooling` | Internal (existing) | Core `McpToolServerConfigurationService` | Low | +| `microsoft-agents-a365-runtime` | Internal (existing) | `OperationResult`, `OperationError` | Low | +| `microsoft-agents-hosting-core` | External (existing) | `TurnContext` | Low | + +### 2.4 Breaking Changes + +**None** - This is a purely additive change. All existing methods remain unchanged. + +--- + +## 3. Task Breakdown by Phase + +--- + +### Phase 1: Foundation & Infrastructure (6-8 hours) + +#### Task 1.1: Analyze OpenAI SDK Types and Create Type Stubs + +| Attribute | Details | +|-----------|---------| +| **Title** | Analyze OpenAI SDK Types and Create Type Stubs | +| **Estimate** | 3-4 hours | +| **Description** | Research the OpenAI Agents SDK to understand the `TResponseInputItem` union type, `Session` protocol, and message types. Create type stubs or protocols for testing if needed. | +| **Acceptance Criteria** | - Document all message types in `TResponseInputItem` union
- Identify attributes for role, content, id, timestamp extraction
- Create mock classes for unit testing
- Verify `Session.get_items()` method signature | +| **Files to Create/Modify** | - `tests/tooling/extensions/openai/conftest.py` (fixtures and mocks) | +| **Dependencies** | None | +| **Testing Requirements** | N/A (research task) | + +--- + +#### Task 1.2: Add openai-agents Dependency + +| Attribute | Details | +|-----------|---------| +| **Title** | Add openai-agents Dependency to pyproject.toml | +| **Estimate** | 1 hour | +| **Description** | Add the `openai-agents` package dependency to the OpenAI extension's `pyproject.toml` with appropriate version constraints. | +| **Acceptance Criteria** | - `openai-agents` added to `[project.dependencies]`
- Version constraint allows flexibility for minor updates
- `pip install` succeeds with new dependency
- No conflicts with existing dependencies | +| **Files to Create/Modify** | - `libraries/microsoft-agents-a365-tooling-extensions-openai/pyproject.toml` | +| **Dependencies** | None | +| **Testing Requirements** | Run `pip install -e .` and verify installation | + +--- + +#### Task 1.3: Create Test Directory Structure and Fixtures + +| Attribute | Details | +|-----------|---------| +| **Title** | Create Test Directory Structure and Shared Fixtures | +| **Estimate** | 2-3 hours | +| **Description** | Create the test directory structure for the OpenAI extension and define shared pytest fixtures for mocking OpenAI types, TurnContext, and common test data. | +| **Acceptance Criteria** | - Test directory structure created: `tests/tooling/extensions/openai/`
- `conftest.py` with fixtures for mock TurnContext, mock OpenAI messages, mock Session
- `__init__.py` files in place
- Fixtures are reusable across test files | +| **Files to Create/Modify** | - `tests/tooling/extensions/openai/__init__.py` (new)
- `tests/tooling/extensions/openai/conftest.py` (new)
- `tests/tooling/extensions/__init__.py` (new if needed) | +| **Dependencies** | Task 1.1 | +| **Testing Requirements** | Verify fixtures work with `pytest --collect-only` | + +--- + +### Phase 2: Core Implementation (12-16 hours) + +#### Task 2.1: Implement Role Extraction Helper + +| Attribute | Details | +|-----------|---------| +| **Title** | Implement `_extract_role()` Private Method | +| **Estimate** | 2 hours | +| **Description** | Implement the private `_extract_role()` method that maps OpenAI message types to `ChatHistoryMessage` roles ("user", "assistant", "system"). Handle unknown types with a warning log and default to "user". | +| **Acceptance Criteria** | - Maps `UserMessage` → "user"
- Maps `AssistantMessage` → "assistant"
- Maps `SystemMessage` → "system"
- Maps `ResponseOutputMessage` with role="assistant" → "assistant"
- Unknown types default to "user" with warning log
- Method is type-hinted | +| **Files to Create/Modify** | - `libraries/microsoft-agents-a365-tooling-extensions-openai/microsoft_agents_a365/tooling/extensions/openai/mcp_tool_registration_service.py` | +| **Dependencies** | Task 1.1, Task 1.2 | +| **Testing Requirements** | Unit tests in Task 3.2 | + +--- + +#### Task 2.2: Implement Content Extraction Helper + +| Attribute | Details | +|-----------|---------| +| **Title** | Implement `_extract_content()` Private Method | +| **Estimate** | 2-3 hours | +| **Description** | Implement the private `_extract_content()` method that extracts text content from OpenAI messages. Handle string content, list content (concatenate text parts), `.text` attribute, and empty/None content with warning log. | +| **Acceptance Criteria** | - String `.content` used directly
- List `.content` concatenates all text parts
- Falls back to `.text` attribute if present
- Returns empty string with warning for empty/None content
- Method is type-hinted | +| **Files to Create/Modify** | - `libraries/microsoft-agents-a365-tooling-extensions-openai/microsoft_agents_a365/tooling/extensions/openai/mcp_tool_registration_service.py` | +| **Dependencies** | Task 1.1, Task 1.2 | +| **Testing Requirements** | Unit tests in Task 3.2 | + +--- + +#### Task 2.3: Implement ID and Timestamp Extraction Helpers + +| Attribute | Details | +|-----------|---------| +| **Title** | Implement `_extract_id()` and `_extract_timestamp()` Private Methods | +| **Estimate** | 2 hours | +| **Description** | Implement helper methods to extract or generate message IDs (UUID if missing) and timestamps (current UTC if missing). | +| **Acceptance Criteria** | - `_extract_id()`: Returns existing ID if present, generates UUID if missing
- `_extract_timestamp()`: Returns existing timestamp if present, uses `datetime.now(timezone.utc)` if missing
- Debug logging for generated values
- Methods are type-hinted | +| **Files to Create/Modify** | - `libraries/microsoft-agents-a365-tooling-extensions-openai/microsoft_agents_a365/tooling/extensions/openai/mcp_tool_registration_service.py` | +| **Dependencies** | Task 1.1 | +| **Testing Requirements** | Unit tests in Task 3.2 | + +--- + +#### Task 2.4: Implement Single Message Conversion + +| Attribute | Details | +|-----------|---------| +| **Title** | Implement `_convert_single_message()` Private Method | +| **Estimate** | 2 hours | +| **Description** | Implement the private method that converts a single `TResponseInputItem` to a `ChatHistoryMessage` using the extraction helpers. | +| **Acceptance Criteria** | - Calls all extraction helpers
- Returns `ChatHistoryMessage` with role, content, id, timestamp
- Returns `None` for unconvertible messages with error log
- Method is type-hinted | +| **Files to Create/Modify** | - `libraries/microsoft-agents-a365-tooling-extensions-openai/microsoft_agents_a365/tooling/extensions/openai/mcp_tool_registration_service.py` | +| **Dependencies** | Task 2.1, Task 2.2, Task 2.3 | +| **Testing Requirements** | Unit tests in Task 3.2 | + +--- + +#### Task 2.5: Implement Batch Message Conversion + +| Attribute | Details | +|-----------|---------| +| **Title** | Implement `_convert_openai_messages_to_chat_history()` Private Method | +| **Estimate** | 1-2 hours | +| **Description** | Implement the private method that converts a list of `TResponseInputItem` messages to a list of `ChatHistoryMessage` objects. Filter out `None` results from unconvertible messages. | +| **Acceptance Criteria** | - Iterates through all messages
- Calls `_convert_single_message()` for each
- Filters out `None` results
- Returns `List[ChatHistoryMessage]`
- Info logging with count of converted messages
- Method is type-hinted | +| **Files to Create/Modify** | - `libraries/microsoft-agents-a365-tooling-extensions-openai/microsoft_agents_a365/tooling/extensions/openai/mcp_tool_registration_service.py` | +| **Dependencies** | Task 2.4 | +| **Testing Requirements** | Unit tests in Task 3.2 | + +--- + +#### Task 2.6: Implement send_chat_history_messages_async Method + +| Attribute | Details | +|-----------|---------| +| **Title** | Implement `send_chat_history_messages_async()` Public Method | +| **Estimate** | 3-4 hours | +| **Description** | Implement the public API method that validates inputs, converts OpenAI messages, and delegates to `McpToolServerConfigurationService.send_chat_history()`. | +| **Acceptance Criteria** | - Validates `turn_context` is not None (raises `ValueError`)
- Validates `messages` is not None (raises `ValueError`)
- Empty list returns `OperationResult.success()` (no-op)
- Sets default `ToolOptions` with orchestrator_name="OpenAI"
- Converts messages using `_convert_openai_messages_to_chat_history()`
- Delegates to `self.config_service.send_chat_history()`
- Returns `OperationResult`
- Catches exceptions and returns `OperationResult.failed()`
- Complete docstring with Args/Returns/Raises | +| **Files to Create/Modify** | - `libraries/microsoft-agents-a365-tooling-extensions-openai/microsoft_agents_a365/tooling/extensions/openai/mcp_tool_registration_service.py` | +| **Dependencies** | Task 2.5 | +| **Testing Requirements** | Unit tests in Task 3.3, Task 3.4 | + +--- + +#### Task 2.7: Implement send_chat_history_async Method + +| Attribute | Details | +|-----------|---------| +| **Title** | Implement `send_chat_history_async()` Public Method | +| **Estimate** | 2 hours | +| **Description** | Implement the session-based public API method that extracts messages from an OpenAI `Session` and delegates to `send_chat_history_messages_async()`. | +| **Acceptance Criteria** | - Validates `turn_context` is not None (raises `ValueError`)
- Validates `session` is not None (raises `ValueError`)
- Calls `session.get_items(limit=limit)` to extract messages
- Delegates to `send_chat_history_messages_async()`
- Returns `OperationResult`
- Catches exceptions and returns `OperationResult.failed()`
- Complete docstring with Args/Returns/Raises | +| **Files to Create/Modify** | - `libraries/microsoft-agents-a365-tooling-extensions-openai/microsoft_agents_a365/tooling/extensions/openai/mcp_tool_registration_service.py` | +| **Dependencies** | Task 2.6 | +| **Testing Requirements** | Unit tests in Task 3.3, Task 3.4 | + +--- + +### Phase 3: Testing (14-20 hours) + +#### Task 3.1: Implement Input Validation Unit Tests + +| Attribute | Details | +|-----------|---------| +| **Title** | Implement Input Validation Unit Tests (UV-01 to UV-09) | +| **Estimate** | 3-4 hours | +| **Description** | Implement unit tests for all input validation scenarios as specified in PRD section 9.2.1. | +| **Acceptance Criteria** | - UV-01: `test_send_chat_history_messages_async_validates_turn_context_none`
- UV-02: `test_send_chat_history_messages_async_validates_messages_none`
- UV-03: `test_send_chat_history_messages_async_empty_list_returns_success`
- UV-04: `test_send_chat_history_messages_async_validates_activity_none`
- UV-05: `test_send_chat_history_messages_async_validates_conversation_id`
- UV-06: `test_send_chat_history_messages_async_validates_message_id`
- UV-07: `test_send_chat_history_messages_async_validates_user_message`
- UV-08: `test_send_chat_history_async_validates_turn_context_none`
- UV-09: `test_send_chat_history_async_validates_session_none`
- All tests pass | +| **Files to Create/Modify** | - `tests/tooling/extensions/openai/test_send_chat_history_async.py` (new) | +| **Dependencies** | Task 2.6, Task 2.7, Task 1.3 | +| **Testing Requirements** | `pytest tests/tooling/extensions/openai/test_send_chat_history_async.py -v` | + +--- + +#### Task 3.2: Implement Conversion Logic Unit Tests + +| Attribute | Details | +|-----------|---------| +| **Title** | Implement Conversion Logic Unit Tests (CV-01 to CV-12) | +| **Estimate** | 4-5 hours | +| **Description** | Implement unit tests for all message conversion scenarios as specified in PRD section 9.2.2. | +| **Acceptance Criteria** | - CV-01: `test_convert_user_message_to_chat_history`
- CV-02: `test_convert_assistant_message_to_chat_history`
- CV-03: `test_convert_system_message_to_chat_history`
- CV-04: `test_convert_message_with_string_content`
- CV-05: `test_convert_message_with_list_content`
- CV-06: `test_convert_message_generates_uuid_when_id_missing`
- CV-07: `test_convert_message_uses_utc_when_timestamp_missing`
- CV-08: `test_convert_message_preserves_existing_id`
- CV-09: `test_convert_message_preserves_existing_timestamp`
- CV-10: `test_convert_unknown_message_type_defaults_to_user`
- CV-11: `test_convert_empty_content_uses_empty_string`
- CV-12: `test_convert_multiple_messages`
- All tests pass | +| **Files to Create/Modify** | - `tests/tooling/extensions/openai/test_message_conversion.py` (new) | +| **Dependencies** | Task 2.1, Task 2.2, Task 2.3, Task 2.4, Task 2.5, Task 1.3 | +| **Testing Requirements** | `pytest tests/tooling/extensions/openai/test_message_conversion.py -v` | + +--- + +#### Task 3.3: Implement Success Path Unit Tests + +| Attribute | Details | +|-----------|---------| +| **Title** | Implement Success Path Unit Tests (SP-01 to SP-07) | +| **Estimate** | 3-4 hours | +| **Description** | Implement unit tests for all success path scenarios as specified in PRD section 9.2.3. | +| **Acceptance Criteria** | - SP-01: `test_send_chat_history_messages_async_success`
- SP-02: `test_send_chat_history_messages_async_with_options`
- SP-03: `test_send_chat_history_messages_async_default_orchestrator_name`
- SP-04: `test_send_chat_history_messages_async_delegates_to_config_service`
- SP-05: `test_send_chat_history_async_success`
- SP-06: `test_send_chat_history_async_with_limit`
- SP-07: `test_send_chat_history_async_delegates_to_send_chat_history_messages`
- All tests pass | +| **Files to Create/Modify** | - `tests/tooling/extensions/openai/test_send_chat_history_async.py` | +| **Dependencies** | Task 2.6, Task 2.7, Task 1.3 | +| **Testing Requirements** | `pytest tests/tooling/extensions/openai/test_send_chat_history_async.py -v` | + +--- + +#### Task 3.4: Implement Error Handling Unit Tests + +| Attribute | Details | +|-----------|---------| +| **Title** | Implement Error Handling Unit Tests (EH-01 to EH-05) | +| **Estimate** | 2-3 hours | +| **Description** | Implement unit tests for all error handling scenarios as specified in PRD section 9.2.4. | +| **Acceptance Criteria** | - EH-01: `test_send_chat_history_messages_async_http_error`
- EH-02: `test_send_chat_history_messages_async_timeout_error`
- EH-03: `test_send_chat_history_messages_async_client_error`
- EH-04: `test_send_chat_history_messages_async_conversion_error`
- EH-05: `test_send_chat_history_async_get_items_error`
- All tests pass | +| **Files to Create/Modify** | - `tests/tooling/extensions/openai/test_send_chat_history_async.py` | +| **Dependencies** | Task 2.6, Task 2.7, Task 1.3 | +| **Testing Requirements** | `pytest tests/tooling/extensions/openai/test_send_chat_history_async.py -v` | + +--- + +#### Task 3.5: Implement Integration Tests + +| Attribute | Details | +|-----------|---------| +| **Title** | Implement Integration Tests (IT-01 to IT-03) | +| **Estimate** | 3-4 hours | +| **Description** | Implement integration tests that verify end-to-end flow with mocked HTTP as specified in PRD section 9.3. | +| **Acceptance Criteria** | - IT-01: `test_send_chat_history_async_end_to_end_success`
- IT-02: `test_send_chat_history_async_end_to_end_server_error`
- IT-03: `test_send_chat_history_async_payload_format`
- Tests use real OpenAI SDK types where possible
- Tests verify full conversion and delegation chain
- All tests pass | +| **Files to Create/Modify** | - `tests/tooling/extensions/openai/test_integration.py` (new) | +| **Dependencies** | Task 2.6, Task 2.7, Task 1.3 | +| **Testing Requirements** | `pytest tests/tooling/extensions/openai/test_integration.py -v` | + +--- + +#### Task 3.6: Verify Test Coverage and Add Performance Benchmarks + +| Attribute | Details | +|-----------|---------| +| **Title** | Verify Test Coverage Targets and Add Performance Benchmarks | +| **Estimate** | 2-3 hours | +| **Description** | Run coverage analysis to ensure ≥95% line coverage and ≥90% branch coverage. Add performance benchmark tests for CI regression detection. | +| **Acceptance Criteria** | - Line coverage ≥95%
- Branch coverage ≥90%
- Performance benchmark: <10ms for 100 messages conversion
- Performance benchmark: <1MB memory for 1000 messages
- Coverage report generated | +| **Files to Create/Modify** | - `tests/tooling/extensions/openai/test_performance.py` (new)
- `pyproject.toml` (add coverage config if needed) | +| **Dependencies** | Task 3.1, Task 3.2, Task 3.3, Task 3.4, Task 3.5 | +| **Testing Requirements** | `pytest tests/tooling/extensions/openai/ --cov --cov-report=html` | + +--- + +### Phase 4: Documentation & Finalization (8-12 hours) + +#### Task 4.1: Add Method Docstrings and Type Hints + +| Attribute | Details | +|-----------|---------| +| **Title** | Complete Method Docstrings and Type Hints | +| **Estimate** | 2-3 hours | +| **Description** | Ensure all public and private methods have complete docstrings following project conventions and full type annotations. | +| **Acceptance Criteria** | - All public methods have docstrings with Args/Returns/Raises
- All private methods have docstrings
- All parameters and return types are type-hinted
- Type checker (`mypy` or `pyright`) passes
- Follows existing project docstring style | +| **Files to Create/Modify** | - `libraries/microsoft-agents-a365-tooling-extensions-openai/microsoft_agents_a365/tooling/extensions/openai/mcp_tool_registration_service.py` | +| **Dependencies** | Task 2.6, Task 2.7 | +| **Testing Requirements** | `mypy` or `pyright` check | + +--- + +#### Task 4.2: Update Package README with Usage Examples + +| Attribute | Details | +|-----------|---------| +| **Title** | Update README.md with Usage Examples | +| **Estimate** | 2-3 hours | +| **Description** | Update the OpenAI extension's README.md with documentation and usage examples for both new methods. | +| **Acceptance Criteria** | - `send_chat_history_async()` documented with code example
- `send_chat_history_messages_async()` documented with code example
- Error handling best practices documented
- Prerequisites and installation instructions updated
- API reference section added | +| **Files to Create/Modify** | - `libraries/microsoft-agents-a365-tooling-extensions-openai/README.md` | +| **Dependencies** | Task 2.6, Task 2.7 | +| **Testing Requirements** | Code examples are syntactically correct | + +--- + +#### Task 4.3: Update Package CHANGELOG + +| Attribute | Details | +|-----------|---------| +| **Title** | Update CHANGELOG.md | +| **Estimate** | 1 hour | +| **Description** | Add changelog entry for the new API methods following the project's changelog format. | +| **Acceptance Criteria** | - New version entry added
- `send_chat_history_async()` listed as new feature
- `send_chat_history_messages_async()` listed as new feature
- Breaking changes section confirms none
- Follows existing changelog format | +| **Files to Create/Modify** | - `libraries/microsoft-agents-a365-tooling-extensions-openai/CHANGELOG.md` (create if not exists) | +| **Dependencies** | Task 2.6, Task 2.7 | +| **Testing Requirements** | N/A | + +--- + +#### Task 4.4: Code Review and Linting + +| Attribute | Details | +|-----------|---------| +| **Title** | Final Code Review and Linting Compliance | +| **Estimate** | 2-3 hours | +| **Description** | Run all linting tools, address any issues, and perform final code review to ensure code quality and consistency with existing patterns. | +| **Acceptance Criteria** | - `ruff check` passes with no errors
- `ruff format` applied
- No new security vulnerabilities
- Code follows existing project patterns
- Self-review completed
- Ready for PR | +| **Files to Create/Modify** | - All modified files | +| **Dependencies** | All previous tasks | +| **Testing Requirements** | `ruff check .` and `ruff format --check .` | + +--- + +#### Task 4.5: Final Integration Verification + +| Attribute | Details | +|-----------|---------| +| **Title** | Final Integration Verification | +| **Estimate** | 1-2 hours | +| **Description** | Perform final end-to-end verification that all tests pass, documentation is complete, and the implementation meets all PRD requirements. | +| **Acceptance Criteria** | - All 33+ unit tests pass
- All 3 integration tests pass
- Coverage targets met
- Documentation complete
- PR checklist completed
- All functional requirements (FR-01 to FR-14) verified
- All acceptance criteria (AC-01 to AC-16) verified | +| **Files to Create/Modify** | N/A | +| **Dependencies** | All previous tasks | +| **Testing Requirements** | Full test suite run | + +--- + +## 4. Task Dependencies + +### 4.1 Dependency Diagram + +``` + ┌──────────────────────┐ + │ Task 1.1 │ + │ Analyze OpenAI │ + │ SDK Types │ + └──────────────────────┘ + │ + ┌───────────────┼───────────────┐ + │ │ │ + ▼ ▼ ▼ + ┌──────────────┐ ┌──────────────┐ ┌──────────────┐ + │ Task 1.2 │ │ Task 1.3 │ │ Task 2.3 │ + │ Add Deps │ │ Test Setup │ │ ID/Time │ + └──────────────┘ └──────────────┘ │ Helpers │ + │ │ └──────────────┘ + │ │ │ + ▼ │ │ + ┌──────────────┐ │ │ + │ Task 2.1 │◄────────┘ │ + │ Role │ │ + │ Extraction │ │ + └──────────────┘ │ + │ │ + ▼ │ + ┌──────────────┐ │ + │ Task 2.2 │ │ + │ Content │ │ + │ Extraction │ │ + └──────────────┘ │ + │ │ + └────────────────┬───────────────┘ + │ + ▼ + ┌──────────────────────┐ + │ Task 2.4 │ + │ Single Message │ + │ Conversion │ + └──────────────────────┘ + │ + ▼ + ┌──────────────────────┐ + │ Task 2.5 │ + │ Batch Conversion │ + └──────────────────────┘ + │ + ▼ + ┌──────────────────────┐ + │ Task 2.6 │ + │ send_chat_history │ + │ _messages_async │ + └──────────────────────┘ + │ + ▼ + ┌──────────────────────┐ + │ Task 2.7 │ + │ send_chat_history │ + │ _async │ + └──────────────────────┘ + │ + ┌───────────────┬───────┴───────┬───────────────┐ + │ │ │ │ + ▼ ▼ ▼ ▼ +┌──────────┐ ┌──────────┐ ┌──────────┐ ┌──────────┐ +│Task 3.1 │ │Task 3.2 │ │Task 3.3 │ │Task 3.4 │ +│Input │ │Conversion│ │Success │ │Error │ +│Validation│ │Tests │ │Path Tests│ │Handling │ +│Tests │ │ │ │ │ │Tests │ +└──────────┘ └──────────┘ └──────────┘ └──────────┘ + │ │ │ │ + └───────────────┴───────┬───────┴───────────────┘ + │ + ▼ + ┌──────────────────────┐ + │ Task 3.5 │ + │ Integration Tests │ + └──────────────────────┘ + │ + ▼ + ┌──────────────────────┐ + │ Task 3.6 │ + │ Coverage & │ + │ Performance │ + └──────────────────────┘ + │ + ┌───────────────┬───────┴───────┬───────────────┐ + │ │ │ │ + ▼ ▼ ▼ ▼ +┌──────────┐ ┌──────────┐ ┌──────────┐ ┌──────────┐ +│Task 4.1 │ │Task 4.2 │ │Task 4.3 │ │Task 4.4 │ +│Docstrings│ │README │ │CHANGELOG │ │Linting │ +└──────────┘ └──────────┘ └──────────┘ └──────────┘ + │ │ │ │ + └───────────────┴───────┬───────┴───────────────┘ + │ + ▼ + ┌──────────────────────┐ + │ Task 4.5 │ + │ Final Verification │ + └──────────────────────┘ +``` + +### 4.2 Critical Path + +The critical path for this implementation is: + +``` +Task 1.1 → Task 1.2 → Task 2.1 → Task 2.2 → Task 2.4 → Task 2.5 → Task 2.6 → Task 2.7 → Task 3.3 → Task 3.6 → Task 4.5 +``` + +**Estimated Critical Path Duration**: 24-32 hours + +### 4.3 Parallelization Opportunities + +The following tasks can be done in parallel: + +| Parallel Group | Tasks | Notes | +|----------------|-------|-------| +| After Task 1.1 | Task 1.2, Task 1.3, Task 2.3 | Infrastructure setup | +| After Task 2.7 | Task 3.1, Task 3.2, Task 3.3, Task 3.4 | Unit test implementation | +| After Task 3.6 | Task 4.1, Task 4.2, Task 4.3, Task 4.4 | Documentation and finalization | + +--- + +## 5. Testing Strategy Overview + +### 5.1 Test Categories Summary + +| Category | Test Count | Coverage Target | Priority | +|----------|------------|-----------------|----------| +| Input Validation (UV) | 9 tests | 100% validation paths | P0 | +| Conversion Logic (CV) | 12 tests | 100% conversion scenarios | P0 | +| Success Path (SP) | 7 tests | All happy paths | P0 | +| Error Handling (EH) | 5 tests | All error conditions | P0 | +| Integration (IT) | 3 tests | End-to-end flows | P0 | +| Performance | 2+ tests | Regression detection | P1 | +| **Total** | **38+ tests** | **≥95% lines, ≥90% branches** | | + +### 5.2 Test File Structure + +``` +tests/ +└── tooling/ + └── extensions/ + └── openai/ + ├── __init__.py + ├── conftest.py # Shared fixtures + ├── test_send_chat_history_async.py # UV, SP, EH tests (both methods) + ├── test_message_conversion.py # CV tests + ├── test_integration.py # IT tests + └── test_performance.py # Performance benchmarks +``` + +### 5.3 Mocking Strategy + +| Component | Mock Approach | +|-----------|---------------| +| OpenAI Message Types | Use `Mock` objects with role/content attributes | +| OpenAI Session | Use `Mock` with `get_items()` returning mock messages | +| TurnContext | Use `Mock(spec=TurnContext)` for interface compliance | +| McpToolServerConfigurationService | Use `AsyncMock` for `send_chat_history()` | +| HTTP Layer | Mock `aiohttp.ClientSession` for integration tests | + +### 5.4 Coverage Requirements + +```python +# pyproject.toml coverage configuration +[tool.pytest.ini_options] +addopts = "--cov=microsoft_agents_a365.tooling.extensions.openai --cov-report=html --cov-fail-under=95" +``` + +--- + +## 6. Risks and Considerations + +### 6.1 Technical Risks + +| Risk | Likelihood | Impact | Mitigation | +|------|------------|--------|------------| +| **OpenAI SDK Breaking Changes** | Medium | High | Pin version, use duck typing, create adapter layer | +| **Type Union Complexity** | Medium | Medium | Comprehensive type checking, runtime isinstance checks | +| **Performance Regression** | Low | Medium | Add benchmark tests in CI | +| **Thread Safety Issues** | Low | High | Stateless design, no shared mutable state | + +### 6.2 Schedule Risks + +| Risk | Likelihood | Impact | Mitigation | +|------|------------|--------|------------| +| **OpenAI SDK Research Takes Longer** | Medium | Medium | Time-box research to 4 hours max | +| **Test Coverage Gap Discovery** | Low | Medium | Continuous coverage monitoring | +| **Code Review Delays** | Low | Low | Early reviewer engagement | + +### 6.3 Integration Risks + +| Risk | Likelihood | Impact | Mitigation | +|------|------------|--------|------------| +| **Core Service API Changes** | Low | High | No planned changes to core service | +| **Dependency Version Conflicts** | Low | Medium | Use flexible version constraints | + +### 6.4 Open Questions to Resolve + +| Question | Resolution Needed By | Impact if Unresolved | +|----------|---------------------|---------------------| +| Full enumeration of `TResponseInputItem` types | Task 1.1 | May miss some message types | +| Session.get_items() async vs sync | Task 1.1 | Implementation approach | +| Content validation (empty string allowed?) | Task 2.2 | May need to relax ChatHistoryMessage validator | + +### 6.5 Assumptions + +1. The `openai-agents` package is available and stable +2. `Session.get_items()` method exists and returns `List[TResponseInputItem]` +3. The core `McpToolServerConfigurationService.send_chat_history()` remains unchanged +4. Empty content strings should be handled gracefully (may require relaxing `ChatHistoryMessage.content` validator) + +--- + +## Appendix: Task Checklist + +### Phase 1: Foundation & Infrastructure +- [ ] Task 1.1: Analyze OpenAI SDK Types and Create Type Stubs +- [ ] Task 1.2: Add openai-agents Dependency +- [ ] Task 1.3: Create Test Directory Structure and Fixtures + +### Phase 2: Core Implementation +- [ ] Task 2.1: Implement `_extract_role()` Private Method +- [ ] Task 2.2: Implement `_extract_content()` Private Method +- [ ] Task 2.3: Implement `_extract_id()` and `_extract_timestamp()` Private Methods +- [ ] Task 2.4: Implement `_convert_single_message()` Private Method +- [ ] Task 2.5: Implement `_convert_openai_messages_to_chat_history()` Private Method +- [ ] Task 2.6: Implement `send_chat_history_messages_async()` Public Method +- [ ] Task 2.7: Implement `send_chat_history_async()` Public Method + +### Phase 3: Testing +- [ ] Task 3.1: Implement Input Validation Unit Tests (UV-01 to UV-09) +- [ ] Task 3.2: Implement Conversion Logic Unit Tests (CV-01 to CV-12) +- [ ] Task 3.3: Implement Success Path Unit Tests (SP-01 to SP-07) +- [ ] Task 3.4: Implement Error Handling Unit Tests (EH-01 to EH-05) +- [ ] Task 3.5: Implement Integration Tests (IT-01 to IT-03) +- [ ] Task 3.6: Verify Test Coverage and Add Performance Benchmarks + +### Phase 4: Documentation & Finalization +- [ ] Task 4.1: Complete Method Docstrings and Type Hints +- [ ] Task 4.2: Update README.md with Usage Examples +- [ ] Task 4.3: Update CHANGELOG.md +- [ ] Task 4.4: Final Code Review and Linting Compliance +- [ ] Task 4.5: Final Integration Verification + +--- + +## Document History + +| Version | Date | Author | Changes | +|---------|------|--------|---------| +| 1.0 | January 21, 2026 | PRD Task Generator Agent | Initial task breakdown | From 05c1608f3cfc20e1792a6c1cda34df40bc14ca98 Mon Sep 17 00:00:00 2001 From: Johan Broberg Date: Wed, 21 Jan 2026 21:14:05 -0800 Subject: [PATCH 04/11] Implement API for OpenAI chat history --- CLAUDE.md | 35 +- docs/prd/openai-send-chat-history-api.md | 46 ++ .../tooling/extensions/openai/__init__.py | 15 +- .../openai/mcp_tool_registration_service.py | 449 +++++++++++++++- .../tooling/extensions/__init__.py | 28 + pyproject.toml | 2 +- tests/tooling/extensions/__init__.py | 4 + tests/tooling/extensions/openai/__init__.py | 4 + tests/tooling/extensions/openai/conftest.py | 295 +++++++++++ .../extensions/openai/test_integration.py | 345 +++++++++++++ .../openai/test_message_conversion.py | 463 +++++++++++++++++ .../openai/test_send_chat_history_async.py | 483 ++++++++++++++++++ 12 files changed, 2157 insertions(+), 12 deletions(-) create mode 100644 libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/extensions/__init__.py create mode 100644 tests/tooling/extensions/__init__.py create mode 100644 tests/tooling/extensions/openai/__init__.py create mode 100644 tests/tooling/extensions/openai/conftest.py create mode 100644 tests/tooling/extensions/openai/test_integration.py create mode 100644 tests/tooling/extensions/openai/test_message_conversion.py create mode 100644 tests/tooling/extensions/openai/test_send_chat_history_async.py diff --git a/CLAUDE.md b/CLAUDE.md index e0772348..be4da0d5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -168,12 +168,45 @@ Place it before imports with one blank line after. ### Python Conventions -- Type hints preferred (Pydantic models heavily used) +- Type hints required on all function parameters and return types - Async/await patterns for I/O operations - 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 +### Type Hints - NEVER Use `Any` + +**CRITICAL: Never use `typing.Any` in this codebase.** Using `Any` defeats the purpose of type checking and can hide bugs. Instead: + +1. **Use actual types from external SDKs** - When integrating with external libraries (OpenAI, LangChain, etc.), import and use their actual types: + ```python + from agents.memory import Session + from agents.items import TResponseInputItem + + async def send_chat_history_async(self, session: Session) -> OperationResult: + ... + ``` + +2. **Use `Union` for known possible types**: + ```python + from typing import Union + MessageType = Union[UserMessage, AssistantMessage, SystemMessage, Dict[str, object]] + ``` + +3. **Use `object` for truly unknown types** that you only pass through: + ```python + def log_item(item: object) -> None: ... + ``` + +4. **Use `Protocol` only as a last resort** - If external types cannot be found or imported, define a Protocol. However, **confirm with the developer first** before proceeding with this approach, as it may indicate a missing dependency or incorrect understanding of the external API. + +**Why this matters:** +- `Any` disables all type checking for that variable +- Bugs that type checkers would catch go unnoticed +- Code readability suffers - developers don't know what types to expect +- Using actual SDK types provides better IDE support and ensures compatibility +- This applies to both production code AND test files + ## CI/CD The `.github/workflows/ci.yml` pipeline: diff --git a/docs/prd/openai-send-chat-history-api.md b/docs/prd/openai-send-chat-history-api.md index 7e7c42b7..2ca0170d 100644 --- a/docs/prd/openai-send-chat-history-api.md +++ b/docs/prd/openai-send-chat-history-api.md @@ -226,6 +226,52 @@ The API SHALL extract text content following this priority: | Type hints | Complete type annotations (PEP 484) | | Pydantic version | ≥2.0 (for model validation) | +### 5.1.1 Type Hints - NEVER Use `Any` + +**CRITICAL**: The use of `typing.Any` is **strictly forbidden** in this codebase. Using `Any` defeats the purpose of type checking and can hide bugs. + +**Required alternatives (in order of preference):** + +| Instead of `Any` | Use | +|------------------|-----| +| External SDK types | Import and use actual types from the SDK (e.g., `Session`, `TResponseInputItem`) | +| Multiple known types | `Union[Type1, Type2, ...]` | +| Pass-through data | `object` | +| Dictionary values | `Dict[str, object]` or specific types | +| Unknown external types (last resort) | `Protocol` - but confirm with developer first | + +**Preferred approach - Use actual SDK types:** + +```python +from agents.memory import Session +from agents.items import TResponseInputItem + +async def send_chat_history_async( + self, + turn_context: TurnContext, + session: Session, # Use actual SDK type +) -> OperationResult: + ... + +async def send_chat_history_messages_async( + self, + turn_context: TurnContext, + messages: List[TResponseInputItem], # Use actual SDK type +) -> OperationResult: + ... +``` + +**Why actual SDK types are preferred:** +- Better IDE support (autocomplete, type checking) +- Ensures compatibility with the external SDK +- Less maintenance burden (no custom protocols to keep in sync) +- Clearer intent for developers reading the code + +**When to use Protocol (last resort only):** +If external types cannot be found or imported, a `Protocol` may be defined. However, this should be rare and requires confirmation with the developer before proceeding, as it may indicate a missing dependency or incorrect understanding of the external API. + +This requirement applies to both production code AND test files. + ### 5.2 OpenAI SDK Compatibility | Requirement | Specification | diff --git a/libraries/microsoft-agents-a365-tooling-extensions-openai/microsoft_agents_a365/tooling/extensions/openai/__init__.py b/libraries/microsoft-agents-a365-tooling-extensions-openai/microsoft_agents_a365/tooling/extensions/openai/__init__.py index c2d53bd0..6e77b7df 100644 --- a/libraries/microsoft-agents-a365-tooling-extensions-openai/microsoft_agents_a365/tooling/extensions/openai/__init__.py +++ b/libraries/microsoft-agents-a365-tooling-extensions-openai/microsoft_agents_a365/tooling/extensions/openai/__init__.py @@ -2,10 +2,21 @@ # Licensed under the MIT License. """ -OpenAI extensions for Microsoft Agent 365 Tooling SDK +OpenAI extensions for Microsoft Agent 365 Tooling SDK. Tooling and utilities specifically for OpenAI framework integration. -Provides OpenAI-specific helper utilities. +Provides OpenAI-specific helper utilities including: +- McpToolRegistrationService: Service for MCP tool registration and chat history management + +For type hints, use the types directly from the OpenAI Agents SDK: +- agents.memory.Session: Protocol for session objects +- agents.items.TResponseInputItem: Type for input message items """ +from .mcp_tool_registration_service import McpToolRegistrationService + __version__ = "1.0.0" + +__all__ = [ + "McpToolRegistrationService", +] diff --git a/libraries/microsoft-agents-a365-tooling-extensions-openai/microsoft_agents_a365/tooling/extensions/openai/mcp_tool_registration_service.py b/libraries/microsoft-agents-a365-tooling-extensions-openai/microsoft_agents_a365/tooling/extensions/openai/mcp_tool_registration_service.py index 1d6e6cc9..17b444d4 100644 --- a/libraries/microsoft-agents-a365-tooling-extensions-openai/microsoft_agents_a365/tooling/extensions/openai/mcp_tool_registration_service.py +++ b/libraries/microsoft-agents-a365-tooling-extensions-openai/microsoft_agents_a365/tooling/extensions/openai/mcp_tool_registration_service.py @@ -1,23 +1,34 @@ -# Copyright (c) Microsoft. All rights reserved. +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. -from typing import Dict, Optional -from dataclasses import dataclass -import logging +""" +MCP Tool Registration Service for OpenAI. -from agents import Agent +This module provides OpenAI-specific extensions for MCP tool registration, +including methods to send chat history from OpenAI Sessions and message lists. +""" -from microsoft_agents.hosting.core import Authorization, TurnContext +import logging +import uuid +from dataclasses import dataclass +from datetime import datetime, timezone +from typing import Dict, List, Optional +from agents import Agent +from agents.items import TResponseInputItem from agents.mcp import ( MCPServerStreamableHttp, MCPServerStreamableHttpParams, ) +from agents.memory import Session +from microsoft_agents.hosting.core import Authorization, TurnContext + +from microsoft_agents_a365.runtime import OperationError, 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, @@ -233,3 +244,425 @@ async def cleanup_all_servers(self): if hasattr(self, "_connected_servers"): await self._cleanup_servers(self._connected_servers) self._connected_servers = [] + + # -------------------------------------------------------------------------- + # SEND CHAT HISTORY - OpenAI-specific implementations + # -------------------------------------------------------------------------- + + async def send_chat_history_async( + self, + turn_context: TurnContext, + session: Session, + limit: Optional[int] = None, + options: Optional[ToolOptions] = None, + ) -> OperationResult: + """ + Extract chat history from an OpenAI Session and send it to the MCP platform. + + This method extracts messages from an OpenAI Session object using get_items() + and sends them to the MCP platform for real-time threat protection. + + Args: + turn_context: TurnContext from the Agents SDK containing conversation info. + Must have a valid activity with conversation.id, activity.id, + and activity.text. + session: OpenAI Session instance to extract messages from. Must support + the get_items() method which returns a list of TResponseInputItem. + limit: Optional maximum number of items to retrieve from session. + If None, retrieves all items. + options: Optional ToolOptions for customization. If not provided, + uses default options with orchestrator_name="OpenAI". + + Returns: + OperationResult indicating success or failure. On success, returns + OperationResult.success(). On failure, returns OperationResult.failed() + with error details. + + Raises: + ValueError: If turn_context is None or session is None. + + Example: + >>> from agents import Agent, Runner + >>> from microsoft_agents_a365.tooling.extensions.openai import ( + ... McpToolRegistrationService + ... ) + >>> + >>> service = McpToolRegistrationService() + >>> agent = Agent(name="my-agent", model="gpt-4") + >>> + >>> # In your agent handler: + >>> async with Runner.run(agent, messages) as result: + ... session = result.session + ... op_result = await service.send_chat_history_async( + ... turn_context, session + ... ) + ... if op_result.succeeded: + ... print("Chat history sent successfully") + """ + # Validate inputs + if turn_context is None: + raise ValueError("turn_context cannot be None") + if session is None: + raise ValueError("session cannot be None") + + try: + # Extract messages from session + self._logger.info("Extracting messages from OpenAI session") + if limit is not None: + messages = session.get_items(limit=limit) + else: + messages = session.get_items() + + self._logger.debug(f"Retrieved {len(messages)} items from session") + + # Delegate to the list-based method + return await self.send_chat_history_messages_async( + turn_context=turn_context, + messages=messages, + options=options, + ) + except ValueError: + # Re-raise validation errors + raise + except Exception as ex: + self._logger.error(f"Failed to send chat history from session: {ex}") + return OperationResult.failed(OperationError(ex)) + + async def send_chat_history_messages_async( + self, + turn_context: TurnContext, + messages: List[TResponseInputItem], + options: Optional[ToolOptions] = None, + ) -> OperationResult: + """ + Send OpenAI chat history messages to the MCP platform for threat protection. + + This method accepts a list of OpenAI TResponseInputItem messages, converts + them to ChatHistoryMessage format, and sends them to the MCP platform. + + Args: + turn_context: TurnContext from the Agents SDK containing conversation info. + Must have a valid activity with conversation.id, activity.id, + and activity.text. + messages: List of OpenAI TResponseInputItem messages to send. Supports + UserMessage, AssistantMessage, SystemMessage, and other OpenAI + message types. + options: Optional ToolOptions for customization. If not provided, + uses default options with orchestrator_name="OpenAI". + + Returns: + OperationResult indicating success or failure. On success, returns + OperationResult.success(). On failure, returns OperationResult.failed() + with error details. + + Raises: + ValueError: If turn_context is None or messages is None. + + Example: + >>> from microsoft_agents_a365.tooling.extensions.openai import ( + ... McpToolRegistrationService + ... ) + >>> + >>> service = McpToolRegistrationService() + >>> messages = [ + ... {"role": "user", "content": "Hello"}, + ... {"role": "assistant", "content": "Hi there!"}, + ... ] + >>> + >>> result = await service.send_chat_history_messages_async( + ... turn_context, messages + ... ) + >>> if result.succeeded: + ... print("Chat history sent successfully") + """ + # Validate inputs + if turn_context is None: + raise ValueError("turn_context cannot be None") + if messages is None: + raise ValueError("messages cannot be None") + + # Handle empty list as no-op + if len(messages) == 0: + self._logger.info("Empty message list provided, returning success") + return OperationResult.success() + + self._logger.info(f"Sending {len(messages)} OpenAI messages as chat history") + + # Set default options + if options is None: + options = ToolOptions(orchestrator_name=self._orchestrator_name) + elif options.orchestrator_name is None: + options.orchestrator_name = self._orchestrator_name + + try: + # Convert OpenAI messages to ChatHistoryMessage format + chat_history_messages = self._convert_openai_messages_to_chat_history(messages) + + if len(chat_history_messages) == 0: + self._logger.warning("No messages could be converted to chat history format") + return OperationResult.success() + + self._logger.debug( + f"Converted {len(chat_history_messages)} messages to ChatHistoryMessage format" + ) + + # Delegate to core service + return await self.config_service.send_chat_history( + turn_context=turn_context, + chat_history_messages=chat_history_messages, + options=options, + ) + except ValueError: + # Re-raise validation errors from the core service + raise + except Exception as ex: + self._logger.error(f"Failed to send chat history messages: {ex}") + return OperationResult.failed(OperationError(ex)) + + # -------------------------------------------------------------------------- + # PRIVATE HELPER METHODS - Message Conversion + # -------------------------------------------------------------------------- + + def _convert_openai_messages_to_chat_history( + self, messages: List[TResponseInputItem] + ) -> List[ChatHistoryMessage]: + """ + Convert a list of OpenAI messages to ChatHistoryMessage format. + + Args: + messages: List of OpenAI TResponseInputItem messages. + + Returns: + List of ChatHistoryMessage objects. Messages that cannot be converted + are filtered out with a warning log. + """ + chat_history_messages: List[ChatHistoryMessage] = [] + + for idx, message in enumerate(messages): + converted = self._convert_single_message(message, idx) + if converted is not None: + chat_history_messages.append(converted) + + self._logger.info( + f"Converted {len(chat_history_messages)} of {len(messages)} messages " + "to ChatHistoryMessage format" + ) + return chat_history_messages + + def _convert_single_message( + self, message: TResponseInputItem, index: int = 0 + ) -> Optional[ChatHistoryMessage]: + """ + Convert a single OpenAI message to ChatHistoryMessage format. + + Args: + message: Single OpenAI TResponseInputItem message. + index: Index of the message in the list (for logging). + + Returns: + ChatHistoryMessage object or None if conversion fails. + """ + try: + role = self._extract_role(message) + content = self._extract_content(message) + msg_id = self._extract_id(message) + timestamp = self._extract_timestamp(message) + + self._logger.debug( + f"Converting message {index}: role={role}, " + f"has_id={msg_id is not None}, has_timestamp={timestamp is not None}" + ) + + # Skip messages with empty content after extraction + # The ChatHistoryMessage validator requires non-empty content + if not content or not content.strip(): + self._logger.warning(f"Message {index} has empty content, skipping") + return None + + return ChatHistoryMessage( + id=msg_id, + role=role, + content=content, + timestamp=timestamp, + ) + except Exception as ex: + self._logger.error(f"Failed to convert message {index}: {ex}") + return None + + def _extract_role(self, message: TResponseInputItem) -> str: + """ + Extract the role from an OpenAI message. + + Role mapping: + - UserMessage or role="user" -> "user" + - AssistantMessage or role="assistant" -> "assistant" + - SystemMessage or role="system" -> "system" + - ResponseOutputMessage with role="assistant" -> "assistant" + - Unknown types -> "user" (default fallback with warning) + + Args: + message: OpenAI message object. + + Returns: + Role string: "user", "assistant", or "system". + """ + # Check for role attribute directly + if hasattr(message, "role"): + role = message.role + if role in ("user", "assistant", "system"): + return role + + # Check message type by class name + type_name = type(message).__name__ + + if "UserMessage" in type_name or "user" in type_name.lower(): + return "user" + elif "AssistantMessage" in type_name or "assistant" in type_name.lower(): + return "assistant" + elif "SystemMessage" in type_name or "system" in type_name.lower(): + return "system" + elif "ResponseOutputMessage" in type_name: + # ResponseOutputMessage typically has role attribute + if hasattr(message, "role") and message.role == "assistant": + return "assistant" + return "assistant" # Default for response output + + # For dict-like objects + if isinstance(message, dict): + role = message.get("role", "") + if role in ("user", "assistant", "system"): + return role + + # Default fallback with warning + self._logger.warning(f"Unknown message type {type_name}, defaulting to 'user' role") + return "user" + + def _extract_content(self, message: TResponseInputItem) -> str: + """ + Extract text content from an OpenAI message. + + Content extraction priority: + 1. If message has .content as string -> use directly + 2. If message has .content as list -> concatenate all text parts + 3. If message has .text attribute -> use directly + 4. If content is empty/None -> return empty string with warning + + Args: + message: OpenAI message object. + + Returns: + Extracted text content as string. + """ + content = "" + + # Try .content attribute first + if hasattr(message, "content"): + raw_content = message.content + + if isinstance(raw_content, str): + content = raw_content + elif isinstance(raw_content, list): + # Concatenate text parts from content list + text_parts = [] + for part in raw_content: + if isinstance(part, str): + text_parts.append(part) + elif hasattr(part, "text"): + text_parts.append(str(part.text)) + elif isinstance(part, dict): + if "text" in part: + text_parts.append(str(part["text"])) + elif part.get("type") == "text" and "text" in part: + text_parts.append(str(part["text"])) + content = " ".join(text_parts) + + # Try .text attribute as fallback + if not content and hasattr(message, "text"): + content = str(message.text) if message.text else "" + + # Try dict-like access + if not content and isinstance(message, dict): + content = message.get("content", "") or message.get("text", "") or "" + if isinstance(content, list): + text_parts = [] + for part in content: + if isinstance(part, str): + text_parts.append(part) + elif isinstance(part, dict) and "text" in part: + text_parts.append(str(part["text"])) + content = " ".join(text_parts) + + if not content: + self._logger.warning("Message has empty content, using empty string") + + return content + + def _extract_id(self, message: TResponseInputItem) -> str: + """ + Extract or generate a unique ID for the message. + + If the message has an existing ID, it is preserved. Otherwise, + a new UUID is generated. + + Args: + message: OpenAI message object. + + Returns: + Message ID as string. + """ + # Try to get existing ID + existing_id = None + + if hasattr(message, "id") and message.id: + existing_id = str(message.id) + elif isinstance(message, dict) and message.get("id"): + existing_id = str(message["id"]) + + if existing_id: + return existing_id + + # Generate new UUID + generated_id = str(uuid.uuid4()) + self._logger.debug(f"Generated UUID {generated_id} for message without ID") + return generated_id + + def _extract_timestamp(self, message: TResponseInputItem) -> datetime: + """ + Extract or generate a timestamp for the message. + + If the message has an existing timestamp, it is preserved. Otherwise, + the current UTC time is used. + + Args: + message: OpenAI message object. + + Returns: + Timestamp as datetime object. + """ + # Try to get existing timestamp + existing_timestamp = None + + if hasattr(message, "timestamp") and message.timestamp: + existing_timestamp = message.timestamp + elif hasattr(message, "created_at") and message.created_at: + existing_timestamp = message.created_at + elif isinstance(message, dict): + existing_timestamp = message.get("timestamp") or message.get("created_at") + + if existing_timestamp: + # Convert to datetime if needed + if isinstance(existing_timestamp, datetime): + return existing_timestamp + elif isinstance(existing_timestamp, (int, float)): + # Unix timestamp + return datetime.fromtimestamp(existing_timestamp, tz=timezone.utc) + elif isinstance(existing_timestamp, str): + # Try ISO format parsing + try: + return datetime.fromisoformat(existing_timestamp.replace("Z", "+00:00")) + except ValueError: + pass + + # Use current UTC time + self._logger.debug("Using current UTC time for message without timestamp") + return datetime.now(timezone.utc) diff --git a/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/extensions/__init__.py b/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/extensions/__init__.py new file mode 100644 index 00000000..7108f9ae --- /dev/null +++ b/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/extensions/__init__.py @@ -0,0 +1,28 @@ +# Copyright (c) Microsoft. All rights reserved. + +""" +Microsoft Agent 365 Tooling Extensions. + +This is a namespace package that allows extension packages to contribute +their modules under the microsoft_agents_a365.tooling.extensions namespace. +""" + +import sys +from pkgutil import extend_path + +# First, try standard pkgutil-style namespace extension +__path__ = extend_path(__path__, __name__) + +# For editable installs with custom finders, we need to manually discover +# extension paths by checking meta_path finders +for finder in sys.meta_path: + # Check if this is an editable finder with namespace support + if hasattr(finder, "find_spec"): + try: + spec = finder.find_spec(__name__, None) + if spec is not None and spec.submodule_search_locations: + for path in spec.submodule_search_locations: + if path not in __path__ and not path.endswith(".__path_hook__"): + __path__.append(path) + except (ImportError, TypeError): + pass diff --git a/pyproject.toml b/pyproject.toml index 4799b21e..294b9239 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -119,7 +119,7 @@ min-file-size = 1 [tool.ruff.lint.per-file-ignores] # Tests can use magic values, assertions, and imports -"tests/*" = ["PLR2004", "S101", "TID252"] +"tests/*" = ["PLR2004", "S101", "TID252", "B903"] "samples/*" = ["B903"] [tool.ruff.format] diff --git a/tests/tooling/extensions/__init__.py b/tests/tooling/extensions/__init__.py new file mode 100644 index 00000000..dc3584eb --- /dev/null +++ b/tests/tooling/extensions/__init__.py @@ -0,0 +1,4 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +"""Tests for tooling extensions.""" diff --git a/tests/tooling/extensions/openai/__init__.py b/tests/tooling/extensions/openai/__init__.py new file mode 100644 index 00000000..1793ec2c --- /dev/null +++ b/tests/tooling/extensions/openai/__init__.py @@ -0,0 +1,4 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +"""Tests for OpenAI tooling extensions.""" diff --git a/tests/tooling/extensions/openai/conftest.py b/tests/tooling/extensions/openai/conftest.py new file mode 100644 index 00000000..e21ce32a --- /dev/null +++ b/tests/tooling/extensions/openai/conftest.py @@ -0,0 +1,295 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +"""Shared pytest fixtures for OpenAI extension tests.""" + +from datetime import UTC, datetime +from typing import Union +from unittest.mock import Mock + +import pytest + +# -------------------------------------------------------------------------- +# TYPE DEFINITIONS +# -------------------------------------------------------------------------- + +# Content can be string, list of content parts, or None (mimics OpenAI SDK) +MessageContent = Union[str, list[object], None] + + +# -------------------------------------------------------------------------- +# MOCK OPENAI MESSAGE CLASSES +# -------------------------------------------------------------------------- + + +class MockUserMessage: + """Mock OpenAI UserMessage for testing.""" + + def __init__( + self, + content: MessageContent = "Hello", + id: str | None = None, + timestamp: datetime | None = None, + ): + self.role = "user" + self.content = content + self.id = id + self.timestamp = timestamp + + +class MockAssistantMessage: + """Mock OpenAI AssistantMessage for testing.""" + + def __init__( + self, + content: MessageContent = "Hi there!", + id: str | None = None, + timestamp: datetime | None = None, + ): + self.role = "assistant" + self.content = content + self.id = id + self.timestamp = timestamp + + +class MockSystemMessage: + """Mock OpenAI SystemMessage for testing.""" + + def __init__( + self, + content: MessageContent = "You are a helpful assistant.", + id: str | None = None, + timestamp: datetime | None = None, + ): + self.role = "system" + self.content = content + self.id = id + self.timestamp = timestamp + + +class MockResponseOutputMessage: + """Mock OpenAI ResponseOutputMessage for testing.""" + + def __init__( + self, + content: MessageContent = "Response from agent", + role: str = "assistant", + id: str | None = None, + timestamp: datetime | None = None, + ): + self.role = role + self.content = content + self.id = id + self.timestamp = timestamp + + +class MockUnknownMessage: + """Mock unknown message type for testing fallback behavior.""" + + def __init__(self, content: MessageContent = "Unknown content"): + self.content = content + + +class MockContentPart: + """Mock content part for list-based content.""" + + def __init__(self, text: str): + self.type = "text" + self.text = text + + +# Type alias for mock messages +MockMessage = Union[ + MockUserMessage, + MockAssistantMessage, + MockSystemMessage, + MockResponseOutputMessage, + MockUnknownMessage, +] + + +class MockSession: + """Mock OpenAI Session for testing.""" + + def __init__(self, items: list[MockMessage] | None = None): + self._items: list[MockMessage] = items or [] + + def get_items(self, limit: int | None = None) -> list[MockMessage]: + """Get items from the session, optionally limited.""" + if limit is not None: + return self._items[:limit] + return self._items + + +# -------------------------------------------------------------------------- +# PYTEST FIXTURES +# -------------------------------------------------------------------------- + + +@pytest.fixture +def mock_turn_context(): + """Create a mock TurnContext with all required fields.""" + from microsoft_agents.hosting.core import TurnContext + + mock_context = Mock(spec=TurnContext) + mock_activity = Mock() + mock_conversation = Mock() + + mock_conversation.id = "conv-123" + mock_activity.conversation = mock_conversation + mock_activity.id = "msg-456" + mock_activity.text = "Hello, how are you?" + + mock_context.activity = mock_activity + return mock_context + + +@pytest.fixture +def mock_turn_context_no_activity(): + """Create a mock TurnContext with no activity.""" + from microsoft_agents.hosting.core import TurnContext + + mock_context = Mock(spec=TurnContext) + mock_context.activity = None + return mock_context + + +@pytest.fixture +def mock_turn_context_no_conversation_id(): + """Create a mock TurnContext with no conversation ID.""" + from microsoft_agents.hosting.core import TurnContext + + mock_context = Mock(spec=TurnContext) + mock_activity = Mock() + mock_activity.conversation = None + mock_activity.id = "msg-456" + mock_activity.text = "Hello" + mock_context.activity = mock_activity + return mock_context + + +@pytest.fixture +def mock_turn_context_no_message_id(): + """Create a mock TurnContext with no message ID.""" + from microsoft_agents.hosting.core import TurnContext + + mock_context = Mock(spec=TurnContext) + mock_activity = Mock() + mock_conversation = Mock() + mock_conversation.id = "conv-123" + mock_activity.conversation = mock_conversation + mock_activity.id = None + mock_activity.text = "Hello" + mock_context.activity = mock_activity + return mock_context + + +@pytest.fixture +def mock_turn_context_no_user_message(): + """Create a mock TurnContext with no user message text.""" + from microsoft_agents.hosting.core import TurnContext + + mock_context = Mock(spec=TurnContext) + mock_activity = Mock() + mock_conversation = Mock() + mock_conversation.id = "conv-123" + mock_activity.conversation = mock_conversation + mock_activity.id = "msg-456" + mock_activity.text = None + mock_context.activity = mock_activity + return mock_context + + +@pytest.fixture +def sample_user_message(): + """Create a sample user message.""" + return MockUserMessage(content="Hello, how are you?") + + +@pytest.fixture +def sample_assistant_message(): + """Create a sample assistant message.""" + return MockAssistantMessage(content="I'm doing great, thank you!") + + +@pytest.fixture +def sample_system_message(): + """Create a sample system message.""" + return MockSystemMessage(content="You are a helpful assistant.") + + +@pytest.fixture +def sample_openai_messages(): + """Create a list of sample OpenAI messages.""" + return [ + MockUserMessage(content="Hello"), + MockAssistantMessage(content="Hi there!"), + MockUserMessage(content="How are you?"), + MockAssistantMessage(content="I'm doing well, thanks for asking!"), + ] + + +@pytest.fixture +def sample_messages_with_ids(): + """Create sample messages with pre-existing IDs.""" + return [ + MockUserMessage(content="Hello", id="user-msg-001"), + MockAssistantMessage(content="Hi!", id="assistant-msg-001"), + ] + + +@pytest.fixture +def sample_messages_with_timestamps(): + """Create sample messages with pre-existing timestamps.""" + timestamp = datetime(2024, 1, 15, 10, 30, 0, tzinfo=UTC) + return [ + MockUserMessage(content="Hello", timestamp=timestamp), + MockAssistantMessage( + content="Hi!", + timestamp=datetime(2024, 1, 15, 10, 30, 5, tzinfo=UTC), + ), + ] + + +@pytest.fixture +def sample_message_with_list_content(): + """Create a message with list-based content.""" + return MockUserMessage(content=[MockContentPart("Hello, "), MockContentPart("how are you?")]) + + +@pytest.fixture +def sample_message_with_empty_content(): + """Create a message with empty content.""" + return MockUserMessage(content="") + + +@pytest.fixture +def sample_message_with_none_content(): + """Create a message with None content.""" + return MockUserMessage(content=None) + + +@pytest.fixture +def sample_unknown_message(): + """Create an unknown message type.""" + return MockUnknownMessage(content="Unknown type content") + + +@pytest.fixture +def mock_session(sample_openai_messages): + """Create a mock OpenAI Session with sample messages.""" + return MockSession(items=sample_openai_messages) + + +@pytest.fixture +def mock_empty_session(): + """Create a mock OpenAI Session with no messages.""" + return MockSession(items=[]) + + +@pytest.fixture +def service(): + """Create a McpToolRegistrationService instance for testing.""" + from microsoft_agents_a365.tooling.extensions.openai import McpToolRegistrationService + + return McpToolRegistrationService() diff --git a/tests/tooling/extensions/openai/test_integration.py b/tests/tooling/extensions/openai/test_integration.py new file mode 100644 index 00000000..8e885c7b --- /dev/null +++ b/tests/tooling/extensions/openai/test_integration.py @@ -0,0 +1,345 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +"""Integration tests for OpenAI send_chat_history methods.""" + +import json +from datetime import UTC, datetime +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest +from microsoft_agents_a365.runtime import OperationResult +from microsoft_agents_a365.tooling.models import ChatHistoryMessage + +from .conftest import ( + MockAssistantMessage, + MockSession, + MockSystemMessage, + MockUserMessage, +) + +# ============================================================================= +# INTEGRATION TESTS (IT-01 to IT-03) +# ============================================================================= + + +class TestEndToEndIntegration: + """Integration tests for end-to-end flow with mocked HTTP.""" + + # IT-01 + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_async_end_to_end_success(self, service, mock_turn_context): + """Test full end-to-end flow: Session -> conversion -> HTTP -> success.""" + # Create a session with realistic messages + messages = [ + MockSystemMessage(content="You are a helpful assistant."), + MockUserMessage(content="What is the capital of France?"), + MockAssistantMessage(content="The capital of France is Paris."), + MockUserMessage(content="And what about Germany?"), + MockAssistantMessage(content="The capital of Germany is Berlin."), + ] + session = MockSession(items=messages) + + # Mock aiohttp.ClientSession + mock_response = AsyncMock() + mock_response.status = 200 + mock_response.text = AsyncMock(return_value="OK") + + with patch("aiohttp.ClientSession") as mock_session_class: + mock_session_instance = MagicMock() + mock_post = AsyncMock() + mock_post.__aenter__.return_value = mock_response + mock_session_instance.post.return_value = mock_post + mock_session_class.return_value.__aenter__.return_value = mock_session_instance + + # Execute + result = await service.send_chat_history_async(mock_turn_context, session) + + # Verify + assert result.succeeded is True + assert len(result.errors) == 0 + + # Verify HTTP call was made + assert mock_session_instance.post.called + + # IT-02 + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_async_end_to_end_server_error( + self, service, mock_turn_context + ): + """Test full end-to-end flow with HTTP 500 error.""" + messages = [ + MockUserMessage(content="Hello"), + MockAssistantMessage(content="Hi there!"), + ] + session = MockSession(items=messages) + + # Mock aiohttp.ClientSession with 500 response + mock_response = AsyncMock() + mock_response.status = 500 + mock_response.text = AsyncMock(return_value="Internal Server Error") + mock_response.request_info = MagicMock() + mock_response.history = () + mock_response.headers = {} + + with patch("aiohttp.ClientSession") as mock_session_class: + mock_session_instance = MagicMock() + mock_post = AsyncMock() + mock_post.__aenter__.return_value = mock_response + mock_session_instance.post.return_value = mock_post + mock_session_class.return_value.__aenter__.return_value = mock_session_instance + + # Execute + result = await service.send_chat_history_async(mock_turn_context, session) + + # Verify failure + assert result.succeeded is False + assert len(result.errors) == 1 + + # IT-03 + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_async_payload_format(self, service, mock_turn_context): + """Test that the JSON payload has the correct structure.""" + messages = [ + MockUserMessage(content="Hello", id="user-001"), + MockAssistantMessage(content="Hi!", id="assistant-001"), + ] + session = MockSession(items=messages) + + mock_response = AsyncMock() + mock_response.status = 200 + mock_response.text = AsyncMock(return_value="OK") + + captured_payload = None + + with patch("aiohttp.ClientSession") as mock_session_class: + mock_session_instance = MagicMock() + + def capture_post(*args, **kwargs): + nonlocal captured_payload + captured_payload = kwargs.get("data") + mock_post = AsyncMock() + mock_post.__aenter__.return_value = mock_response + return mock_post + + mock_session_instance.post.side_effect = capture_post + mock_session_class.return_value.__aenter__.return_value = mock_session_instance + + # Execute + result = await service.send_chat_history_async(mock_turn_context, session) + + # Verify success + assert result.succeeded is True + + # Verify payload structure + assert captured_payload is not None + payload = json.loads(captured_payload) + + # Check required fields + assert "conversationId" in payload + assert "messageId" in payload + assert "userMessage" in payload + assert "chatHistory" in payload + + # Check conversation context from turn_context + assert payload["conversationId"] == "conv-123" + assert payload["messageId"] == "msg-456" + assert payload["userMessage"] == "Hello, how are you?" + + # Check chat history + chat_history = payload["chatHistory"] + assert len(chat_history) == 2 + + # Verify first message (user) + assert chat_history[0]["role"] == "user" + assert chat_history[0]["content"] == "Hello" + assert chat_history[0]["id"] == "user-001" + + # Verify second message (assistant) + assert chat_history[1]["role"] == "assistant" + assert chat_history[1]["content"] == "Hi!" + assert chat_history[1]["id"] == "assistant-001" + + +class TestConversionChainIntegration: + """Integration tests for the full conversion chain.""" + + @pytest.mark.asyncio + @pytest.mark.unit + async def test_messages_converted_to_chat_history_message_type( + self, service, mock_turn_context, sample_openai_messages + ): + """Test that OpenAI messages are converted to ChatHistoryMessage instances.""" + captured_messages = None + + with patch.object( + service.config_service, + "send_chat_history", + new_callable=AsyncMock, + ) as mock_send: + mock_send.return_value = OperationResult.success() + + def capture_args(*args, **kwargs): + nonlocal captured_messages + captured_messages = kwargs.get("chat_history_messages") + return OperationResult.success() + + mock_send.side_effect = capture_args + + await service.send_chat_history_messages_async( + mock_turn_context, sample_openai_messages + ) + + # Verify all messages are ChatHistoryMessage instances + assert captured_messages is not None + for msg in captured_messages: + assert isinstance(msg, ChatHistoryMessage) + + @pytest.mark.asyncio + @pytest.mark.unit + async def test_session_extraction_and_conversion_chain(self, service, mock_turn_context): + """Test the full chain: session.get_items() -> conversion -> send.""" + timestamp = datetime(2024, 1, 15, 10, 30, 0, tzinfo=UTC) + messages = [ + MockUserMessage(content="Query", id="q-1", timestamp=timestamp), + MockAssistantMessage(content="Response", id="r-1", timestamp=timestamp), + ] + session = MockSession(items=messages) + + captured_messages = None + + with patch.object( + service.config_service, + "send_chat_history", + new_callable=AsyncMock, + ) as mock_send: + + def capture_args(*args, **kwargs): + nonlocal captured_messages + captured_messages = kwargs.get("chat_history_messages") + return OperationResult.success() + + mock_send.side_effect = capture_args + + result = await service.send_chat_history_async(mock_turn_context, session) + + # Verify success + assert result.succeeded is True + + # Verify messages were extracted and converted + assert captured_messages is not None + assert len(captured_messages) == 2 + + # Verify IDs were preserved + assert captured_messages[0].id == "q-1" + assert captured_messages[1].id == "r-1" + + # Verify timestamps were preserved + assert captured_messages[0].timestamp == timestamp + assert captured_messages[1].timestamp == timestamp + + +class TestLimitParameterIntegration: + """Integration tests for the limit parameter.""" + + @pytest.mark.asyncio + @pytest.mark.unit + async def test_limit_restricts_session_items(self, service, mock_turn_context): + """Test that limit parameter correctly restricts items from session.""" + # Create session with many messages + messages = [MockUserMessage(content=f"Msg {i}") for i in range(100)] + session = MockSession(items=messages) + + captured_messages = None + + with patch.object( + service.config_service, + "send_chat_history", + new_callable=AsyncMock, + ) as mock_send: + + def capture_args(*args, **kwargs): + nonlocal captured_messages + captured_messages = kwargs.get("chat_history_messages") + return OperationResult.success() + + mock_send.side_effect = capture_args + + # Send with limit + result = await service.send_chat_history_async(mock_turn_context, session, limit=10) + + assert result.succeeded is True + assert captured_messages is not None + assert len(captured_messages) == 10 + + @pytest.mark.asyncio + @pytest.mark.unit + async def test_no_limit_sends_all_items(self, service, mock_turn_context): + """Test that no limit sends all session items.""" + messages = [MockUserMessage(content=f"Msg {i}") for i in range(50)] + session = MockSession(items=messages) + + captured_messages = None + + with patch.object( + service.config_service, + "send_chat_history", + new_callable=AsyncMock, + ) as mock_send: + + def capture_args(*args, **kwargs): + nonlocal captured_messages + captured_messages = kwargs.get("chat_history_messages") + return OperationResult.success() + + mock_send.side_effect = capture_args + + # Send without limit + result = await service.send_chat_history_async(mock_turn_context, session) + + assert result.succeeded is True + assert captured_messages is not None + assert len(captured_messages) == 50 + + +class TestHeadersIntegration: + """Integration tests for HTTP headers.""" + + @pytest.mark.asyncio + @pytest.mark.unit + async def test_user_agent_header_includes_orchestrator_name( + self, service, mock_turn_context, sample_openai_messages + ): + """Test that User-Agent header includes orchestrator name.""" + captured_headers = None + + mock_response = AsyncMock() + mock_response.status = 200 + mock_response.text = AsyncMock(return_value="OK") + + with patch("aiohttp.ClientSession") as mock_session_class: + mock_session_instance = MagicMock() + + def capture_post(*args, **kwargs): + nonlocal captured_headers + captured_headers = kwargs.get("headers") + mock_post = AsyncMock() + mock_post.__aenter__.return_value = mock_response + return mock_post + + mock_session_instance.post.side_effect = capture_post + mock_session_class.return_value.__aenter__.return_value = mock_session_instance + + await service.send_chat_history_messages_async( + mock_turn_context, sample_openai_messages + ) + + # Verify headers + assert captured_headers is not None + assert "User-Agent" in captured_headers + # User agent should contain OpenAI or orchestrator info + user_agent = captured_headers["User-Agent"] + assert "OpenAI" in user_agent or "microsoft-agents" in user_agent.lower() diff --git a/tests/tooling/extensions/openai/test_message_conversion.py b/tests/tooling/extensions/openai/test_message_conversion.py new file mode 100644 index 00000000..f42a12f9 --- /dev/null +++ b/tests/tooling/extensions/openai/test_message_conversion.py @@ -0,0 +1,463 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +"""Unit tests for message conversion logic in McpToolRegistrationService.""" + +import uuid +from datetime import UTC, datetime +from unittest.mock import Mock + +import pytest + +from .conftest import ( + MockAssistantMessage, + MockContentPart, + MockResponseOutputMessage, + MockSystemMessage, + MockUnknownMessage, + MockUserMessage, +) + +# ============================================================================= +# CONVERSION TESTS (CV-01 to CV-12) +# ============================================================================= + + +class TestRoleConversion: + """Tests for role extraction and mapping.""" + + # CV-01 + @pytest.mark.unit + def test_convert_user_message_to_chat_history(self, service): + """Test that UserMessage converts with role='user'.""" + message = MockUserMessage(content="Hello from user") + + result = service._convert_single_message(message) + + assert result is not None + assert result.role == "user" + assert result.content == "Hello from user" + + # CV-02 + @pytest.mark.unit + def test_convert_assistant_message_to_chat_history(self, service): + """Test that AssistantMessage converts with role='assistant'.""" + message = MockAssistantMessage(content="Hello from assistant") + + result = service._convert_single_message(message) + + assert result is not None + assert result.role == "assistant" + assert result.content == "Hello from assistant" + + # CV-03 + @pytest.mark.unit + def test_convert_system_message_to_chat_history(self, service): + """Test that SystemMessage converts with role='system'.""" + message = MockSystemMessage(content="System instructions") + + result = service._convert_single_message(message) + + assert result is not None + assert result.role == "system" + assert result.content == "System instructions" + + # CV-10 + @pytest.mark.unit + def test_convert_unknown_message_type_defaults_to_user(self, service): + """Test that unknown message type defaults to 'user' role.""" + message = MockUnknownMessage(content="Unknown type content") + + result = service._convert_single_message(message) + + assert result is not None + assert result.role == "user" + assert result.content == "Unknown type content" + + @pytest.mark.unit + def test_convert_response_output_message_to_assistant(self, service): + """Test that ResponseOutputMessage converts with role='assistant'.""" + message = MockResponseOutputMessage(content="Response content", role="assistant") + + result = service._convert_single_message(message) + + assert result is not None + assert result.role == "assistant" + + @pytest.mark.unit + def test_extract_role_from_dict_message(self, service): + """Test role extraction from dict-like message.""" + message = {"role": "user", "content": "Hello"} + + role = service._extract_role(message) + + assert role == "user" + + @pytest.mark.unit + def test_extract_role_from_dict_assistant(self, service): + """Test role extraction from dict with assistant role.""" + message = {"role": "assistant", "content": "Hi"} + + role = service._extract_role(message) + + assert role == "assistant" + + @pytest.mark.unit + def test_extract_role_from_dict_system(self, service): + """Test role extraction from dict with system role.""" + message = {"role": "system", "content": "Instructions"} + + role = service._extract_role(message) + + assert role == "system" + + +class TestContentExtraction: + """Tests for content extraction from messages.""" + + # CV-04 + @pytest.mark.unit + def test_convert_message_with_string_content(self, service): + """Test that string content is extracted directly.""" + message = MockUserMessage(content="Simple string content") + + result = service._convert_single_message(message) + + assert result is not None + assert result.content == "Simple string content" + + # CV-05 + @pytest.mark.unit + def test_convert_message_with_list_content(self, service): + """Test that list content is concatenated.""" + message = MockUserMessage(content=[MockContentPart("Hello, "), MockContentPart("world!")]) + + result = service._convert_single_message(message) + + assert result is not None + assert result.content == "Hello, world!" + + # CV-11 + @pytest.mark.unit + def test_convert_empty_content_uses_empty_string(self, service): + """Test that empty content is handled - message skipped due to validator.""" + message = MockUserMessage(content="") + + result = service._convert_single_message(message) + + # Empty content should cause the message to be skipped + assert result is None + + @pytest.mark.unit + def test_extract_content_from_text_attribute(self, service): + """Test content extraction from .text attribute as fallback.""" + message = Mock() + message.content = None + message.text = "Content from text attribute" + + content = service._extract_content(message) + + assert content == "Content from text attribute" + + @pytest.mark.unit + def test_extract_content_from_dict(self, service): + """Test content extraction from dict message.""" + message = {"role": "user", "content": "Dict content"} + + content = service._extract_content(message) + + assert content == "Dict content" + + @pytest.mark.unit + def test_extract_content_from_dict_with_list(self, service): + """Test content extraction from dict with list content.""" + message = { + "role": "user", + "content": [{"type": "text", "text": "Part 1"}, {"text": "Part 2"}], + } + + content = service._extract_content(message) + + assert "Part 1" in content + assert "Part 2" in content + + @pytest.mark.unit + def test_extract_content_concatenates_string_parts(self, service): + """Test that string parts in list are concatenated.""" + message = Mock() + message.content = ["Hello", " ", "world"] + + content = service._extract_content(message) + + assert content == "Hello world" + + +class TestIdExtraction: + """Tests for ID extraction and generation.""" + + # CV-06 + @pytest.mark.unit + def test_convert_message_generates_uuid_when_id_missing(self, service): + """Test that UUID is generated for messages without ID.""" + message = MockUserMessage(content="No ID message", id=None) + + result = service._convert_single_message(message) + + assert result is not None + assert result.id is not None + # Verify it's a valid UUID format + try: + uuid.UUID(result.id) + is_valid_uuid = True + except ValueError: + is_valid_uuid = False + assert is_valid_uuid + + # CV-08 + @pytest.mark.unit + def test_convert_message_preserves_existing_id(self, service): + """Test that existing ID is preserved.""" + message = MockUserMessage(content="Has ID", id="existing-id-123") + + result = service._convert_single_message(message) + + assert result is not None + assert result.id == "existing-id-123" + + @pytest.mark.unit + def test_extract_id_from_dict(self, service): + """Test ID extraction from dict message.""" + message = {"role": "user", "content": "Hello", "id": "dict-id-456"} + + msg_id = service._extract_id(message) + + assert msg_id == "dict-id-456" + + @pytest.mark.unit + def test_extract_id_generates_uuid_for_dict_without_id(self, service): + """Test UUID generation for dict without ID.""" + message = {"role": "user", "content": "Hello"} + + msg_id = service._extract_id(message) + + # Should be a valid UUID + try: + uuid.UUID(msg_id) + is_valid_uuid = True + except ValueError: + is_valid_uuid = False + assert is_valid_uuid + + +class TestTimestampExtraction: + """Tests for timestamp extraction and generation.""" + + # CV-07 + @pytest.mark.unit + def test_convert_message_uses_utc_when_timestamp_missing(self, service): + """Test that current UTC timestamp is used when missing.""" + message = MockUserMessage(content="No timestamp", timestamp=None) + before = datetime.now(UTC) + + result = service._convert_single_message(message) + + after = datetime.now(UTC) + + assert result is not None + assert result.timestamp is not None + assert before <= result.timestamp <= after + + # CV-09 + @pytest.mark.unit + def test_convert_message_preserves_existing_timestamp(self, service): + """Test that existing timestamp is preserved.""" + timestamp = datetime(2024, 1, 15, 10, 30, 0, tzinfo=UTC) + message = MockUserMessage(content="Has timestamp", timestamp=timestamp) + + result = service._convert_single_message(message) + + assert result is not None + assert result.timestamp == timestamp + + @pytest.mark.unit + def test_extract_timestamp_from_created_at(self, service): + """Test timestamp extraction from created_at attribute.""" + timestamp = datetime(2024, 6, 1, 12, 0, 0, tzinfo=UTC) + message = Mock() + message.timestamp = None + message.created_at = timestamp + + result = service._extract_timestamp(message) + + assert result == timestamp + + @pytest.mark.unit + def test_extract_timestamp_from_unix_timestamp(self, service): + """Test timestamp extraction from Unix timestamp.""" + unix_ts = 1704067200 # 2024-01-01 00:00:00 UTC + message = Mock() + message.timestamp = unix_ts + message.created_at = None + + result = service._extract_timestamp(message) + + assert result.year == 2024 + assert result.month == 1 + assert result.day == 1 + + @pytest.mark.unit + def test_extract_timestamp_from_iso_string(self, service): + """Test timestamp extraction from ISO format string.""" + message = Mock() + message.timestamp = "2024-03-15T14:30:00Z" + message.created_at = None + + result = service._extract_timestamp(message) + + assert result.year == 2024 + assert result.month == 3 + assert result.day == 15 + + +class TestBatchConversion: + """Tests for batch message conversion.""" + + # CV-12 + @pytest.mark.unit + def test_convert_multiple_messages(self, service, sample_openai_messages): + """Test that multiple messages are converted correctly.""" + result = service._convert_openai_messages_to_chat_history(sample_openai_messages) + + assert len(result) == len(sample_openai_messages) + + # Check alternating roles + assert result[0].role == "user" + assert result[1].role == "assistant" + assert result[2].role == "user" + assert result[3].role == "assistant" + + @pytest.mark.unit + def test_convert_filters_out_empty_content_messages(self, service): + """Test that messages with empty content are filtered out.""" + messages = [ + MockUserMessage(content="Valid content"), + MockUserMessage(content=""), # Should be filtered + MockAssistantMessage(content="Also valid"), + ] + + result = service._convert_openai_messages_to_chat_history(messages) + + # Only 2 messages should be converted (empty one filtered) + assert len(result) == 2 + assert result[0].content == "Valid content" + assert result[1].content == "Also valid" + + @pytest.mark.unit + def test_convert_handles_mixed_message_types(self, service): + """Test conversion of mixed message types.""" + messages = [ + MockSystemMessage(content="System prompt"), + MockUserMessage(content="User query"), + MockAssistantMessage(content="Assistant response"), + MockResponseOutputMessage(content="Output message"), + ] + + result = service._convert_openai_messages_to_chat_history(messages) + + assert len(result) == 4 + assert result[0].role == "system" + assert result[1].role == "user" + assert result[2].role == "assistant" + assert result[3].role == "assistant" + + @pytest.mark.unit + def test_convert_empty_list_returns_empty_list(self, service): + """Test that empty input returns empty output.""" + result = service._convert_openai_messages_to_chat_history([]) + + assert result == [] + + @pytest.mark.unit + def test_all_converted_messages_have_ids(self, service, sample_openai_messages): + """Test that all converted messages have IDs.""" + result = service._convert_openai_messages_to_chat_history(sample_openai_messages) + + for msg in result: + assert msg.id is not None + assert len(msg.id) > 0 + + @pytest.mark.unit + def test_all_converted_messages_have_timestamps(self, service, sample_openai_messages): + """Test that all converted messages have timestamps.""" + result = service._convert_openai_messages_to_chat_history(sample_openai_messages) + + for msg in result: + assert msg.timestamp is not None + + +class TestDictMessageConversion: + """Tests for dict-based message conversion.""" + + @pytest.mark.unit + def test_convert_dict_user_message(self, service): + """Test conversion of dict-based user message.""" + message = {"role": "user", "content": "Hello from dict"} + + result = service._convert_single_message(message) + + assert result is not None + assert result.role == "user" + assert result.content == "Hello from dict" + + @pytest.mark.unit + def test_convert_dict_with_id_and_timestamp(self, service): + """Test conversion of dict with ID and timestamp.""" + message = { + "role": "assistant", + "content": "Response", + "id": "dict-msg-id", + "timestamp": "2024-01-15T10:30:00Z", + } + + result = service._convert_single_message(message) + + assert result is not None + assert result.id == "dict-msg-id" + assert result.timestamp.year == 2024 + + +class TestEdgeCases: + """Tests for edge cases and boundary conditions.""" + + @pytest.mark.unit + def test_message_with_only_whitespace_content_skipped(self, service): + """Test that messages with only whitespace are skipped.""" + message = MockUserMessage(content=" ") + + result = service._convert_single_message(message) + + assert result is None + + @pytest.mark.unit + def test_message_with_none_content_skipped(self, service): + """Test that messages with None content are skipped.""" + message = Mock() + message.role = "user" + message.content = None + message.text = None + message.id = None + message.timestamp = None + + result = service._convert_single_message(message) + + assert result is None + + @pytest.mark.unit + def test_conversion_preserves_message_order(self, service): + """Test that message order is preserved during conversion.""" + messages = [MockUserMessage(content=f"Message {i}") for i in range(10)] + + result = service._convert_openai_messages_to_chat_history(messages) + + for i, msg in enumerate(result): + assert msg.content == f"Message {i}" diff --git a/tests/tooling/extensions/openai/test_send_chat_history_async.py b/tests/tooling/extensions/openai/test_send_chat_history_async.py new file mode 100644 index 00000000..02046dc8 --- /dev/null +++ b/tests/tooling/extensions/openai/test_send_chat_history_async.py @@ -0,0 +1,483 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +"""Unit tests for send_chat_history_async and send_chat_history_messages_async methods.""" + +from unittest.mock import AsyncMock, MagicMock, Mock, patch + +import pytest +from microsoft_agents_a365.runtime import OperationResult +from microsoft_agents_a365.tooling.models import ToolOptions + +from .conftest import ( + MockSession, + MockUserMessage, +) + +# ============================================================================= +# INPUT VALIDATION TESTS (UV-01 to UV-09) +# ============================================================================= + + +class TestInputValidation: + """Tests for input validation in send_chat_history methods.""" + + # UV-01 + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_messages_async_validates_turn_context_none( + self, service, sample_openai_messages + ): + """Test that send_chat_history_messages_async raises ValueError when turn_context is None.""" + with pytest.raises(ValueError, match="turn_context cannot be None"): + await service.send_chat_history_messages_async(None, sample_openai_messages) + + # UV-02 + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_messages_async_validates_messages_none( + self, service, mock_turn_context + ): + """Test that send_chat_history_messages_async raises ValueError when messages is None.""" + with pytest.raises(ValueError, match="messages cannot be None"): + await service.send_chat_history_messages_async(mock_turn_context, None) + + # UV-03 + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_messages_async_empty_list_returns_success( + self, service, mock_turn_context + ): + """Test that empty message list returns success (no-op).""" + result = await service.send_chat_history_messages_async(mock_turn_context, []) + + assert result.succeeded is True + assert len(result.errors) == 0 + + # UV-04 + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_messages_async_validates_activity_none( + self, service, mock_turn_context_no_activity, sample_openai_messages + ): + """Test that send_chat_history_messages_async validates turn_context.activity.""" + with patch.object( + service.config_service, + "send_chat_history", + new_callable=AsyncMock, + ) as mock_send: + mock_send.side_effect = ValueError("turn_context.activity cannot be None") + + with pytest.raises(ValueError, match="turn_context.activity cannot be None"): + await service.send_chat_history_messages_async( + mock_turn_context_no_activity, sample_openai_messages + ) + + # UV-05 + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_messages_async_validates_conversation_id( + self, service, mock_turn_context_no_conversation_id, sample_openai_messages + ): + """Test that send_chat_history_messages_async validates conversation_id.""" + with patch.object( + service.config_service, + "send_chat_history", + new_callable=AsyncMock, + ) as mock_send: + mock_send.side_effect = ValueError("conversation_id cannot be empty or None") + + with pytest.raises(ValueError, match="conversation_id cannot be empty"): + await service.send_chat_history_messages_async( + mock_turn_context_no_conversation_id, sample_openai_messages + ) + + # UV-06 + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_messages_async_validates_message_id( + self, service, mock_turn_context_no_message_id, sample_openai_messages + ): + """Test that send_chat_history_messages_async validates message_id.""" + with patch.object( + service.config_service, + "send_chat_history", + new_callable=AsyncMock, + ) as mock_send: + mock_send.side_effect = ValueError("message_id cannot be empty or None") + + with pytest.raises(ValueError, match="message_id cannot be empty"): + await service.send_chat_history_messages_async( + mock_turn_context_no_message_id, sample_openai_messages + ) + + # UV-07 + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_messages_async_validates_user_message( + self, service, mock_turn_context_no_user_message, sample_openai_messages + ): + """Test that send_chat_history_messages_async validates user_message text.""" + with patch.object( + service.config_service, + "send_chat_history", + new_callable=AsyncMock, + ) as mock_send: + mock_send.side_effect = ValueError("user_message cannot be empty or None") + + with pytest.raises(ValueError, match="user_message cannot be empty"): + await service.send_chat_history_messages_async( + mock_turn_context_no_user_message, sample_openai_messages + ) + + # UV-08 + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_async_validates_turn_context_none(self, service, mock_session): + """Test that send_chat_history_async raises ValueError when turn_context is None.""" + with pytest.raises(ValueError, match="turn_context cannot be None"): + await service.send_chat_history_async(None, mock_session) + + # UV-09 + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_async_validates_session_none(self, service, mock_turn_context): + """Test that send_chat_history_async raises ValueError when session is None.""" + with pytest.raises(ValueError, match="session cannot be None"): + await service.send_chat_history_async(mock_turn_context, None) + + +# ============================================================================= +# SUCCESS PATH TESTS (SP-01 to SP-07) +# ============================================================================= + + +class TestSuccessPath: + """Tests for successful execution paths.""" + + # SP-01 + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_messages_async_success( + self, service, mock_turn_context, sample_openai_messages + ): + """Test successful send_chat_history_messages_async call.""" + with patch.object( + service.config_service, + "send_chat_history", + new_callable=AsyncMock, + ) as mock_send: + mock_send.return_value = OperationResult.success() + + result = await service.send_chat_history_messages_async( + mock_turn_context, sample_openai_messages + ) + + assert result.succeeded is True + assert len(result.errors) == 0 + mock_send.assert_called_once() + + # SP-02 + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_messages_async_with_options( + self, service, mock_turn_context, sample_openai_messages + ): + """Test send_chat_history_messages_async with custom ToolOptions.""" + custom_options = ToolOptions(orchestrator_name="CustomOrchestrator") + + with patch.object( + service.config_service, + "send_chat_history", + new_callable=AsyncMock, + ) as mock_send: + mock_send.return_value = OperationResult.success() + + result = await service.send_chat_history_messages_async( + mock_turn_context, sample_openai_messages, options=custom_options + ) + + assert result.succeeded is True + # Verify options were passed through + call_args = mock_send.call_args + assert call_args.kwargs["options"].orchestrator_name == "CustomOrchestrator" + + # SP-03 + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_messages_async_default_orchestrator_name( + self, service, mock_turn_context, sample_openai_messages + ): + """Test that default orchestrator name is set to 'OpenAI'.""" + with patch.object( + service.config_service, + "send_chat_history", + new_callable=AsyncMock, + ) as mock_send: + mock_send.return_value = OperationResult.success() + + await service.send_chat_history_messages_async( + mock_turn_context, sample_openai_messages + ) + + # Verify default orchestrator name + call_args = mock_send.call_args + assert call_args.kwargs["options"].orchestrator_name == "OpenAI" + + # SP-04 + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_messages_async_delegates_to_config_service( + self, service, mock_turn_context, sample_openai_messages + ): + """Test that send_chat_history_messages_async delegates to config_service.""" + with patch.object( + service.config_service, + "send_chat_history", + new_callable=AsyncMock, + ) as mock_send: + mock_send.return_value = OperationResult.success() + + await service.send_chat_history_messages_async( + mock_turn_context, sample_openai_messages + ) + + # Verify delegation + mock_send.assert_called_once() + call_args = mock_send.call_args + + # Check turn_context was passed + assert call_args.kwargs["turn_context"] == mock_turn_context + + # Check chat_history_messages were converted + chat_history = call_args.kwargs["chat_history_messages"] + assert len(chat_history) == len(sample_openai_messages) + + # SP-05 + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_async_success(self, service, mock_turn_context, mock_session): + """Test successful send_chat_history_async call.""" + with patch.object( + service.config_service, + "send_chat_history", + new_callable=AsyncMock, + ) as mock_send: + mock_send.return_value = OperationResult.success() + + result = await service.send_chat_history_async(mock_turn_context, mock_session) + + assert result.succeeded is True + mock_send.assert_called_once() + + # SP-06 + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_async_with_limit(self, service, mock_turn_context): + """Test send_chat_history_async with limit parameter.""" + # Create session with many messages + messages = [MockUserMessage(content=f"Message {i}") for i in range(10)] + session = MockSession(items=messages) + + with patch.object( + service.config_service, + "send_chat_history", + new_callable=AsyncMock, + ) as mock_send: + mock_send.return_value = OperationResult.success() + + result = await service.send_chat_history_async(mock_turn_context, session, limit=5) + + assert result.succeeded is True + + # Verify only limited messages were sent + call_args = mock_send.call_args + chat_history = call_args.kwargs["chat_history_messages"] + assert len(chat_history) == 5 + + # SP-07 + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_async_delegates_to_send_chat_history_messages( + self, service, mock_turn_context, mock_session + ): + """Test that send_chat_history_async calls send_chat_history_messages_async.""" + with patch.object( + service, + "send_chat_history_messages_async", + new_callable=AsyncMock, + ) as mock_method: + mock_method.return_value = OperationResult.success() + + await service.send_chat_history_async(mock_turn_context, mock_session) + + mock_method.assert_called_once() + call_args = mock_method.call_args + assert call_args.kwargs["turn_context"] == mock_turn_context + + +# ============================================================================= +# ERROR HANDLING TESTS (EH-01 to EH-05) +# ============================================================================= + + +class TestErrorHandling: + """Tests for error handling scenarios.""" + + # EH-01 + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_messages_async_http_error( + self, service, mock_turn_context, sample_openai_messages + ): + """Test send_chat_history_messages_async handles HTTP errors.""" + + with patch.object( + service.config_service, + "send_chat_history", + new_callable=AsyncMock, + ) as mock_send: + mock_send.return_value = OperationResult.failed( + MagicMock(message="HTTP 500: Internal Server Error") + ) + + result = await service.send_chat_history_messages_async( + mock_turn_context, sample_openai_messages + ) + + assert result.succeeded is False + + # EH-02 + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_messages_async_timeout_error( + self, service, mock_turn_context, sample_openai_messages + ): + """Test send_chat_history_messages_async handles timeout errors.""" + with patch.object( + service.config_service, + "send_chat_history", + new_callable=AsyncMock, + ) as mock_send: + mock_send.side_effect = TimeoutError("Request timed out") + + result = await service.send_chat_history_messages_async( + mock_turn_context, sample_openai_messages + ) + + assert result.succeeded is False + assert len(result.errors) == 1 + + # EH-03 + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_messages_async_client_error( + self, service, mock_turn_context, sample_openai_messages + ): + """Test send_chat_history_messages_async handles network/client errors.""" + import aiohttp + + with patch.object( + service.config_service, + "send_chat_history", + new_callable=AsyncMock, + ) as mock_send: + mock_send.side_effect = aiohttp.ClientError("Connection failed") + + result = await service.send_chat_history_messages_async( + mock_turn_context, sample_openai_messages + ) + + assert result.succeeded is False + assert len(result.errors) == 1 + + # EH-04 + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_messages_async_conversion_error( + self, service, mock_turn_context + ): + """Test send_chat_history_messages_async handles conversion errors gracefully.""" + # Create a message that might cause conversion issues but still has content + problematic_message = MockUserMessage(content="Valid content") + + with patch.object( + service.config_service, + "send_chat_history", + new_callable=AsyncMock, + ) as mock_send: + mock_send.return_value = OperationResult.success() + + # Should not raise, should handle gracefully + result = await service.send_chat_history_messages_async( + mock_turn_context, [problematic_message] + ) + + assert result.succeeded is True + + # EH-05 + @pytest.mark.asyncio + @pytest.mark.unit + async def test_send_chat_history_async_get_items_error(self, service, mock_turn_context): + """Test send_chat_history_async handles session.get_items() errors.""" + # Create a mock session that raises an error + mock_session = Mock() + mock_session.get_items.side_effect = Exception("Session error") + + result = await service.send_chat_history_async(mock_turn_context, mock_session) + + assert result.succeeded is False + assert len(result.errors) == 1 + + +# ============================================================================= +# ORCHESTRATOR NAME HANDLING TESTS +# ============================================================================= + + +class TestOrchestratorNameHandling: + """Tests for orchestrator name handling in options.""" + + @pytest.mark.asyncio + @pytest.mark.unit + async def test_options_with_none_orchestrator_name_gets_default( + self, service, mock_turn_context, sample_openai_messages + ): + """Test that options with None orchestrator_name get default value.""" + options = ToolOptions(orchestrator_name=None) + + with patch.object( + service.config_service, + "send_chat_history", + new_callable=AsyncMock, + ) as mock_send: + mock_send.return_value = OperationResult.success() + + await service.send_chat_history_messages_async( + mock_turn_context, sample_openai_messages, options=options + ) + + call_args = mock_send.call_args + assert call_args.kwargs["options"].orchestrator_name == "OpenAI" + + @pytest.mark.asyncio + @pytest.mark.unit + async def test_options_preserves_custom_orchestrator_name( + self, service, mock_turn_context, sample_openai_messages + ): + """Test that custom orchestrator name is preserved.""" + options = ToolOptions(orchestrator_name="MyCustomOrchestrator") + + with patch.object( + service.config_service, + "send_chat_history", + new_callable=AsyncMock, + ) as mock_send: + mock_send.return_value = OperationResult.success() + + await service.send_chat_history_messages_async( + mock_turn_context, sample_openai_messages, options=options + ) + + call_args = mock_send.call_args + assert call_args.kwargs["options"].orchestrator_name == "MyCustomOrchestrator" From 2e969b51d9cba920c9e6a4e9bf52076f2cbac0b1 Mon Sep 17 00:00:00 2001 From: Johan Broberg Date: Thu, 22 Jan 2026 20:28:46 -0800 Subject: [PATCH 05/11] Add comments to clarify error handling in namespace package finder --- .../microsoft_agents_a365/tooling/extensions/__init__.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/extensions/__init__.py b/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/extensions/__init__.py index 7108f9ae..5c1bc402 100644 --- a/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/extensions/__init__.py +++ b/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/extensions/__init__.py @@ -25,4 +25,6 @@ if path not in __path__ and not path.endswith(".__path_hook__"): __path__.append(path) except (ImportError, TypeError): + # Silently skip finders that don't support our find_spec call signature + # or fail to locate the module - we'll try other finders in meta_path pass From 4cb074f2f860178a78c6e857dbaa50a8f4ddd42f Mon Sep 17 00:00:00 2001 From: Johan Broberg Date: Sat, 24 Jan 2026 19:50:04 -0800 Subject: [PATCH 06/11] Refactor send_chat_history methods and add comprehensive tests - Removed unnecessary async suffix from send_chat_history methods for consistency. - Updated docstrings and comments for clarity. - Introduced end-to-end tests for send_chat_history methods with mocked HTTP responses. - Added unit tests for input validation, success paths, error handling, and orchestrator name handling. - Ensured thread safety and isolation for concurrent calls to send_chat_history methods. - Improved overall test coverage and reliability of the service. --- CLAUDE.md | 2 +- docs/design.md | 44 +- .../prd/openai-send-chat-history-api-tasks.md | 695 ----------- docs/prd/openai-send-chat-history-api.md | 1027 ----------------- .../openai/mcp_tool_registration_service.py | 11 +- .../tooling/extensions/__init__.py | 18 +- .../{test_integration.py => test_e2e.py} | 58 +- ...ory_async.py => test_send_chat_history.py} | 177 +-- 8 files changed, 202 insertions(+), 1830 deletions(-) delete mode 100644 docs/prd/openai-send-chat-history-api-tasks.md delete mode 100644 docs/prd/openai-send-chat-history-api.md rename tests/tooling/extensions/openai/{test_integration.py => test_e2e.py} (86%) rename tests/tooling/extensions/openai/{test_send_chat_history_async.py => test_send_chat_history.py} (67%) diff --git a/CLAUDE.md b/CLAUDE.md index be4da0d5..b273e8dd 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -183,7 +183,7 @@ Place it before imports with one blank line after. from agents.memory import Session from agents.items import TResponseInputItem - async def send_chat_history_async(self, session: Session) -> OperationResult: + async def send_chat_history(self, session: Session) -> OperationResult: ... ``` diff --git a/docs/design.md b/docs/design.md index 3ebd5c9f..9f107784 100644 --- a/docs/design.md +++ b/docs/design.md @@ -227,9 +227,51 @@ Framework-specific adapters for MCP tool integration: |---------|---------|------------| | `extensions-agentframework` | Adapt MCP tools to Microsoft Agents SDK | [design.md](../libraries/microsoft-agents-a365-tooling-extensions-agentframework/docs/design.md) | | `extensions-azureaifoundry` | Azure AI Foundry tool integration | [design.md](../libraries/microsoft-agents-a365-tooling-extensions-azureaifoundry/docs/design.md) | -| `extensions-openai` | OpenAI function calling integration | [design.md](../libraries/microsoft-agents-a365-tooling-extensions-openai/docs/design.md) | +| `extensions-openai` | OpenAI function calling integration and chat history | [design.md](../libraries/microsoft-agents-a365-tooling-extensions-openai/docs/design.md) | | `extensions-semantickernel` | Semantic Kernel plugin integration | [design.md](../libraries/microsoft-agents-a365-tooling-extensions-semantickernel/docs/design.md) | +#### OpenAI Extension: Chat History API + +The OpenAI tooling extension provides methods to send chat history to the MCP platform for real-time threat protection: + +**Key Classes:** + +| Class | Purpose | +|-------|---------| +| `McpToolRegistrationService` | MCP tool registration and chat history management | + +**Methods:** + +| Method | Purpose | +|--------|---------| +| `send_chat_history(turn_context, session, limit, options)` | Extract messages from OpenAI Session and send to MCP platform | +| `send_chat_history_messages(turn_context, messages, options)` | Send a list of OpenAI TResponseInputItem messages to MCP platform | + +**Usage Example:** + +```python +from agents import Agent, Runner +from microsoft_agents_a365.tooling.extensions.openai import McpToolRegistrationService + +service = McpToolRegistrationService() +agent = Agent(name="my-agent", model="gpt-4") + +# In your agent handler: +async with Runner.run(agent, messages) as result: + session = result.session + + # Option 1: Send from Session object + op_result = await service.send_chat_history(turn_context, session) + + # Option 2: Send from message list + op_result = await service.send_chat_history_messages(turn_context, messages) + + if op_result.succeeded: + print("Chat history sent successfully") +``` + +The methods convert OpenAI message types to `ChatHistoryMessage` format and delegate to the core `McpToolServerConfigurationService.send_chat_history()` method. + ### 6. Notifications (`microsoft-agents-a365-notifications`) > **Detailed documentation**: [libraries/microsoft-agents-a365-notifications/docs/design.md](../libraries/microsoft-agents-a365-notifications/docs/design.md) diff --git a/docs/prd/openai-send-chat-history-api-tasks.md b/docs/prd/openai-send-chat-history-api-tasks.md deleted file mode 100644 index 1bc6fb97..00000000 --- a/docs/prd/openai-send-chat-history-api-tasks.md +++ /dev/null @@ -1,695 +0,0 @@ -# Task Breakdown: OpenAI `send_chat_history_messages_async` API - -| **Document Information** | | -|--------------------------|----------------------------------------------| -| **PRD Reference** | [openai-send-chat-history-api.md](openai-send-chat-history-api.md) | -| **Created** | January 21, 2026 | -| **Target Package** | `microsoft-agents-a365-tooling-extensions-openai` | -| **Estimated Total Effort** | 40-56 hours | - ---- - -## Table of Contents - -1. [Executive Summary](#1-executive-summary) -2. [Architecture Impact Analysis](#2-architecture-impact-analysis) -3. [Task Breakdown by Phase](#3-task-breakdown-by-phase) -4. [Task Dependencies](#4-task-dependencies) -5. [Testing Strategy Overview](#5-testing-strategy-overview) -6. [Risks and Considerations](#6-risks-and-considerations) -7. [Appendix: Task Checklist](#appendix-task-checklist) - ---- - -## 1. Executive Summary - -### 1.1 Overview - -This task breakdown covers the implementation of two new async APIs for the OpenAI orchestrator extension: - -1. **`send_chat_history_async(turn_context, session, limit, options)`** - Extracts messages from an OpenAI `Session` object and delegates to `send_chat_history_messages_async`. This is the primary/most common use case. - -2. **`send_chat_history_messages_async(turn_context, messages, options)`** - Accepts a list of OpenAI `TResponseInputItem` messages and converts them to `ChatHistoryMessage` format before sending to the MCP platform. - -### 1.2 Key Implementation Points - -| Aspect | Details | -|--------|---------| -| **Target File** | `libraries/microsoft-agents-a365-tooling-extensions-openai/microsoft_agents_a365/tooling/extensions/openai/mcp_tool_registration_service.py` | -| **Delegation Target** | `McpToolServerConfigurationService.send_chat_history()` in `microsoft-agents-a365-tooling` | -| **Return Type** | `OperationResult` (success/failure pattern) | -| **Error Handling** | `ValueError` for validation, `OperationResult.failed()` for runtime errors | - -### 1.3 Scope Summary - -- **In Scope**: Two new public methods, message conversion logic, comprehensive unit/integration tests -- **Out of Scope**: Changes to core `McpToolServerConfigurationService`, synchronous wrappers, other orchestrator extensions - -### 1.4 Effort Estimation - -| Phase | Estimated Hours | -|-------|-----------------| -| Phase 1: Foundation & Infrastructure | 6-8 hours | -| Phase 2: Core Implementation | 12-16 hours | -| Phase 3: Testing | 14-20 hours | -| Phase 4: Documentation & Finalization | 8-12 hours | -| **Total** | **40-56 hours** | - ---- - -## 2. Architecture Impact Analysis - -### 2.1 Component Diagram - -``` -┌─────────────────────────────────────────────────────────────────────────────┐ -│ Developer Application │ -└─────────────────────────────────────────────────────────────────────────────┘ - │ - │ calls - ▼ -┌─────────────────────────────────────────────────────────────────────────────┐ -│ microsoft-agents-a365-tooling-extensions-openai │ -│ ┌───────────────────────────────────────────────────────────────────────┐ │ -│ │ McpToolRegistrationService │ │ -│ │ ┌─────────────────────────────────────────────────────────────────┐ │ │ -│ │ │ NEW: send_chat_history_async() │ │ │ -│ │ │ NEW: send_chat_history_messages_async() │ │ │ -│ │ │ NEW: _convert_openai_messages_to_chat_history() │ │ │ -│ │ │ NEW: _convert_single_message() │ │ │ -│ │ │ NEW: _extract_role() / _extract_content() / _extract_id() │ │ │ -│ │ │ EXISTING: add_tool_servers_to_agent() │ │ │ -│ │ └─────────────────────────────────────────────────────────────────┘ │ │ -│ └───────────────────────────────────────────────────────────────────────┘ │ -└─────────────────────────────────────────────────────────────────────────────┘ - │ - │ delegates to - ▼ -┌─────────────────────────────────────────────────────────────────────────────┐ -│ microsoft-agents-a365-tooling (core package) │ -│ ┌───────────────────────────────────────────────────────────────────────┐ │ -│ │ McpToolServerConfigurationService │ │ -│ │ ┌─────────────────────────────────────────────────────────────────┐ │ │ -│ │ │ EXISTING: send_chat_history() │ │ │ -│ │ └─────────────────────────────────────────────────────────────────┘ │ │ -│ └───────────────────────────────────────────────────────────────────────┘ │ -└─────────────────────────────────────────────────────────────────────────────┘ - │ - │ HTTP POST - ▼ -┌─────────────────────────────────────────────────────────────────────────────┐ -│ MCP Platform │ -│ /real-time-threat-protection/chat-message │ -└─────────────────────────────────────────────────────────────────────────────┘ -``` - -### 2.2 Files Impacted - -| File | Impact Type | Changes | -|------|-------------|---------| -| `mcp_tool_registration_service.py` | **Modified** | Add 2 public methods, 5+ private helper methods | -| `__init__.py` (openai extension) | **Modified** | Ensure exports are correct | -| `pyproject.toml` (openai extension) | **Modified** | Add `openai-agents` dependency if not present | -| `tests/tooling/extensions/openai/test_send_chat_history_async.py` | **New** | Main API unit tests | -| `tests/tooling/extensions/openai/test_message_conversion.py` | **New** | Conversion logic tests | -| `tests/tooling/extensions/openai/conftest.py` | **New** | Shared pytest fixtures | -| `README.md` (openai extension) | **Modified** | Add usage documentation | - -### 2.3 Dependency Analysis - -| Dependency | Type | Purpose | Risk | -|------------|------|---------|------| -| `openai-agents` | External (new) | OpenAI SDK types (`TResponseInputItem`, `Session`) | Medium - API stability | -| `microsoft-agents-a365-tooling` | Internal (existing) | Core `McpToolServerConfigurationService` | Low | -| `microsoft-agents-a365-runtime` | Internal (existing) | `OperationResult`, `OperationError` | Low | -| `microsoft-agents-hosting-core` | External (existing) | `TurnContext` | Low | - -### 2.4 Breaking Changes - -**None** - This is a purely additive change. All existing methods remain unchanged. - ---- - -## 3. Task Breakdown by Phase - ---- - -### Phase 1: Foundation & Infrastructure (6-8 hours) - -#### Task 1.1: Analyze OpenAI SDK Types and Create Type Stubs - -| Attribute | Details | -|-----------|---------| -| **Title** | Analyze OpenAI SDK Types and Create Type Stubs | -| **Estimate** | 3-4 hours | -| **Description** | Research the OpenAI Agents SDK to understand the `TResponseInputItem` union type, `Session` protocol, and message types. Create type stubs or protocols for testing if needed. | -| **Acceptance Criteria** | - Document all message types in `TResponseInputItem` union
- Identify attributes for role, content, id, timestamp extraction
- Create mock classes for unit testing
- Verify `Session.get_items()` method signature | -| **Files to Create/Modify** | - `tests/tooling/extensions/openai/conftest.py` (fixtures and mocks) | -| **Dependencies** | None | -| **Testing Requirements** | N/A (research task) | - ---- - -#### Task 1.2: Add openai-agents Dependency - -| Attribute | Details | -|-----------|---------| -| **Title** | Add openai-agents Dependency to pyproject.toml | -| **Estimate** | 1 hour | -| **Description** | Add the `openai-agents` package dependency to the OpenAI extension's `pyproject.toml` with appropriate version constraints. | -| **Acceptance Criteria** | - `openai-agents` added to `[project.dependencies]`
- Version constraint allows flexibility for minor updates
- `pip install` succeeds with new dependency
- No conflicts with existing dependencies | -| **Files to Create/Modify** | - `libraries/microsoft-agents-a365-tooling-extensions-openai/pyproject.toml` | -| **Dependencies** | None | -| **Testing Requirements** | Run `pip install -e .` and verify installation | - ---- - -#### Task 1.3: Create Test Directory Structure and Fixtures - -| Attribute | Details | -|-----------|---------| -| **Title** | Create Test Directory Structure and Shared Fixtures | -| **Estimate** | 2-3 hours | -| **Description** | Create the test directory structure for the OpenAI extension and define shared pytest fixtures for mocking OpenAI types, TurnContext, and common test data. | -| **Acceptance Criteria** | - Test directory structure created: `tests/tooling/extensions/openai/`
- `conftest.py` with fixtures for mock TurnContext, mock OpenAI messages, mock Session
- `__init__.py` files in place
- Fixtures are reusable across test files | -| **Files to Create/Modify** | - `tests/tooling/extensions/openai/__init__.py` (new)
- `tests/tooling/extensions/openai/conftest.py` (new)
- `tests/tooling/extensions/__init__.py` (new if needed) | -| **Dependencies** | Task 1.1 | -| **Testing Requirements** | Verify fixtures work with `pytest --collect-only` | - ---- - -### Phase 2: Core Implementation (12-16 hours) - -#### Task 2.1: Implement Role Extraction Helper - -| Attribute | Details | -|-----------|---------| -| **Title** | Implement `_extract_role()` Private Method | -| **Estimate** | 2 hours | -| **Description** | Implement the private `_extract_role()` method that maps OpenAI message types to `ChatHistoryMessage` roles ("user", "assistant", "system"). Handle unknown types with a warning log and default to "user". | -| **Acceptance Criteria** | - Maps `UserMessage` → "user"
- Maps `AssistantMessage` → "assistant"
- Maps `SystemMessage` → "system"
- Maps `ResponseOutputMessage` with role="assistant" → "assistant"
- Unknown types default to "user" with warning log
- Method is type-hinted | -| **Files to Create/Modify** | - `libraries/microsoft-agents-a365-tooling-extensions-openai/microsoft_agents_a365/tooling/extensions/openai/mcp_tool_registration_service.py` | -| **Dependencies** | Task 1.1, Task 1.2 | -| **Testing Requirements** | Unit tests in Task 3.2 | - ---- - -#### Task 2.2: Implement Content Extraction Helper - -| Attribute | Details | -|-----------|---------| -| **Title** | Implement `_extract_content()` Private Method | -| **Estimate** | 2-3 hours | -| **Description** | Implement the private `_extract_content()` method that extracts text content from OpenAI messages. Handle string content, list content (concatenate text parts), `.text` attribute, and empty/None content with warning log. | -| **Acceptance Criteria** | - String `.content` used directly
- List `.content` concatenates all text parts
- Falls back to `.text` attribute if present
- Returns empty string with warning for empty/None content
- Method is type-hinted | -| **Files to Create/Modify** | - `libraries/microsoft-agents-a365-tooling-extensions-openai/microsoft_agents_a365/tooling/extensions/openai/mcp_tool_registration_service.py` | -| **Dependencies** | Task 1.1, Task 1.2 | -| **Testing Requirements** | Unit tests in Task 3.2 | - ---- - -#### Task 2.3: Implement ID and Timestamp Extraction Helpers - -| Attribute | Details | -|-----------|---------| -| **Title** | Implement `_extract_id()` and `_extract_timestamp()` Private Methods | -| **Estimate** | 2 hours | -| **Description** | Implement helper methods to extract or generate message IDs (UUID if missing) and timestamps (current UTC if missing). | -| **Acceptance Criteria** | - `_extract_id()`: Returns existing ID if present, generates UUID if missing
- `_extract_timestamp()`: Returns existing timestamp if present, uses `datetime.now(timezone.utc)` if missing
- Debug logging for generated values
- Methods are type-hinted | -| **Files to Create/Modify** | - `libraries/microsoft-agents-a365-tooling-extensions-openai/microsoft_agents_a365/tooling/extensions/openai/mcp_tool_registration_service.py` | -| **Dependencies** | Task 1.1 | -| **Testing Requirements** | Unit tests in Task 3.2 | - ---- - -#### Task 2.4: Implement Single Message Conversion - -| Attribute | Details | -|-----------|---------| -| **Title** | Implement `_convert_single_message()` Private Method | -| **Estimate** | 2 hours | -| **Description** | Implement the private method that converts a single `TResponseInputItem` to a `ChatHistoryMessage` using the extraction helpers. | -| **Acceptance Criteria** | - Calls all extraction helpers
- Returns `ChatHistoryMessage` with role, content, id, timestamp
- Returns `None` for unconvertible messages with error log
- Method is type-hinted | -| **Files to Create/Modify** | - `libraries/microsoft-agents-a365-tooling-extensions-openai/microsoft_agents_a365/tooling/extensions/openai/mcp_tool_registration_service.py` | -| **Dependencies** | Task 2.1, Task 2.2, Task 2.3 | -| **Testing Requirements** | Unit tests in Task 3.2 | - ---- - -#### Task 2.5: Implement Batch Message Conversion - -| Attribute | Details | -|-----------|---------| -| **Title** | Implement `_convert_openai_messages_to_chat_history()` Private Method | -| **Estimate** | 1-2 hours | -| **Description** | Implement the private method that converts a list of `TResponseInputItem` messages to a list of `ChatHistoryMessage` objects. Filter out `None` results from unconvertible messages. | -| **Acceptance Criteria** | - Iterates through all messages
- Calls `_convert_single_message()` for each
- Filters out `None` results
- Returns `List[ChatHistoryMessage]`
- Info logging with count of converted messages
- Method is type-hinted | -| **Files to Create/Modify** | - `libraries/microsoft-agents-a365-tooling-extensions-openai/microsoft_agents_a365/tooling/extensions/openai/mcp_tool_registration_service.py` | -| **Dependencies** | Task 2.4 | -| **Testing Requirements** | Unit tests in Task 3.2 | - ---- - -#### Task 2.6: Implement send_chat_history_messages_async Method - -| Attribute | Details | -|-----------|---------| -| **Title** | Implement `send_chat_history_messages_async()` Public Method | -| **Estimate** | 3-4 hours | -| **Description** | Implement the public API method that validates inputs, converts OpenAI messages, and delegates to `McpToolServerConfigurationService.send_chat_history()`. | -| **Acceptance Criteria** | - Validates `turn_context` is not None (raises `ValueError`)
- Validates `messages` is not None (raises `ValueError`)
- Empty list returns `OperationResult.success()` (no-op)
- Sets default `ToolOptions` with orchestrator_name="OpenAI"
- Converts messages using `_convert_openai_messages_to_chat_history()`
- Delegates to `self.config_service.send_chat_history()`
- Returns `OperationResult`
- Catches exceptions and returns `OperationResult.failed()`
- Complete docstring with Args/Returns/Raises | -| **Files to Create/Modify** | - `libraries/microsoft-agents-a365-tooling-extensions-openai/microsoft_agents_a365/tooling/extensions/openai/mcp_tool_registration_service.py` | -| **Dependencies** | Task 2.5 | -| **Testing Requirements** | Unit tests in Task 3.3, Task 3.4 | - ---- - -#### Task 2.7: Implement send_chat_history_async Method - -| Attribute | Details | -|-----------|---------| -| **Title** | Implement `send_chat_history_async()` Public Method | -| **Estimate** | 2 hours | -| **Description** | Implement the session-based public API method that extracts messages from an OpenAI `Session` and delegates to `send_chat_history_messages_async()`. | -| **Acceptance Criteria** | - Validates `turn_context` is not None (raises `ValueError`)
- Validates `session` is not None (raises `ValueError`)
- Calls `session.get_items(limit=limit)` to extract messages
- Delegates to `send_chat_history_messages_async()`
- Returns `OperationResult`
- Catches exceptions and returns `OperationResult.failed()`
- Complete docstring with Args/Returns/Raises | -| **Files to Create/Modify** | - `libraries/microsoft-agents-a365-tooling-extensions-openai/microsoft_agents_a365/tooling/extensions/openai/mcp_tool_registration_service.py` | -| **Dependencies** | Task 2.6 | -| **Testing Requirements** | Unit tests in Task 3.3, Task 3.4 | - ---- - -### Phase 3: Testing (14-20 hours) - -#### Task 3.1: Implement Input Validation Unit Tests - -| Attribute | Details | -|-----------|---------| -| **Title** | Implement Input Validation Unit Tests (UV-01 to UV-09) | -| **Estimate** | 3-4 hours | -| **Description** | Implement unit tests for all input validation scenarios as specified in PRD section 9.2.1. | -| **Acceptance Criteria** | - UV-01: `test_send_chat_history_messages_async_validates_turn_context_none`
- UV-02: `test_send_chat_history_messages_async_validates_messages_none`
- UV-03: `test_send_chat_history_messages_async_empty_list_returns_success`
- UV-04: `test_send_chat_history_messages_async_validates_activity_none`
- UV-05: `test_send_chat_history_messages_async_validates_conversation_id`
- UV-06: `test_send_chat_history_messages_async_validates_message_id`
- UV-07: `test_send_chat_history_messages_async_validates_user_message`
- UV-08: `test_send_chat_history_async_validates_turn_context_none`
- UV-09: `test_send_chat_history_async_validates_session_none`
- All tests pass | -| **Files to Create/Modify** | - `tests/tooling/extensions/openai/test_send_chat_history_async.py` (new) | -| **Dependencies** | Task 2.6, Task 2.7, Task 1.3 | -| **Testing Requirements** | `pytest tests/tooling/extensions/openai/test_send_chat_history_async.py -v` | - ---- - -#### Task 3.2: Implement Conversion Logic Unit Tests - -| Attribute | Details | -|-----------|---------| -| **Title** | Implement Conversion Logic Unit Tests (CV-01 to CV-12) | -| **Estimate** | 4-5 hours | -| **Description** | Implement unit tests for all message conversion scenarios as specified in PRD section 9.2.2. | -| **Acceptance Criteria** | - CV-01: `test_convert_user_message_to_chat_history`
- CV-02: `test_convert_assistant_message_to_chat_history`
- CV-03: `test_convert_system_message_to_chat_history`
- CV-04: `test_convert_message_with_string_content`
- CV-05: `test_convert_message_with_list_content`
- CV-06: `test_convert_message_generates_uuid_when_id_missing`
- CV-07: `test_convert_message_uses_utc_when_timestamp_missing`
- CV-08: `test_convert_message_preserves_existing_id`
- CV-09: `test_convert_message_preserves_existing_timestamp`
- CV-10: `test_convert_unknown_message_type_defaults_to_user`
- CV-11: `test_convert_empty_content_uses_empty_string`
- CV-12: `test_convert_multiple_messages`
- All tests pass | -| **Files to Create/Modify** | - `tests/tooling/extensions/openai/test_message_conversion.py` (new) | -| **Dependencies** | Task 2.1, Task 2.2, Task 2.3, Task 2.4, Task 2.5, Task 1.3 | -| **Testing Requirements** | `pytest tests/tooling/extensions/openai/test_message_conversion.py -v` | - ---- - -#### Task 3.3: Implement Success Path Unit Tests - -| Attribute | Details | -|-----------|---------| -| **Title** | Implement Success Path Unit Tests (SP-01 to SP-07) | -| **Estimate** | 3-4 hours | -| **Description** | Implement unit tests for all success path scenarios as specified in PRD section 9.2.3. | -| **Acceptance Criteria** | - SP-01: `test_send_chat_history_messages_async_success`
- SP-02: `test_send_chat_history_messages_async_with_options`
- SP-03: `test_send_chat_history_messages_async_default_orchestrator_name`
- SP-04: `test_send_chat_history_messages_async_delegates_to_config_service`
- SP-05: `test_send_chat_history_async_success`
- SP-06: `test_send_chat_history_async_with_limit`
- SP-07: `test_send_chat_history_async_delegates_to_send_chat_history_messages`
- All tests pass | -| **Files to Create/Modify** | - `tests/tooling/extensions/openai/test_send_chat_history_async.py` | -| **Dependencies** | Task 2.6, Task 2.7, Task 1.3 | -| **Testing Requirements** | `pytest tests/tooling/extensions/openai/test_send_chat_history_async.py -v` | - ---- - -#### Task 3.4: Implement Error Handling Unit Tests - -| Attribute | Details | -|-----------|---------| -| **Title** | Implement Error Handling Unit Tests (EH-01 to EH-05) | -| **Estimate** | 2-3 hours | -| **Description** | Implement unit tests for all error handling scenarios as specified in PRD section 9.2.4. | -| **Acceptance Criteria** | - EH-01: `test_send_chat_history_messages_async_http_error`
- EH-02: `test_send_chat_history_messages_async_timeout_error`
- EH-03: `test_send_chat_history_messages_async_client_error`
- EH-04: `test_send_chat_history_messages_async_conversion_error`
- EH-05: `test_send_chat_history_async_get_items_error`
- All tests pass | -| **Files to Create/Modify** | - `tests/tooling/extensions/openai/test_send_chat_history_async.py` | -| **Dependencies** | Task 2.6, Task 2.7, Task 1.3 | -| **Testing Requirements** | `pytest tests/tooling/extensions/openai/test_send_chat_history_async.py -v` | - ---- - -#### Task 3.5: Implement Integration Tests - -| Attribute | Details | -|-----------|---------| -| **Title** | Implement Integration Tests (IT-01 to IT-03) | -| **Estimate** | 3-4 hours | -| **Description** | Implement integration tests that verify end-to-end flow with mocked HTTP as specified in PRD section 9.3. | -| **Acceptance Criteria** | - IT-01: `test_send_chat_history_async_end_to_end_success`
- IT-02: `test_send_chat_history_async_end_to_end_server_error`
- IT-03: `test_send_chat_history_async_payload_format`
- Tests use real OpenAI SDK types where possible
- Tests verify full conversion and delegation chain
- All tests pass | -| **Files to Create/Modify** | - `tests/tooling/extensions/openai/test_integration.py` (new) | -| **Dependencies** | Task 2.6, Task 2.7, Task 1.3 | -| **Testing Requirements** | `pytest tests/tooling/extensions/openai/test_integration.py -v` | - ---- - -#### Task 3.6: Verify Test Coverage and Add Performance Benchmarks - -| Attribute | Details | -|-----------|---------| -| **Title** | Verify Test Coverage Targets and Add Performance Benchmarks | -| **Estimate** | 2-3 hours | -| **Description** | Run coverage analysis to ensure ≥95% line coverage and ≥90% branch coverage. Add performance benchmark tests for CI regression detection. | -| **Acceptance Criteria** | - Line coverage ≥95%
- Branch coverage ≥90%
- Performance benchmark: <10ms for 100 messages conversion
- Performance benchmark: <1MB memory for 1000 messages
- Coverage report generated | -| **Files to Create/Modify** | - `tests/tooling/extensions/openai/test_performance.py` (new)
- `pyproject.toml` (add coverage config if needed) | -| **Dependencies** | Task 3.1, Task 3.2, Task 3.3, Task 3.4, Task 3.5 | -| **Testing Requirements** | `pytest tests/tooling/extensions/openai/ --cov --cov-report=html` | - ---- - -### Phase 4: Documentation & Finalization (8-12 hours) - -#### Task 4.1: Add Method Docstrings and Type Hints - -| Attribute | Details | -|-----------|---------| -| **Title** | Complete Method Docstrings and Type Hints | -| **Estimate** | 2-3 hours | -| **Description** | Ensure all public and private methods have complete docstrings following project conventions and full type annotations. | -| **Acceptance Criteria** | - All public methods have docstrings with Args/Returns/Raises
- All private methods have docstrings
- All parameters and return types are type-hinted
- Type checker (`mypy` or `pyright`) passes
- Follows existing project docstring style | -| **Files to Create/Modify** | - `libraries/microsoft-agents-a365-tooling-extensions-openai/microsoft_agents_a365/tooling/extensions/openai/mcp_tool_registration_service.py` | -| **Dependencies** | Task 2.6, Task 2.7 | -| **Testing Requirements** | `mypy` or `pyright` check | - ---- - -#### Task 4.2: Update Package README with Usage Examples - -| Attribute | Details | -|-----------|---------| -| **Title** | Update README.md with Usage Examples | -| **Estimate** | 2-3 hours | -| **Description** | Update the OpenAI extension's README.md with documentation and usage examples for both new methods. | -| **Acceptance Criteria** | - `send_chat_history_async()` documented with code example
- `send_chat_history_messages_async()` documented with code example
- Error handling best practices documented
- Prerequisites and installation instructions updated
- API reference section added | -| **Files to Create/Modify** | - `libraries/microsoft-agents-a365-tooling-extensions-openai/README.md` | -| **Dependencies** | Task 2.6, Task 2.7 | -| **Testing Requirements** | Code examples are syntactically correct | - ---- - -#### Task 4.3: Update Package CHANGELOG - -| Attribute | Details | -|-----------|---------| -| **Title** | Update CHANGELOG.md | -| **Estimate** | 1 hour | -| **Description** | Add changelog entry for the new API methods following the project's changelog format. | -| **Acceptance Criteria** | - New version entry added
- `send_chat_history_async()` listed as new feature
- `send_chat_history_messages_async()` listed as new feature
- Breaking changes section confirms none
- Follows existing changelog format | -| **Files to Create/Modify** | - `libraries/microsoft-agents-a365-tooling-extensions-openai/CHANGELOG.md` (create if not exists) | -| **Dependencies** | Task 2.6, Task 2.7 | -| **Testing Requirements** | N/A | - ---- - -#### Task 4.4: Code Review and Linting - -| Attribute | Details | -|-----------|---------| -| **Title** | Final Code Review and Linting Compliance | -| **Estimate** | 2-3 hours | -| **Description** | Run all linting tools, address any issues, and perform final code review to ensure code quality and consistency with existing patterns. | -| **Acceptance Criteria** | - `ruff check` passes with no errors
- `ruff format` applied
- No new security vulnerabilities
- Code follows existing project patterns
- Self-review completed
- Ready for PR | -| **Files to Create/Modify** | - All modified files | -| **Dependencies** | All previous tasks | -| **Testing Requirements** | `ruff check .` and `ruff format --check .` | - ---- - -#### Task 4.5: Final Integration Verification - -| Attribute | Details | -|-----------|---------| -| **Title** | Final Integration Verification | -| **Estimate** | 1-2 hours | -| **Description** | Perform final end-to-end verification that all tests pass, documentation is complete, and the implementation meets all PRD requirements. | -| **Acceptance Criteria** | - All 33+ unit tests pass
- All 3 integration tests pass
- Coverage targets met
- Documentation complete
- PR checklist completed
- All functional requirements (FR-01 to FR-14) verified
- All acceptance criteria (AC-01 to AC-16) verified | -| **Files to Create/Modify** | N/A | -| **Dependencies** | All previous tasks | -| **Testing Requirements** | Full test suite run | - ---- - -## 4. Task Dependencies - -### 4.1 Dependency Diagram - -``` - ┌──────────────────────┐ - │ Task 1.1 │ - │ Analyze OpenAI │ - │ SDK Types │ - └──────────────────────┘ - │ - ┌───────────────┼───────────────┐ - │ │ │ - ▼ ▼ ▼ - ┌──────────────┐ ┌──────────────┐ ┌──────────────┐ - │ Task 1.2 │ │ Task 1.3 │ │ Task 2.3 │ - │ Add Deps │ │ Test Setup │ │ ID/Time │ - └──────────────┘ └──────────────┘ │ Helpers │ - │ │ └──────────────┘ - │ │ │ - ▼ │ │ - ┌──────────────┐ │ │ - │ Task 2.1 │◄────────┘ │ - │ Role │ │ - │ Extraction │ │ - └──────────────┘ │ - │ │ - ▼ │ - ┌──────────────┐ │ - │ Task 2.2 │ │ - │ Content │ │ - │ Extraction │ │ - └──────────────┘ │ - │ │ - └────────────────┬───────────────┘ - │ - ▼ - ┌──────────────────────┐ - │ Task 2.4 │ - │ Single Message │ - │ Conversion │ - └──────────────────────┘ - │ - ▼ - ┌──────────────────────┐ - │ Task 2.5 │ - │ Batch Conversion │ - └──────────────────────┘ - │ - ▼ - ┌──────────────────────┐ - │ Task 2.6 │ - │ send_chat_history │ - │ _messages_async │ - └──────────────────────┘ - │ - ▼ - ┌──────────────────────┐ - │ Task 2.7 │ - │ send_chat_history │ - │ _async │ - └──────────────────────┘ - │ - ┌───────────────┬───────┴───────┬───────────────┐ - │ │ │ │ - ▼ ▼ ▼ ▼ -┌──────────┐ ┌──────────┐ ┌──────────┐ ┌──────────┐ -│Task 3.1 │ │Task 3.2 │ │Task 3.3 │ │Task 3.4 │ -│Input │ │Conversion│ │Success │ │Error │ -│Validation│ │Tests │ │Path Tests│ │Handling │ -│Tests │ │ │ │ │ │Tests │ -└──────────┘ └──────────┘ └──────────┘ └──────────┘ - │ │ │ │ - └───────────────┴───────┬───────┴───────────────┘ - │ - ▼ - ┌──────────────────────┐ - │ Task 3.5 │ - │ Integration Tests │ - └──────────────────────┘ - │ - ▼ - ┌──────────────────────┐ - │ Task 3.6 │ - │ Coverage & │ - │ Performance │ - └──────────────────────┘ - │ - ┌───────────────┬───────┴───────┬───────────────┐ - │ │ │ │ - ▼ ▼ ▼ ▼ -┌──────────┐ ┌──────────┐ ┌──────────┐ ┌──────────┐ -│Task 4.1 │ │Task 4.2 │ │Task 4.3 │ │Task 4.4 │ -│Docstrings│ │README │ │CHANGELOG │ │Linting │ -└──────────┘ └──────────┘ └──────────┘ └──────────┘ - │ │ │ │ - └───────────────┴───────┬───────┴───────────────┘ - │ - ▼ - ┌──────────────────────┐ - │ Task 4.5 │ - │ Final Verification │ - └──────────────────────┘ -``` - -### 4.2 Critical Path - -The critical path for this implementation is: - -``` -Task 1.1 → Task 1.2 → Task 2.1 → Task 2.2 → Task 2.4 → Task 2.5 → Task 2.6 → Task 2.7 → Task 3.3 → Task 3.6 → Task 4.5 -``` - -**Estimated Critical Path Duration**: 24-32 hours - -### 4.3 Parallelization Opportunities - -The following tasks can be done in parallel: - -| Parallel Group | Tasks | Notes | -|----------------|-------|-------| -| After Task 1.1 | Task 1.2, Task 1.3, Task 2.3 | Infrastructure setup | -| After Task 2.7 | Task 3.1, Task 3.2, Task 3.3, Task 3.4 | Unit test implementation | -| After Task 3.6 | Task 4.1, Task 4.2, Task 4.3, Task 4.4 | Documentation and finalization | - ---- - -## 5. Testing Strategy Overview - -### 5.1 Test Categories Summary - -| Category | Test Count | Coverage Target | Priority | -|----------|------------|-----------------|----------| -| Input Validation (UV) | 9 tests | 100% validation paths | P0 | -| Conversion Logic (CV) | 12 tests | 100% conversion scenarios | P0 | -| Success Path (SP) | 7 tests | All happy paths | P0 | -| Error Handling (EH) | 5 tests | All error conditions | P0 | -| Integration (IT) | 3 tests | End-to-end flows | P0 | -| Performance | 2+ tests | Regression detection | P1 | -| **Total** | **38+ tests** | **≥95% lines, ≥90% branches** | | - -### 5.2 Test File Structure - -``` -tests/ -└── tooling/ - └── extensions/ - └── openai/ - ├── __init__.py - ├── conftest.py # Shared fixtures - ├── test_send_chat_history_async.py # UV, SP, EH tests (both methods) - ├── test_message_conversion.py # CV tests - ├── test_integration.py # IT tests - └── test_performance.py # Performance benchmarks -``` - -### 5.3 Mocking Strategy - -| Component | Mock Approach | -|-----------|---------------| -| OpenAI Message Types | Use `Mock` objects with role/content attributes | -| OpenAI Session | Use `Mock` with `get_items()` returning mock messages | -| TurnContext | Use `Mock(spec=TurnContext)` for interface compliance | -| McpToolServerConfigurationService | Use `AsyncMock` for `send_chat_history()` | -| HTTP Layer | Mock `aiohttp.ClientSession` for integration tests | - -### 5.4 Coverage Requirements - -```python -# pyproject.toml coverage configuration -[tool.pytest.ini_options] -addopts = "--cov=microsoft_agents_a365.tooling.extensions.openai --cov-report=html --cov-fail-under=95" -``` - ---- - -## 6. Risks and Considerations - -### 6.1 Technical Risks - -| Risk | Likelihood | Impact | Mitigation | -|------|------------|--------|------------| -| **OpenAI SDK Breaking Changes** | Medium | High | Pin version, use duck typing, create adapter layer | -| **Type Union Complexity** | Medium | Medium | Comprehensive type checking, runtime isinstance checks | -| **Performance Regression** | Low | Medium | Add benchmark tests in CI | -| **Thread Safety Issues** | Low | High | Stateless design, no shared mutable state | - -### 6.2 Schedule Risks - -| Risk | Likelihood | Impact | Mitigation | -|------|------------|--------|------------| -| **OpenAI SDK Research Takes Longer** | Medium | Medium | Time-box research to 4 hours max | -| **Test Coverage Gap Discovery** | Low | Medium | Continuous coverage monitoring | -| **Code Review Delays** | Low | Low | Early reviewer engagement | - -### 6.3 Integration Risks - -| Risk | Likelihood | Impact | Mitigation | -|------|------------|--------|------------| -| **Core Service API Changes** | Low | High | No planned changes to core service | -| **Dependency Version Conflicts** | Low | Medium | Use flexible version constraints | - -### 6.4 Open Questions to Resolve - -| Question | Resolution Needed By | Impact if Unresolved | -|----------|---------------------|---------------------| -| Full enumeration of `TResponseInputItem` types | Task 1.1 | May miss some message types | -| Session.get_items() async vs sync | Task 1.1 | Implementation approach | -| Content validation (empty string allowed?) | Task 2.2 | May need to relax ChatHistoryMessage validator | - -### 6.5 Assumptions - -1. The `openai-agents` package is available and stable -2. `Session.get_items()` method exists and returns `List[TResponseInputItem]` -3. The core `McpToolServerConfigurationService.send_chat_history()` remains unchanged -4. Empty content strings should be handled gracefully (may require relaxing `ChatHistoryMessage.content` validator) - ---- - -## Appendix: Task Checklist - -### Phase 1: Foundation & Infrastructure -- [ ] Task 1.1: Analyze OpenAI SDK Types and Create Type Stubs -- [ ] Task 1.2: Add openai-agents Dependency -- [ ] Task 1.3: Create Test Directory Structure and Fixtures - -### Phase 2: Core Implementation -- [ ] Task 2.1: Implement `_extract_role()` Private Method -- [ ] Task 2.2: Implement `_extract_content()` Private Method -- [ ] Task 2.3: Implement `_extract_id()` and `_extract_timestamp()` Private Methods -- [ ] Task 2.4: Implement `_convert_single_message()` Private Method -- [ ] Task 2.5: Implement `_convert_openai_messages_to_chat_history()` Private Method -- [ ] Task 2.6: Implement `send_chat_history_messages_async()` Public Method -- [ ] Task 2.7: Implement `send_chat_history_async()` Public Method - -### Phase 3: Testing -- [ ] Task 3.1: Implement Input Validation Unit Tests (UV-01 to UV-09) -- [ ] Task 3.2: Implement Conversion Logic Unit Tests (CV-01 to CV-12) -- [ ] Task 3.3: Implement Success Path Unit Tests (SP-01 to SP-07) -- [ ] Task 3.4: Implement Error Handling Unit Tests (EH-01 to EH-05) -- [ ] Task 3.5: Implement Integration Tests (IT-01 to IT-03) -- [ ] Task 3.6: Verify Test Coverage and Add Performance Benchmarks - -### Phase 4: Documentation & Finalization -- [ ] Task 4.1: Complete Method Docstrings and Type Hints -- [ ] Task 4.2: Update README.md with Usage Examples -- [ ] Task 4.3: Update CHANGELOG.md -- [ ] Task 4.4: Final Code Review and Linting Compliance -- [ ] Task 4.5: Final Integration Verification - ---- - -## Document History - -| Version | Date | Author | Changes | -|---------|------|--------|---------| -| 1.0 | January 21, 2026 | PRD Task Generator Agent | Initial task breakdown | diff --git a/docs/prd/openai-send-chat-history-api.md b/docs/prd/openai-send-chat-history-api.md deleted file mode 100644 index 2ca0170d..00000000 --- a/docs/prd/openai-send-chat-history-api.md +++ /dev/null @@ -1,1027 +0,0 @@ -# Product Requirements Document (PRD) - -## OpenAI `send_chat_history_async` API for Agent365-python SDK - -| **Document Information** | | -|--------------------------|----------------------------------------------| -| **Version** | 1.0 | -| **Status** | Draft | -| **Author** | Agent365 Python SDK Team | -| **Created** | January 21, 2026 | -| **Last Updated** | January 21, 2026 | -| **Target Package** | `microsoft-agents-a365-tooling-extensions-openai` | - ---- - -## Table of Contents - -1. [Overview and Business Justification](#1-overview-and-business-justification) -2. [Objectives](#2-objectives) -3. [User Stories](#3-user-stories) -4. [Functional Requirements](#4-functional-requirements) -5. [Technical Requirements](#5-technical-requirements) -6. [Package Impact Analysis](#6-package-impact-analysis) -7. [API Design](#7-api-design) -8. [Observability Requirements](#8-observability-requirements) -9. [Testing Strategy](#9-testing-strategy) -10. [Acceptance Criteria](#10-acceptance-criteria) -11. [Non-Functional Requirements](#11-non-functional-requirements) -12. [Dependencies](#12-dependencies) -13. [Risks and Mitigations](#13-risks-and-mitigations) -14. [Open Questions](#14-open-questions) - ---- - -## 1. Overview and Business Justification - -### 1.1 Problem Statement - -The Agent365-python SDK currently provides a core `send_chat_history` method in `McpToolServerConfigurationService` that sends conversation history to the MCP platform for real-time threat protection. However, this method requires developers using the OpenAI Agents SDK to manually convert OpenAI-native types (such as `Session`, `TResponseInputItem`, `UserMessage`, `AssistantMessage`, etc.) to the SDK's `ChatHistoryMessage` format. - -This manual conversion creates: -- **Developer friction**: Extra boilerplate code to transform OpenAI types -- **Inconsistency risk**: Different developers may implement conversion logic differently -- **Error-prone integrations**: Missing ID or timestamp handling may vary -- **Feature parity gap**: The .NET SDK already provides framework-specific `SendChatHistoryAsync` methods for Agent Framework (PR #171) and Semantic Kernel (PR #173) - -### 1.2.1 Method Naming Convention - -This PRD defines two methods with the following naming convention: -- **`send_chat_history_messages_async`**: Accepts a list of OpenAI `TResponseInputItem` messages directly -- **`send_chat_history_async`**: Extracts messages from an OpenAI `Session` object (the more common use case) - -### 1.2 Proposed Solution - -Implement OpenAI-specific chat history APIs in the `microsoft-agents-a365-tooling-extensions-openai` package that: -- Accepts OpenAI SDK native types directly (Session protocol items, message types) -- Handles all conversion logic internally -- Provides a seamless developer experience for OpenAI Agents SDK users -- Maintains feature parity with the .NET SDK - -### 1.3 Business Value - -| Value Driver | Description | -|--------------|-------------| -| **Developer Experience** | Reduces integration effort from ~20 lines of conversion code to a single method call | -| **Adoption** | Lowers barrier to entry for OpenAI SDK developers adopting Agent365 | -| **Consistency** | Ensures standardized handling of missing IDs, timestamps, and role mapping | -| **Feature Parity** | Aligns Python SDK capabilities with .NET SDK | -| **Enterprise Readiness** | Enables real-time threat protection for OpenAI-based agents with minimal effort | - -### 1.4 Success Metrics - -| Metric | Target | -|--------|--------| -| API adoption rate | 80% of OpenAI extension users use `send_chat_history_async` or `send_chat_history_messages_async` within 3 months | -| Code reduction | Average 15+ lines of boilerplate eliminated per integration | -| Test coverage | ≥95% line coverage, ≥90% branch coverage | -| Documentation completeness | 100% of public APIs documented with examples | - ---- - -## 2. Objectives - -### 2.1 Primary Objectives - -1. **O1**: Provide OpenAI-native API for sending chat history to the MCP platform -2. **O2**: Support OpenAI Session protocol items (`TResponseInputItem` types) -3. **O3**: Support direct list of OpenAI message types -4. **O4**: Maintain backward compatibility with existing `McpToolServerConfigurationService` -5. **O5**: Achieve feature parity with .NET SDK implementation - -### 2.2 Secondary Objectives - -1. **O6**: Provide extensible design for future OpenAI SDK version support -2. **O7**: Enable observability integration for tracing chat history operations -3. **O8**: Support both synchronous and asynchronous usage patterns - -### 2.3 Out of Scope - -- Modifications to the core `McpToolServerConfigurationService` -- Support for other orchestrator SDKs (covered by separate extensions) -- Persistent storage of chat history (handled by MCP platform) -- Chat history retrieval APIs (read operations) - ---- - -## 3. User Stories - -### 3.1 Primary User Stories - -| ID | User Story | Priority | Acceptance Criteria | -|----|------------|----------|---------------------| -| **US-01** | As an OpenAI agent developer, I want to send my agent's Session history to the MCP platform so that my conversations are protected by real-time threat detection | P0 | Session items are converted and sent successfully | -| **US-02** | As an OpenAI agent developer, I want to send a list of messages to the MCP platform without manual conversion so that I can focus on agent logic | P0 | List of OpenAI messages converts and sends correctly | -| **US-03** | As an OpenAI agent developer, I want missing message IDs to be auto-generated so that I don't need to track IDs manually | P0 | UUIDs generated for messages without IDs | -| **US-04** | As an OpenAI agent developer, I want missing timestamps to use current UTC time so that all messages have valid timestamps | P0 | Current UTC timestamp used when not provided | -| **US-05** | As an OpenAI agent developer, I want to receive clear success/failure results so that I can handle errors appropriately | P0 | `OperationResult` returned with error details on failure | -| **US-05a** | As an OpenAI agent developer, I want to send my Session's conversation history directly so that I don't need to extract messages manually | P0 | `send_chat_history_async` extracts and sends Session items | - -### 3.2 Secondary User Stories - -| ID | User Story | Priority | Acceptance Criteria | -|----|------------|----------|---------------------| -| **US-06** | As an OpenAI agent developer, I want to pass custom `ToolOptions` so that I can customize orchestrator identification | P1 | ToolOptions parameter accepted and applied | -| **US-07** | As an OpenAI agent developer, I want detailed logging of conversion operations so that I can debug issues | P1 | Debug-level logs for conversion operations | -| **US-08** | As an OpenAI agent developer, I want support for all standard OpenAI message roles so that system, user, and assistant messages are handled | P1 | All roles mapped correctly | - ---- - -## 4. Functional Requirements - -### 4.1 Core Functional Requirements - -| ID | Requirement | Priority | Verification Method | -|----|-------------|----------|---------------------| -| **FR-01** | The API SHALL accept OpenAI `TResponseInputItem` types and convert them to `ChatHistoryMessage` | P0 | Unit tests | -| **FR-02** | The API SHALL support `UserMessage`, `AssistantMessage`, and `SystemMessage` OpenAI types | P0 | Unit tests | -| **FR-03** | The API SHALL generate a UUID for messages without an ID | P0 | Unit tests | -| **FR-04** | The API SHALL use `datetime.now(timezone.utc)` for messages without a timestamp | P0 | Unit tests | -| **FR-05** | The API SHALL delegate to `McpToolServerConfigurationService.send_chat_history` | P0 | Integration tests | -| **FR-06** | The API SHALL return `OperationResult` indicating success or failure | P0 | Unit tests | -| **FR-07** | The API SHALL validate that `turn_context` is not None | P0 | Unit tests | -| **FR-08** | The API SHALL allow empty message lists (no-op, return success) | P0 | Unit tests | -| **FR-09** | The API SHALL map OpenAI roles to `ChatHistoryMessage` roles ("user", "assistant", "system") | P0 | Unit tests | -| **FR-10** | The API SHALL extract text content from OpenAI message content arrays | P0 | Unit tests | -| **FR-11** | The API SHALL provide a `send_chat_history_async` method that accepts an OpenAI Session | P0 | Unit tests | -| **FR-12** | The `send_chat_history_async` method SHALL call `session.get_items()` to retrieve messages | P0 | Unit tests | -| **FR-13** | The `send_chat_history_async` method SHALL support an optional `limit` parameter for `get_items()` | P1 | Unit tests | -| **FR-14** | The API SHALL include all item types from the session without filtering | P0 | Unit tests | - -### 4.2 Method Signatures - -The implementation SHALL provide the following method overloads: - -```python -# Primary method - extracts messages from OpenAI Session (most common use case) -async def send_chat_history_async( - self, - turn_context: TurnContext, - session: Session, - limit: Optional[int] = None, - options: Optional[ToolOptions] = None, -) -> OperationResult: - """ - Extracts chat history from an OpenAI Session and sends it to the MCP platform. - - Args: - turn_context: TurnContext from the Agents SDK containing conversation info. - session: OpenAI Session instance to extract messages from. - limit: Optional maximum number of items to retrieve from session. - If None, retrieves all items. - options: Optional ToolOptions for customization. - - Returns: - OperationResult indicating success or failure. - """ - -# Secondary method - accepts list of OpenAI message types directly -async def send_chat_history_messages_async( - self, - turn_context: TurnContext, - messages: List[TResponseInputItem], - options: Optional[ToolOptions] = None, -) -> OperationResult: - """ - Sends OpenAI chat history to the MCP platform for threat protection. - - Args: - turn_context: TurnContext from the Agents SDK containing conversation info. - messages: List of OpenAI TResponseInputItem messages to send. - options: Optional ToolOptions for customization. - - Returns: - OperationResult indicating success or failure. - """ -``` - -### 4.3 Role Mapping - -| OpenAI Type | ChatHistoryMessage Role | -|-------------|------------------------| -| `UserMessage` | `"user"` | -| `AssistantMessage` | `"assistant"` | -| `SystemMessage` | `"system"` | -| `ResponseOutputMessage` with role="assistant" | `"assistant"` | -| Other/Unknown | `"user"` (default fallback with warning log) | - -### 4.4 Content Extraction - -The API SHALL extract text content following this priority: -1. If message has `.content` as string → use directly -2. If message has `.content` as list → concatenate all text parts -3. If message has `.text` attribute → use directly -4. If content is empty/None → use empty string with warning log - ---- - -## 5. Technical Requirements - -### 5.1 Language and Runtime - -| Requirement | Specification | -|-------------|---------------| -| Python version | ≥3.10 | -| Async support | Full `async/await` pattern | -| Type hints | Complete type annotations (PEP 484) | -| Pydantic version | ≥2.0 (for model validation) | - -### 5.1.1 Type Hints - NEVER Use `Any` - -**CRITICAL**: The use of `typing.Any` is **strictly forbidden** in this codebase. Using `Any` defeats the purpose of type checking and can hide bugs. - -**Required alternatives (in order of preference):** - -| Instead of `Any` | Use | -|------------------|-----| -| External SDK types | Import and use actual types from the SDK (e.g., `Session`, `TResponseInputItem`) | -| Multiple known types | `Union[Type1, Type2, ...]` | -| Pass-through data | `object` | -| Dictionary values | `Dict[str, object]` or specific types | -| Unknown external types (last resort) | `Protocol` - but confirm with developer first | - -**Preferred approach - Use actual SDK types:** - -```python -from agents.memory import Session -from agents.items import TResponseInputItem - -async def send_chat_history_async( - self, - turn_context: TurnContext, - session: Session, # Use actual SDK type -) -> OperationResult: - ... - -async def send_chat_history_messages_async( - self, - turn_context: TurnContext, - messages: List[TResponseInputItem], # Use actual SDK type -) -> OperationResult: - ... -``` - -**Why actual SDK types are preferred:** -- Better IDE support (autocomplete, type checking) -- Ensures compatibility with the external SDK -- Less maintenance burden (no custom protocols to keep in sync) -- Clearer intent for developers reading the code - -**When to use Protocol (last resort only):** -If external types cannot be found or imported, a `Protocol` may be defined. However, this should be rare and requires confirmation with the developer before proceeding, as it may indicate a missing dependency or incorrect understanding of the external API. - -This requirement applies to both production code AND test files. - -### 5.2 OpenAI SDK Compatibility - -| Requirement | Specification | -|-------------|---------------| -| OpenAI Agents SDK | Compatible with `openai-agents` package | -| Message types | Support `TResponseInputItem` union type | -| Session protocol | Support items from `Session.get_items()` | - -### 5.3 Error Handling - -| Error Condition | Expected Behavior | -|-----------------|-------------------| -| `turn_context` is None | Raise `ValueError` with descriptive message | -| `messages` is None | Raise `ValueError` with descriptive message | -| `messages` is empty list | Return `OperationResult.success()` (no-op) | -| `turn_context.activity` is None | Raise `ValueError` with descriptive message | -| Missing conversation ID | Raise `ValueError` with field path | -| Missing message ID | Raise `ValueError` with field path | -| Missing user message text | Raise `ValueError` with field path | -| HTTP error from MCP platform | Return `OperationResult.failed()` with error | -| Network timeout | Return `OperationResult.failed()` with error | -| Conversion error | Return `OperationResult.failed()` with error | - -### 5.4 Thread Safety - -- The service class SHALL be thread-safe for concurrent calls -- State SHALL NOT be shared between method invocations -- Logger instance SHALL be initialized per instance - ---- - -## 6. Package Impact Analysis - -### 6.1 Modified Package - -**Package**: `microsoft-agents-a365-tooling-extensions-openai` - -| File | Change Type | Description | -|------|-------------|-------------| -| `mcp_tool_registration_service.py` | Modified | Add `send_chat_history_async` method | -| `__init__.py` | Modified | Export new types if needed | - -### 6.2 New Files - -| File | Purpose | -|------|---------| -| `tests/tooling/extensions/openai/test_send_chat_history_async.py` | Unit tests | -| `tests/tooling/extensions/openai/test_message_conversion.py` | Conversion logic tests | - -### 6.3 Dependency Changes - -**New Dependencies (in `pyproject.toml`)**: - -```toml -[project.dependencies] -# Existing dependencies... -openai-agents = ">=0.1.0" # For type definitions -``` - -### 6.4 Package Exports - -Update `__init__.py` to export: -- `McpToolRegistrationService` (already exported, method added) - ---- - -## 7. API Design - -### 7.1 Class Diagram - -``` -┌─────────────────────────────────────────────────────────────────────┐ -│ McpToolRegistrationService │ -├─────────────────────────────────────────────────────────────────────┤ -│ - _orchestrator_name: str = "OpenAI" │ -│ - _logger: logging.Logger │ -│ - config_service: McpToolServerConfigurationService │ -├─────────────────────────────────────────────────────────────────────┤ -│ + __init__(logger: Optional[Logger]) │ -│ + add_tool_servers_to_agent(...) -> Agent │ -│ + send_chat_history_async( │ -│ turn_context: TurnContext, │ -│ session: Session, │ -│ limit: Optional[int], │ -│ options: Optional[ToolOptions] │ -│ ) -> OperationResult │ -│ + send_chat_history_messages_async( │ -│ turn_context: TurnContext, │ -│ messages: List[TResponseInputItem], │ -│ options: Optional[ToolOptions] │ -│ ) -> OperationResult │ -│ - _convert_openai_messages_to_chat_history( │ -│ messages: List[TResponseInputItem] │ -│ ) -> List[ChatHistoryMessage] │ -│ - _convert_single_message( │ -│ message: TResponseInputItem │ -│ ) -> Optional[ChatHistoryMessage] │ -│ - _extract_role(message: TResponseInputItem) -> str │ -│ - _extract_content(message: TResponseInputItem) -> str │ -│ - _extract_id(message: TResponseInputItem) -> str │ -│ - _extract_timestamp(message: TResponseInputItem) -> datetime │ -└─────────────────────────────────────────────────────────────────────┘ - │ - │ delegates to - ▼ -┌─────────────────────────────────────────────────────────────────────┐ -│ McpToolServerConfigurationService │ -├─────────────────────────────────────────────────────────────────────┤ -│ + send_chat_history( │ -│ turn_context: TurnContext, │ -│ chat_history_messages: List[ChatHistoryMessage], │ -│ options: Optional[ToolOptions] │ -│ ) -> OperationResult │ -└─────────────────────────────────────────────────────────────────────┘ -``` - -### 7.2 Sequence Diagram - -``` -Developer McpToolRegistrationService McpToolServerConfigurationService MCP Platform - │ │ │ │ - │ send_chat_history_ │ │ │ - │ messages_async │ │ │ - │──────────────────────>│ │ │ - │ │ │ │ - │ │ validate inputs │ │ - │ │──────────────┐ │ │ - │ │ │ │ │ - │ │<─────────────┘ │ │ - │ │ │ │ - │ │ _convert_openai_messages_to_chat_history │ - │ │──────────────┐ │ │ - │ │ │ for each message: │ - │ │ │ - extract role │ - │ │ │ - extract content │ - │ │ │ - extract/generate ID │ - │ │ │ - extract/generate timestamp │ - │ │<─────────────┘ │ │ - │ │ │ │ - │ │ send_chat_history │ │ - │ │─────────────────────────────>│ │ - │ │ │ │ - │ │ │ POST /chat-message │ - │ │ │──────────────────────────────>│ - │ │ │ │ - │ │ │ HTTP 200 / Error │ - │ │ │<──────────────────────────────│ - │ │ │ │ - │ │ OperationResult │ │ - │ │<─────────────────────────────│ │ - │ │ │ │ - │ OperationResult │ │ │ - │<──────────────────────│ │ │ - │ │ │ │ -``` - -### 7.3 Data Models - -#### 7.3.1 Existing Models (No Changes) - -```python -# From microsoft_agents_a365.tooling.models - -class ChatHistoryMessage(BaseModel): - """Represents a single message in the chat history.""" - - id: Optional[str] = Field(default=None) - role: Literal["user", "assistant", "system"] - content: str - timestamp: Optional[datetime] = Field(default=None) - - -class ToolOptions: - """Configuration options for tooling operations.""" - - orchestrator_name: Optional[str] -``` - -#### 7.3.2 OpenAI Types (External, for reference) - -```python -# From openai-agents SDK (external types for reference) - -TResponseInputItem = Union[ - EasyInputMessage, - ResponseOutputMessage, - ResponseFileSearchToolCall, - ResponseFunctionWebSearch, - ResponseComputerToolCall, - ResponseFunctionToolCall, - ResponseFunctionToolCallOutput, - ResponseReasoningItem, - ItemReference, - # ... other types -] - -class UserMessage: - role: Literal["user"] - content: Union[str, List[ContentPart]] - -class AssistantMessage: - role: Literal["assistant"] - content: Union[str, List[ContentPart]] - -class SystemMessage: - role: Literal["system"] - content: Union[str, List[ContentPart]] -``` - -### 7.4 Method Implementation Pseudocode - -```python -async def send_chat_history_async( - self, - turn_context: TurnContext, - session: Session, - limit: Optional[int] = None, - options: Optional[ToolOptions] = None, -) -> OperationResult: - """ - Extracts chat history from an OpenAI Session and sends it to the MCP platform. - """ - # Validate inputs - if turn_context is None: - raise ValueError("turn_context cannot be None") - if session is None: - raise ValueError("session cannot be None") - - try: - # Extract messages from session - messages = await session.get_items(limit=limit) - - # Delegate to the list-based method - return await self.send_chat_history_messages_async( - turn_context=turn_context, - messages=messages, - options=options, - ) - except ValueError: - # Re-raise validation errors - raise - except Exception as ex: - self._logger.error(f"Failed to send chat history: {ex}") - return OperationResult.failed(OperationError(ex)) - - -async def send_chat_history_messages_async( - self, - turn_context: TurnContext, - messages: List[TResponseInputItem], - options: Optional[ToolOptions] = None, -) -> OperationResult: - """ - Sends OpenAI chat history to the MCP platform for threat protection. - """ - # Validate inputs - if turn_context is None: - raise ValueError("turn_context cannot be None") - if messages is None: - raise ValueError("messages cannot be None") - - # Handle empty list as no-op - if len(messages) == 0: - self._logger.info("Empty message list provided, returning success") - return OperationResult.success() - - # Set default options - if options is None: - options = ToolOptions(orchestrator_name=self._orchestrator_name) - elif options.orchestrator_name is None: - options.orchestrator_name = self._orchestrator_name - - try: - # Convert OpenAI messages to ChatHistoryMessage format - chat_history_messages = self._convert_openai_messages_to_chat_history(messages) - - # Delegate to core service - return await self.config_service.send_chat_history( - turn_context=turn_context, - chat_history_messages=chat_history_messages, - options=options, - ) - except ValueError: - # Re-raise validation errors - raise - except Exception as ex: - self._logger.error(f"Failed to send chat history messages: {ex}") - return OperationResult.failed(OperationError(ex)) -``` - ---- - -## 8. Observability Requirements - -### 8.1 Logging Requirements - -| Level | When | Message Template | -|-------|------|------------------| -| INFO | Method entry | `"Sending {count} OpenAI messages as chat history"` | -| INFO | Success | `"Successfully sent chat history with {count} messages"` | -| DEBUG | Per-message conversion | `"Converting message: role={role}, has_id={has_id}, has_timestamp={has_ts}"` | -| DEBUG | ID generation | `"Generated UUID {id} for message without ID"` | -| DEBUG | Timestamp generation | `"Using current UTC time for message without timestamp"` | -| WARNING | Unknown role | `"Unknown message type {type}, defaulting to 'user' role"` | -| WARNING | Empty content | `"Message has empty content, using empty string"` | -| ERROR | Conversion failure | `"Failed to convert message: {error}"` | -| ERROR | Send failure | `"Failed to send chat history: {error}"` | - -### 8.2 Metrics (Future Enhancement) - -| Metric Name | Type | Description | -|-------------|------|-------------| -| `a365.tooling.openai.send_chat_history.count` | Counter | Total number of send operations | -| `a365.tooling.openai.send_chat_history.success` | Counter | Successful send operations | -| `a365.tooling.openai.send_chat_history.failure` | Counter | Failed send operations | -| `a365.tooling.openai.send_chat_history.messages` | Histogram | Messages per batch | -| `a365.tooling.openai.send_chat_history.duration_ms` | Histogram | Operation duration | - -### 8.3 Tracing Integration - -The method SHOULD integrate with the existing observability framework: - -```python -# Future enhancement - integrate with ExecuteToolScope -from microsoft_agents_a365.observability.core import ExecuteToolScope, ToolCallDetails - -with ExecuteToolScope.start( - tool_call_details=ToolCallDetails( - tool_name="send_chat_history", - tool_arguments={"message_count": len(messages)}, - ) -) as scope: - result = await self._send_internal(...) - scope.record_response(str(result)) -``` - ---- - -## 9. Testing Strategy - -### 9.1 Test Categories - -| Category | Coverage Target | Focus | -|----------|-----------------|-------| -| Unit Tests | ≥95% lines | Method logic, conversion, validation | -| Integration Tests | Key flows | End-to-end with mocked HTTP | -| Edge Case Tests | 100% identified cases | Null handling, empty content, unknown types | - -### 9.2 Unit Test Cases - -#### 9.2.1 Input Validation Tests - -| Test ID | Test Name | Description | -|---------|-----------|-------------| -| UV-01 | `test_send_chat_history_messages_async_validates_turn_context_none` | Verify ValueError when turn_context is None | -| UV-02 | `test_send_chat_history_messages_async_validates_messages_none` | Verify ValueError when messages is None | -| UV-03 | `test_send_chat_history_messages_async_empty_list_returns_success` | Verify empty list returns success (no-op) | -| UV-04 | `test_send_chat_history_messages_async_validates_activity_none` | Verify ValueError when activity is None | -| UV-05 | `test_send_chat_history_messages_async_validates_conversation_id` | Verify ValueError when conversation.id missing | -| UV-06 | `test_send_chat_history_messages_async_validates_message_id` | Verify ValueError when activity.id missing | -| UV-07 | `test_send_chat_history_messages_async_validates_user_message` | Verify ValueError when activity.text missing | -| UV-08 | `test_send_chat_history_async_validates_turn_context_none` | Verify ValueError when turn_context is None | -| UV-09 | `test_send_chat_history_async_validates_session_none` | Verify ValueError when session is None | - -#### 9.2.2 Conversion Tests - -| Test ID | Test Name | Description | -|---------|-----------|-------------| -| CV-01 | `test_convert_user_message_to_chat_history` | UserMessage converts with role="user" | -| CV-02 | `test_convert_assistant_message_to_chat_history` | AssistantMessage converts with role="assistant" | -| CV-03 | `test_convert_system_message_to_chat_history` | SystemMessage converts with role="system" | -| CV-04 | `test_convert_message_with_string_content` | String content extracted directly | -| CV-05 | `test_convert_message_with_list_content` | List content concatenated | -| CV-06 | `test_convert_message_generates_uuid_when_id_missing` | UUID generated for missing ID | -| CV-07 | `test_convert_message_uses_utc_when_timestamp_missing` | UTC timestamp used when missing | -| CV-08 | `test_convert_message_preserves_existing_id` | Existing ID preserved | -| CV-09 | `test_convert_message_preserves_existing_timestamp` | Existing timestamp preserved | -| CV-10 | `test_convert_unknown_message_type_defaults_to_user` | Unknown type defaults to user role | -| CV-11 | `test_convert_empty_content_uses_empty_string` | Empty content handled gracefully | -| CV-12 | `test_convert_multiple_messages` | Multiple messages converted correctly | - -#### 9.2.3 Success Path Tests - -| Test ID | Test Name | Description | -|---------|-----------|-------------| -| SP-01 | `test_send_chat_history_messages_async_success` | Successful send returns succeeded=True | -| SP-02 | `test_send_chat_history_messages_async_with_options` | Custom ToolOptions applied | -| SP-03 | `test_send_chat_history_messages_async_default_orchestrator_name` | Default orchestrator name set | -| SP-04 | `test_send_chat_history_messages_async_delegates_to_config_service` | Delegation verified | -| SP-05 | `test_send_chat_history_async_success` | Session messages extracted and sent | -| SP-06 | `test_send_chat_history_async_with_limit` | Limit parameter passed to get_items | -| SP-07 | `test_send_chat_history_async_delegates_to_send_chat_history_messages` | Delegation verified | - -#### 9.2.4 Error Handling Tests - -| Test ID | Test Name | Description | -|---------|-----------|-------------| -| EH-01 | `test_send_chat_history_messages_async_http_error` | HTTP error returns failed result | -| EH-02 | `test_send_chat_history_messages_async_timeout_error` | Timeout returns failed result | -| EH-03 | `test_send_chat_history_messages_async_client_error` | Network error returns failed result | -| EH-04 | `test_send_chat_history_messages_async_conversion_error` | Conversion error returns failed result | -| EH-05 | `test_send_chat_history_async_get_items_error` | Session get_items error returns failed result | - -### 9.3 Integration Test Cases - -| Test ID | Test Name | Description | -|---------|-----------|-------------| -| IT-01 | `test_send_chat_history_async_end_to_end_success` | Full flow with mocked HTTP | -| IT-02 | `test_send_chat_history_async_end_to_end_server_error` | Full flow with HTTP 500 | -| IT-03 | `test_send_chat_history_async_payload_format` | Verify JSON payload structure | - -### 9.4 Test File Structure - -``` -tests/ -└── tooling/ - └── extensions/ - └── openai/ - ├── __init__.py - ├── test_send_chat_history_async.py # Main API tests - ├── test_message_conversion.py # Conversion logic tests - └── conftest.py # Shared fixtures -``` - -### 9.5 Sample Test Code - -```python -# tests/tooling/extensions/openai/test_send_chat_history_async.py - -import pytest -from datetime import datetime, timezone -from unittest.mock import AsyncMock, Mock, patch -import uuid - -from microsoft_agents.hosting.core import TurnContext -from microsoft_agents_a365.tooling.extensions.openai import McpToolRegistrationService -from microsoft_agents_a365.runtime import OperationResult - - -class TestSendChatHistoryMessagesAsync: - """Tests for send_chat_history_messages_async method.""" - - @pytest.fixture - def service(self): - """Create McpToolRegistrationService instance.""" - return McpToolRegistrationService() - - @pytest.fixture - def mock_turn_context(self): - """Create a mock TurnContext.""" - mock_context = Mock(spec=TurnContext) - mock_activity = Mock() - mock_conversation = Mock() - mock_conversation.id = "conv-123" - mock_activity.conversation = mock_conversation - mock_activity.id = "msg-456" - mock_activity.text = "Hello, how are you?" - mock_context.activity = mock_activity - return mock_context - - @pytest.fixture - def sample_openai_messages(self): - """Create sample OpenAI messages.""" - # Mock OpenAI message types - user_msg = Mock() - user_msg.role = "user" - user_msg.content = "Hello" - - assistant_msg = Mock() - assistant_msg.role = "assistant" - assistant_msg.content = "Hi there!" - - return [user_msg, assistant_msg] - - @pytest.mark.asyncio - async def test_send_chat_history_messages_async_validates_turn_context_none( - self, service, sample_openai_messages - ): - """Test that send_chat_history_messages_async validates turn_context.""" - with pytest.raises(ValueError, match="turn_context cannot be None"): - await service.send_chat_history_messages_async(None, sample_openai_messages) - - @pytest.mark.asyncio - async def test_send_chat_history_messages_async_success( - self, service, mock_turn_context, sample_openai_messages - ): - """Test successful send_chat_history_messages_async call.""" - with patch.object( - service.config_service, - 'send_chat_history', - new_callable=AsyncMock - ) as mock_send: - mock_send.return_value = OperationResult.success() - - result = await service.send_chat_history_messages_async( - mock_turn_context, - sample_openai_messages - ) - - assert result.succeeded is True - mock_send.assert_called_once() -``` - ---- - -## 10. Acceptance Criteria - -### 10.1 Functional Acceptance Criteria - -| ID | Criterion | Verification | -|----|-----------|--------------| -| AC-01 | `send_chat_history_async` method exists in `McpToolRegistrationService` | Code review | -| AC-01a | `send_chat_history_messages_async` method exists in `McpToolRegistrationService` | Code review | -| AC-02 | Method accepts `List[TResponseInputItem]` parameter | Type checker passes | -| AC-03 | Method returns `OperationResult` | Unit tests pass | -| AC-04 | Missing message IDs are generated as UUIDs | Unit tests pass | -| AC-05 | Missing timestamps use current UTC time | Unit tests pass | -| AC-06 | All OpenAI role types map correctly | Unit tests pass | -| AC-07 | Validation errors raise `ValueError` with descriptive messages | Unit tests pass | -| AC-07a | Empty message list returns `OperationResult.success()` | Unit tests pass | -| AC-08 | HTTP errors return `OperationResult.failed()` | Unit tests pass | -| AC-08a | `send_chat_history_async` calls `session.get_items()` correctly | Unit tests pass | - -### 10.2 Non-Functional Acceptance Criteria - -| ID | Criterion | Verification | -|----|-----------|--------------| -| AC-09 | Unit test coverage ≥95% | Coverage report | -| AC-10 | All public methods have docstrings | Code review | -| AC-11 | Type hints on all parameters and returns | Type checker passes | -| AC-12 | No new linting errors | `ruff check` passes | -| AC-13 | Code follows existing patterns | Code review | - -### 10.3 Documentation Acceptance Criteria - -| ID | Criterion | Verification | -|----|-----------|--------------| -| AC-14 | Method has complete docstring with Args/Returns/Raises | Code review | -| AC-15 | README.md updated with usage example | Documentation review | -| AC-16 | CHANGELOG.md updated | Documentation review | - ---- - -## 11. Non-Functional Requirements - -### 11.1 Performance - -| Requirement | Target | -|-------------|--------| -| Conversion overhead | <10ms for 100 messages | -| Memory overhead | <1MB for 1000 messages | -| No blocking calls | All I/O is async | -| Batch size | No limit - endpoint assumed to handle unlimited size | -| Performance benchmarks | Required in CI for regression detection | - -### 11.2 Reliability - -| Requirement | Target | -|-------------|--------| -| Graceful degradation | Return `OperationResult.failed()` on errors | -| No data loss | Messages not modified in place | -| Idempotent IDs | Same message without ID gets different UUID each call | - -### 11.3 Maintainability - -| Requirement | Target | -|-------------|--------| -| Cyclomatic complexity | <10 per method | -| Method length | <50 lines | -| Single responsibility | One public method, helper methods for conversion | - -### 11.4 Security - -| Requirement | Description | -|-------------|-------------| -| No credential logging | Auth tokens never logged | -| Input sanitization | Content not modified, passed through | -| Secure defaults | HTTPS enforced by underlying service | - ---- - -## 12. Dependencies - -### 12.1 Internal Dependencies - -| Package | Version | Purpose | -|---------|---------|---------| -| `microsoft-agents-a365-tooling` | ≥1.0.0 | Core `McpToolServerConfigurationService` | -| `microsoft-agents-a365-runtime` | ≥1.0.0 | `OperationResult`, `OperationError` | - -### 12.2 External Dependencies - -| Package | Version | Purpose | -|---------|---------|---------| -| `openai-agents` | ≥0.1.0 | OpenAI message types (`TResponseInputItem`) | -| `microsoft-agents-hosting-core` | ≥0.1.0 | `TurnContext`, `Authorization` | -| `pydantic` | ≥2.0.0 | Model validation | - -### 12.3 Development Dependencies - -| Package | Version | Purpose | -|---------|---------|---------| -| `pytest` | ≥7.0.0 | Test framework | -| `pytest-asyncio` | ≥0.21.0 | Async test support | -| `pytest-cov` | ≥4.0.0 | Coverage reporting | - ---- - -## 13. Risks and Mitigations - -### 13.1 Technical Risks - -| Risk | Likelihood | Impact | Mitigation | -|------|------------|--------|------------| -| OpenAI SDK type changes | Medium | High | Use duck typing, version pin, adapter pattern | -| Performance regression | Low | Medium | Benchmark tests, lazy evaluation | -| Thread safety issues | Low | High | Stateless design, no shared state | - -### 13.2 Schedule Risks - -| Risk | Likelihood | Impact | Mitigation | -|------|------------|--------|------------| -| Scope creep | Medium | Medium | Clear scope definition, change control | -| Testing delays | Medium | Medium | Parallel test development | -| Review delays | Low | Medium | Early reviewer engagement | - -### 13.3 Integration Risks - -| Risk | Likelihood | Impact | Mitigation | -|------|------------|--------|------------| -| Breaking existing API | Low | High | No changes to existing methods | -| Dependency conflicts | Low | Medium | Version ranges, compatibility testing | - ---- - -## 14. Open Questions - -### 14.1 Design Questions - -| ID | Question | Status | Decision | -|----|----------|--------|----------| -| OQ-01 | Should we support OpenAI Session protocol directly in addition to message lists? | **Resolved** | **Yes** - `send_chat_history_async` extracts messages from Session, `send_chat_history_messages_async` accepts message list directly | -| OQ-02 | Should we add a synchronous wrapper `send_chat_history` for convenience? | **Deferred** | Skip for now - evaluate in future iteration | -| OQ-03 | Should empty message lists be allowed (no-op) vs. raising ValueError? | **Resolved** | **Empty list allowed** - Return success with no-op | -| OQ-04 | Should we support custom ID generators (injectable)? | **Resolved** | **No** - Use UUID by default for simplicity | - -### 14.2 Implementation Questions - -| ID | Question | Status | Decision | -|----|----------|--------|----------| -| OQ-05 | Which specific OpenAI message types need support beyond User/Assistant/System? | Open | Need to enumerate `TResponseInputItem` union | -| OQ-06 | Should tool call/result messages be included in chat history? | **Resolved** | **Yes** - Include all items retrieved, no filtering required | -| OQ-07 | What is the maximum batch size for messages? | **Resolved** | **No limit assumed** - Assume endpoint handles unlimited size, no batching required | - -### 14.3 Testing Questions - -| ID | Question | Status | Decision | -|----|----------|--------|----------| -| OQ-08 | Should we add performance benchmarks to CI? | **Resolved** | **Yes** - Add performance benchmarks for regression detection | -| OQ-09 | Should we mock OpenAI types or use real SDK types in tests? | **Resolved** | **Yes** - Mock for unit tests, real SDK types for integration tests | - ---- - -## Appendix A: .NET SDK Reference - -### A.1 Agent Framework Implementation (PR #171) - -The .NET implementation provides four method overloads: - -```csharp -// Method 1: IEnumerable -Task SendChatHistoryAsync( - TurnContext turnContext, - IEnumerable chatMessages, - ToolOptions? options = null, - CancellationToken cancellationToken = default); - -// Method 2: ChatMessageStore -Task SendChatHistoryAsync( - TurnContext turnContext, - ChatMessageStore chatMessageStore, - ToolOptions? options = null, - CancellationToken cancellationToken = default); -``` - -Key implementation details: -- Generates `Guid.NewGuid()` for missing IDs -- Uses `DateTime.UtcNow` for missing timestamps -- Converts to `ChatHistoryMessage[]` array for the core API - -### A.2 Semantic Kernel Implementation (PR #173) - -```csharp -// Method 1: ChatHistory -Task SendChatHistoryAsync( - TurnContext turnContext, - ChatHistory chatHistory, - ToolOptions? options = null, - CancellationToken cancellationToken = default); - -// Method 2: IEnumerable -Task SendChatHistoryAsync( - TurnContext turnContext, - IEnumerable chatHistory, - ToolOptions? options = null, - CancellationToken cancellationToken = default); -``` - ---- - -## Appendix B: OpenAI SDK Type Reference - -### B.1 TResponseInputItem Union Type - -The `TResponseInputItem` is a union type in the OpenAI Agents SDK that includes: - -```python -TResponseInputItem = Union[ - EasyInputMessage, # Simple input message - ResponseOutputMessage, # Response from agent - ResponseFileSearchToolCall, # File search tool call - ResponseFunctionWebSearch, # Web search function - ResponseComputerToolCall, # Computer use tool call - ResponseFunctionToolCall, # Function tool call - ResponseFunctionToolCallOutput, # Tool call output - ResponseReasoningItem, # Reasoning step - ItemReference, # Reference to another item - # ... potentially more types -] -``` - -### B.2 Message Content Types - -```python -# Content can be string or list of parts -ContentPart = Union[ - TextContentPart, - ImageContentPart, - AudioContentPart, -] - -class TextContentPart: - type: Literal["text"] - text: str -``` - ---- - -## Document History - -| Version | Date | Author | Changes | -|---------|------|--------|---------| -| 1.0 | 2026-01-21 | Agent365 Python SDK Team | Initial draft | diff --git a/libraries/microsoft-agents-a365-tooling-extensions-openai/microsoft_agents_a365/tooling/extensions/openai/mcp_tool_registration_service.py b/libraries/microsoft-agents-a365-tooling-extensions-openai/microsoft_agents_a365/tooling/extensions/openai/mcp_tool_registration_service.py index 17b444d4..c1d2ea79 100644 --- a/libraries/microsoft-agents-a365-tooling-extensions-openai/microsoft_agents_a365/tooling/extensions/openai/mcp_tool_registration_service.py +++ b/libraries/microsoft-agents-a365-tooling-extensions-openai/microsoft_agents_a365/tooling/extensions/openai/mcp_tool_registration_service.py @@ -35,7 +35,6 @@ ) -# TODO: This is not needed. Remove this. @dataclass class MCPServerInfo: """Information about an MCP server""" @@ -249,7 +248,7 @@ async def cleanup_all_servers(self): # SEND CHAT HISTORY - OpenAI-specific implementations # -------------------------------------------------------------------------- - async def send_chat_history_async( + async def send_chat_history( self, turn_context: TurnContext, session: Session, @@ -293,7 +292,7 @@ async def send_chat_history_async( >>> # In your agent handler: >>> async with Runner.run(agent, messages) as result: ... session = result.session - ... op_result = await service.send_chat_history_async( + ... op_result = await service.send_chat_history( ... turn_context, session ... ) ... if op_result.succeeded: @@ -316,7 +315,7 @@ async def send_chat_history_async( self._logger.debug(f"Retrieved {len(messages)} items from session") # Delegate to the list-based method - return await self.send_chat_history_messages_async( + return await self.send_chat_history_messages( turn_context=turn_context, messages=messages, options=options, @@ -328,7 +327,7 @@ async def send_chat_history_async( self._logger.error(f"Failed to send chat history from session: {ex}") return OperationResult.failed(OperationError(ex)) - async def send_chat_history_messages_async( + async def send_chat_history_messages( self, turn_context: TurnContext, messages: List[TResponseInputItem], @@ -369,7 +368,7 @@ async def send_chat_history_messages_async( ... {"role": "assistant", "content": "Hi there!"}, ... ] >>> - >>> result = await service.send_chat_history_messages_async( + >>> result = await service.send_chat_history_messages( ... turn_context, messages ... ) >>> if result.succeeded: diff --git a/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/extensions/__init__.py b/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/extensions/__init__.py index 5c1bc402..61568bec 100644 --- a/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/extensions/__init__.py +++ b/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/extensions/__init__.py @@ -1,22 +1,20 @@ -# Copyright (c) Microsoft. All rights reserved. +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. -""" -Microsoft Agent 365 Tooling Extensions. +"""Microsoft Agent 365 Tooling Extensions namespace package. -This is a namespace package that allows extension packages to contribute -their modules under the microsoft_agents_a365.tooling.extensions namespace. +This file enables the `microsoft_agents_a365.tooling.extensions` namespace +to span multiple installed packages (e.g., extensions-openai, extensions-agentframework). """ import sys from pkgutil import extend_path -# First, try standard pkgutil-style namespace extension +# Standard pkgutil-style namespace extension __path__ = extend_path(__path__, __name__) -# For editable installs with custom finders, we need to manually discover -# extension paths by checking meta_path finders +# For editable installs with custom finders, manually discover extension paths for finder in sys.meta_path: - # Check if this is an editable finder with namespace support if hasattr(finder, "find_spec"): try: spec = finder.find_spec(__name__, None) @@ -25,6 +23,4 @@ if path not in __path__ and not path.endswith(".__path_hook__"): __path__.append(path) except (ImportError, TypeError): - # Silently skip finders that don't support our find_spec call signature - # or fail to locate the module - we'll try other finders in meta_path pass diff --git a/tests/tooling/extensions/openai/test_integration.py b/tests/tooling/extensions/openai/test_e2e.py similarity index 86% rename from tests/tooling/extensions/openai/test_integration.py rename to tests/tooling/extensions/openai/test_e2e.py index 8e885c7b..c595b185 100644 --- a/tests/tooling/extensions/openai/test_integration.py +++ b/tests/tooling/extensions/openai/test_e2e.py @@ -1,7 +1,12 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. -"""Integration tests for OpenAI send_chat_history methods.""" +"""End-to-end tests for OpenAI send_chat_history methods with mocked HTTP. + +These tests verify the complete flow from Session/messages through conversion +to the HTTP call, using mocked HTTP responses. They are marked as unit tests +because they use mocks and don't require real external services. +""" import json from datetime import UTC, datetime @@ -19,17 +24,22 @@ ) # ============================================================================= -# INTEGRATION TESTS (IT-01 to IT-03) +# END-TO-END TESTS WITH MOCKED HTTP (E2E-01 to E2E-03) # ============================================================================= -class TestEndToEndIntegration: - """Integration tests for end-to-end flow with mocked HTTP.""" +class TestEndToEndWithMockedHttp: + """End-to-end tests with mocked HTTP dependencies. + + These tests verify the complete flow through the service but use mocked + HTTP responses. They are marked as unit tests since no real network + calls are made. + """ - # IT-01 + # E2E-01 @pytest.mark.asyncio @pytest.mark.unit - async def test_send_chat_history_async_end_to_end_success(self, service, mock_turn_context): + async def test_send_chat_history_e2e_success(self, service, mock_turn_context): """Test full end-to-end flow: Session -> conversion -> HTTP -> success.""" # Create a session with realistic messages messages = [ @@ -54,7 +64,7 @@ async def test_send_chat_history_async_end_to_end_success(self, service, mock_tu mock_session_class.return_value.__aenter__.return_value = mock_session_instance # Execute - result = await service.send_chat_history_async(mock_turn_context, session) + result = await service.send_chat_history(mock_turn_context, session) # Verify assert result.succeeded is True @@ -63,10 +73,10 @@ async def test_send_chat_history_async_end_to_end_success(self, service, mock_tu # Verify HTTP call was made assert mock_session_instance.post.called - # IT-02 + # E2E-02 @pytest.mark.asyncio @pytest.mark.unit - async def test_send_chat_history_async_end_to_end_server_error( + async def test_send_chat_history_e2e_server_error( self, service, mock_turn_context ): """Test full end-to-end flow with HTTP 500 error.""" @@ -92,16 +102,16 @@ async def test_send_chat_history_async_end_to_end_server_error( mock_session_class.return_value.__aenter__.return_value = mock_session_instance # Execute - result = await service.send_chat_history_async(mock_turn_context, session) + result = await service.send_chat_history(mock_turn_context, session) # Verify failure assert result.succeeded is False assert len(result.errors) == 1 - # IT-03 + # E2E-03 @pytest.mark.asyncio @pytest.mark.unit - async def test_send_chat_history_async_payload_format(self, service, mock_turn_context): + async def test_send_chat_history_e2e_payload_format(self, service, mock_turn_context): """Test that the JSON payload has the correct structure.""" messages = [ MockUserMessage(content="Hello", id="user-001"), @@ -129,7 +139,7 @@ def capture_post(*args, **kwargs): mock_session_class.return_value.__aenter__.return_value = mock_session_instance # Execute - result = await service.send_chat_history_async(mock_turn_context, session) + result = await service.send_chat_history(mock_turn_context, session) # Verify success assert result.succeeded is True @@ -164,8 +174,8 @@ def capture_post(*args, **kwargs): assert chat_history[1]["id"] == "assistant-001" -class TestConversionChainIntegration: - """Integration tests for the full conversion chain.""" +class TestConversionChainE2E: + """End-to-end tests for the full conversion chain with mocked dependencies.""" @pytest.mark.asyncio @pytest.mark.unit @@ -189,7 +199,7 @@ def capture_args(*args, **kwargs): mock_send.side_effect = capture_args - await service.send_chat_history_messages_async( + await service.send_chat_history_messages( mock_turn_context, sample_openai_messages ) @@ -224,7 +234,7 @@ def capture_args(*args, **kwargs): mock_send.side_effect = capture_args - result = await service.send_chat_history_async(mock_turn_context, session) + result = await service.send_chat_history(mock_turn_context, session) # Verify success assert result.succeeded is True @@ -242,8 +252,8 @@ def capture_args(*args, **kwargs): assert captured_messages[1].timestamp == timestamp -class TestLimitParameterIntegration: - """Integration tests for the limit parameter.""" +class TestLimitParameterE2E: + """End-to-end tests for the limit parameter with mocked dependencies.""" @pytest.mark.asyncio @pytest.mark.unit @@ -269,7 +279,7 @@ def capture_args(*args, **kwargs): mock_send.side_effect = capture_args # Send with limit - result = await service.send_chat_history_async(mock_turn_context, session, limit=10) + result = await service.send_chat_history(mock_turn_context, session, limit=10) assert result.succeeded is True assert captured_messages is not None @@ -298,15 +308,15 @@ def capture_args(*args, **kwargs): mock_send.side_effect = capture_args # Send without limit - result = await service.send_chat_history_async(mock_turn_context, session) + result = await service.send_chat_history(mock_turn_context, session) assert result.succeeded is True assert captured_messages is not None assert len(captured_messages) == 50 -class TestHeadersIntegration: - """Integration tests for HTTP headers.""" +class TestHeadersE2E: + """End-to-end tests for HTTP headers with mocked dependencies.""" @pytest.mark.asyncio @pytest.mark.unit @@ -333,7 +343,7 @@ def capture_post(*args, **kwargs): mock_session_instance.post.side_effect = capture_post mock_session_class.return_value.__aenter__.return_value = mock_session_instance - await service.send_chat_history_messages_async( + await service.send_chat_history_messages( mock_turn_context, sample_openai_messages ) diff --git a/tests/tooling/extensions/openai/test_send_chat_history_async.py b/tests/tooling/extensions/openai/test_send_chat_history.py similarity index 67% rename from tests/tooling/extensions/openai/test_send_chat_history_async.py rename to tests/tooling/extensions/openai/test_send_chat_history.py index 02046dc8..e1693a8c 100644 --- a/tests/tooling/extensions/openai/test_send_chat_history_async.py +++ b/tests/tooling/extensions/openai/test_send_chat_history.py @@ -1,7 +1,7 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. -"""Unit tests for send_chat_history_async and send_chat_history_messages_async methods.""" +"""Unit tests for send_chat_history and send_chat_history_messages methods.""" from unittest.mock import AsyncMock, MagicMock, Mock, patch @@ -25,31 +25,31 @@ class TestInputValidation: # UV-01 @pytest.mark.asyncio @pytest.mark.unit - async def test_send_chat_history_messages_async_validates_turn_context_none( + async def test_send_chat_history_messages_validates_turn_context_none( self, service, sample_openai_messages ): - """Test that send_chat_history_messages_async raises ValueError when turn_context is None.""" + """Test that send_chat_history_messages raises ValueError when turn_context is None.""" with pytest.raises(ValueError, match="turn_context cannot be None"): - await service.send_chat_history_messages_async(None, sample_openai_messages) + await service.send_chat_history_messages(None, sample_openai_messages) # UV-02 @pytest.mark.asyncio @pytest.mark.unit - async def test_send_chat_history_messages_async_validates_messages_none( + async def test_send_chat_history_messages_validates_messages_none( self, service, mock_turn_context ): - """Test that send_chat_history_messages_async raises ValueError when messages is None.""" + """Test that send_chat_history_messages raises ValueError when messages is None.""" with pytest.raises(ValueError, match="messages cannot be None"): - await service.send_chat_history_messages_async(mock_turn_context, None) + await service.send_chat_history_messages(mock_turn_context, None) # UV-03 @pytest.mark.asyncio @pytest.mark.unit - async def test_send_chat_history_messages_async_empty_list_returns_success( + async def test_send_chat_history_messages_empty_list_returns_success( self, service, mock_turn_context ): """Test that empty message list returns success (no-op).""" - result = await service.send_chat_history_messages_async(mock_turn_context, []) + result = await service.send_chat_history_messages(mock_turn_context, []) assert result.succeeded is True assert len(result.errors) == 0 @@ -57,10 +57,10 @@ async def test_send_chat_history_messages_async_empty_list_returns_success( # UV-04 @pytest.mark.asyncio @pytest.mark.unit - async def test_send_chat_history_messages_async_validates_activity_none( + async def test_send_chat_history_messages_validates_activity_none( self, service, mock_turn_context_no_activity, sample_openai_messages ): - """Test that send_chat_history_messages_async validates turn_context.activity.""" + """Test that send_chat_history_messages validates turn_context.activity.""" with patch.object( service.config_service, "send_chat_history", @@ -69,17 +69,17 @@ async def test_send_chat_history_messages_async_validates_activity_none( mock_send.side_effect = ValueError("turn_context.activity cannot be None") with pytest.raises(ValueError, match="turn_context.activity cannot be None"): - await service.send_chat_history_messages_async( + await service.send_chat_history_messages( mock_turn_context_no_activity, sample_openai_messages ) # UV-05 @pytest.mark.asyncio @pytest.mark.unit - async def test_send_chat_history_messages_async_validates_conversation_id( + async def test_send_chat_history_messages_validates_conversation_id( self, service, mock_turn_context_no_conversation_id, sample_openai_messages ): - """Test that send_chat_history_messages_async validates conversation_id.""" + """Test that send_chat_history_messages validates conversation_id.""" with patch.object( service.config_service, "send_chat_history", @@ -88,17 +88,17 @@ async def test_send_chat_history_messages_async_validates_conversation_id( mock_send.side_effect = ValueError("conversation_id cannot be empty or None") with pytest.raises(ValueError, match="conversation_id cannot be empty"): - await service.send_chat_history_messages_async( + await service.send_chat_history_messages( mock_turn_context_no_conversation_id, sample_openai_messages ) # UV-06 @pytest.mark.asyncio @pytest.mark.unit - async def test_send_chat_history_messages_async_validates_message_id( + async def test_send_chat_history_messages_validates_message_id( self, service, mock_turn_context_no_message_id, sample_openai_messages ): - """Test that send_chat_history_messages_async validates message_id.""" + """Test that send_chat_history_messages validates message_id.""" with patch.object( service.config_service, "send_chat_history", @@ -107,17 +107,17 @@ async def test_send_chat_history_messages_async_validates_message_id( mock_send.side_effect = ValueError("message_id cannot be empty or None") with pytest.raises(ValueError, match="message_id cannot be empty"): - await service.send_chat_history_messages_async( + await service.send_chat_history_messages( mock_turn_context_no_message_id, sample_openai_messages ) # UV-07 @pytest.mark.asyncio @pytest.mark.unit - async def test_send_chat_history_messages_async_validates_user_message( + async def test_send_chat_history_messages_validates_user_message( self, service, mock_turn_context_no_user_message, sample_openai_messages ): - """Test that send_chat_history_messages_async validates user_message text.""" + """Test that send_chat_history_messages validates user_message text.""" with patch.object( service.config_service, "send_chat_history", @@ -126,25 +126,25 @@ async def test_send_chat_history_messages_async_validates_user_message( mock_send.side_effect = ValueError("user_message cannot be empty or None") with pytest.raises(ValueError, match="user_message cannot be empty"): - await service.send_chat_history_messages_async( + await service.send_chat_history_messages( mock_turn_context_no_user_message, sample_openai_messages ) # UV-08 @pytest.mark.asyncio @pytest.mark.unit - async def test_send_chat_history_async_validates_turn_context_none(self, service, mock_session): - """Test that send_chat_history_async raises ValueError when turn_context is None.""" + async def test_send_chat_history_validates_turn_context_none(self, service, mock_session): + """Test that send_chat_history raises ValueError when turn_context is None.""" with pytest.raises(ValueError, match="turn_context cannot be None"): - await service.send_chat_history_async(None, mock_session) + await service.send_chat_history(None, mock_session) # UV-09 @pytest.mark.asyncio @pytest.mark.unit - async def test_send_chat_history_async_validates_session_none(self, service, mock_turn_context): - """Test that send_chat_history_async raises ValueError when session is None.""" + async def test_send_chat_history_validates_session_none(self, service, mock_turn_context): + """Test that send_chat_history raises ValueError when session is None.""" with pytest.raises(ValueError, match="session cannot be None"): - await service.send_chat_history_async(mock_turn_context, None) + await service.send_chat_history(mock_turn_context, None) # ============================================================================= @@ -158,10 +158,10 @@ class TestSuccessPath: # SP-01 @pytest.mark.asyncio @pytest.mark.unit - async def test_send_chat_history_messages_async_success( + async def test_send_chat_history_messages_success( self, service, mock_turn_context, sample_openai_messages ): - """Test successful send_chat_history_messages_async call.""" + """Test successful send_chat_history_messages call.""" with patch.object( service.config_service, "send_chat_history", @@ -169,7 +169,7 @@ async def test_send_chat_history_messages_async_success( ) as mock_send: mock_send.return_value = OperationResult.success() - result = await service.send_chat_history_messages_async( + result = await service.send_chat_history_messages( mock_turn_context, sample_openai_messages ) @@ -180,10 +180,10 @@ async def test_send_chat_history_messages_async_success( # SP-02 @pytest.mark.asyncio @pytest.mark.unit - async def test_send_chat_history_messages_async_with_options( + async def test_send_chat_history_messages_with_options( self, service, mock_turn_context, sample_openai_messages ): - """Test send_chat_history_messages_async with custom ToolOptions.""" + """Test send_chat_history_messages with custom ToolOptions.""" custom_options = ToolOptions(orchestrator_name="CustomOrchestrator") with patch.object( @@ -193,7 +193,7 @@ async def test_send_chat_history_messages_async_with_options( ) as mock_send: mock_send.return_value = OperationResult.success() - result = await service.send_chat_history_messages_async( + result = await service.send_chat_history_messages( mock_turn_context, sample_openai_messages, options=custom_options ) @@ -205,7 +205,7 @@ async def test_send_chat_history_messages_async_with_options( # SP-03 @pytest.mark.asyncio @pytest.mark.unit - async def test_send_chat_history_messages_async_default_orchestrator_name( + async def test_send_chat_history_messages_default_orchestrator_name( self, service, mock_turn_context, sample_openai_messages ): """Test that default orchestrator name is set to 'OpenAI'.""" @@ -216,7 +216,7 @@ async def test_send_chat_history_messages_async_default_orchestrator_name( ) as mock_send: mock_send.return_value = OperationResult.success() - await service.send_chat_history_messages_async( + await service.send_chat_history_messages( mock_turn_context, sample_openai_messages ) @@ -227,10 +227,10 @@ async def test_send_chat_history_messages_async_default_orchestrator_name( # SP-04 @pytest.mark.asyncio @pytest.mark.unit - async def test_send_chat_history_messages_async_delegates_to_config_service( + async def test_send_chat_history_messages_delegates_to_config_service( self, service, mock_turn_context, sample_openai_messages ): - """Test that send_chat_history_messages_async delegates to config_service.""" + """Test that send_chat_history_messages delegates to config_service.""" with patch.object( service.config_service, "send_chat_history", @@ -238,7 +238,7 @@ async def test_send_chat_history_messages_async_delegates_to_config_service( ) as mock_send: mock_send.return_value = OperationResult.success() - await service.send_chat_history_messages_async( + await service.send_chat_history_messages( mock_turn_context, sample_openai_messages ) @@ -256,8 +256,8 @@ async def test_send_chat_history_messages_async_delegates_to_config_service( # SP-05 @pytest.mark.asyncio @pytest.mark.unit - async def test_send_chat_history_async_success(self, service, mock_turn_context, mock_session): - """Test successful send_chat_history_async call.""" + async def test_send_chat_history_success(self, service, mock_turn_context, mock_session): + """Test successful send_chat_history call.""" with patch.object( service.config_service, "send_chat_history", @@ -265,7 +265,7 @@ async def test_send_chat_history_async_success(self, service, mock_turn_context, ) as mock_send: mock_send.return_value = OperationResult.success() - result = await service.send_chat_history_async(mock_turn_context, mock_session) + result = await service.send_chat_history(mock_turn_context, mock_session) assert result.succeeded is True mock_send.assert_called_once() @@ -273,8 +273,8 @@ async def test_send_chat_history_async_success(self, service, mock_turn_context, # SP-06 @pytest.mark.asyncio @pytest.mark.unit - async def test_send_chat_history_async_with_limit(self, service, mock_turn_context): - """Test send_chat_history_async with limit parameter.""" + async def test_send_chat_history_with_limit(self, service, mock_turn_context): + """Test send_chat_history with limit parameter.""" # Create session with many messages messages = [MockUserMessage(content=f"Message {i}") for i in range(10)] session = MockSession(items=messages) @@ -286,7 +286,7 @@ async def test_send_chat_history_async_with_limit(self, service, mock_turn_conte ) as mock_send: mock_send.return_value = OperationResult.success() - result = await service.send_chat_history_async(mock_turn_context, session, limit=5) + result = await service.send_chat_history(mock_turn_context, session, limit=5) assert result.succeeded is True @@ -298,18 +298,18 @@ async def test_send_chat_history_async_with_limit(self, service, mock_turn_conte # SP-07 @pytest.mark.asyncio @pytest.mark.unit - async def test_send_chat_history_async_delegates_to_send_chat_history_messages( + async def test_send_chat_history_delegates_to_send_chat_history_messages( self, service, mock_turn_context, mock_session ): - """Test that send_chat_history_async calls send_chat_history_messages_async.""" + """Test that send_chat_history calls send_chat_history_messages.""" with patch.object( service, - "send_chat_history_messages_async", + "send_chat_history_messages", new_callable=AsyncMock, ) as mock_method: mock_method.return_value = OperationResult.success() - await service.send_chat_history_async(mock_turn_context, mock_session) + await service.send_chat_history(mock_turn_context, mock_session) mock_method.assert_called_once() call_args = mock_method.call_args @@ -327,10 +327,10 @@ class TestErrorHandling: # EH-01 @pytest.mark.asyncio @pytest.mark.unit - async def test_send_chat_history_messages_async_http_error( + async def test_send_chat_history_messages_http_error( self, service, mock_turn_context, sample_openai_messages ): - """Test send_chat_history_messages_async handles HTTP errors.""" + """Test send_chat_history_messages handles HTTP errors.""" with patch.object( service.config_service, @@ -341,7 +341,7 @@ async def test_send_chat_history_messages_async_http_error( MagicMock(message="HTTP 500: Internal Server Error") ) - result = await service.send_chat_history_messages_async( + result = await service.send_chat_history_messages( mock_turn_context, sample_openai_messages ) @@ -350,10 +350,10 @@ async def test_send_chat_history_messages_async_http_error( # EH-02 @pytest.mark.asyncio @pytest.mark.unit - async def test_send_chat_history_messages_async_timeout_error( + async def test_send_chat_history_messages_timeout_error( self, service, mock_turn_context, sample_openai_messages ): - """Test send_chat_history_messages_async handles timeout errors.""" + """Test send_chat_history_messages handles timeout errors.""" with patch.object( service.config_service, "send_chat_history", @@ -361,7 +361,7 @@ async def test_send_chat_history_messages_async_timeout_error( ) as mock_send: mock_send.side_effect = TimeoutError("Request timed out") - result = await service.send_chat_history_messages_async( + result = await service.send_chat_history_messages( mock_turn_context, sample_openai_messages ) @@ -371,10 +371,10 @@ async def test_send_chat_history_messages_async_timeout_error( # EH-03 @pytest.mark.asyncio @pytest.mark.unit - async def test_send_chat_history_messages_async_client_error( + async def test_send_chat_history_messages_client_error( self, service, mock_turn_context, sample_openai_messages ): - """Test send_chat_history_messages_async handles network/client errors.""" + """Test send_chat_history_messages handles network/client errors.""" import aiohttp with patch.object( @@ -384,7 +384,7 @@ async def test_send_chat_history_messages_async_client_error( ) as mock_send: mock_send.side_effect = aiohttp.ClientError("Connection failed") - result = await service.send_chat_history_messages_async( + result = await service.send_chat_history_messages( mock_turn_context, sample_openai_messages ) @@ -394,10 +394,10 @@ async def test_send_chat_history_messages_async_client_error( # EH-04 @pytest.mark.asyncio @pytest.mark.unit - async def test_send_chat_history_messages_async_conversion_error( + async def test_send_chat_history_messages_conversion_error( self, service, mock_turn_context ): - """Test send_chat_history_messages_async handles conversion errors gracefully.""" + """Test send_chat_history_messages handles conversion errors gracefully.""" # Create a message that might cause conversion issues but still has content problematic_message = MockUserMessage(content="Valid content") @@ -409,7 +409,7 @@ async def test_send_chat_history_messages_async_conversion_error( mock_send.return_value = OperationResult.success() # Should not raise, should handle gracefully - result = await service.send_chat_history_messages_async( + result = await service.send_chat_history_messages( mock_turn_context, [problematic_message] ) @@ -418,13 +418,13 @@ async def test_send_chat_history_messages_async_conversion_error( # EH-05 @pytest.mark.asyncio @pytest.mark.unit - async def test_send_chat_history_async_get_items_error(self, service, mock_turn_context): - """Test send_chat_history_async handles session.get_items() errors.""" + async def test_send_chat_history_get_items_error(self, service, mock_turn_context): + """Test send_chat_history handles session.get_items() errors.""" # Create a mock session that raises an error mock_session = Mock() mock_session.get_items.side_effect = Exception("Session error") - result = await service.send_chat_history_async(mock_turn_context, mock_session) + result = await service.send_chat_history(mock_turn_context, mock_session) assert result.succeeded is False assert len(result.errors) == 1 @@ -453,7 +453,7 @@ async def test_options_with_none_orchestrator_name_gets_default( ) as mock_send: mock_send.return_value = OperationResult.success() - await service.send_chat_history_messages_async( + await service.send_chat_history_messages( mock_turn_context, sample_openai_messages, options=options ) @@ -475,9 +475,56 @@ async def test_options_preserves_custom_orchestrator_name( ) as mock_send: mock_send.return_value = OperationResult.success() - await service.send_chat_history_messages_async( + await service.send_chat_history_messages( mock_turn_context, sample_openai_messages, options=options ) call_args = mock_send.call_args assert call_args.kwargs["options"].orchestrator_name == "MyCustomOrchestrator" + + +# ============================================================================= +# THREAD SAFETY / CONCURRENT CALLS TESTS +# ============================================================================= + + +class TestConcurrentCalls: + """Tests for thread safety and concurrent call isolation.""" + + @pytest.mark.asyncio + @pytest.mark.unit + async def test_concurrent_calls_do_not_interfere(self, service, mock_turn_context): + """Test that concurrent calls to send_chat_history_messages are isolated.""" + import asyncio + + messages1 = [MockUserMessage(content="Message set 1")] + messages2 = [MockUserMessage(content="Message set 2")] + + captured_payloads: list[list[object]] = [] + + with patch.object( + service.config_service, + "send_chat_history", + new_callable=AsyncMock, + ) as mock_send: + + async def capture_and_succeed(*args: object, **kwargs: object) -> OperationResult: + captured_payloads.append(kwargs.get("chat_history_messages")) + await asyncio.sleep(0.01) # Simulate async work + return OperationResult.success() + + mock_send.side_effect = capture_and_succeed + + # Run concurrently + results = await asyncio.gather( + service.send_chat_history_messages(mock_turn_context, messages1), + service.send_chat_history_messages(mock_turn_context, messages2), + ) + + # Both should succeed independently + assert all(r.succeeded for r in results) + assert len(captured_payloads) == 2 + # Verify no cross-contamination + contents = [p[0].content for p in captured_payloads] + assert "Message set 1" in contents + assert "Message set 2" in contents From 4aee605bfac863e6438d5901f64427e7a6e6c872 Mon Sep 17 00:00:00 2001 From: Johan Broberg Date: Sat, 24 Jan 2026 19:59:13 -0800 Subject: [PATCH 07/11] Address code review feedback: type annotations, test fixes, and code quality - CRM-003: Add return type annotation to add_tool_servers_to_agent - CRM-004: Add type annotations to _cleanup_servers and cleanup_all_servers - CRM-006: Fix conversion error test to actually test error handling - CRM-007: Update implicit boolean check to explicit None check - CRM-008: Remove redundant local import of Agent - CRM-010: Use bare raise instead of raise e - CRM-011: Rename test to match behavior (skips_message not uses_empty_string) Co-Authored-By: Claude Opus 4.5 --- .../openai/mcp_tool_registration_service.py | 12 +++++------- .../openai/test_message_conversion.py | 4 ++-- .../openai/test_send_chat_history.py | 18 ++++++++---------- 3 files changed, 15 insertions(+), 19 deletions(-) diff --git a/libraries/microsoft-agents-a365-tooling-extensions-openai/microsoft_agents_a365/tooling/extensions/openai/mcp_tool_registration_service.py b/libraries/microsoft-agents-a365-tooling-extensions-openai/microsoft_agents_a365/tooling/extensions/openai/mcp_tool_registration_service.py index c1d2ea79..2f9efae3 100644 --- a/libraries/microsoft-agents-a365-tooling-extensions-openai/microsoft_agents_a365/tooling/extensions/openai/mcp_tool_registration_service.py +++ b/libraries/microsoft-agents-a365-tooling-extensions-openai/microsoft_agents_a365/tooling/extensions/openai/mcp_tool_registration_service.py @@ -69,7 +69,7 @@ async def add_tool_servers_to_agent( auth_handler_name: str, context: TurnContext, auth_token: Optional[str] = None, - ): + ) -> Agent: """ Add new MCP servers to the agent by creating a new Agent instance. @@ -88,7 +88,7 @@ async def add_tool_servers_to_agent( New Agent instance with all MCP servers, or original agent if no new servers """ - if not auth_token: + if auth_token is None: scopes = get_mcp_platform_authentication_scope() authToken = await auth.exchange_token(context, scopes, auth_handler_name) auth_token = authToken.token @@ -190,8 +190,6 @@ async def add_tool_servers_to_agent( all_mcp_servers = existing_mcp_servers + new_mcp_servers # Recreate the agent with all MCP servers - from agents import Agent - new_agent = Agent( name=agent.name, model=agent.model, @@ -223,12 +221,12 @@ async def add_tool_servers_to_agent( # Clean up connected servers if agent creation fails self._logger.error(f"Failed to recreate agent with new MCP servers: {e}") await self._cleanup_servers(connected_servers) - raise e + raise self._logger.info("No new MCP servers to add to agent") return agent - async def _cleanup_servers(self, servers): + async def _cleanup_servers(self, servers: List[MCPServerStreamableHttp]) -> None: """Clean up connected MCP servers""" for server in servers: try: @@ -238,7 +236,7 @@ async def _cleanup_servers(self, servers): # Log cleanup errors but don't raise them self._logger.debug(f"Error during server cleanup: {e}") - async def cleanup_all_servers(self): + async def cleanup_all_servers(self) -> None: """Clean up all connected MCP servers""" if hasattr(self, "_connected_servers"): await self._cleanup_servers(self._connected_servers) diff --git a/tests/tooling/extensions/openai/test_message_conversion.py b/tests/tooling/extensions/openai/test_message_conversion.py index f42a12f9..df2a97f7 100644 --- a/tests/tooling/extensions/openai/test_message_conversion.py +++ b/tests/tooling/extensions/openai/test_message_conversion.py @@ -139,8 +139,8 @@ def test_convert_message_with_list_content(self, service): # CV-11 @pytest.mark.unit - def test_convert_empty_content_uses_empty_string(self, service): - """Test that empty content is handled - message skipped due to validator.""" + def test_convert_empty_content_skips_message(self, service): + """Test that messages with empty content are skipped during conversion.""" message = MockUserMessage(content="") result = service._convert_single_message(message) diff --git a/tests/tooling/extensions/openai/test_send_chat_history.py b/tests/tooling/extensions/openai/test_send_chat_history.py index e1693a8c..474de346 100644 --- a/tests/tooling/extensions/openai/test_send_chat_history.py +++ b/tests/tooling/extensions/openai/test_send_chat_history.py @@ -398,22 +398,20 @@ async def test_send_chat_history_messages_conversion_error( self, service, mock_turn_context ): """Test send_chat_history_messages handles conversion errors gracefully.""" - # Create a message that might cause conversion issues but still has content - problematic_message = MockUserMessage(content="Valid content") + sample_messages = [MockUserMessage(content="Hello")] with patch.object( - service.config_service, - "send_chat_history", - new_callable=AsyncMock, - ) as mock_send: - mock_send.return_value = OperationResult.success() + service, "_convert_openai_messages_to_chat_history" + ) as mock_convert: + mock_convert.side_effect = Exception("Conversion failed") - # Should not raise, should handle gracefully result = await service.send_chat_history_messages( - mock_turn_context, [problematic_message] + mock_turn_context, sample_messages ) - assert result.succeeded is True + assert result.succeeded is False + assert len(result.errors) == 1 + assert "Conversion failed" in str(result.errors[0].message) # EH-05 @pytest.mark.asyncio From 1087cd54b5afcca356b6bc9314e2cadbd8390b48 Mon Sep 17 00:00:00 2001 From: Johan Broberg Date: Sat, 24 Jan 2026 20:10:59 -0800 Subject: [PATCH 08/11] Refactor type annotations to use TypeAlias for MessageContent and MockMessage --- tests/tooling/extensions/openai/conftest.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/tooling/extensions/openai/conftest.py b/tests/tooling/extensions/openai/conftest.py index e21ce32a..9a6f108e 100644 --- a/tests/tooling/extensions/openai/conftest.py +++ b/tests/tooling/extensions/openai/conftest.py @@ -4,7 +4,7 @@ """Shared pytest fixtures for OpenAI extension tests.""" from datetime import UTC, datetime -from typing import Union +from typing import TypeAlias from unittest.mock import Mock import pytest @@ -14,7 +14,7 @@ # -------------------------------------------------------------------------- # Content can be string, list of content parts, or None (mimics OpenAI SDK) -MessageContent = Union[str, list[object], None] +MessageContent: TypeAlias = str | list[object] | None # -------------------------------------------------------------------------- @@ -99,13 +99,13 @@ def __init__(self, text: str): # Type alias for mock messages -MockMessage = Union[ - MockUserMessage, - MockAssistantMessage, - MockSystemMessage, - MockResponseOutputMessage, - MockUnknownMessage, -] +MockMessage: TypeAlias = ( + MockUserMessage + | MockAssistantMessage + | MockSystemMessage + | MockResponseOutputMessage + | MockUnknownMessage +) class MockSession: From 255bee38b0429f6cdbd2338db891aebd09af4596 Mon Sep 17 00:00:00 2001 From: Johan Broberg Date: Sat, 24 Jan 2026 20:20:26 -0800 Subject: [PATCH 09/11] Refactor send_chat_history_messages calls for improved readability --- tests/tooling/extensions/openai/test_e2e.py | 12 +++-------- .../openai/test_send_chat_history.py | 20 +++++-------------- 2 files changed, 8 insertions(+), 24 deletions(-) diff --git a/tests/tooling/extensions/openai/test_e2e.py b/tests/tooling/extensions/openai/test_e2e.py index c595b185..a33ec8e5 100644 --- a/tests/tooling/extensions/openai/test_e2e.py +++ b/tests/tooling/extensions/openai/test_e2e.py @@ -76,9 +76,7 @@ async def test_send_chat_history_e2e_success(self, service, mock_turn_context): # E2E-02 @pytest.mark.asyncio @pytest.mark.unit - async def test_send_chat_history_e2e_server_error( - self, service, mock_turn_context - ): + async def test_send_chat_history_e2e_server_error(self, service, mock_turn_context): """Test full end-to-end flow with HTTP 500 error.""" messages = [ MockUserMessage(content="Hello"), @@ -199,9 +197,7 @@ def capture_args(*args, **kwargs): mock_send.side_effect = capture_args - await service.send_chat_history_messages( - mock_turn_context, sample_openai_messages - ) + await service.send_chat_history_messages(mock_turn_context, sample_openai_messages) # Verify all messages are ChatHistoryMessage instances assert captured_messages is not None @@ -343,9 +339,7 @@ def capture_post(*args, **kwargs): mock_session_instance.post.side_effect = capture_post mock_session_class.return_value.__aenter__.return_value = mock_session_instance - await service.send_chat_history_messages( - mock_turn_context, sample_openai_messages - ) + await service.send_chat_history_messages(mock_turn_context, sample_openai_messages) # Verify headers assert captured_headers is not None diff --git a/tests/tooling/extensions/openai/test_send_chat_history.py b/tests/tooling/extensions/openai/test_send_chat_history.py index 474de346..9d7b9f12 100644 --- a/tests/tooling/extensions/openai/test_send_chat_history.py +++ b/tests/tooling/extensions/openai/test_send_chat_history.py @@ -216,9 +216,7 @@ async def test_send_chat_history_messages_default_orchestrator_name( ) as mock_send: mock_send.return_value = OperationResult.success() - await service.send_chat_history_messages( - mock_turn_context, sample_openai_messages - ) + await service.send_chat_history_messages(mock_turn_context, sample_openai_messages) # Verify default orchestrator name call_args = mock_send.call_args @@ -238,9 +236,7 @@ async def test_send_chat_history_messages_delegates_to_config_service( ) as mock_send: mock_send.return_value = OperationResult.success() - await service.send_chat_history_messages( - mock_turn_context, sample_openai_messages - ) + await service.send_chat_history_messages(mock_turn_context, sample_openai_messages) # Verify delegation mock_send.assert_called_once() @@ -394,20 +390,14 @@ async def test_send_chat_history_messages_client_error( # EH-04 @pytest.mark.asyncio @pytest.mark.unit - async def test_send_chat_history_messages_conversion_error( - self, service, mock_turn_context - ): + async def test_send_chat_history_messages_conversion_error(self, service, mock_turn_context): """Test send_chat_history_messages handles conversion errors gracefully.""" sample_messages = [MockUserMessage(content="Hello")] - with patch.object( - service, "_convert_openai_messages_to_chat_history" - ) as mock_convert: + with patch.object(service, "_convert_openai_messages_to_chat_history") as mock_convert: mock_convert.side_effect = Exception("Conversion failed") - result = await service.send_chat_history_messages( - mock_turn_context, sample_messages - ) + result = await service.send_chat_history_messages(mock_turn_context, sample_messages) assert result.succeeded is False assert len(result.errors) == 1 From 2cc4a3b6e51ccf741f809c59de17040b9001fb02 Mon Sep 17 00:00:00 2001 From: Johan Broberg Date: Sat, 24 Jan 2026 20:33:01 -0800 Subject: [PATCH 10/11] Update libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/extensions/__init__.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../microsoft_agents_a365/tooling/extensions/__init__.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/extensions/__init__.py b/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/extensions/__init__.py index 61568bec..3e7da8a5 100644 --- a/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/extensions/__init__.py +++ b/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/extensions/__init__.py @@ -23,4 +23,6 @@ if path not in __path__ and not path.endswith(".__path_hook__"): __path__.append(path) except (ImportError, TypeError): + # Some meta path finders may not support this namespace and can raise + # ImportError or TypeError; ignore these and continue discovering paths. pass From fa3cf8812e93a122a6af19ac362ff861ef8f3beb Mon Sep 17 00:00:00 2001 From: Johan Broberg Date: Sat, 24 Jan 2026 20:38:45 -0800 Subject: [PATCH 11/11] Apply suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../tooling/extensions/openai/mcp_tool_registration_service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/microsoft-agents-a365-tooling-extensions-openai/microsoft_agents_a365/tooling/extensions/openai/mcp_tool_registration_service.py b/libraries/microsoft-agents-a365-tooling-extensions-openai/microsoft_agents_a365/tooling/extensions/openai/mcp_tool_registration_service.py index 2f9efae3..a62db7ee 100644 --- a/libraries/microsoft-agents-a365-tooling-extensions-openai/microsoft_agents_a365/tooling/extensions/openai/mcp_tool_registration_service.py +++ b/libraries/microsoft-agents-a365-tooling-extensions-openai/microsoft_agents_a365/tooling/extensions/openai/mcp_tool_registration_service.py @@ -88,7 +88,7 @@ async def add_tool_servers_to_agent( New Agent instance with all MCP servers, or original agent if no new servers """ - if auth_token is None: + if auth_token is None or auth_token.strip() == "": scopes = get_mcp_platform_authentication_scope() authToken = await auth.exchange_token(context, scopes, auth_handler_name) auth_token = authToken.token