From 598b381e3ddda7cdb8f996c2cc7dbcf0a057445b Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Mon, 9 Mar 2026 12:21:52 -0600 Subject: [PATCH] Added fallback for sandbox spec service (#13317) Co-authored-by: openhands --- .../app_conversation_router.py | 9 +-- .../test_app_conversation_skills_endpoint.py | 70 +++++++++++++++++++ 2 files changed, 75 insertions(+), 4 deletions(-) diff --git a/openhands/app_server/app_conversation/app_conversation_router.py b/openhands/app_server/app_conversation/app_conversation_router.py index 5a08d1d193..757c40de5d 100644 --- a/openhands/app_server/app_conversation/app_conversation_router.py +++ b/openhands/app_server/app_conversation/app_conversation_router.py @@ -507,10 +507,11 @@ async def get_conversation_skills( sandbox.sandbox_spec_id ) if not sandbox_spec: - return JSONResponse( - status_code=status.HTTP_404_NOT_FOUND, - content={'error': 'Sandbox spec not found'}, - ) + # TODO: This is a temporary work around for the fact that we don't store previous + # sandbox spec versions when updating OpenHands. When the SandboxSpecServices + # transition to truly multi sandbox spec model this should raise a 404 error + logger.warning('Sandbox spec not found - using default.') + sandbox_spec = await sandbox_spec_service.get_default_sandbox_spec() # Get the agent server URL if not sandbox.exposed_urls: 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 e84412bcd0..ed7fedd43d 100644 --- a/tests/unit/app_server/test_app_conversation_skills_endpoint.py +++ b/tests/unit/app_server/test_app_conversation_skills_endpoint.py @@ -501,3 +501,73 @@ class TestGetConversationSkills: data = json.loads(content) assert 'skills' in data assert len(data['skills']) == 0 + + async def test_get_skills_uses_default_sandbox_spec_when_not_found(self): + """Test endpoint uses default sandbox spec when original spec is not found. + + Arrange: Setup sandbox with spec that returns None, but default spec available + Act: Call get_conversation_skills endpoint + Assert: Response is successful using default sandbox spec + """ + # Arrange + 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.RUNNING, + ) + + mock_sandbox = SandboxInfo( + id=sandbox_id, + created_by_user_id='test-user', + status=SandboxStatus.RUNNING, + sandbox_spec_id=str(uuid4()), + session_api_key='test-api-key', + exposed_urls=[ + ExposedUrl(name=AGENT_SERVER, url='http://localhost:8000', port=8000) + ], + ) + + # Default sandbox spec to be used as fallback + default_sandbox_spec = SandboxSpecInfo( + id=str(uuid4()), command=None, working_dir='/workspace' + ) + + mock_user_context = MagicMock(spec=UserContext) + mock_app_conversation_service = _make_service_mock( + user_context=mock_user_context, + conversation_return=mock_conversation, + skills_return=[], + ) + + mock_sandbox_service = MagicMock() + mock_sandbox_service.get_sandbox = AsyncMock(return_value=mock_sandbox) + + mock_sandbox_spec_service = MagicMock() + # Original sandbox spec lookup returns None + mock_sandbox_spec_service.get_sandbox_spec = AsyncMock(return_value=None) + # Default sandbox spec is available + mock_sandbox_spec_service.get_default_sandbox_spec = AsyncMock( + return_value=default_sandbox_spec + ) + + # Act + response = await get_conversation_skills( + conversation_id=conversation_id, + app_conversation_service=mock_app_conversation_service, + sandbox_service=mock_sandbox_service, + sandbox_spec_service=mock_sandbox_spec_service, + ) + + # Assert + assert response.status_code == status.HTTP_200_OK + # Verify that get_default_sandbox_spec was called as fallback + mock_sandbox_spec_service.get_default_sandbox_spec.assert_called_once() + content = response.body.decode('utf-8') + import json + + data = json.loads(content) + assert 'skills' in data