Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
42 changes: 26 additions & 16 deletions openhands-agent-server/openhands/agent_server/profiles_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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")

Expand Down Expand Up @@ -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)"
Expand Down
20 changes: 12 additions & 8 deletions openhands-agent-server/openhands/agent_server/settings_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)

Expand All @@ -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}}}
Expand Down Expand Up @@ -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"
),
)

Expand Down Expand Up @@ -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,
)

Expand Down
15 changes: 13 additions & 2 deletions openhands-sdk/openhands/sdk/settings/api_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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:
Expand Down Expand Up @@ -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 ────────────────────────────────────────────────────
Expand Down
27 changes: 26 additions & 1 deletion tests/agent_server/test_profiles_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 ─────────────────────────────────────────────────────────


Expand Down Expand Up @@ -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
45 changes: 44 additions & 1 deletion tests/agent_server/test_settings_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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) ─────────────────────
Expand Down
Loading