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 8896f584f..60530f191 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 @@ -134,6 +134,7 @@ from openhands.sdk.secret import LookupSecret, StaticSecret from openhands.sdk.settings import ACPAgentSettings from openhands.sdk.subagent import get_registered_agent_definitions +from openhands.sdk.tool import Tool from openhands.sdk.tool.builtins import SwitchLLMTool from openhands.sdk.utils.redact import ( redact_api_key_literals, @@ -141,6 +142,7 @@ sanitize_config, ) from openhands.sdk.workspace.remote.async_remote_workspace import AsyncRemoteWorkspace +from openhands.tools.child_conversation import StartChildConversationTool from openhands.tools.preset.default import ( get_default_tools, register_builtins_agents, @@ -2171,6 +2173,21 @@ async def _build_start_conversation_request_for_user( ) if user.agent_settings.enable_sub_agents: agent_definitions = list(get_registered_agent_definitions()) + if self.web_url: + # Server-side child launches: the tool runs inside the sandbox + # and calls back into this app server, which provisions the + # child through the normal lifecycle (see webhook_router). + tools.append( + Tool( + name=StartChildConversationTool.name, + params={ + 'launch_url': ( + f'{self.web_url}/api/v1/webhooks/conversations/' + f'{conversation_id}/children' + ) + }, + ) + ) # --- build AgentSettings and create agent --------------------------- configured_agent_settings = user.agent_settings.model_copy( diff --git a/openhands/app_server/event_callback/webhook_router.py b/openhands/app_server/event_callback/webhook_router.py index 5c9d5d484..a1c811a96 100644 --- a/openhands/app_server/event_callback/webhook_router.py +++ b/openhands/app_server/event_callback/webhook_router.py @@ -22,7 +22,13 @@ from pydantic import SecretStr from openhands import tools # type: ignore[attr-defined] -from openhands.agent_server.models import ConversationInfo, Success +from openhands.agent_server.models import ( + ConversationInfo, + StartChildConversationRequest, + StartChildConversationResponse, + Success, + TextContent, +) from openhands.analytics import get_analytics_service, resolve_analytics_context from openhands.app_server import shared from openhands.app_server.app_conversation.app_conversation_info_service import ( @@ -31,7 +37,11 @@ from openhands.app_server.app_conversation.app_conversation_models import ( ACP_SERVER_TAG_KEY, AppConversationInfo, + AppConversationStartRequest, + AppConversationStartTask, + AppConversationStartTaskStatus, ConversationTrigger, + SendMessageRequest, ) from openhands.app_server.config import ( depends_app_conversation_info_service, @@ -624,6 +634,112 @@ async def _resolve_user_context(user_id: str | None) -> AuthUserContext: return AuthUserContext(user_auth=user_auth) +async def _resolve_conversation_org_id(conversation_id: UUID) -> UUID | None: + """Organization that owns *conversation_id* in SaaS; ``None`` in OSS mode. + + Imported lazily, like the daily-quota helpers: the SaaS storage layer only + exists in the enterprise deployment. + """ + try: + from sqlalchemy import select + + from storage.database import a_session_maker + from storage.stored_conversation_metadata_saas import ( + StoredConversationMetadataSaas, + ) + except ImportError: + return None + + async with a_session_maker() as session: + result = await session.execute( + select(StoredConversationMetadataSaas.org_id).where( + StoredConversationMetadataSaas.conversation_id == str(conversation_id) + ) + ) + return result.scalar_one_or_none() + + +@router.post( + '/conversations/{conversation_id}/children', + status_code=status.HTTP_201_CREATED, + responses={ + 404: {'description': 'Parent conversation not found'}, + 500: {'description': 'Child conversation failed to start'}, + }, +) +async def start_child_conversation( + conversation_id: UUID, + request: StartChildConversationRequest, + sandbox_record: SandboxRecord = Depends(valid_sandbox), +) -> StartChildConversationResponse: + """Launcher behind the ``start_child_conversation`` tool for Cloud parents. + + Called by the agent-server inside the parent's sandbox. The child is + provisioned through the normal app-conversation lifecycle as the parent's + owner, in the parent's organization, and linked via + ``parent_conversation_id``; sandbox, repository, branch and model are + inherited from the parent. ``isolation`` is not applicable here: children + share the parent's sandbox and its workspace layout. + """ + state = InjectorState() + setattr(state, USER_CONTEXT_ATTR, ADMIN) + async with get_app_conversation_info_service(state) as info_service: + parent = await info_service.get_app_conversation_info(conversation_id) + if parent is None: + raise HTTPException( + status.HTTP_404_NOT_FOUND, detail='Parent conversation not found' + ) + if ( + parent.created_by_user_id != sandbox_record.created_by_user_id + or parent.sandbox_id != sandbox_record.id + ): + raise AuthError() + + user_context = await _resolve_user_context(parent.created_by_user_id) + # Scope the launch to the parent's organization rather than the owner's + # currently selected one, so the lookup, quota and SaaS metadata all agree. + org_id = await _resolve_conversation_org_id(parent.id) + set_org_override = getattr( + user_context.user_auth, 'set_effective_org_id_override', None + ) + if org_id is not None and callable(set_org_override): + set_org_override(org_id) + + start_request = AppConversationStartRequest( + parent_conversation_id=parent.id, + initial_message=SendMessageRequest( + role='user', content=[TextContent(text=request.task)] + ), + title=request.title, + ) + start_state = InjectorState() + setattr(start_state, USER_CONTEXT_ATTR, user_context) + task: AppConversationStartTask | None = None + async with get_app_conversation_service(start_state) as app_conversation_service: + async for task in app_conversation_service.start_app_conversation( + start_request + ): + if task.status == AppConversationStartTaskStatus.ERROR: + raise HTTPException( + status.HTTP_500_INTERNAL_SERVER_ERROR, + detail=task.detail or 'Child conversation failed to start', + ) + if task is None or task.app_conversation_id is None: + raise HTTPException( + status.HTTP_500_INTERNAL_SERVER_ERROR, + detail='Child conversation did not become ready', + ) + + web_url = get_global_config().web_url + return StartChildConversationResponse( + conversation_id=task.app_conversation_id, + parent_conversation_id=parent.id, + status=task.status.value, + title=request.title, + url=f'{web_url}/conversations/{task.app_conversation_id}' if web_url else None, + ) + + @router.get('/secrets') async def get_secret( access_token: str = Depends(APIKeyHeader(name='X-Access-Token', auto_error=False)), 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 25f977aa9..c2ee1baac 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 @@ -1799,6 +1799,75 @@ async def test_build_request_without_remote_workspace(self, _mock_tools): assert isinstance(result, StartConversationRequest) assert result.conversation_id == conversation_id + @patch( + 'openhands.app_server.app_conversation.live_status_app_conversation_service.get_default_tools', + return_value=[], + ) + @pytest.mark.asyncio + async def test_build_request_adds_child_conversation_launcher(self, _mock_tools): + """Cloud agents get start_child_conversation pointed back at this app server.""" + self.mock_user_context.get_user_info.return_value = self.mock_user + + real_llm = LLM(model='gpt-4', api_key=SecretStr('test-key')) + self.service._setup_secrets_for_git_providers = AsyncMock(return_value={}) + self.service._configure_llm_and_mcp = AsyncMock(return_value=(real_llm, {})) + conversation_id = uuid4() + + result = await self.service._build_start_conversation_request_for_user( + user=self.mock_user, + sandbox=self.mock_sandbox, + conversation_id=conversation_id, + initial_message=None, + system_message_suffix=None, + git_provider=None, + working_dir='/test/dir', + remote_workspace=None, + ) + + launchers = [ + tool + for tool in result.agent.tools + if tool.name == 'start_child_conversation' + ] + assert len(launchers) == 1 + assert launchers[0].params == { + 'launch_url': ( + 'https://test.example.com/api/v1/webhooks/conversations/' + f'{conversation_id}/children' + ) + } + + @patch( + 'openhands.app_server.app_conversation.live_status_app_conversation_service.get_default_tools', + return_value=[], + ) + @pytest.mark.asyncio + async def test_build_request_skips_child_conversation_launcher_without_web_url( + self, _mock_tools + ): + """Without a reachable app server URL the tool is not offered at all.""" + self.mock_user_context.get_user_info.return_value = self.mock_user + self.service.web_url = None + + real_llm = LLM(model='gpt-4', api_key=SecretStr('test-key')) + self.service._setup_secrets_for_git_providers = AsyncMock(return_value={}) + self.service._configure_llm_and_mcp = AsyncMock(return_value=(real_llm, {})) + + result = await self.service._build_start_conversation_request_for_user( + user=self.mock_user, + sandbox=self.mock_sandbox, + conversation_id=uuid4(), + initial_message=None, + system_message_suffix=None, + git_provider=None, + working_dir='/test/dir', + remote_workspace=None, + ) + + assert all( + tool.name != 'start_child_conversation' for tool in result.agent.tools + ) + @patch( 'openhands.app_server.app_conversation.live_status_app_conversation_service.get_default_tools', return_value=[], diff --git a/tests/unit/app_server/test_webhook_router_child_conversation.py b/tests/unit/app_server/test_webhook_router_child_conversation.py new file mode 100644 index 000000000..80b286e4a --- /dev/null +++ b/tests/unit/app_server/test_webhook_router_child_conversation.py @@ -0,0 +1,237 @@ +"""Tests for the sandbox-authenticated child conversation launcher. + +``POST /api/v1/webhooks/conversations/{id}/children`` is the Cloud launcher +behind the ``start_child_conversation`` tool: one call must provision exactly +one child through the normal app-conversation lifecycle, linked to its parent. +""" + +from contextlib import asynccontextmanager +from types import SimpleNamespace +from unittest.mock import AsyncMock, MagicMock, patch +from uuid import UUID, uuid4 + +import pytest +from fastapi import HTTPException + +from openhands.agent_server.models import StartChildConversationRequest +from openhands.app_server.app_conversation.app_conversation_models import ( + AppConversationInfo, + AppConversationStartTask, + AppConversationStartTaskStatus, +) +from openhands.app_server.errors import AuthError +from openhands.app_server.event_callback.webhook_router import ( + start_child_conversation, +) +from openhands.app_server.sandbox.sandbox_models import SandboxRecord +from openhands.app_server.user.auth_user_context import AuthUserContext +from openhands.app_server.user_auth.default_user_auth import DefaultUserAuth + +ROUTER = 'openhands.app_server.event_callback.webhook_router' +CHILD_ID = uuid4() +READY_SEQUENCE = [ + (AppConversationStartTaskStatus.WORKING, None), + (AppConversationStartTaskStatus.READY, None), +] + + +class _OrgAwareUserAuth(DefaultUserAuth): + """DefaultUserAuth plus the SaaS org-override hook, to observe scoping.""" + + override_org_id: UUID | None = None + + def set_effective_org_id_override(self, org_id: UUID | None) -> None: + self.override_org_id = org_id + + +class _FakeAppConversationService: + """Records start requests and replays a scripted start-task sequence.""" + + def __init__(self, statuses): + self.statuses = statuses + self.requests = [] + + async def start_app_conversation(self, request): + self.requests.append(request) + for status, detail in self.statuses: + task = AppConversationStartTask( + created_by_user_id='user_123', + request=request, + status=status, + detail=detail, + ) + if status == AppConversationStartTaskStatus.READY: + task.app_conversation_id = CHILD_ID + yield task + + +@pytest.fixture +def sandbox_record() -> SandboxRecord: + return SandboxRecord(id='sandbox_123', created_by_user_id='user_123') + + +@pytest.fixture +def parent() -> AppConversationInfo: + return AppConversationInfo( + id=uuid4(), + title='Parent', + sandbox_id='sandbox_123', + created_by_user_id='user_123', + selected_repository='org/repo', + ) + + +@pytest.fixture +def user_auth() -> _OrgAwareUserAuth: + return _OrgAwareUserAuth() + + +@asynccontextmanager +async def _launcher(parent, app_service, user_auth, org_id=None): + """Wire the endpoint's collaborators to in-memory fakes.""" + + @asynccontextmanager + async def info_service_ctx(state, request=None): + service = AsyncMock() + service.get_app_conversation_info.return_value = parent + yield service + + @asynccontextmanager + async def app_service_ctx(state, request=None): + yield app_service + + with ( + patch(f'{ROUTER}.get_app_conversation_info_service', info_service_ctx), + patch(f'{ROUTER}.get_app_conversation_service', app_service_ctx), + patch( + f'{ROUTER}._resolve_user_context', + AsyncMock(return_value=AuthUserContext(user_auth=user_auth)), + ), + patch(f'{ROUTER}._resolve_conversation_org_id', AsyncMock(return_value=org_id)), + patch( + f'{ROUTER}.get_global_config', + MagicMock(return_value=SimpleNamespace(web_url='https://app.example')), + ), + ): + yield + + +@pytest.mark.asyncio +async def test_provisions_exactly_one_child_linked_to_parent( + sandbox_record, parent, user_auth +): + # Arrange + app_service = _FakeAppConversationService(READY_SEQUENCE) + request = StartChildConversationRequest(task='Write the docs', title='Docs') + + # Act + async with _launcher(parent, app_service, user_auth): + await start_child_conversation( + parent.id, request, sandbox_record=sandbox_record + ) + + # Assert + assert len(app_service.requests) == 1 + start_request = app_service.requests[0] + assert start_request.parent_conversation_id == parent.id + assert start_request.title == 'Docs' + assert start_request.initial_message is not None + assert start_request.initial_message.content[0].text == 'Write the docs' + + +@pytest.mark.asyncio +async def test_scopes_launch_to_parent_organization(sandbox_record, parent, user_auth): + # Arrange + app_service = _FakeAppConversationService(READY_SEQUENCE) + org_id = uuid4() + + # Act + async with _launcher(parent, app_service, user_auth, org_id=org_id): + await start_child_conversation( + parent.id, + StartChildConversationRequest(task='Write the docs'), + sandbox_record=sandbox_record, + ) + + # Assert + assert user_auth.override_org_id == org_id + + +@pytest.mark.asyncio +async def test_returns_child_identity_status_and_url(sandbox_record, parent, user_auth): + # Arrange + app_service = _FakeAppConversationService(READY_SEQUENCE) + + # Act + async with _launcher(parent, app_service, user_auth): + response = await start_child_conversation( + parent.id, + StartChildConversationRequest(task='Write the docs', title='Docs'), + sandbox_record=sandbox_record, + ) + + # Assert + assert response.conversation_id == CHILD_ID + assert response.parent_conversation_id == parent.id + assert response.status == 'READY' + assert response.title == 'Docs' + assert response.url == f'https://app.example/conversations/{CHILD_ID}' + + +@pytest.mark.asyncio +async def test_missing_parent_is_404(sandbox_record, user_auth): + # Arrange + app_service = _FakeAppConversationService(READY_SEQUENCE) + + # Act / Assert + async with _launcher(None, app_service, user_auth): + with pytest.raises(HTTPException) as exc_info: + await start_child_conversation( + uuid4(), + StartChildConversationRequest(task='anything'), + sandbox_record=sandbox_record, + ) + + assert exc_info.value.status_code == 404 + assert app_service.requests == [] + + +@pytest.mark.asyncio +async def test_parent_from_another_sandbox_is_rejected(parent, user_auth): + # Arrange + app_service = _FakeAppConversationService(READY_SEQUENCE) + other_sandbox = SandboxRecord(id='sandbox_999', created_by_user_id='user_123') + + # Act / Assert + async with _launcher(parent, app_service, user_auth): + with pytest.raises(AuthError): + await start_child_conversation( + parent.id, + StartChildConversationRequest(task='anything'), + sandbox_record=other_sandbox, + ) + + assert app_service.requests == [] + + +@pytest.mark.asyncio +async def test_failed_start_is_surfaced_with_detail(sandbox_record, parent, user_auth): + # Arrange + app_service = _FakeAppConversationService( + [ + (AppConversationStartTaskStatus.WORKING, None), + (AppConversationStartTaskStatus.ERROR, 'sandbox exploded'), + ] + ) + + # Act / Assert + async with _launcher(parent, app_service, user_auth): + with pytest.raises(HTTPException) as exc_info: + await start_child_conversation( + parent.id, + StartChildConversationRequest(task='anything'), + sandbox_record=sandbox_record, + ) + + assert exc_info.value.status_code == 500 + assert exc_info.value.detail == 'sandbox exploded'