From 0527c46bba98754e32dbff8d2c75eaf4829fe9d1 Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Fri, 13 Mar 2026 11:24:58 -0600 Subject: [PATCH] Add sandbox_id__eq filter to AppConversationService search and count methods (#13387) Co-authored-by: openhands --- .../app_conversation_router.py | 10 + .../app_conversation_service.py | 2 + .../live_status_app_conversation_service.py | 4 + .../test_app_conversation_router.py | 171 +++++++++++++++++- ...st_live_status_app_conversation_service.py | 114 ++++++++++++ 5 files changed, 299 insertions(+), 2 deletions(-) diff --git a/openhands/app_server/app_conversation/app_conversation_router.py b/openhands/app_server/app_conversation/app_conversation_router.py index 54fc7c16b3..50a8497a85 100644 --- a/openhands/app_server/app_conversation/app_conversation_router.py +++ b/openhands/app_server/app_conversation/app_conversation_router.py @@ -117,6 +117,10 @@ async def search_app_conversations( datetime | None, Query(title='Filter by updated_at less than this datetime'), ] = None, + sandbox_id__eq: Annotated[ + str | None, + Query(title='Filter by exact sandbox_id'), + ] = None, page_id: Annotated[ str | None, Query(title='Optional next_page_id from the previously returned page'), @@ -148,6 +152,7 @@ async def search_app_conversations( created_at__lt=created_at__lt, updated_at__gte=updated_at__gte, updated_at__lt=updated_at__lt, + sandbox_id__eq=sandbox_id__eq, page_id=page_id, limit=limit, include_sub_conversations=include_sub_conversations, @@ -176,6 +181,10 @@ async def count_app_conversations( datetime | None, Query(title='Filter by updated_at less than this datetime'), ] = None, + sandbox_id__eq: Annotated[ + str | None, + Query(title='Filter by exact sandbox_id'), + ] = None, app_conversation_service: AppConversationService = ( app_conversation_service_dependency ), @@ -187,6 +196,7 @@ async def count_app_conversations( created_at__lt=created_at__lt, updated_at__gte=updated_at__gte, updated_at__lt=updated_at__lt, + sandbox_id__eq=sandbox_id__eq, ) diff --git a/openhands/app_server/app_conversation/app_conversation_service.py b/openhands/app_server/app_conversation/app_conversation_service.py index 97918c3267..1f955cac9c 100644 --- a/openhands/app_server/app_conversation/app_conversation_service.py +++ b/openhands/app_server/app_conversation/app_conversation_service.py @@ -29,6 +29,7 @@ class AppConversationService(ABC): created_at__lt: datetime | None = None, updated_at__gte: datetime | None = None, updated_at__lt: datetime | None = None, + sandbox_id__eq: str | None = None, sort_order: AppConversationSortOrder = AppConversationSortOrder.CREATED_AT_DESC, page_id: str | None = None, limit: int = 100, @@ -44,6 +45,7 @@ class AppConversationService(ABC): created_at__lt: datetime | None = None, updated_at__gte: datetime | None = None, updated_at__lt: datetime | None = None, + sandbox_id__eq: str | None = None, ) -> int: """Count sandboxed conversations.""" diff --git a/openhands/app_server/app_conversation/live_status_app_conversation_service.py b/openhands/app_server/app_conversation/live_status_app_conversation_service.py index 97b92c6aa5..94b5740329 100644 --- a/openhands/app_server/app_conversation/live_status_app_conversation_service.py +++ b/openhands/app_server/app_conversation/live_status_app_conversation_service.py @@ -142,6 +142,7 @@ class LiveStatusAppConversationService(AppConversationServiceBase): created_at__lt: datetime | None = None, updated_at__gte: datetime | None = None, updated_at__lt: datetime | None = None, + sandbox_id__eq: str | None = None, sort_order: AppConversationSortOrder = AppConversationSortOrder.CREATED_AT_DESC, page_id: str | None = None, limit: int = 20, @@ -154,6 +155,7 @@ class LiveStatusAppConversationService(AppConversationServiceBase): created_at__lt=created_at__lt, updated_at__gte=updated_at__gte, updated_at__lt=updated_at__lt, + sandbox_id__eq=sandbox_id__eq, sort_order=sort_order, page_id=page_id, limit=limit, @@ -171,6 +173,7 @@ class LiveStatusAppConversationService(AppConversationServiceBase): created_at__lt: datetime | None = None, updated_at__gte: datetime | None = None, updated_at__lt: datetime | None = None, + sandbox_id__eq: str | None = None, ) -> int: return await self.app_conversation_info_service.count_app_conversation_info( title__contains=title__contains, @@ -178,6 +181,7 @@ class LiveStatusAppConversationService(AppConversationServiceBase): created_at__lt=created_at__lt, updated_at__gte=updated_at__gte, updated_at__lt=updated_at__lt, + sandbox_id__eq=sandbox_id__eq, ) async def get_app_conversation( diff --git a/tests/unit/app_server/test_app_conversation_router.py b/tests/unit/app_server/test_app_conversation_router.py index d052537d96..c655d4b422 100644 --- a/tests/unit/app_server/test_app_conversation_router.py +++ b/tests/unit/app_server/test_app_conversation_router.py @@ -12,21 +12,28 @@ from fastapi import HTTPException, status from openhands.app_server.app_conversation.app_conversation_models import ( AppConversation, + AppConversationPage, ) from openhands.app_server.app_conversation.app_conversation_router import ( batch_get_app_conversations, + count_app_conversations, + search_app_conversations, ) from openhands.app_server.sandbox.sandbox_models import SandboxStatus -def _make_mock_app_conversation(conversation_id=None, user_id='test-user'): +def _make_mock_app_conversation( + conversation_id=None, user_id='test-user', sandbox_id=None +): """Create a mock AppConversation for testing.""" if conversation_id is None: conversation_id = uuid4() + if sandbox_id is None: + sandbox_id = str(uuid4()) return AppConversation( id=conversation_id, created_by_user_id=user_id, - sandbox_id=str(uuid4()), + sandbox_id=sandbox_id, sandbox_status=SandboxStatus.RUNNING, ) @@ -34,11 +41,17 @@ def _make_mock_app_conversation(conversation_id=None, user_id='test-user'): def _make_mock_service( get_conversation_return=None, batch_get_return=None, + search_return=None, + count_return=0, ): """Create a mock AppConversationService for testing.""" service = MagicMock() service.get_app_conversation = AsyncMock(return_value=get_conversation_return) service.batch_get_app_conversations = AsyncMock(return_value=batch_get_return or []) + service.search_app_conversations = AsyncMock( + return_value=search_return or AppConversationPage(items=[]) + ) + service.count_app_conversations = AsyncMock(return_value=count_return) return service @@ -207,3 +220,157 @@ class TestBatchGetAppConversations: assert result[0] is not None assert result[0].id == uuid1 assert result[1] is None + + +@pytest.mark.asyncio +class TestSearchAppConversations: + """Test suite for search_app_conversations endpoint.""" + + async def test_search_with_sandbox_id_filter(self): + """Test that sandbox_id__eq filter is passed to the service. + + Arrange: Create mock service and specific sandbox_id + Act: Call search_app_conversations with sandbox_id__eq + Assert: Service is called with the sandbox_id__eq parameter + """ + # Arrange + sandbox_id = 'test-sandbox-123' + mock_conversation = _make_mock_app_conversation(sandbox_id=sandbox_id) + mock_service = _make_mock_service( + search_return=AppConversationPage(items=[mock_conversation]) + ) + + # Act + result = await search_app_conversations( + sandbox_id__eq=sandbox_id, + app_conversation_service=mock_service, + ) + + # Assert + mock_service.search_app_conversations.assert_called_once() + call_kwargs = mock_service.search_app_conversations.call_args[1] + assert call_kwargs.get('sandbox_id__eq') == sandbox_id + assert len(result.items) == 1 + assert result.items[0].sandbox_id == sandbox_id + + async def test_search_without_sandbox_id_filter(self): + """Test that sandbox_id__eq defaults to None when not provided. + + Arrange: Create mock service + Act: Call search_app_conversations without sandbox_id__eq + Assert: Service is called with sandbox_id__eq=None + """ + # Arrange + mock_service = _make_mock_service() + + # Act + await search_app_conversations( + app_conversation_service=mock_service, + ) + + # Assert + mock_service.search_app_conversations.assert_called_once() + call_kwargs = mock_service.search_app_conversations.call_args[1] + assert call_kwargs.get('sandbox_id__eq') is None + + async def test_search_with_sandbox_id_and_other_filters(self): + """Test that sandbox_id__eq works correctly with other filters. + + Arrange: Create mock service + Act: Call search_app_conversations with sandbox_id__eq and other filters + Assert: Service is called with all parameters correctly + """ + # Arrange + sandbox_id = 'test-sandbox-456' + mock_service = _make_mock_service() + + # Act + await search_app_conversations( + title__contains='test', + sandbox_id__eq=sandbox_id, + limit=50, + include_sub_conversations=True, + app_conversation_service=mock_service, + ) + + # Assert + mock_service.search_app_conversations.assert_called_once() + call_kwargs = mock_service.search_app_conversations.call_args[1] + assert call_kwargs.get('sandbox_id__eq') == sandbox_id + assert call_kwargs.get('title__contains') == 'test' + assert call_kwargs.get('limit') == 50 + assert call_kwargs.get('include_sub_conversations') is True + + +@pytest.mark.asyncio +class TestCountAppConversations: + """Test suite for count_app_conversations endpoint.""" + + async def test_count_with_sandbox_id_filter(self): + """Test that sandbox_id__eq filter is passed to the service. + + Arrange: Create mock service with count return value + Act: Call count_app_conversations with sandbox_id__eq + Assert: Service is called with the sandbox_id__eq parameter + """ + # Arrange + sandbox_id = 'test-sandbox-789' + mock_service = _make_mock_service(count_return=5) + + # Act + result = await count_app_conversations( + sandbox_id__eq=sandbox_id, + app_conversation_service=mock_service, + ) + + # Assert + mock_service.count_app_conversations.assert_called_once() + call_kwargs = mock_service.count_app_conversations.call_args[1] + assert call_kwargs.get('sandbox_id__eq') == sandbox_id + assert result == 5 + + async def test_count_without_sandbox_id_filter(self): + """Test that sandbox_id__eq defaults to None when not provided. + + Arrange: Create mock service + Act: Call count_app_conversations without sandbox_id__eq + Assert: Service is called with sandbox_id__eq=None + """ + # Arrange + mock_service = _make_mock_service(count_return=10) + + # Act + result = await count_app_conversations( + app_conversation_service=mock_service, + ) + + # Assert + mock_service.count_app_conversations.assert_called_once() + call_kwargs = mock_service.count_app_conversations.call_args[1] + assert call_kwargs.get('sandbox_id__eq') is None + assert result == 10 + + async def test_count_with_sandbox_id_and_other_filters(self): + """Test that sandbox_id__eq works correctly with other filters. + + Arrange: Create mock service + Act: Call count_app_conversations with sandbox_id__eq and other filters + Assert: Service is called with all parameters correctly + """ + # Arrange + sandbox_id = 'test-sandbox-abc' + mock_service = _make_mock_service(count_return=3) + + # Act + result = await count_app_conversations( + title__contains='test', + sandbox_id__eq=sandbox_id, + app_conversation_service=mock_service, + ) + + # Assert + mock_service.count_app_conversations.assert_called_once() + call_kwargs = mock_service.count_app_conversations.call_args[1] + assert call_kwargs.get('sandbox_id__eq') == sandbox_id + assert call_kwargs.get('title__contains') == 'test' + assert result == 3 diff --git a/tests/unit/app_server/test_live_status_app_conversation_service.py b/tests/unit/app_server/test_live_status_app_conversation_service.py index d9f79f79e2..ad9b4edb46 100644 --- a/tests/unit/app_server/test_live_status_app_conversation_service.py +++ b/tests/unit/app_server/test_live_status_app_conversation_service.py @@ -2097,6 +2097,120 @@ class TestLiveStatusAppConversationService: assert captured['workspace_working_dir'] == '/workspace/project' + @pytest.mark.asyncio + async def test_search_app_conversations_with_sandbox_id_filter(self): + """Test that search_app_conversations passes sandbox_id__eq to the info service. + + This verifies that the sandbox_id filter is correctly propagated through + the service layer to the underlying info service. + """ + from openhands.app_server.app_conversation.app_conversation_models import ( + AppConversationInfoPage, + ) + + # Create test data with different sandbox IDs + sandbox_id_alpha = 'sandbox-alpha-123' + sandbox_id_beta = 'sandbox-beta-456' + + conv_alpha = AppConversationInfo( + id=uuid4(), + created_by_user_id=None, + sandbox_id=sandbox_id_alpha, + title='Alpha Conversation', + ) + conv_beta = AppConversationInfo( + id=uuid4(), + created_by_user_id=None, + sandbox_id=sandbox_id_beta, + title='Beta Conversation', + ) + + # Mock the info service to return filtered results based on sandbox_id__eq + async def mock_search(sandbox_id__eq=None, **kwargs): + if sandbox_id__eq == sandbox_id_alpha: + return AppConversationInfoPage(items=[conv_alpha]) + elif sandbox_id__eq == sandbox_id_beta: + return AppConversationInfoPage(items=[conv_beta]) + else: + return AppConversationInfoPage(items=[conv_alpha, conv_beta]) + + self.mock_app_conversation_info_service.search_app_conversation_info = ( + AsyncMock(side_effect=mock_search) + ) + + # Mock sandbox service to return running status for sandbox lookups + self.mock_sandbox_service.batch_get_sandboxes = AsyncMock(return_value=[]) + + # Test filtering by sandbox_id_alpha + result = await self.service.search_app_conversations( + sandbox_id__eq=sandbox_id_alpha + ) + + # Verify the info service was called with the correct sandbox_id__eq + self.mock_app_conversation_info_service.search_app_conversation_info.assert_called() + call_kwargs = self.mock_app_conversation_info_service.search_app_conversation_info.call_args[ + 1 + ] + assert call_kwargs.get('sandbox_id__eq') == sandbox_id_alpha + + # Verify only alpha conversation is returned + assert len(result.items) == 1 + assert result.items[0].sandbox_id == sandbox_id_alpha + + @pytest.mark.asyncio + async def test_count_app_conversations_with_sandbox_id_filter(self): + """Test that count_app_conversations passes sandbox_id__eq to the info service. + + This verifies that the sandbox_id filter is correctly propagated through + the service layer to the underlying info service for count operations. + """ + sandbox_id = 'sandbox-count-test-789' + + # Mock the info service to return count based on sandbox_id__eq + async def mock_count(sandbox_id__eq=None, **kwargs): + if sandbox_id__eq == sandbox_id: + return 3 # 3 conversations match this sandbox + else: + return 10 # 10 total conversations + + self.mock_app_conversation_info_service.count_app_conversation_info = AsyncMock( + side_effect=mock_count + ) + + # Test counting with sandbox_id filter + result = await self.service.count_app_conversations(sandbox_id__eq=sandbox_id) + + # Verify the info service was called with the correct sandbox_id__eq + self.mock_app_conversation_info_service.count_app_conversation_info.assert_called_once() + call_kwargs = self.mock_app_conversation_info_service.count_app_conversation_info.call_args[ + 1 + ] + assert call_kwargs.get('sandbox_id__eq') == sandbox_id + + # Verify filtered count is returned + assert result == 3 + + @pytest.mark.asyncio + async def test_search_app_conversations_sandbox_id_filter_returns_empty(self): + """Test that search with non-matching sandbox_id returns empty results.""" + from openhands.app_server.app_conversation.app_conversation_models import ( + AppConversationInfoPage, + ) + + # Mock the info service to return empty for non-matching sandbox + self.mock_app_conversation_info_service.search_app_conversation_info = ( + AsyncMock(return_value=AppConversationInfoPage(items=[])) + ) + self.mock_sandbox_service.batch_get_sandboxes = AsyncMock(return_value=[]) + + # Test filtering by non-existent sandbox_id + result = await self.service.search_app_conversations( + sandbox_id__eq='non-existent-sandbox' + ) + + # Verify empty results + assert len(result.items) == 0 + class TestPluginHandling: """Test cases for plugin-related functionality in LiveStatusAppConversationService."""