diff --git a/openhands-agent-server/openhands/agent_server/conversation_router.py b/openhands-agent-server/openhands/agent_server/conversation_router.py index aeaca118d4..ed1caf86e1 100644 --- a/openhands-agent-server/openhands/agent_server/conversation_router.py +++ b/openhands-agent-server/openhands/agent_server/conversation_router.py @@ -41,6 +41,7 @@ ) from openhands.sdk import LLM, Agent, TextContent from openhands.sdk.conversation.state import ConversationExecutionStatus +from openhands.sdk.profiles.resolver import DanglingMcpServerRef, ProfileNotFound from openhands.sdk.tool.client_tool import ClientToolRegistrationError from openhands.sdk.workspace import LocalWorkspace from openhands.tools.preset.default import get_default_tools @@ -197,6 +198,13 @@ async def start_conversation( """Start a conversation in the local environment.""" try: info, is_new = await conversation_service.start_conversation(request) + except ProfileNotFound as e: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=str(e)) from e + except DanglingMcpServerRef as e: + raise HTTPException( + status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, + detail={"message": str(e), "dangling_mcp_server_refs": e.missing}, + ) from e except ClientToolRegistrationError as e: raise HTTPException( status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, detail=str(e) diff --git a/openhands-agent-server/openhands/agent_server/conversation_service.py b/openhands-agent-server/openhands/agent_server/conversation_service.py index b588377769..f884562ac9 100644 --- a/openhands-agent-server/openhands/agent_server/conversation_service.py +++ b/openhands-agent-server/openhands/agent_server/conversation_service.py @@ -21,6 +21,7 @@ ConversationInfo, ConversationPage, ConversationSortOrder, + LaunchedProfile, StartConversationRequest, StoredConversation, UpdateConversationRequest, @@ -240,6 +241,59 @@ def _prepare_request_workspace( logger = logging.getLogger(__name__) +def _resolve_agent_from_profile( + profile_id: "UUID", + cipher: "Cipher | None", + mcp_config: "Any", +) -> "tuple[AgentBase, LaunchedProfile]": + """Load and resolve an agent profile by id, returning the built agent + provenance. + + Runs synchronously (call via ``asyncio.to_thread`` from async context). + + Args: + mcp_config: Global MCP config already loaded by the caller using the + server's cipher. Passed explicitly so this free function never + touches the settings-store singleton (which may not have been + initialised with the correct cipher yet). + + Raises: + ProfileNotFound: No stored profile has ``profile_id``. + DanglingMcpServerRef: A referenced MCP server is absent from the global config. + ValueError: Profile load or settings validation failure. + """ + from openhands.sdk.llm.llm_profile_store import LLMProfileStore + from openhands.sdk.profiles.agent_profile_store import AgentProfileStore + from openhands.sdk.profiles.resolver import ProfileNotFound, resolve_agent_profile + + store = AgentProfileStore() + profile_name = store.name_for_id(profile_id) + if profile_name is None: + raise ProfileNotFound(f"Agent profile with id '{profile_id}' not found") + + try: + profile = store.load(profile_name, cipher=cipher) + except FileNotFoundError: + raise ProfileNotFound( + f"Agent profile '{profile_name}' (id={profile_id}) not found" + ) + except ValueError as exc: + raise ValueError( + f"Failed to load agent profile '{profile_name}': {exc}" + ) from exc + + llm_store = LLMProfileStore() + try: + settings_config = resolve_agent_profile( + profile, llm_store=llm_store, mcp_config=mcp_config, cipher=cipher + ) + except (TypeError, ValueError) as exc: + raise ValueError(f"Profile '{profile_name}' failed to resolve: {exc}") from exc + + agent = settings_config.create_agent() + launched = LaunchedProfile(profile_id=profile.id, revision=profile.revision) + return agent, launched + + def _compose_conversation_info( stored: StoredConversation, state: ConversationState ) -> ConversationInfo: @@ -309,6 +363,7 @@ def _compose_conversation_info( available_models=available_models, supports_runtime_model_switch=supports_runtime_model_switch, client_tools=stored.client_tools, + launched_profile=stored.launched_profile, ) @@ -612,6 +667,28 @@ async def _start_conversation( ) return conversation_info, False + # Profile resolution must happen before _prepare_request_workspace (which + # asserts request.agent is not None) and before model_dump so the resolved + # agent is captured in request_data. + launched_profile: LaunchedProfile | None = None + if request.agent_profile_id is not None: + # get_settings_store() is safe here: get_instance() initialises the + # singleton with the server cipher before any conversation can start. + from openhands.agent_server.persistence import ( + PersistedSettings, + get_settings_store, + ) + + settings = get_settings_store().load() or PersistedSettings() + mcp_config = settings.agent_settings.mcp_config + resolved_agent, launched_profile = await asyncio.to_thread( + _resolve_agent_from_profile, + request.agent_profile_id, + self.cipher, + mcp_config, + ) + request = request.model_copy(update={"agent": resolved_agent}) + request = _prepare_request_workspace(request, conversation_id) # Dynamically register tools from client's registry @@ -674,7 +751,13 @@ async def _start_conversation( # serialize to plain strings. Pass expose_secrets=True so StaticSecret values # are preserved through the round-trip; the dict is only used in-process to # construct StoredConversation, not sent over the network. - request_data = request.model_dump(mode="json", context={"expose_secrets": True}) + # agent_profile_id is excluded: it was resolved into `launched_profile` + # above and must not re-trigger the mutual-exclusivity validator. + request_data = request.model_dump( + mode="json", + context={"expose_secrets": True}, + exclude={"agent_profile_id"}, + ) # If secrets_encrypted=True, the agent's secrets (e.g., LLM api_key) are # cipher-encrypted and need decryption during model validation. Pass the @@ -686,11 +769,23 @@ async def _start_conversation( "Set OH_SECRET_KEY environment variable." ) stored = StoredConversation.model_validate( - {"id": conversation_id, **request_data}, + { + "id": conversation_id, + **request_data, + "launched_profile": ( + launched_profile.model_dump(mode="json") + if launched_profile is not None + else None + ), + }, context={"cipher": self.cipher}, ) else: - stored = StoredConversation(id=conversation_id, **request_data) + stored = StoredConversation( + id=conversation_id, + launched_profile=launched_profile, + **request_data, + ) event_service = await self._start_event_service(stored) initial_message = request.initial_message if initial_message: @@ -1099,6 +1194,11 @@ async def __aexit__(self, exc_type, exc_value, traceback): @classmethod def get_instance(cls, config: Config) -> "ConversationService": + # Initialise the settings-store singleton with the server cipher before + # any conversation handler can call get_settings_store() without config. + from openhands.agent_server.persistence import get_settings_store + + get_settings_store(config) return ConversationService( conversations_dir=config.conversations_path, webhook_specs=config.webhooks, diff --git a/openhands-agent-server/openhands/agent_server/models.py b/openhands-agent-server/openhands/agent_server/models.py index 35da059e8f..244dcaa0d7 100644 --- a/openhands-agent-server/openhands/agent_server/models.py +++ b/openhands-agent-server/openhands/agent_server/models.py @@ -28,6 +28,7 @@ TextContent as TextContent, ) from openhands.sdk.llm.utils.metrics import MetricsSnapshot +from openhands.sdk.profiles.agent_profile import LaunchedProfile as LaunchedProfile from openhands.sdk.secret import SecretSource from openhands.sdk.security.analyzer import SecurityAnalyzerBase from openhands.sdk.security.confirmation_policy import ( @@ -78,6 +79,10 @@ class StoredConversation(StartConversationRequest): Extends StartConversationRequest with server-assigned fields. """ + # agent_profile_id is resolved into launched_profile at creation; exclude from + # the persistence payload so it does not re-appear in meta.json. + agent_profile_id: UUID | None = Field(default=None, exclude=True) + id: OpenHandsUUID title: str | None = Field( default=None, description="User-defined title for the conversation" @@ -85,6 +90,15 @@ class StoredConversation(StartConversationRequest): metrics: MetricsSnapshot | None = None created_at: datetime = Field(default_factory=utc_now) updated_at: datetime = Field(default_factory=utc_now) + launched_profile: LaunchedProfile | None = Field( + default=None, + description=( + "Provenance snapshot of the agent profile that launched this " + "conversation. Set at creation when `agent_profile_id` is supplied; " + "``None`` for conversations started directly with `agent` or " + "`agent_settings`." + ), + ) class _ConversationInfoBase(BaseModel): @@ -234,6 +248,16 @@ class _ConversationInfoBase(BaseModel): "and before the conversation has started a session." ), ) + launched_profile: LaunchedProfile | None = Field( + default=None, + description=( + "Provenance snapshot of the agent profile that launched this " + "conversation. Set at creation when the conversation was started via " + "``agent_profile_id``; ``None`` for conversations started directly " + "with ``agent`` or ``agent_settings``. Clients use this to identify " + "which profile is current without fragile settings-comparison." + ), + ) class ConversationInfo(_ConversationInfoBase): diff --git a/openhands-sdk/openhands/sdk/conversation/request.py b/openhands-sdk/openhands/sdk/conversation/request.py index c97c498f41..60d2245657 100644 --- a/openhands-sdk/openhands/sdk/conversation/request.py +++ b/openhands-sdk/openhands/sdk/conversation/request.py @@ -11,7 +11,14 @@ from typing import Annotated, Any, Literal, cast from uuid import UUID -from pydantic import BaseModel, Discriminator, Field, Tag, model_validator +from pydantic import ( + BaseModel, + Discriminator, + Field, + Tag, + field_serializer, + model_validator, +) from openhands.sdk.agent.acp_agent import ACPAgent as ACPAgent from openhands.sdk.agent.agent import Agent as Agent @@ -234,6 +241,16 @@ class StartConversationRequest(BaseModel): "used to construct the concrete agent." ), ) + agent_profile_id: UUID | None = Field( + default=None, + description=( + "Optional agent profile ID. When set, the agent-server resolves the " + "referenced profile server-side (stores + cipher are required) and " + "builds the agent from it. Mutually exclusive with `agent` and " + "`agent_settings`. The SDK validator enforces exclusivity only — " + "resolution happens in conversation_service, not here." + ), + ) agent: AgentBase = Field(default=cast(AgentBase, None)) @model_validator(mode="before") @@ -242,28 +259,46 @@ def _populate_agent_from_settings(cls, data: Any) -> Any: if not isinstance(data, dict): return data payload = dict(data) - if payload.get("agent") is None and payload.get("agent_settings") is not None: - from openhands.sdk.settings.model import validate_agent_settings + has_profile_id = payload.get("agent_profile_id") is not None + has_agent = payload.get("agent") is not None + has_agent_settings = payload.get("agent_settings") is not None + if has_profile_id and (has_agent or has_agent_settings): + raise ValueError( + "`agent_profile_id` is mutually exclusive with" + " `agent` and `agent_settings`" + ) + if not has_profile_id: + if payload.get("agent") is None and has_agent_settings: + from openhands.sdk.settings.model import validate_agent_settings - try: - payload["agent"] = validate_agent_settings( - payload["agent_settings"] - ).create_agent() - except (TypeError, ValueError) as exc: - raise ValueError(str(exc)) from exc - elif isinstance(payload.get("agent"), dict): - agent_payload = dict(payload["agent"]) - if "kind" not in agent_payload and "llm" in agent_payload: - agent_payload["kind"] = "Agent" - payload["agent"] = agent_payload + try: + payload["agent"] = validate_agent_settings( + payload["agent_settings"] + ).create_agent() + except (TypeError, ValueError) as exc: + raise ValueError(str(exc)) from exc + elif isinstance(payload.get("agent"), dict): + agent_payload = dict(payload["agent"]) + if "kind" not in agent_payload and "llm" in agent_payload: + agent_payload["kind"] = "Agent" + payload["agent"] = agent_payload return payload @model_validator(mode="after") def _require_agent(self) -> StartConversationRequest: - if self.agent is None: - raise ValueError("Either `agent` or `agent_settings` must be provided") + if self.agent is None and self.agent_profile_id is None: + raise ValueError( + "One of `agent`, `agent_settings`, or" + " `agent_profile_id` must be provided" + ) return self + @field_serializer("agent", mode="wrap") + def _serialize_agent(self, value: AgentBase | None, handler: Any) -> Any: + if value is None: + return None + return handler(value) + class StartACPConversationRequest(StartConversationRequest): """Deprecated compatibility alias for ACP-capable start requests. diff --git a/openhands-sdk/openhands/sdk/profiles/__init__.py b/openhands-sdk/openhands/sdk/profiles/__init__.py index 1800a6c04c..5339c96be4 100644 --- a/openhands-sdk/openhands/sdk/profiles/__init__.py +++ b/openhands-sdk/openhands/sdk/profiles/__init__.py @@ -5,6 +5,7 @@ ACPAgentProfile, AgentProfile, AgentProfileBase, + LaunchedProfile, OpenHandsAgentProfile, ProfileVerificationSettings, validate_agent_profile, @@ -37,6 +38,7 @@ "AgentProfileDiagnostics", "AgentProfileStore", "DanglingMcpServerRef", + "LaunchedProfile", "OpenHandsAgentProfile", "ProfileLimitExceeded", "ProfileNotFound", diff --git a/openhands-sdk/openhands/sdk/profiles/agent_profile.py b/openhands-sdk/openhands/sdk/profiles/agent_profile.py index 83197b8d8a..27bc5d6097 100644 --- a/openhands-sdk/openhands/sdk/profiles/agent_profile.py +++ b/openhands-sdk/openhands/sdk/profiles/agent_profile.py @@ -209,6 +209,23 @@ class ACPAgentProfile(AgentProfileBase): ) +class LaunchedProfile(BaseModel): + """Provenance snapshot recorded when a profile launches a conversation. + + Stored on ``StoredConversation`` and projected onto ``ConversationInfo`` so + ts-client ``deriveSwitchPlan`` can identify which profile is current without + fragile settings-comparison. See #3720. + """ + + profile_id: UUID = Field( + description="Stable id of the profile that launched the conversation." + ) + revision: int = Field( + ge=0, + description="Revision of the profile at launch time.", + ) + + def _agent_profile_discriminator(value: Any) -> str: """Discriminator for :data:`AgentProfile` — defaults to ``'openhands'``. diff --git a/openhands-sdk/openhands/sdk/profiles/agent_profile_store.py b/openhands-sdk/openhands/sdk/profiles/agent_profile_store.py index ebad0ef199..bfd1c3a012 100644 --- a/openhands-sdk/openhands/sdk/profiles/agent_profile_store.py +++ b/openhands-sdk/openhands/sdk/profiles/agent_profile_store.py @@ -17,6 +17,8 @@ if TYPE_CHECKING: + from uuid import UUID + from openhands.sdk.profiles.agent_profile import ( ACPAgentProfile, OpenHandsAgentProfile, @@ -305,3 +307,16 @@ def list_summaries(self) -> list[dict[str, Any]]: } ) return summaries + + def name_for_id(self, profile_id: str | UUID) -> str | None: + """Return the stored name for a stable profile id, or ``None`` if not found. + + Scans ``list_summaries()`` under the lock so the lookup is consistent + with the on-disk state at the time of the call. Mirrors the id→name + resolution that used to be open-coded by each caller. + """ + target = str(profile_id) + for summary in self.list_summaries(): + if str(summary.get("id")) == target: + return str(summary["name"]) + return None diff --git a/tests/agent_server/test_agent_profile_conv_start.py b/tests/agent_server/test_agent_profile_conv_start.py new file mode 100644 index 0000000000..df90bccb1a --- /dev/null +++ b/tests/agent_server/test_agent_profile_conv_start.py @@ -0,0 +1,491 @@ +"""Tests for agent_profile_id at conversation start + LaunchedProfile provenance. + +Covers: +- start-from-profile (OpenHands + ACP paths) +- mutual-exclusivity validation (SDK layer) +- unknown-id 404 / dangling-ref 422 (router layer) +- LaunchedProfile provenance round-trip through StoredConversation +""" + +from __future__ import annotations + +from datetime import UTC, datetime +from typing import Any +from unittest.mock import AsyncMock, MagicMock, patch +from uuid import UUID, uuid4 + +import pytest +from fastapi import FastAPI +from fastapi.testclient import TestClient +from pydantic import ValidationError + +from openhands.agent_server.config import Config +from openhands.agent_server.conversation_router import conversation_router +from openhands.agent_server.conversation_service import ConversationService +from openhands.agent_server.dependencies import get_conversation_service +from openhands.agent_server.event_service import EventService +from openhands.agent_server.models import ( + ConversationInfo, + LaunchedProfile, + StartConversationRequest, + StoredConversation, +) +from openhands.sdk import LLM, Agent +from openhands.sdk.conversation.state import ( + ConversationExecutionStatus, + ConversationState, +) +from openhands.sdk.profiles.agent_profile import ( + ACPAgentProfile, + OpenHandsAgentProfile, +) +from openhands.sdk.profiles.resolver import DanglingMcpServerRef, ProfileNotFound +from openhands.sdk.workspace import LocalWorkspace + + +# --------------------------------------------------------------------------- +# Fixtures +# --------------------------------------------------------------------------- + + +@pytest.fixture +def client(): + """TestClient with no auth — conversations router only.""" + app = FastAPI() + app.include_router(conversation_router, prefix="/api") + app.state.config = Config( + static_files_path=None, session_api_keys=[], secret_key=None + ) + return TestClient(app) + + +@pytest.fixture +def mock_conversation_service(): + return AsyncMock(spec=ConversationService) + + +def _make_openhands_profile(profile_id: UUID | None = None) -> OpenHandsAgentProfile: + return OpenHandsAgentProfile( + id=profile_id or uuid4(), + name="my-profile", + revision=3, + llm_profile_ref="default", + ) + + +def _make_acp_profile(profile_id: UUID | None = None) -> ACPAgentProfile: + return ACPAgentProfile( + id=profile_id or uuid4(), + name="acp-profile", + revision=1, + acp_server="claude-code", + ) + + +def _make_agent() -> Agent: + return Agent(llm=LLM(model="gpt-4o", usage_id="llm"), tools=[]) + + +# --------------------------------------------------------------------------- +# SDK-layer: mutual exclusivity (StartConversationRequest) +# --------------------------------------------------------------------------- + + +class TestStartConversationRequestValidation: + def test_agent_profile_id_alone_is_valid(self): + req = StartConversationRequest( + agent_profile_id=uuid4(), + workspace=LocalWorkspace(working_dir="/tmp"), + ) + assert req.agent_profile_id is not None + assert req.agent is None + + def test_agent_alone_is_valid(self): + req = StartConversationRequest( + agent=_make_agent(), + workspace=LocalWorkspace(working_dir="/tmp"), + ) + assert req.agent is not None + assert req.agent_profile_id is None + + def test_agent_profile_id_and_agent_is_invalid(self): + with pytest.raises(ValidationError, match="mutually exclusive"): + StartConversationRequest( + agent_profile_id=uuid4(), + agent=_make_agent(), + workspace=LocalWorkspace(working_dir="/tmp"), + ) + + def test_agent_profile_id_and_agent_settings_is_invalid(self): + with pytest.raises(ValidationError, match="mutually exclusive"): + StartConversationRequest( + agent_profile_id=uuid4(), + agent_settings={ + "agent_kind": "openhands", + "llm": {"model": "gpt-4o", "usage_id": "llm"}, + }, + workspace=LocalWorkspace(working_dir="/tmp"), + ) + + def test_no_agent_source_is_invalid(self): + with pytest.raises(ValidationError, match="agent_profile_id"): + StartConversationRequest(workspace=LocalWorkspace(working_dir="/tmp")) + + def test_agent_profile_id_present_in_request_payload(self): + """agent_profile_id must survive model_dump() for HTTP transport.""" + profile_id = uuid4() + req = StartConversationRequest( + agent_profile_id=profile_id, + workspace=LocalWorkspace(working_dir="/tmp"), + ) + dumped = req.model_dump(mode="json") + assert "agent_profile_id" in dumped + assert dumped["agent_profile_id"] == str(profile_id) + + +# --------------------------------------------------------------------------- +# Service-layer: _resolve_agent_from_profile helper +# --------------------------------------------------------------------------- + +# The helper does local imports inside the function body; patch at the source modules. +_STORE_PATH = "openhands.sdk.profiles.agent_profile_store.AgentProfileStore" +_LLM_STORE_PATH = "openhands.sdk.llm.llm_profile_store.LLMProfileStore" +_RESOLVE_PATH = "openhands.sdk.profiles.resolver.resolve_agent_profile" + + +class TestResolveAgentFromProfile: + def test_unknown_id_raises_profile_not_found(self): + from openhands.agent_server.conversation_service import ( + _resolve_agent_from_profile, + ) + + with patch(_STORE_PATH) as MockStore: + MockStore.return_value.name_for_id.return_value = None + with pytest.raises(ProfileNotFound, match="not found"): + _resolve_agent_from_profile(uuid4(), cipher=None, mcp_config=None) + + def test_openhands_profile_resolves_to_agent_and_stamps_launched(self): + from openhands.agent_server.conversation_service import ( + _resolve_agent_from_profile, + ) + + profile = _make_openhands_profile() + agent = _make_agent() + + with ( + patch(_STORE_PATH) as MockStore, + patch(_LLM_STORE_PATH), + patch(_RESOLVE_PATH) as MockResolve, + ): + store_inst = MockStore.return_value + store_inst.name_for_id.return_value = profile.name + store_inst.load.return_value = profile + + mock_config = MagicMock() + mock_config.create_agent.return_value = agent + MockResolve.return_value = mock_config + + result_agent, launched = _resolve_agent_from_profile( + profile.id, cipher=None, mcp_config=None + ) + + assert result_agent is agent + assert launched.profile_id == profile.id + assert launched.revision == profile.revision + + def test_dangling_mcp_server_ref_propagates(self): + from openhands.agent_server.conversation_service import ( + _resolve_agent_from_profile, + ) + + profile = _make_openhands_profile() + with ( + patch(_STORE_PATH) as MockStore, + patch(_LLM_STORE_PATH), + patch(_RESOLVE_PATH) as MockResolve, + ): + store_inst = MockStore.return_value + store_inst.name_for_id.return_value = profile.name + store_inst.load.return_value = profile + MockResolve.side_effect = DanglingMcpServerRef(["missing-server"]) + + with pytest.raises(DanglingMcpServerRef) as exc_info: + _resolve_agent_from_profile(profile.id, cipher=None, mcp_config=None) + assert "missing-server" in exc_info.value.missing + + def test_acp_profile_resolves_to_acp_agent(self): + from openhands.agent_server.conversation_service import ( + _resolve_agent_from_profile, + ) + from openhands.sdk.agent.acp_agent import ACPAgent + + profile = _make_acp_profile() + acp_agent = MagicMock(spec=ACPAgent) + + with ( + patch(_STORE_PATH) as MockStore, + patch(_LLM_STORE_PATH), + patch(_RESOLVE_PATH) as MockResolve, + ): + store_inst = MockStore.return_value + store_inst.name_for_id.return_value = profile.name + store_inst.load.return_value = profile + mock_config = MagicMock() + mock_config.create_agent.return_value = acp_agent + MockResolve.return_value = mock_config + + result_agent, launched = _resolve_agent_from_profile( + profile.id, cipher=None, mcp_config=None + ) + + assert result_agent is acp_agent + assert launched.profile_id == profile.id + assert launched.revision == profile.revision + + +# --------------------------------------------------------------------------- +# Service-layer: conversation start with agent_profile_id +# --------------------------------------------------------------------------- + + +class TestConversationServiceStartFromProfile: + @pytest.mark.asyncio + async def test_start_from_profile_stamps_launched_profile_on_stored(self, tmp_path): + """_start_conversation passes launched_profile to StoredConversation.""" + profile_id = uuid4() + agent = _make_agent() + launched_profile = LaunchedProfile(profile_id=profile_id, revision=5) + request = StartConversationRequest( + agent_profile_id=profile_id, + workspace=LocalWorkspace(working_dir=str(tmp_path)), + ) + + captured: dict[str, Any] = {} + mock_state = ConversationState( + id=uuid4(), + agent=agent, + workspace=request.workspace, + execution_status=ConversationExecutionStatus.IDLE, + ) + + with patch( + "openhands.agent_server.conversation_service._resolve_agent_from_profile", + return_value=(agent, launched_profile), + ): + service = ConversationService(conversations_dir=tmp_path) + service._event_services = {} + + with patch.object( + service, "_start_event_service", new_callable=AsyncMock + ) as mock_ses: + mock_es = AsyncMock(spec=EventService) + mock_es.get_state.return_value = mock_state + mock_es.stored = MagicMock( + launched_profile=launched_profile, + client_tools=[], + title=None, + metrics=None, + created_at=datetime.now(UTC), + updated_at=datetime.now(UTC), + ) + + async def capture_start(stored): + captured["stored"] = stored + return mock_es + + mock_ses.side_effect = capture_start + + info, is_new = await service.start_conversation(request) + + stored = captured.get("stored") + assert stored is not None, "StoredConversation was not captured" + assert stored.launched_profile is not None + assert stored.launched_profile.profile_id == profile_id + assert stored.launched_profile.revision == 5 + # The resolved agent (not None) must be present + assert stored.agent is not None + + @pytest.mark.asyncio + async def test_profile_not_found_propagates(self, tmp_path): + request = StartConversationRequest( + agent_profile_id=uuid4(), + workspace=LocalWorkspace(working_dir=str(tmp_path)), + ) + + with patch( + "openhands.agent_server.conversation_service._resolve_agent_from_profile", + side_effect=ProfileNotFound("profile not found"), + ): + service = ConversationService(conversations_dir=tmp_path) + service._event_services = {} + + with pytest.raises(ProfileNotFound): + await service.start_conversation(request) + + @pytest.mark.asyncio + async def test_dangling_ref_propagates_from_service(self, tmp_path): + request = StartConversationRequest( + agent_profile_id=uuid4(), + workspace=LocalWorkspace(working_dir=str(tmp_path)), + ) + + with patch( + "openhands.agent_server.conversation_service._resolve_agent_from_profile", + side_effect=DanglingMcpServerRef(["mcp-server-x"]), + ): + service = ConversationService(conversations_dir=tmp_path) + service._event_services = {} + + with pytest.raises(DanglingMcpServerRef) as exc_info: + await service.start_conversation(request) + assert "mcp-server-x" in exc_info.value.missing + + +# --------------------------------------------------------------------------- +# Router-layer: HTTP error mapping +# --------------------------------------------------------------------------- + + +class TestConversationRouterProfileErrors: + def test_profile_not_found_returns_404(self, client, mock_conversation_service): + mock_conversation_service.start_conversation.side_effect = ProfileNotFound( + "Agent profile with id 'abc' not found" + ) + client.app.dependency_overrides[get_conversation_service] = lambda: ( + mock_conversation_service + ) + + payload = { + "agent_profile_id": str(uuid4()), + "workspace": {"working_dir": "/tmp/test", "kind": "LocalWorkspace"}, + } + resp = client.post("/api/conversations", json=payload) + assert resp.status_code == 404 + assert "not found" in resp.json().get("detail", "").lower() + + def test_dangling_mcp_server_ref_returns_422( + self, client, mock_conversation_service + ): + mock_conversation_service.start_conversation.side_effect = DanglingMcpServerRef( + ["missing-server", "another-missing"] + ) + client.app.dependency_overrides[get_conversation_service] = lambda: ( + mock_conversation_service + ) + + payload = { + "agent_profile_id": str(uuid4()), + "workspace": {"working_dir": "/tmp/test", "kind": "LocalWorkspace"}, + } + resp = client.post("/api/conversations", json=payload) + assert resp.status_code == 422 + detail = resp.json().get("detail", {}) + assert "dangling_mcp_server_refs" in detail + assert "missing-server" in detail["dangling_mcp_server_refs"] + + +# --------------------------------------------------------------------------- +# Provenance round-trip: LaunchedProfile survives serialization +# --------------------------------------------------------------------------- + + +class TestLaunchedProfileRoundTrip: + def test_launched_profile_survives_stored_conversation_round_trip(self): + """LaunchedProfile survives model_dump/model_validate round-trip.""" + profile_id = uuid4() + lp = LaunchedProfile(profile_id=profile_id, revision=7) + stored = StoredConversation( + id=uuid4(), + agent=_make_agent(), + workspace=LocalWorkspace(working_dir="/tmp"), + launched_profile=lp, + ) + + dumped = stored.model_dump(mode="json") + assert dumped["launched_profile"] is not None + assert dumped["launched_profile"]["profile_id"] == str(profile_id) + assert dumped["launched_profile"]["revision"] == 7 + + reloaded = StoredConversation.model_validate({"id": str(stored.id), **dumped}) + assert reloaded.launched_profile is not None + assert reloaded.launched_profile.profile_id == profile_id + assert reloaded.launched_profile.revision == 7 + + def test_stored_conversation_without_profile_has_none(self): + stored = StoredConversation( + id=uuid4(), + agent=_make_agent(), + workspace=LocalWorkspace(working_dir="/tmp"), + ) + assert stored.launched_profile is None + + def test_agent_profile_id_excluded_from_stored_conversation_persistence(self): + """Regression: agent_profile_id must NOT appear in StoredConversation payload. + + StartConversationRequest.model_dump() includes agent_profile_id for HTTP + transport. _start_conversation excludes it before building StoredConversation + (the field is resolved into launched_profile); this test verifies that a + StoredConversation built from a resolved request contains neither the raw + profile UUID nor re-exposes it. + """ + profile_id = uuid4() + # Simulate the resolved state: agent is set, agent_profile_id excluded. + request = StartConversationRequest( + agent_profile_id=profile_id, + workspace=LocalWorkspace(working_dir="/tmp"), + ) + # Mirror what _start_conversation does: exclude agent_profile_id from + # the persistence payload before constructing StoredConversation. + request_data = request.model_dump(mode="json", exclude={"agent_profile_id"}) + agent = _make_agent() + request_data["agent"] = agent.model_dump(mode="json") + stored = StoredConversation(id=uuid4(), **request_data) + dumped = stored.model_dump(mode="json") + assert "agent_profile_id" not in dumped + + def test_launched_profile_in_conversation_info(self): + profile_id = uuid4() + lp = LaunchedProfile(profile_id=profile_id, revision=3) + now = datetime.now(UTC) + info = ConversationInfo( + id=uuid4(), + agent=_make_agent(), + workspace=LocalWorkspace(working_dir="/tmp"), + execution_status=ConversationExecutionStatus.IDLE, + created_at=now, + updated_at=now, + launched_profile=lp, + ) + assert info.launched_profile is not None + assert info.launched_profile.profile_id == profile_id + assert info.launched_profile.revision == 3 + + def test_conversation_info_without_profile_is_none(self): + now = datetime.now(UTC) + info = ConversationInfo( + id=uuid4(), + agent=_make_agent(), + workspace=LocalWorkspace(working_dir="/tmp"), + execution_status=ConversationExecutionStatus.IDLE, + created_at=now, + updated_at=now, + ) + assert info.launched_profile is None + + def test_launched_profile_survives_json_serialization(self, tmp_path): + """Simulate meta.json round-trip: dump → write → read → validate.""" + profile_id = uuid4() + lp = LaunchedProfile(profile_id=profile_id, revision=5) + stored = StoredConversation( + id=uuid4(), + agent=_make_agent(), + workspace=LocalWorkspace(working_dir=str(tmp_path)), + launched_profile=lp, + ) + meta_file = tmp_path / "meta.json" + meta_file.write_text(stored.model_dump_json()) + + reloaded = StoredConversation.model_validate_json(meta_file.read_text()) + assert reloaded.launched_profile is not None + assert reloaded.launched_profile.profile_id == profile_id + assert reloaded.launched_profile.revision == 5