diff --git a/openhands/app_server/app_conversation/app_conversation_router.py b/openhands/app_server/app_conversation/app_conversation_router.py index d3ad901db7..582de93761 100644 --- a/openhands/app_server/app_conversation/app_conversation_router.py +++ b/openhands/app_server/app_conversation/app_conversation_router.py @@ -115,7 +115,7 @@ async def _get_agent_server_context( app_conversation_service: AppConversationService, sandbox_service: SandboxService, sandbox_spec_service: SandboxSpecService, -) -> AgentServerContext | JSONResponse: +) -> AgentServerContext | JSONResponse | None: """Get the agent server context for a conversation. This helper retrieves all necessary information to communicate with the @@ -129,7 +129,8 @@ async def _get_agent_server_context( sandbox_spec_service: Service for sandbox spec operations Returns: - AgentServerContext if successful, or JSONResponse with error details. + AgentServerContext if successful, JSONResponse(404) if conversation + not found, or None if sandbox is not running (e.g. closed conversation). """ # Get the conversation info conversation = await app_conversation_service.get_app_conversation(conversation_id) @@ -141,12 +142,19 @@ async def _get_agent_server_context( # Get the sandbox info sandbox = await sandbox_service.get_sandbox(conversation.sandbox_id) - if not sandbox or sandbox.status != SandboxStatus.RUNNING: + if not sandbox: return JSONResponse( status_code=status.HTTP_404_NOT_FOUND, - content={ - 'error': f'Sandbox not found or not running for conversation {conversation_id}' - }, + content={'error': f'Sandbox not found for conversation {conversation_id}'}, + ) + # Return None for paused sandboxes (closed conversation) + if sandbox.status == SandboxStatus.PAUSED: + return None + # Return 404 for other non-running states (STARTING, ERROR, MISSING) + if sandbox.status != SandboxStatus.RUNNING: + return JSONResponse( + status_code=status.HTTP_404_NOT_FOUND, + content={'error': f'Sandbox not ready for conversation {conversation_id}'}, ) # Get the sandbox spec to find the working directory @@ -587,6 +595,7 @@ async def get_conversation_skills( Returns: JSONResponse: A JSON response containing the list of skills. + Returns an empty list if the sandbox is not running. """ try: # Get agent server context (conversation, sandbox, sandbox_spec, agent_server_url) @@ -598,6 +607,8 @@ async def get_conversation_skills( ) if isinstance(ctx, JSONResponse): return ctx + if ctx is None: + return JSONResponse(status_code=status.HTTP_200_OK, content={'skills': []}) # Load skills from all sources logger.info(f'Loading skills for conversation {conversation_id}') @@ -685,6 +696,7 @@ async def get_conversation_hooks( Returns: JSONResponse: A JSON response containing the list of hook event types. + Returns an empty list if the sandbox is not running. """ try: # Get agent server context (conversation, sandbox, sandbox_spec, agent_server_url) @@ -696,6 +708,8 @@ async def get_conversation_hooks( ) if isinstance(ctx, JSONResponse): return ctx + if ctx is None: + return JSONResponse(status_code=status.HTTP_200_OK, content={'hooks': []}) from openhands.app_server.app_conversation.hook_loader import ( fetch_hooks_from_agent_server, diff --git a/tests/unit/app_server/test_app_conversation_hooks_endpoint.py b/tests/unit/app_server/test_app_conversation_hooks_endpoint.py index ba67c4b488..ffc8c54d37 100644 --- a/tests/unit/app_server/test_app_conversation_hooks_endpoint.py +++ b/tests/unit/app_server/test_app_conversation_hooks_endpoint.py @@ -263,7 +263,7 @@ class TestGetConversationHooks: assert response.status_code == status.HTTP_404_NOT_FOUND - async def test_get_hooks_returns_404_when_sandbox_not_running(self): + async def test_get_hooks_returns_404_when_sandbox_not_found(self): conversation_id = uuid4() sandbox_id = str(uuid4()) @@ -291,3 +291,44 @@ class TestGetConversationHooks: ) assert response.status_code == status.HTTP_404_NOT_FOUND + + async def test_get_hooks_returns_empty_list_when_sandbox_paused(self): + conversation_id = uuid4() + sandbox_id = str(uuid4()) + + mock_conversation = AppConversation( + id=conversation_id, + created_by_user_id='test-user', + sandbox_id=sandbox_id, + sandbox_status=SandboxStatus.PAUSED, + ) + + mock_sandbox = SandboxInfo( + id=sandbox_id, + created_by_user_id='test-user', + status=SandboxStatus.PAUSED, + sandbox_spec_id=str(uuid4()), + session_api_key='test-api-key', + ) + + mock_app_conversation_service = MagicMock() + mock_app_conversation_service.get_app_conversation = AsyncMock( + return_value=mock_conversation + ) + + mock_sandbox_service = MagicMock() + mock_sandbox_service.get_sandbox = AsyncMock(return_value=mock_sandbox) + + response = await get_conversation_hooks( + conversation_id=conversation_id, + app_conversation_service=mock_app_conversation_service, + sandbox_service=mock_sandbox_service, + sandbox_spec_service=MagicMock(), + httpx_client=AsyncMock(spec=httpx.AsyncClient), + ) + + assert response.status_code == status.HTTP_200_OK + import json + + data = json.loads(response.body.decode('utf-8')) + assert data == {'hooks': []} diff --git a/tests/unit/app_server/test_app_conversation_skills_endpoint.py b/tests/unit/app_server/test_app_conversation_skills_endpoint.py index ed7fedd43d..6b601cf9db 100644 --- a/tests/unit/app_server/test_app_conversation_skills_endpoint.py +++ b/tests/unit/app_server/test_app_conversation_skills_endpoint.py @@ -203,7 +203,7 @@ class TestGetConversationSkills: Arrange: Setup conversation but no sandbox Act: Call get_conversation_skills endpoint - Assert: Response is 404 with sandbox error message + Assert: Response is 404 """ # Arrange conversation_id = uuid4() @@ -237,19 +237,13 @@ class TestGetConversationSkills: # Assert assert response.status_code == status.HTTP_404_NOT_FOUND - content = response.body.decode('utf-8') - import json - data = json.loads(content) - assert 'error' in data - assert 'Sandbox not found' in data['error'] + async def test_get_skills_returns_empty_list_when_sandbox_paused(self): + """Test endpoint returns empty skills when sandbox is PAUSED (closed conversation). - async def test_get_skills_returns_404_when_sandbox_not_running(self): - """Test endpoint returns 404 when sandbox is not in RUNNING state. - - Arrange: Setup conversation with stopped sandbox + Arrange: Setup conversation with paused sandbox Act: Call get_conversation_skills endpoint - Assert: Response is 404 with sandbox not running message + Assert: Response is 200 with empty skills list """ # Arrange conversation_id = uuid4() @@ -290,13 +284,12 @@ class TestGetConversationSkills: ) # Assert - assert response.status_code == status.HTTP_404_NOT_FOUND + assert response.status_code == status.HTTP_200_OK content = response.body.decode('utf-8') import json data = json.loads(content) - assert 'error' in data - assert 'not running' in data['error'] + assert data == {'skills': []} async def test_get_skills_handles_task_trigger_skills(self): """Test endpoint correctly handles skills with TaskTrigger.