diff --git a/openhands-agent-server/openhands/agent_server/persistence/models.py b/openhands-agent-server/openhands/agent_server/persistence/models.py index bb55df5745..6772cf3783 100644 --- a/openhands-agent-server/openhands/agent_server/persistence/models.py +++ b/openhands-agent-server/openhands/agent_server/persistence/models.py @@ -289,8 +289,8 @@ def from_persisted( Schema-version history: - - **v1**: ``agent_settings`` + ``conversation_settings`` only. - Missing ``misc_settings`` defaults to an empty dict. + - **v1**: ``agent_settings`` + ``conversation_settings`` plus + ``active_profile``. - **v2** (current): adds the opaque ``misc_settings`` container. """ if not isinstance(data, dict): diff --git a/openhands-agent-server/openhands/agent_server/profiles_router.py b/openhands-agent-server/openhands/agent_server/profiles_router.py index f2f6d882c9..5a2b5963ff 100644 --- a/openhands-agent-server/openhands/agent_server/profiles_router.py +++ b/openhands-agent-server/openhands/agent_server/profiles_router.py @@ -105,6 +105,23 @@ def _has_api_key(llm: LLM) -> bool: return bool(llm.api_key.get_secret_value().strip()) +def _set_active_profile_if_matches( + request: Request, old_name: str, new_name: str | None +) -> bool: + config = get_config(request) + settings_store = get_settings_store(config) + settings = settings_store.load() or PersistedSettings() + if settings.active_profile != old_name: + return False + + def update_active(settings: PersistedSettings) -> PersistedSettings: + settings.active_profile = new_name + return settings + + settings_store.update(update_active) + return True + + @profiles_router.get("", response_model=ProfileListResponse) async def list_profiles(request: Request) -> ProfileListResponse: """List all saved LLM profiles. @@ -207,11 +224,15 @@ async def save_profile( @profiles_router.delete("/{name}", response_model=ProfileMutationResponse) -async def delete_profile(name: ProfileName) -> ProfileMutationResponse: +async def delete_profile( + request: Request, name: ProfileName +) -> ProfileMutationResponse: """Delete a saved profile (idempotent).""" store = LLMProfileStore() with _store_errors(): store.delete(name) + if _set_active_profile_if_matches(request, name, None): + logger.info(f"Cleared active_profile for deleted profile '{name}'") logger.info(f"Deleted profile '{name}'") return ProfileMutationResponse(name=name, message=f"Profile '{name}' deleted") @@ -245,21 +266,10 @@ async def rename_profile( detail=f"Profile '{body.new_name}' already exists", ) - # Update active_profile if the renamed profile was the active one - if name != body.new_name: - config = get_config(request) - settings_store = get_settings_store(config) - settings = settings_store.load() or PersistedSettings() - - if settings.active_profile == name: - new_name = body.new_name - - def update_active(s: PersistedSettings) -> PersistedSettings: - s.active_profile = new_name - return s - - settings_store.update(update_active) - logger.info(f"Updated active_profile from '{name}' to '{new_name}'") + if name != body.new_name and _set_active_profile_if_matches( + request, name, body.new_name + ): + logger.info(f"Updated active_profile from '{name}' to '{body.new_name}'") if name == body.new_name: message = f"Profile '{name}' unchanged (same name)" diff --git a/openhands-agent-server/openhands/agent_server/settings_router.py b/openhands-agent-server/openhands/agent_server/settings_router.py index a09bd467cc..5c3b410a58 100644 --- a/openhands-agent-server/openhands/agent_server/settings_router.py +++ b/openhands-agent-server/openhands/agent_server/settings_router.py @@ -160,6 +160,7 @@ async def get_settings(request: Request) -> SettingsResponse: mode="json" ), llm_api_key_is_set=settings.llm_api_key_is_set, + active_profile=settings.active_profile, misc_settings=settings.misc_settings, ) @@ -170,12 +171,12 @@ async def update_settings( ) -> SettingsResponse: """Update settings with partial changes. - Accepts ``agent_settings_diff``, ``conversation_settings_diff``, and/or - ``misc_settings_diff`` for incremental updates. All three are deep-merged; - nested objects merge recursively, and a ``null`` value **inside a nested - map deletes that entry** — the "unset" primitive that lets a client - remove a single map key without round-tripping the whole map. To drop one - ACP env-var:: + Accepts ``agent_settings_diff``, ``conversation_settings_diff``, + ``misc_settings_diff``, and/or ``active_profile`` for incremental updates. + The three ``*_settings_diff`` fields are deep-merged; nested objects merge + recursively, and a ``null`` value **inside a nested map deletes that entry** + — the "unset" primitive that lets a client remove a single map key without + round-tripping the whole map. To drop one ACP env-var:: PATCH /api/settings {"agent_settings_diff": {"acp_env": {"STALE_KEY": null}}} @@ -203,14 +204,16 @@ async def update_settings( store = get_settings_store(config) update_data = payload.model_dump(exclude_none=True) + if "active_profile" in payload.model_fields_set: + update_data["active_profile"] = payload.active_profile if not update_data: # No updates provided - this is a client error raise HTTPException( status_code=400, detail=( "At least one of agent_settings_diff, " - "conversation_settings_diff, or misc_settings_diff " - "must be provided" + "conversation_settings_diff, misc_settings_diff, " + "or active_profile must be provided" ), ) @@ -265,6 +268,7 @@ def apply_update(settings: PersistedSettings) -> PersistedSettings: agent_settings=settings.agent_settings.model_dump(mode="json"), conversation_settings=settings.conversation_settings.model_dump(mode="json"), llm_api_key_is_set=settings.llm_api_key_is_set, + active_profile=settings.active_profile, misc_settings=settings.misc_settings, ) diff --git a/openhands-sdk/openhands/sdk/settings/api_models.py b/openhands-sdk/openhands/sdk/settings/api_models.py index 5d3a386979..52504941fe 100644 --- a/openhands-sdk/openhands/sdk/settings/api_models.py +++ b/openhands-sdk/openhands/sdk/settings/api_models.py @@ -30,6 +30,8 @@ from pydantic import BaseModel, Field, SecretStr +from openhands.sdk.llm.llm_profile_store import PROFILE_NAME_PATTERN + if TYPE_CHECKING: from .model import AgentSettingsConfig, ConversationSettings @@ -42,8 +44,8 @@ class SettingsResponse(BaseModel): """Response model for GET /api/settings. Contains the full settings payload including agent configuration, - conversation settings, miscellaneous frontend-owned settings, and a flag - indicating whether an LLM API key is set. + conversation settings, active LLM profile, miscellaneous frontend-owned + settings, and a flag indicating whether an LLM API key is set. The ``agent_settings`` and ``conversation_settings`` fields are raw dicts because the server controls secret serialization via context. Use the @@ -63,6 +65,10 @@ class SettingsResponse(BaseModel): agent_settings: dict[str, Any] conversation_settings: dict[str, Any] llm_api_key_is_set: bool + active_profile: str | None = Field( + default=None, + description="Name of the currently active LLM profile, if one is selected.", + ) misc_settings: dict[str, Any] = Field(default_factory=dict) def get_agent_settings(self) -> AgentSettingsConfig: @@ -102,6 +108,11 @@ class SettingsUpdateRequest(BaseModel): agent_settings_diff: dict[str, Any] | None = None conversation_settings_diff: dict[str, Any] | None = None misc_settings_diff: dict[str, Any] | None = None + active_profile: str | None = Field( + default=None, + pattern=PROFILE_NAME_PATTERN, + description="Name of the active LLM profile to persist; null clears it.", + ) # ── Secrets API Models ──────────────────────────────────────────────────── diff --git a/tests/agent_server/test_profiles_router.py b/tests/agent_server/test_profiles_router.py index 328304f443..45ff6c2dc8 100644 --- a/tests/agent_server/test_profiles_router.py +++ b/tests/agent_server/test_profiles_router.py @@ -241,6 +241,29 @@ def test_delete_profile_idempotent(client): assert body["name"] == "nonexistent" +def test_delete_active_profile_clears_active_profile(client, store): + """Deleting the active profile clears active_profile in settings.""" + llm = LLM(model="gpt-4o") + store.save("active-profile", llm) + store.save("other-profile", llm) + activate_response = client.post("/api/profiles/active-profile/activate") + assert activate_response.status_code == 200 + assert client.get("/api/settings").json()["active_profile"] == "active-profile" + + response = client.delete("/api/profiles/active-profile") + + assert response.status_code == 200 + settings_response = client.get("/api/settings") + assert settings_response.status_code == 200 + assert settings_response.json()["active_profile"] is None + + profiles_response = client.get("/api/profiles") + assert profiles_response.status_code == 200 + body = profiles_response.json() + assert body["active_profile"] is None + assert {profile["name"] for profile in body["profiles"]} == {"other-profile"} + + # ── Rename Profile ───────────────────────────────────────────────────────── @@ -1115,4 +1138,6 @@ def test_list_profiles_no_auto_create_after_deleting_active_profile(client, stor response = client.get("/api/profiles") assert response.status_code == 200 - assert response.json()["profiles"] == [] + body = response.json() + assert body["profiles"] == [] + assert body["active_profile"] is None diff --git a/tests/agent_server/test_settings_router.py b/tests/agent_server/test_settings_router.py index a4410810ea..0a883813fa 100644 --- a/tests/agent_server/test_settings_router.py +++ b/tests/agent_server/test_settings_router.py @@ -133,6 +133,7 @@ def test_get_settings_returns_default_settings(client_with_settings): assert "conversation_settings" in body assert "llm_api_key_is_set" in body assert body["llm_api_key_is_set"] is False + assert body["active_profile"] is None def test_get_settings_migrates_legacy_openhands_settings_and_resaves_current( @@ -214,6 +215,7 @@ def test_get_settings_migrates_legacy_openhands_settings_and_resaves_current( ) assert response.status_code == 200 body = response.json() + assert body["active_profile"] == "legacy-profile" agent_settings = body["agent_settings"] assert agent_settings["schema_version"] == AGENT_SETTINGS_SCHEMA_VERSION assert agent_settings["agent_kind"] == "openhands" @@ -483,6 +485,43 @@ def test_patch_settings_updates_llm_config(client_with_settings): assert body["llm_api_key_is_set"] is True +def test_patch_settings_updates_active_profile(client_with_settings): + """PATCH /api/settings can update and clear the active LLM profile.""" + response = client_with_settings.patch( + "/api/settings", + json={"active_profile": "fast-profile"}, + ) + + assert response.status_code == 200 + assert response.json()["active_profile"] == "fast-profile" + + refetch = client_with_settings.get("/api/settings") + assert refetch.status_code == 200 + assert refetch.json()["active_profile"] == "fast-profile" + + clear_response = client_with_settings.patch( + "/api/settings", + json={"active_profile": None}, + ) + + assert clear_response.status_code == 200 + assert clear_response.json()["active_profile"] is None + + refetch = client_with_settings.get("/api/settings") + assert refetch.status_code == 200 + assert refetch.json()["active_profile"] is None + + +def test_patch_settings_rejects_invalid_active_profile(client_with_settings): + """PATCH /api/settings validates active profile names.""" + response = client_with_settings.patch( + "/api/settings", + json={"active_profile": "not a valid profile"}, + ) + + assert response.status_code == 422 + + def test_patch_settings_updates_condenser_config(client_with_settings): """PATCH /api/settings can update condenser constructor settings.""" response = client_with_settings.patch( @@ -600,7 +639,11 @@ def test_patch_settings_empty_payload_returns_400(client_with_settings): response = client_with_settings.patch("/api/settings", json={}) assert response.status_code == 400 - assert "At least one of" in response.json()["detail"] + assert response.json()["detail"] == ( + "At least one of agent_settings_diff, " + "conversation_settings_diff, misc_settings_diff, " + "or active_profile must be provided" + ) # ── misc_settings (opaque frontend-owned container) ─────────────────────