refactor(settings): nest AppPreferences under new misc_settings container

openhands-agent-server/openhands/agent_server/persistence/models.py

@@ -24,7 +24,6 @@
 
 from openhands.sdk.settings import (
     AgentSettingsConfig,
-    AppPreferences,
     ConversationSettings,
     default_agent_settings,
     validate_agent_settings,
@@ -35,23 +34,23 @@
 class SettingsUpdatePayload(TypedDict, total=False):
     """Typed payload for PersistedSettings.update() method.
 
-    The ``*_diff`` dicts are deep-merged via :func:`_deep_merge`: nested
+    All three ``*_diff`` dicts are deep-merged via :func:`_deep_merge`: nested
     objects merge recursively, and a ``None`` value *inside a nested map*
     deletes that entry (the "unset" primitive) — e.g. send
     ``{"acp_env": {"NAME": None}}`` to drop one env-var without re-sending the
     whole map. A ``None`` on a top-level *field* is not treated as delete; it
     flows to validation as before.
 
-    ``app_preferences_diff`` is a shallow overlay — fields present in the diff
-    overwrite the persisted values, fields absent are left alone. There is no
-    deep-merge or "unset" semantic because :class:`AppPreferences` has no
-    nested maps; ``disabled_skills`` is a list and callers expect a list write
-    to replace, not merge.
+    ``misc_settings_diff`` is deep-merged into the persisted ``misc_settings``
+    block. The agent-server treats ``misc_settings`` as opaque
+    frontend-owned data (it persists and merges, but does not interpret), so
+    any shape the client chooses is valid; lists are replaced wholesale by
+    the deep-merge.
     """
 
     agent_settings_diff: dict[str, Any]
     conversation_settings_diff: dict[str, Any]
-    app_preferences_diff: dict[str, Any]
+    misc_settings_diff: dict[str, Any]
     active_profile: str | None
 
 
@@ -118,10 +117,11 @@ class PersistedSettings(BaseModel):
     The ``active_profile`` field tracks which LLM profile was last activated,
     allowing frontends to display which profile is currently in use.
 
-    The ``app_preferences`` field stores frontend app-level preferences
-    (language, sound notifications, analytics consent, git identity,
-    disabled skills) that don't affect agent execution. See
-    :class:`openhands.sdk.settings.AppPreferences`.
+    The ``misc_settings`` field is an opaque dict the agent-server persists
+    on behalf of the frontend. The agent-server never reads its contents and
+    has no schema for it; clients are free to store any JSON-serializable
+    structure they need (e.g. app/UI preferences, analytics consent, git
+    identity used for in-conversation commits, etc.).
     """
 
     schema_version: int = Field(
@@ -137,12 +137,12 @@ class PersistedSettings(BaseModel):
         default=None,
         description="Name of the currently active LLM profile.",
     )
-    app_preferences: AppPreferences = Field(
-        default_factory=AppPreferences,
+    misc_settings: dict[str, Any] = Field(
+        default_factory=dict,
         description=(
-            "Frontend app-level user preferences (language, sound notifications, "
-            "analytics opt-in, git identity, disabled skills). Persisted but not "
-            "interpreted by the agent-server."
+            "Opaque dict the agent-server persists on behalf of the frontend. "
+            "Updated through misc_settings_diff (deep-merged); contents are "
+            "never read or validated by the agent-server."
         ),
     )
 
@@ -194,7 +194,7 @@ def update(self, payload: SettingsUpdatePayload) -> None:
         agent_update = payload.get("agent_settings_diff")
         conv_update = payload.get("conversation_settings_diff")
 
-        # Phase 1: Validate both updates before any mutations
+        # Phase 1: Validate all updates before any mutations
         new_agent: AgentSettingsConfig | None = None
         new_conv: ConversationSettings | None = None
         agent_merged: dict | None = None
@@ -253,28 +253,23 @@ def update(self, payload: SettingsUpdatePayload) -> None:
                         f"Failed to update conversation settings: {type(e).__name__}"
                     ) from None
 
-            # Validate app_preferences before mutating anything else
-            prefs_update = payload.get("app_preferences_diff")
-            new_prefs: AppPreferences | None = None
-            if isinstance(prefs_update, dict):
-                merged_prefs = {
-                    **self.app_preferences.model_dump(mode="json"),
-                    **prefs_update,
-                }
-                try:
-                    new_prefs = AppPreferences.model_validate(merged_prefs)
-                except Exception as e:
-                    raise ValueError(
-                        f"Failed to update app preferences: {type(e).__name__}"
-                    ) from None
+            # ``misc_settings`` is opaque: deep-merge without schema
+            # validation. The agent-server doesn't interpret what's inside,
+            # and ``misc_settings`` is not a secret container — the merged
+            # dict is therefore stored directly without the post-commit
+            # clear-down used by ``agent_settings`` / ``conversation_settings``.
+            misc_update = payload.get("misc_settings_diff")
+            new_misc: dict[str, Any] | None = None
+            if isinstance(misc_update, dict):
+                new_misc = _deep_merge(self.misc_settings, misc_update)
 
             # Phase 2: Apply validated changes atomically
             if new_agent is not None:
                 self.agent_settings = new_agent
             if new_conv is not None:
                 self.conversation_settings = new_conv
-            if new_prefs is not None:
-                self.app_preferences = new_prefs
+            if new_misc is not None:
+                self.misc_settings = new_misc
 
             # Update active_profile if explicitly provided (including None to clear)
             if "active_profile" in payload:
@@ -290,7 +285,14 @@ def update(self, payload: SettingsUpdatePayload) -> None:
     def from_persisted(
         cls, data: Any, *, context: dict[str, Any] | None = None
     ) -> PersistedSettings:
-        """Load persisted settings, applying top-level and nested migrations."""
+        """Load persisted settings.
+
+        Schema-version history:
+
+        - **v1**: ``agent_settings`` + ``conversation_settings`` only.
+          Missing ``misc_settings`` defaults to an empty dict.
+        - **v2** (current): adds the opaque ``misc_settings`` container.
+        """
         if not isinstance(data, dict):
             return cls.model_validate(data, context=context)
 
@@ -304,6 +306,7 @@ def from_persisted(
                 f"{version} is newer than supported version "
                 f"{PERSISTED_SETTINGS_SCHEMA_VERSION}"
             )
+
         payload["schema_version"] = PERSISTED_SETTINGS_SCHEMA_VERSION
         return cls.model_validate(payload, context=context)
 

openhands-agent-server/openhands/agent_server/settings_router.py

@@ -160,7 +160,7 @@ async def get_settings(request: Request) -> SettingsResponse:
                 mode="json"
             ),
             llm_api_key_is_set=settings.llm_api_key_is_set,
-            app_preferences=settings.app_preferences,
+            misc_settings=settings.misc_settings,
         )
 
 
@@ -171,11 +171,11 @@ async def update_settings(
     """Update settings with partial changes.
 
     Accepts ``agent_settings_diff``, ``conversation_settings_diff``, and/or
-    ``app_preferences_diff`` for incremental updates. The two ``*_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::
+    ``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::
 
         PATCH /api/settings
         {"agent_settings_diff": {"acp_env": {"STALE_KEY": null}}}
@@ -189,10 +189,10 @@ async def update_settings(
     is **not** an unset — it flows to model validation as before, so it still
     fails loudly rather than silently resetting the field to its default.
 
-    ``app_preferences_diff`` is a shallow overlay (no deep merge, no unset
-    semantics): each provided field overwrites the persisted value, and
-    omitted fields are left alone. ``disabled_skills`` lists are replaced
-    wholesale rather than merged.
+    ``misc_settings_diff`` is deep-merged into the persisted ``misc_settings``
+    block. The agent-server treats ``misc_settings`` as opaque frontend-owned
+    data: nested dicts are merged recursively, lists are replaced wholesale,
+    and the contents are never read or validated server-side.
 
     Uses file locking to prevent concurrent updates from overwriting each other.
 
@@ -209,7 +209,7 @@ async def update_settings(
             status_code=400,
             detail=(
                 "At least one of agent_settings_diff, "
-                "conversation_settings_diff, or app_preferences_diff "
+                "conversation_settings_diff, or misc_settings_diff "
                 "must be provided"
             ),
         )
@@ -231,7 +231,7 @@ def apply_update(settings: PersistedSettings) -> PersistedSettings:
                 "conversation_settings_modified": (
                     "conversation_settings_diff" in update_data
                 ),
-                "app_preferences_modified": "app_preferences_diff" in update_data,
+                "misc_settings_modified": "misc_settings_diff" in update_data,
             },
         )
     except (ValueError, ValidationError):
