Address review 3: 500 on settings corruption, UUID validation, comment trim

- activate: a corrupted/mis-keyed settings file (RuntimeError) is a server-side
  integrity failure → 500, not 409. Test added.
- SettingsUpdateRequest.active_agent_profile_id gains a UUID format pattern so
  malformed pointers are rejected at the HTTP layer (mirrors active_profile).
- Drop a diff-narrative comment in PersistedSettings.update().

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: Simon Rosenberg

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

@@ -512,10 +512,12 @@ def set_pointer(settings: PersistedSettings) -> PersistedSettings:
         logger.error("Failed to activate agent profile - file I/O error")
         raise HTTPException(status_code=500, detail="Failed to activate agent profile")
     except RuntimeError as e:
+        # A corrupted / mis-keyed settings file is a server-side integrity
+        # failure, not a client conflict.
         logger.error(f"Failed to activate agent profile: {e}")
         raise HTTPException(
-            status_code=status.HTTP_409_CONFLICT,
-            detail="Settings file is corrupted or encrypted with a different key",
+            status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
+            detail="Failed to activate agent profile",
         )
 
     logger.info(f"Activated agent profile id '{profile_id}'")

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

@@ -254,11 +254,9 @@ def update(self, payload: SettingsUpdatePayload) -> None:
             if new_misc is not None:
                 self.misc_settings = new_misc
 
-            # Update active_profile if explicitly provided (including None to clear)
+            # Update pointers if explicitly provided (including None to clear)
             if "active_profile" in payload:
                 self.active_profile = payload["active_profile"]
-
-            # Same for the AgentProfile pointer (including None to clear)
             if "active_agent_profile_id" in payload:
                 self.active_agent_profile_id = payload["active_agent_profile_id"]
         finally:

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

@@ -33,6 +33,14 @@
 from openhands.sdk.llm.llm_profile_store import PROFILE_NAME_PATTERN
 
 
+# An AgentProfile's stable id is a UUID (the pointer target); reject malformed
+# values at the HTTP layer, mirroring ``active_profile``'s name pattern.
+UUID_PATTERN = (
+    r"^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-"
+    r"[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$"
+)
+
+
 if TYPE_CHECKING:
     from .model import AgentSettingsConfig, ConversationSettings
 
@@ -119,6 +127,7 @@ class SettingsUpdateRequest(BaseModel):
     )
     active_agent_profile_id: str | None = Field(
         default=None,
+        pattern=UUID_PATTERN,
         description="Stable id of the active AgentProfile to persist; null clears it.",
     )
 

tests/agent_server/test_agent_profiles_router.py

@@ -375,6 +375,21 @@ def test_activate_unknown_id_returns_404(client, store):
     assert response.status_code == 404
 
 
+def test_activate_settings_corruption_returns_500(client, store, monkeypatch):
+    """A corrupted/mis-keyed settings file is a server-side failure (500)."""
+    from openhands.agent_server.persistence.store import FileSettingsStore
+
+    store.save(OpenHandsAgentProfile(name="p", llm_profile_ref="x"))
+    profile_id = client.get("/api/agent-profiles/p").json()["profile"]["id"]
+
+    def boom(self, *args, **kwargs):
+        raise RuntimeError("settings file corrupted")
+
+    monkeypatch.setattr(FileSettingsStore, "update", boom)
+    response = client.post(f"/api/agent-profiles/{profile_id}/activate")
+    assert response.status_code == 500
+
+
 # ── Seed fidelity (migration preserves the user's launch config) ────────────
 
 

tests/agent_server/test_settings_router.py

@@ -524,14 +524,15 @@ def test_patch_settings_rejects_invalid_active_profile(client_with_settings):
 
 def test_patch_settings_active_agent_profile_id_independent(client_with_settings):
     """active_agent_profile_id sets/clears independently of active_profile."""
+    agent_id = "12345678-1234-1234-1234-1234567890ab"
     set_response = client_with_settings.patch(
         "/api/settings",
-        json={"active_profile": "fast-profile", "active_agent_profile_id": "abc-123"},
+        json={"active_profile": "fast-profile", "active_agent_profile_id": agent_id},
     )
     assert set_response.status_code == 200
     body = set_response.json()
     assert body["active_profile"] == "fast-profile"
-    assert body["active_agent_profile_id"] == "abc-123"
+    assert body["active_agent_profile_id"] == agent_id
 
     # Clearing the agent pointer must leave the LLM profile pointer untouched.
     clear_response = client_with_settings.patch(
@@ -548,6 +549,15 @@ def test_patch_settings_active_agent_profile_id_independent(client_with_settings
     assert refetch["active_profile"] == "fast-profile"
 
 
+def test_patch_settings_rejects_malformed_active_agent_profile_id(client_with_settings):
+    """A non-UUID active_agent_profile_id is rejected at the HTTP layer."""
+    response = client_with_settings.patch(
+        "/api/settings",
+        json={"active_agent_profile_id": "not-a-uuid"},
+    )
+    assert response.status_code == 422
+
+
 def test_existing_settings_load_with_null_active_agent_profile_id(
     temp_persistence_dir, config_with_settings
 ):