From 8d888601c8db72e6b29197978257e00eca61f1a2 Mon Sep 17 00:00:00 2001 From: hieptl Date: Thu, 17 Sep 2026 14:54:19 +0700 Subject: [PATCH] feat(app-conversation): launch child conversations server-side for Cloud parents A Cloud parent has no way to start a child conversation today. Agent Canvas registers its browser-executed child launch tool only for parents running on a local agent server, and AppConversationStartRequest cannot carry client tools. Add POST /api/v1/webhooks/conversations/{conversation_id}/children as the Cloud launcher for the SDK's start_child_conversation tool. It authenticates with the sandbox session key like the other webhook callbacks, checks that the parent belongs to the calling sandbox, and starts the child through start_app_conversation as the parent's owner, scoped to the parent's organization. parent_conversation_id is set on the start request, so the child inherits the parent's sandbox, repository, branch and model, and the usual quota and secrets handling applies. The response carries the child's id, launch status and URL; a failed start comes back as a 500 with the start task detail. Attach start_child_conversation to non-planner OpenHands agents whenever web_url is configured, with launch_url pointing at that endpoint, so the tool runs inside the sandbox and calls back into the app server instead of depending on a browser. This needs an openhands-sdk release that ships start_child_conversation; the dependency pins are bumped once that release exists. --- .../live_status_app_conversation_service.py | 17 ++ .../event_callback/webhook_router.py | 118 ++++++++- ...st_live_status_app_conversation_service.py | 69 +++++ .../test_webhook_router_child_conversation.py | 237 ++++++++++++++++++ 4 files changed, 440 insertions(+), 1 deletion(-) create mode 100644 tests/unit/app_server/test_webhook_router_child_conversation.py 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'