From ae6e1150bc9abdcb6af5a55b5216689e4f1672c7 Mon Sep 17 00:00:00 2001 From: Simon Rosenberg Date: Thu, 18 Jun 2026 11:03:00 +0200 Subject: [PATCH 1/6] [AgentProfile][agent-server] agent_profile_id at conversation start + LaunchedProfile provenance (#3720) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Accept `agent_profile_id: UUID | None` on `StartConversationRequest` (mutually exclusive with `agent`/`agent_settings`, carry-only `exclude=True`). The SDK validator enforces exclusivity; resolution runs server-side so stores and the cipher are available. Resolution path (`_resolve_agent_from_profile`): - look up profile name from stable UUID via `AgentProfileStore.list_summaries()` - load and decrypt the profile (`AgentProfileStore.load`) - pull global `mcp_config` from `PersistedSettings` (already-decrypted by the file-store cipher context) - call `resolve_agent_profile()` → `AgentSettingsConfig.create_agent()` - return the built `AgentBase` and a `LaunchedProfile(id, revision)` provenance `_start_conversation` calls this via `asyncio.to_thread()` before the agent- requirement check, injects the resolved agent onto the request, and stamps the provenance on `StoredConversation`. `_compose_conversation_info` projects it onto `ConversationInfo`. `ProfileNotFound` maps to 404; `DanglingMcpServerRef` maps to 422 in the router. `LaunchedProfile` is a new declared field on both `StoredConversation` and `ConversationInfo` so ts-client `deriveSwitchPlan` can identify the active profile without fragile settings-comparison. Tests (20): SDK mutual-exclusivity, `_resolve_agent_from_profile` helper (OpenHands, ACP, unknown-id, dangling-ref), service-layer propagation, router HTTP mapping, and `LaunchedProfile` JSON round-trip. Co-Authored-By: Claude Sonnet 4.6 --- .../agent_server/conversation_router.py | 8 + .../agent_server/conversation_service.py | 96 +++- .../openhands/agent_server/models.py | 20 + .../openhands/sdk/conversation/request.py | 53 +- .../openhands/sdk/profiles/__init__.py | 2 + .../openhands/sdk/profiles/agent_profile.py | 17 + .../test_agent_profile_conv_start.py | 485 ++++++++++++++++++ 7 files changed, 664 insertions(+), 17 deletions(-) create mode 100644 tests/agent_server/test_agent_profile_conv_start.py 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..16aa860093 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,72 @@ def _prepare_request_workspace( logger = logging.getLogger(__name__) +def _resolve_agent_from_profile( + profile_id: "UUID", + cipher: "Cipher | None", +) -> "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). + + 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.agent_server.persistence import PersistedSettings, get_settings_store + from openhands.sdk.llm.llm_profile_store import LLMProfileStore + from openhands.sdk.profiles.agent_profile_store import AgentProfileStore + from openhands.sdk.profiles.resolver import ( + DanglingMcpServerRef as _DanglingMcpServerRef, + ProfileNotFound, + resolve_agent_profile, + ) + + # Find the profile name from its stable id. + store = AgentProfileStore() + profile_name: str | None = None + for summary in store.list_summaries(): + if str(summary.get("id")) == str(profile_id): + profile_name = summary.get("name") + break + + 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 + + # Global MCP config — already decrypted by the FileSettingsStore cipher context. + settings_store = get_settings_store() + settings = settings_store.load() or PersistedSettings() + mcp_config = settings.agent_settings.mcp_config + + llm_store = LLMProfileStore() + try: + settings_config = resolve_agent_profile( + profile, llm_store=llm_store, mcp_config=mcp_config, cipher=cipher + ) + except _DanglingMcpServerRef: + raise + except ProfileNotFound: + raise + 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 +376,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 +680,18 @@ 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: + resolved_agent, launched_profile = await asyncio.to_thread( + _resolve_agent_from_profile, + request.agent_profile_id, + self.cipher, + ) + request = request.model_copy(update={"agent": resolved_agent}) + request = _prepare_request_workspace(request, conversation_id) # Dynamically register tools from client's registry @@ -686,11 +766,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: diff --git a/openhands-agent-server/openhands/agent_server/models.py b/openhands-agent-server/openhands/agent_server/models.py index 35da059e8f..4d24641ba7 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 ( @@ -85,6 +86,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 +244,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..36076fcd2b 100644 --- a/openhands-sdk/openhands/sdk/conversation/request.py +++ b/openhands-sdk/openhands/sdk/conversation/request.py @@ -234,6 +234,17 @@ class StartConversationRequest(BaseModel): "used to construct the concrete agent." ), ) + agent_profile_id: UUID | None = Field( + default=None, + exclude=True, + 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,26 +253,38 @@ 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 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/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..16e7fb5c29 --- /dev/null +++ b/tests/agent_server/test_agent_profile_conv_start.py @@ -0,0 +1,485 @@ +"""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_is_excluded_from_model_dump(self): + """agent_profile_id is carry-only (exclude=True).""" + agent = _make_agent() + req = StartConversationRequest( + agent=agent, + workspace=LocalWorkspace(working_dir="/tmp"), + ) + dumped = req.model_dump(mode="json") + assert "agent_profile_id" not in dumped + + +# --------------------------------------------------------------------------- +# 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" +_SETTINGS_STORE_PATH = "openhands.agent_server.persistence.get_settings_store" +_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.list_summaries.return_value = [] + with pytest.raises(ProfileNotFound, match="not found"): + _resolve_agent_from_profile(uuid4(), cipher=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(_SETTINGS_STORE_PATH) as MockSettings, + patch(_RESOLVE_PATH) as MockResolve, + ): + store_inst = MockStore.return_value + store_inst.list_summaries.return_value = [ + {"id": str(profile.id), "name": profile.name} + ] + store_inst.load.return_value = profile + + MockSettings.return_value.load.return_value = MagicMock( + agent_settings=MagicMock(mcp_config=None) + ) + 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 + ) + + 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(_SETTINGS_STORE_PATH) as MockSettings, + patch(_RESOLVE_PATH) as MockResolve, + ): + store_inst = MockStore.return_value + store_inst.list_summaries.return_value = [ + {"id": str(profile.id), "name": profile.name} + ] + store_inst.load.return_value = profile + MockSettings.return_value.load.return_value = MagicMock( + agent_settings=MagicMock(mcp_config=None) + ) + MockResolve.side_effect = DanglingMcpServerRef(["missing-server"]) + + with pytest.raises(DanglingMcpServerRef) as exc_info: + _resolve_agent_from_profile(profile.id, cipher=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(_SETTINGS_STORE_PATH) as MockSettings, + patch(_RESOLVE_PATH) as MockResolve, + ): + store_inst = MockStore.return_value + store_inst.list_summaries.return_value = [ + {"id": str(profile.id), "name": profile.name} + ] + store_inst.load.return_value = profile + MockSettings.return_value.load.return_value = MagicMock( + agent_settings=MagicMock(mcp_config=None) + ) + 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 + ) + + 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_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 From c2b35a0b42ffa9370484626f201bb71cea35d16e Mon Sep 17 00:00:00 2001 From: Simon Rosenberg Date: Thu, 18 Jun 2026 11:50:51 +0200 Subject: [PATCH 2/6] =?UTF-8?q?[AgentProfile]=20add=20AgentProfileStore.na?= =?UTF-8?q?me=5Ffor=5Fid(),=20consolidate=20scattered=20id=E2=86=92name=20?= =?UTF-8?q?scans?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three callers open-coded the same O(n) list_summaries scan to resolve a stable UUID to a profile name. Add a single store method so callers don't each re-implement it: store.name_for_id(profile_id) -> str | None Update _resolve_agent_from_profile in conversation_service to use it, and fix test mocks accordingly (mock name_for_id instead of list_summaries). Co-Authored-By: Claude Sonnet 4.6 --- .../agent_server/conversation_service.py | 8 +------- .../openhands/sdk/profiles/agent_profile_store.py | 15 +++++++++++++++ .../agent_server/test_agent_profile_conv_start.py | 14 ++++---------- 3 files changed, 20 insertions(+), 17 deletions(-) diff --git a/openhands-agent-server/openhands/agent_server/conversation_service.py b/openhands-agent-server/openhands/agent_server/conversation_service.py index 16aa860093..bf66f565cd 100644 --- a/openhands-agent-server/openhands/agent_server/conversation_service.py +++ b/openhands-agent-server/openhands/agent_server/conversation_service.py @@ -263,14 +263,8 @@ def _resolve_agent_from_profile( resolve_agent_profile, ) - # Find the profile name from its stable id. store = AgentProfileStore() - profile_name: str | None = None - for summary in store.list_summaries(): - if str(summary.get("id")) == str(profile_id): - profile_name = summary.get("name") - break - + profile_name = store.name_for_id(profile_id) if profile_name is None: raise ProfileNotFound(f"Agent profile with id '{profile_id}' not found") 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 index 16e7fb5c29..3365625b28 100644 --- a/tests/agent_server/test_agent_profile_conv_start.py +++ b/tests/agent_server/test_agent_profile_conv_start.py @@ -160,7 +160,7 @@ def test_unknown_id_raises_profile_not_found(self): ) with patch(_STORE_PATH) as MockStore: - MockStore.return_value.list_summaries.return_value = [] + MockStore.return_value.name_for_id.return_value = None with pytest.raises(ProfileNotFound, match="not found"): _resolve_agent_from_profile(uuid4(), cipher=None) @@ -179,9 +179,7 @@ def test_openhands_profile_resolves_to_agent_and_stamps_launched(self): patch(_RESOLVE_PATH) as MockResolve, ): store_inst = MockStore.return_value - store_inst.list_summaries.return_value = [ - {"id": str(profile.id), "name": profile.name} - ] + store_inst.name_for_id.return_value = profile.name store_inst.load.return_value = profile MockSettings.return_value.load.return_value = MagicMock( @@ -212,9 +210,7 @@ def test_dangling_mcp_server_ref_propagates(self): patch(_RESOLVE_PATH) as MockResolve, ): store_inst = MockStore.return_value - store_inst.list_summaries.return_value = [ - {"id": str(profile.id), "name": profile.name} - ] + store_inst.name_for_id.return_value = profile.name store_inst.load.return_value = profile MockSettings.return_value.load.return_value = MagicMock( agent_settings=MagicMock(mcp_config=None) @@ -241,9 +237,7 @@ def test_acp_profile_resolves_to_acp_agent(self): patch(_RESOLVE_PATH) as MockResolve, ): store_inst = MockStore.return_value - store_inst.list_summaries.return_value = [ - {"id": str(profile.id), "name": profile.name} - ] + store_inst.name_for_id.return_value = profile.name store_inst.load.return_value = profile MockSettings.return_value.load.return_value = MagicMock( agent_settings=MagicMock(mcp_config=None) From 474950f7f3ed87825ebb39d3b4197045c816c967 Mon Sep 17 00:00:00 2001 From: Simon Rosenberg Date: Thu, 18 Jun 2026 12:03:55 +0200 Subject: [PATCH 3/6] fix: guard agent field serializer against None when profile-only request is dumped `StartConversationRequest` with `agent_profile_id` set holds `agent=None` (the server resolves it). Pydantic's `model_dump` calls the `AgentBase` model_serializer with `self=None`, crashing. The `@field_serializer("agent", mode="wrap")` short-circuits before the model_serializer fires and returns `None` directly. Co-Authored-By: Claude Sonnet 4.6 --- .../openhands/sdk/conversation/request.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/openhands-sdk/openhands/sdk/conversation/request.py b/openhands-sdk/openhands/sdk/conversation/request.py index 36076fcd2b..ff0ff6c0e9 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 @@ -287,6 +294,12 @@ def _require_agent(self) -> StartConversationRequest: ) 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. From 5b3e6dd83558561e7eb097c19b1fed453052c19f Mon Sep 17 00:00:00 2001 From: Simon Rosenberg Date: Thu, 18 Jun 2026 12:25:11 +0200 Subject: [PATCH 4/6] fix: derive mcp_config from service cipher instead of unconfigured singleton MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The free function _resolve_agent_from_profile called get_settings_store() without a Config, so if it ran before any other handler had initialised the singleton the cipher was missing and MCP config values stayed encrypted. Extract _load_mcp_config() which builds a short-lived FileSettingsStore from the ConversationService's conversations_dir + cipher — exactly mirroring _get_persistence_dir() in the store module — and pass the result in as an explicit mcp_config parameter, removing the singleton dependency from the resolver entirely. Co-Authored-By: Claude Sonnet 4.6 --- .../agent_server/conversation_service.py | 46 ++++++++++++++++--- .../test_agent_profile_conv_start.py | 21 ++------- 2 files changed, 44 insertions(+), 23 deletions(-) diff --git a/openhands-agent-server/openhands/agent_server/conversation_service.py b/openhands-agent-server/openhands/agent_server/conversation_service.py index bf66f565cd..0686169717 100644 --- a/openhands-agent-server/openhands/agent_server/conversation_service.py +++ b/openhands-agent-server/openhands/agent_server/conversation_service.py @@ -241,20 +241,54 @@ def _prepare_request_workspace( logger = logging.getLogger(__name__) +def _load_mcp_config( + conversations_dir: Path, + cipher: "Cipher | None", +) -> "Any": + """Load the global MCP config using the server's cipher. + + Derives the persistence dir from ``conversations_dir`` (or ``OH_PERSISTENCE_DIR`` + env var), exactly mirroring :func:`_get_persistence_dir` in the store module, + and builds a short-lived ``FileSettingsStore`` instance with the caller-supplied + cipher. This avoids touching the singleton — which may not yet have been + initialised with the correct cipher if this is the first settings-store access + after boot. + """ + import os + + from openhands.agent_server.persistence import FileSettingsStore, PersistedSettings + + env_dir = os.environ.get("OH_PERSISTENCE_DIR") + persistence_dir = ( + Path(env_dir) if env_dir else conversations_dir.parent / ".openhands" + ) + settings = ( + FileSettingsStore(persistence_dir=persistence_dir, cipher=cipher).load() + or PersistedSettings() + ) + return settings.agent_settings.mcp_config + + 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.agent_server.persistence import PersistedSettings, get_settings_store from openhands.sdk.llm.llm_profile_store import LLMProfileStore from openhands.sdk.profiles.agent_profile_store import AgentProfileStore from openhands.sdk.profiles.resolver import ( @@ -279,11 +313,6 @@ def _resolve_agent_from_profile( f"Failed to load agent profile '{profile_name}': {exc}" ) from exc - # Global MCP config — already decrypted by the FileSettingsStore cipher context. - settings_store = get_settings_store() - settings = settings_store.load() or PersistedSettings() - mcp_config = settings.agent_settings.mcp_config - llm_store = LLMProfileStore() try: settings_config = resolve_agent_profile( @@ -679,10 +708,15 @@ async def _start_conversation( # agent is captured in request_data. launched_profile: LaunchedProfile | None = None if request.agent_profile_id is not None: + # Load mcp_config using the service's own cipher so the free function + # never touches the settings-store singleton, which may not yet be + # initialised with the server cipher if this is the first request. + mcp_config = _load_mcp_config(self.conversations_dir, self.cipher) 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}) diff --git a/tests/agent_server/test_agent_profile_conv_start.py b/tests/agent_server/test_agent_profile_conv_start.py index 3365625b28..0dcb4c6259 100644 --- a/tests/agent_server/test_agent_profile_conv_start.py +++ b/tests/agent_server/test_agent_profile_conv_start.py @@ -149,7 +149,6 @@ def test_agent_profile_id_is_excluded_from_model_dump(self): # 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" -_SETTINGS_STORE_PATH = "openhands.agent_server.persistence.get_settings_store" _RESOLVE_PATH = "openhands.sdk.profiles.resolver.resolve_agent_profile" @@ -162,7 +161,7 @@ def test_unknown_id_raises_profile_not_found(self): 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) + _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 ( @@ -175,22 +174,18 @@ def test_openhands_profile_resolves_to_agent_and_stamps_launched(self): with ( patch(_STORE_PATH) as MockStore, patch(_LLM_STORE_PATH), - patch(_SETTINGS_STORE_PATH) as MockSettings, 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 - MockSettings.return_value.load.return_value = MagicMock( - agent_settings=MagicMock(mcp_config=None) - ) 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 + profile.id, cipher=None, mcp_config=None ) assert result_agent is agent @@ -206,19 +201,15 @@ def test_dangling_mcp_server_ref_propagates(self): with ( patch(_STORE_PATH) as MockStore, patch(_LLM_STORE_PATH), - patch(_SETTINGS_STORE_PATH) as MockSettings, 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 - MockSettings.return_value.load.return_value = MagicMock( - agent_settings=MagicMock(mcp_config=None) - ) MockResolve.side_effect = DanglingMcpServerRef(["missing-server"]) with pytest.raises(DanglingMcpServerRef) as exc_info: - _resolve_agent_from_profile(profile.id, cipher=None) + _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): @@ -233,21 +224,17 @@ def test_acp_profile_resolves_to_acp_agent(self): with ( patch(_STORE_PATH) as MockStore, patch(_LLM_STORE_PATH), - patch(_SETTINGS_STORE_PATH) as MockSettings, 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 - MockSettings.return_value.load.return_value = MagicMock( - agent_settings=MagicMock(mcp_config=None) - ) 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 + profile.id, cipher=None, mcp_config=None ) assert result_agent is acp_agent From 9074b800f7967316c9ca64e052ce759ff5843cac Mon Sep 17 00:00:00 2001 From: Simon Rosenberg Date: Thu, 18 Jun 2026 12:46:57 +0200 Subject: [PATCH 5/6] chore: address PR review feedback (#3784) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Drop dead `except ProfileNotFound: raise` / `except DanglingMcpServerRef: raise` blocks — these exceptions are not ValueError/TypeError so they were no-ops; let them propagate naturally. - Drop `_load_mcp_config` helper and its duplicated persistence-dir derivation. Instead, initialise the settings-store singleton with the server cipher in `ConversationService.get_instance(config)` (one-time, before any request can run), then call `get_settings_store()` in `_start_conversation` — no duplication, correct cipher guaranteed. Co-Authored-By: Claude Sonnet 4.6 --- .../agent_server/conversation_service.py | 56 +++++-------------- 1 file changed, 15 insertions(+), 41 deletions(-) diff --git a/openhands-agent-server/openhands/agent_server/conversation_service.py b/openhands-agent-server/openhands/agent_server/conversation_service.py index 0686169717..4c85b4d8a5 100644 --- a/openhands-agent-server/openhands/agent_server/conversation_service.py +++ b/openhands-agent-server/openhands/agent_server/conversation_service.py @@ -241,34 +241,6 @@ def _prepare_request_workspace( logger = logging.getLogger(__name__) -def _load_mcp_config( - conversations_dir: Path, - cipher: "Cipher | None", -) -> "Any": - """Load the global MCP config using the server's cipher. - - Derives the persistence dir from ``conversations_dir`` (or ``OH_PERSISTENCE_DIR`` - env var), exactly mirroring :func:`_get_persistence_dir` in the store module, - and builds a short-lived ``FileSettingsStore`` instance with the caller-supplied - cipher. This avoids touching the singleton — which may not yet have been - initialised with the correct cipher if this is the first settings-store access - after boot. - """ - import os - - from openhands.agent_server.persistence import FileSettingsStore, PersistedSettings - - env_dir = os.environ.get("OH_PERSISTENCE_DIR") - persistence_dir = ( - Path(env_dir) if env_dir else conversations_dir.parent / ".openhands" - ) - settings = ( - FileSettingsStore(persistence_dir=persistence_dir, cipher=cipher).load() - or PersistedSettings() - ) - return settings.agent_settings.mcp_config - - def _resolve_agent_from_profile( profile_id: "UUID", cipher: "Cipher | None", @@ -291,11 +263,7 @@ def _resolve_agent_from_profile( """ from openhands.sdk.llm.llm_profile_store import LLMProfileStore from openhands.sdk.profiles.agent_profile_store import AgentProfileStore - from openhands.sdk.profiles.resolver import ( - DanglingMcpServerRef as _DanglingMcpServerRef, - ProfileNotFound, - resolve_agent_profile, - ) + from openhands.sdk.profiles.resolver import ProfileNotFound, resolve_agent_profile store = AgentProfileStore() profile_name = store.name_for_id(profile_id) @@ -318,10 +286,6 @@ def _resolve_agent_from_profile( settings_config = resolve_agent_profile( profile, llm_store=llm_store, mcp_config=mcp_config, cipher=cipher ) - except _DanglingMcpServerRef: - raise - except ProfileNotFound: - raise except (TypeError, ValueError) as exc: raise ValueError(f"Profile '{profile_name}' failed to resolve: {exc}") from exc @@ -708,10 +672,15 @@ async def _start_conversation( # agent is captured in request_data. launched_profile: LaunchedProfile | None = None if request.agent_profile_id is not None: - # Load mcp_config using the service's own cipher so the free function - # never touches the settings-store singleton, which may not yet be - # initialised with the server cipher if this is the first request. - mcp_config = _load_mcp_config(self.conversations_dir, self.cipher) + # 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, @@ -1219,6 +1188,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, From 7726004067ab9b2b20536b4951f20c6bdd15fdbf Mon Sep 17 00:00:00 2001 From: Simon Rosenberg Date: Thu, 18 Jun 2026 12:59:36 +0200 Subject: [PATCH 6/6] fix: keep agent_profile_id serializable on StartConversationRequest (#3784) Remove exclude=True from agent_profile_id on the API request model so model_dump() includes it for HTTP transport. Override the field with exclude=True on StoredConversation (the persistence model) instead, and exclude it explicitly when building the request_data dict so the mutual-exclusivity validator is not triggered after resolution. Add regression tests covering both the transport and persistence paths. Co-Authored-By: Claude Sonnet 4.6 --- .../agent_server/conversation_service.py | 8 ++++- .../openhands/agent_server/models.py | 4 +++ .../openhands/sdk/conversation/request.py | 1 - .../test_agent_profile_conv_start.py | 35 ++++++++++++++++--- 4 files changed, 41 insertions(+), 7 deletions(-) diff --git a/openhands-agent-server/openhands/agent_server/conversation_service.py b/openhands-agent-server/openhands/agent_server/conversation_service.py index 4c85b4d8a5..f884562ac9 100644 --- a/openhands-agent-server/openhands/agent_server/conversation_service.py +++ b/openhands-agent-server/openhands/agent_server/conversation_service.py @@ -751,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 diff --git a/openhands-agent-server/openhands/agent_server/models.py b/openhands-agent-server/openhands/agent_server/models.py index 4d24641ba7..244dcaa0d7 100644 --- a/openhands-agent-server/openhands/agent_server/models.py +++ b/openhands-agent-server/openhands/agent_server/models.py @@ -79,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" diff --git a/openhands-sdk/openhands/sdk/conversation/request.py b/openhands-sdk/openhands/sdk/conversation/request.py index ff0ff6c0e9..60d2245657 100644 --- a/openhands-sdk/openhands/sdk/conversation/request.py +++ b/openhands-sdk/openhands/sdk/conversation/request.py @@ -243,7 +243,6 @@ class StartConversationRequest(BaseModel): ) agent_profile_id: UUID | None = Field( default=None, - exclude=True, description=( "Optional agent profile ID. When set, the agent-server resolves the " "referenced profile server-side (stores + cipher are required) and " diff --git a/tests/agent_server/test_agent_profile_conv_start.py b/tests/agent_server/test_agent_profile_conv_start.py index 0dcb4c6259..df90bccb1a 100644 --- a/tests/agent_server/test_agent_profile_conv_start.py +++ b/tests/agent_server/test_agent_profile_conv_start.py @@ -131,15 +131,16 @@ 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_is_excluded_from_model_dump(self): - """agent_profile_id is carry-only (exclude=True).""" - agent = _make_agent() + 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=agent, + agent_profile_id=profile_id, workspace=LocalWorkspace(working_dir="/tmp"), ) dumped = req.model_dump(mode="json") - assert "agent_profile_id" not in dumped + assert "agent_profile_id" in dumped + assert dumped["agent_profile_id"] == str(profile_id) # --------------------------------------------------------------------------- @@ -418,6 +419,30 @@ def test_stored_conversation_without_profile_has_none(self): ) 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)