@@ -265,7 +265,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,
-        app_preferences=settings.app_preferences,
+        misc_settings=settings.misc_settings,
     )
 
 

openhands-sdk/openhands/sdk/settings/__init__.py

@@ -13,7 +13,6 @@
     get_acp_provider,
 )
 from .api_models import (
-    AppPreferences,
     SecretCreateRequest,
     SecretItemResponse,
     SecretsListResponse,
@@ -88,7 +87,6 @@
     "ACPFileSecretSpec",
     "ACPModelOption",
     "ACPProviderInfo",
-    "AppPreferences",
     "build_session_model_meta",
     "default_acp_file_secrets",
     "AGENT_SETTINGS_SCHEMA_VERSION",

openhands-sdk/openhands/sdk/settings/api_models.py

@@ -28,58 +28,22 @@
 
 from typing import TYPE_CHECKING, Any
 
-from pydantic import BaseModel, ConfigDict, Field, SecretStr
+from pydantic import BaseModel, Field, SecretStr
 
 
 if TYPE_CHECKING:
     from .model import AgentSettingsConfig, ConversationSettings
 
 
-# ── App Preferences ───────────────────────────────────────────────────────
-
-
-class AppPreferences(BaseModel):
-    """Frontend app-level user preferences that don't affect agent execution.
-
-    These fields are app/UI-level metadata (preferred language, sound
-    notifications, analytics opt-in, git identity used for in-conversation
-    commits) plus the list of skills the user has disabled. The agent-server
-    persists them alongside agent/conversation settings but does not interpret
-    them — the cloud equivalent (``POST /api/v1/settings``) accepts the same
-    keys at the top level, so frontends can use a single shape for both
-    backends.
-
-    Field semantics:
-
-    - ``language``, ``git_user_name``, ``git_user_email``: ``None`` means "no
-      preference set" (the frontend can fall back to its own default).
-    - ``user_consents_to_analytics``: tri-state — ``None`` means "not yet
-      asked", ``True``/``False`` are explicit answers.
-    - ``enable_sound_notifications``: ``None`` means "use frontend default";
-      ``True``/``False`` are explicit user choices.
-    - ``disabled_skills``: list of skill identifiers the user has disabled.
-      Defaults to an empty list (no skills disabled).
-    """
-
-    language: str | None = None
-    user_consents_to_analytics: bool | None = None
-    enable_sound_notifications: bool | None = None
-    git_user_name: str | None = None
-    git_user_email: str | None = None
-    disabled_skills: list[str] = Field(default_factory=list)
-
-    model_config = ConfigDict(extra="ignore")
-
-
 # ── Settings API Models ───────────────────────────────────────────────────
 
 
 class SettingsResponse(BaseModel):
     """Response model for GET /api/settings.
 
     Contains the full settings payload including agent configuration,
-    conversation settings, app-level user preferences, and a flag indicating
-    if an LLM API key is set.
+    conversation settings, 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
@@ -90,13 +54,16 @@ class SettingsResponse(BaseModel):
         response = SettingsResponse.model_validate(api_response.json())
         agent = response.get_agent_settings()  # Returns AgentSettingsConfig
         conv = response.get_conversation_settings()  # Returns ConversationSettings
-        prefs = response.app_preferences  # Already typed
+
+    ``misc_settings`` is an opaque container for frontend-owned data that the
+    agent-server persists but does not interpret — see the docstring of
+    :class:`PersistedSettings.misc_settings`.
     """
 
     agent_settings: dict[str, Any]
     conversation_settings: dict[str, Any]
     llm_api_key_is_set: bool
-    app_preferences: AppPreferences = Field(default_factory=AppPreferences)
+    misc_settings: dict[str, Any] = Field(default_factory=dict)
 
     def get_agent_settings(self) -> AgentSettingsConfig:
         """Parse and validate ``agent_settings`` into a typed model.
@@ -124,19 +91,17 @@ class SettingsUpdateRequest(BaseModel):
     """Request model for PATCH /api/settings.
 
     Supports partial updates via diff objects that are deep-merged with
-    existing settings.
-
-    ``app_preferences_diff`` accepts a partial :class:`AppPreferences` dict;
-    fields present in the diff are written through, fields omitted are left
-    untouched. The diff is *not* deep-merged because :class:`AppPreferences`
-    has no nested maps — every field is a scalar or a list, and a list
-    overwrite (rather than merge) is what callers expect for
-    ``disabled_skills``.
+    existing settings. ``misc_settings_diff`` is deep-merged into the
+    persisted ``misc_settings`` block with the same semantics as
+    ``agent_settings_diff`` and ``conversation_settings_diff``: nested dicts
+    merge recursively, and lists are replaced wholesale rather than merged.
+    Because ``misc_settings`` is opaque to the agent-server, callers are
+    responsible for the shape of what they store there.
     """
 
     agent_settings_diff: dict[str, Any] | None = None
     conversation_settings_diff: dict[str, Any] | None = None
-    app_preferences_diff: dict[str, Any] | None = None
+    misc_settings_diff: dict[str, Any] | None = None
 
 
 # ── Secrets API Models ────────────────────────────────────────────────────