diff --git a/enterprise/storage/saas_settings_store.py b/enterprise/storage/saas_settings_store.py index bbc8c82fc3..a9e96a2da2 100644 --- a/enterprise/storage/saas_settings_store.py +++ b/enterprise/storage/saas_settings_store.py @@ -191,17 +191,15 @@ class SaasSettingsStore(SettingsStore): if hasattr(model, key): setattr(model, key, value) - # Map Settings fields to Org fields with 'default_' prefix - # The generic loop above doesn't update these because Org uses - # 'default_llm_model' not 'llm_model', etc. - # Use exclude_unset to only update explicitly-set fields (allows clearing with null) - settings_data = item.model_dump(exclude_unset=True) - if 'llm_model' in settings_data: - org.default_llm_model = settings_data['llm_model'] - if 'llm_base_url' in settings_data: - org.default_llm_base_url = settings_data['llm_base_url'] - if 'max_iterations' in settings_data: - org.default_max_iterations = settings_data['max_iterations'] + # Map explicitly provided SDK-managed settings onto Org defaults. + # These values now live in item.agent_settings, so inspect the + # dotted keys directly instead of relying on model_dump(). + if 'llm.model' in item.agent_settings: + org.default_llm_model = item.llm_model + if 'llm.base_url' in item.agent_settings: + org.default_llm_base_url = item.llm_base_url + if 'max_iterations' in item.agent_settings: + org.default_max_iterations = item.max_iterations # Propagate LLM settings to all org members # This ensures all members see the same LLM configuration when an admin saves diff --git a/enterprise/tests/unit/test_saas_settings_store.py b/enterprise/tests/unit/test_saas_settings_store.py index e936b10b6d..52f7f54106 100644 --- a/enterprise/tests/unit/test_saas_settings_store.py +++ b/enterprise/tests/unit/test_saas_settings_store.py @@ -76,9 +76,35 @@ def settings_store(async_session_maker, mock_config): del item_dict['email_verified'] if 'secrets_store' in item_dict: del item_dict['secrets_store'] + if 'agent_settings' in item_dict: + del item_dict['agent_settings'] + + legacy_fields = { + 'agent': item.agent, + 'llm_model': item.llm_model, + 'llm_api_key': item.llm_api_key.get_secret_value() + if item.llm_api_key + else None, + 'llm_base_url': item.llm_base_url, + 'max_iterations': item.max_iterations, + 'confirmation_mode': item.confirmation_mode, + 'security_analyzer': item.security_analyzer, + 'enable_default_condenser': item.enable_default_condenser, + 'condenser_max_size': item.condenser_max_size, + } + item_dict.update( + { + key: value + for key, value in legacy_fields.items() + if value is not None + } + ) + + sdk_settings_values = item.sdk_settings_values # Encrypt the data before storing store._encrypt_kwargs(item_dict) + item_dict['sdk_settings_values'] = sdk_settings_values # Continue with the original implementation from sqlalchemy import select @@ -130,10 +156,8 @@ async def test_store_and_load_keycloak_user(settings_store): # Load and verify settings loaded_settings = await settings_store.load() assert loaded_settings is not None - assert loaded_settings.sdk_settings_values == { - 'critic_mode': 'all_actions', - 'enable_critic': True, - } + assert loaded_settings.sdk_settings_values['critic_mode'] == 'all_actions' + assert loaded_settings.sdk_settings_values['enable_critic'] is True assert loaded_settings.llm_api_key.get_secret_value() == 'secret_key' assert loaded_settings.agent == 'smith' diff --git a/openhands/storage/data_models/settings.py b/openhands/storage/data_models/settings.py index d41b88cb58..4515c3b67c 100644 --- a/openhands/storage/data_models/settings.py +++ b/openhands/storage/data_models/settings.py @@ -57,7 +57,8 @@ class Settings(BaseModel): SDK-managed fields (LLM config, condenser, verification, agent) live exclusively in ``agent_settings`` using dotted keys such as - ``llm.model``. Convenience properties provide typed read access. + ``llm.model``. Convenience properties provide typed access while + preserving backward-compatible assignment semantics for legacy code. """ language: str | None = None @@ -90,52 +91,105 @@ class Settings(BaseModel): validate_assignment=True, ) + def _set_agent_setting(self, key: str, value: Any) -> None: + if value is None: + self.agent_settings.pop(key, None) + return + if isinstance(value, SecretStr): + self.agent_settings[key] = value.get_secret_value() + return + self.agent_settings[key] = value + # ------------------------------------------------------------------ - # Convenience read accessors into agent_settings + # Convenience accessors into agent_settings # ------------------------------------------------------------------ @property def llm_model(self) -> str | None: return self.agent_settings.get('llm.model') + @llm_model.setter + def llm_model(self, value: str | None) -> None: + self._set_agent_setting('llm.model', value) + @property def llm_api_key(self) -> SecretStr | None: val = self.agent_settings.get('llm.api_key') return SecretStr(val) if val else None + @llm_api_key.setter + def llm_api_key(self, value: SecretStr | str | None) -> None: + self._set_agent_setting('llm.api_key', value) + @property def llm_base_url(self) -> str | None: return self.agent_settings.get('llm.base_url') + @llm_base_url.setter + def llm_base_url(self, value: str | None) -> None: + self._set_agent_setting('llm.base_url', value) + @property def agent(self) -> str | None: return self.agent_settings.get('agent') + @agent.setter + def agent(self, value: str | None) -> None: + self._set_agent_setting('agent', value) + @property def confirmation_mode(self) -> bool | None: return self.agent_settings.get('verification.confirmation_mode') + @confirmation_mode.setter + def confirmation_mode(self, value: bool | None) -> None: + self._set_agent_setting('verification.confirmation_mode', value) + @property def security_analyzer(self) -> str | None: return self.agent_settings.get('verification.security_analyzer') + @security_analyzer.setter + def security_analyzer(self, value: str | None) -> None: + self._set_agent_setting('verification.security_analyzer', value) + @property def max_iterations(self) -> int | None: return self.agent_settings.get('max_iterations') + @max_iterations.setter + def max_iterations(self, value: int | None) -> None: + self._set_agent_setting('max_iterations', value) + @property def enable_default_condenser(self) -> bool: return self.agent_settings.get('condenser.enabled', True) + @enable_default_condenser.setter + def enable_default_condenser(self, value: bool | None) -> None: + self._set_agent_setting('condenser.enabled', value) + @property def condenser_max_size(self) -> int | None: return self.agent_settings.get('condenser.max_size') + @condenser_max_size.setter + def condenser_max_size(self, value: int | None) -> None: + self._set_agent_setting('condenser.max_size', value) + @property def llm_api_key_is_set(self) -> bool: val = self.agent_settings.get('llm.api_key') return bool(val and str(val).strip()) + @property + def sdk_settings_values(self) -> dict[str, Any]: + return self.agent_settings + + @sdk_settings_values.setter + def sdk_settings_values(self, value: dict[str, Any] | None) -> None: + self.agent_settings = dict(value or {}) + # ------------------------------------------------------------------ # Serialization # ------------------------------------------------------------------ @@ -172,6 +226,11 @@ class Settings(BaseModel): agent_vals: dict[str, Any] = dict(data.get('agent_settings') or {}) + legacy_agent_vals = data.pop('sdk_settings_values', None) + if isinstance(legacy_agent_vals, dict): + for key, value in legacy_agent_vals.items(): + agent_vals.setdefault(key, value) + for flat_key, dotted_key in _LEGACY_FLAT_TO_SDK.items(): if flat_key in data and dotted_key not in agent_vals: value = data[flat_key]