diff --git a/frontend/src/constants/settings-nav.tsx b/frontend/src/constants/settings-nav.tsx index 5a8cb4ee82..59051d6cc5 100644 --- a/frontend/src/constants/settings-nav.tsx +++ b/frontend/src/constants/settings-nav.tsx @@ -1,7 +1,6 @@ import { FiUsers, FiBriefcase } from "react-icons/fi"; import CreditCardIcon from "#/icons/credit-card.svg?react"; import KeyIcon from "#/icons/key.svg?react"; -import LightbulbIcon from "#/icons/lightbulb.svg?react"; import LockIcon from "#/icons/lock.svg?react"; import MemoryIcon from "#/icons/memory_icon.svg?react"; import ServerProcessIcon from "#/icons/server-process.svg?react"; @@ -37,20 +36,15 @@ export const SAAS_NAV_ITEMS: SettingsNavItem[] = [ to: "/settings", text: "COMMON$LANGUAGE_MODEL_LLM", }, - { - icon: , - to: "/settings/security", - text: "SETTINGS$NAV_SECURITY", - }, { icon: , to: "/settings/condenser", text: "SETTINGS$NAV_CONDENSER", }, { - icon: , - to: "/settings/critic", - text: "SETTINGS$NAV_CRITIC", + icon: , + to: "/settings/verification", + text: "SETTINGS$NAV_VERIFICATION", }, { icon: , @@ -90,20 +84,15 @@ export const OSS_NAV_ITEMS: SettingsNavItem[] = [ to: "/settings", text: "SETTINGS$NAV_LLM", }, - { - icon: , - to: "/settings/security", - text: "SETTINGS$NAV_SECURITY", - }, { icon: , to: "/settings/condenser", text: "SETTINGS$NAV_CONDENSER", }, { - icon: , - to: "/settings/critic", - text: "SETTINGS$NAV_CRITIC", + icon: , + to: "/settings/verification", + text: "SETTINGS$NAV_VERIFICATION", }, { icon: , diff --git a/frontend/src/i18n/declaration.ts b/frontend/src/i18n/declaration.ts index 957a894747..62d7119ff8 100644 --- a/frontend/src/i18n/declaration.ts +++ b/frontend/src/i18n/declaration.ts @@ -136,9 +136,8 @@ export enum I18nKey { SETTINGS$GITLAB_INSTALLING_WEBHOOK = "SETTINGS$GITLAB_INSTALLING_WEBHOOK", SETTINGS$GITLAB = "SETTINGS$GITLAB", SETTINGS$NAV_CONDENSER = "SETTINGS$NAV_CONDENSER", - SETTINGS$NAV_CRITIC = "SETTINGS$NAV_CRITIC", SETTINGS$NAV_LLM = "SETTINGS$NAV_LLM", - SETTINGS$NAV_SECURITY = "SETTINGS$NAV_SECURITY", + SETTINGS$NAV_VERIFICATION = "SETTINGS$NAV_VERIFICATION", GIT$MERGE_REQUEST = "GIT$MERGE_REQUEST", GIT$GITLAB_API = "GIT$GITLAB_API", GIT$PULL_REQUEST = "GIT$PULL_REQUEST", diff --git a/frontend/src/i18n/translation.json b/frontend/src/i18n/translation.json index d5a8bc7a25..6cd6a59539 100644 --- a/frontend/src/i18n/translation.json +++ b/frontend/src/i18n/translation.json @@ -2175,22 +2175,6 @@ "de": "Kondensator", "uk": "Конденсатор" }, - "SETTINGS$NAV_CRITIC": { - "en": "Critic", - "ja": "クリティック", - "zh-CN": "评审", - "zh-TW": "評審", - "ko-KR": "크리틱", - "no": "Kritiker", - "it": "Critico", - "pt": "Crítico", - "es": "Crítico", - "ar": "الناقد", - "fr": "Critique", - "tr": "Eleştirmen", - "de": "Kritiker", - "uk": "Критик" - }, "SETTINGS$NAV_LLM": { "en": "LLM", "ja": "LLM", @@ -2207,21 +2191,21 @@ "de": "LLM", "uk": "LLM" }, - "SETTINGS$NAV_SECURITY": { - "en": "Security", - "ja": "セキュリティ", - "zh-CN": "安全", - "zh-TW": "安全", - "ko-KR": "보안", - "no": "Sikkerhet", - "it": "Sicurezza", - "pt": "Segurança", - "es": "Seguridad", - "ar": "الأمان", - "fr": "Sécurité", - "tr": "Güvenlik", - "de": "Sicherheit", - "uk": "Безпека" + "SETTINGS$NAV_VERIFICATION": { + "en": "Verification", + "ja": "検証", + "zh-CN": "验证", + "zh-TW": "驗證", + "ko-KR": "검증", + "no": "Verifisering", + "it": "Verifica", + "pt": "Verificação", + "es": "Verificación", + "ar": "التحقق", + "fr": "Vérification", + "tr": "Doğrulama", + "de": "Verifizierung", + "uk": "Верифікація" }, "GIT$MERGE_REQUEST": { "en": "Merge Request", diff --git a/frontend/src/routes.ts b/frontend/src/routes.ts index 7e841f2413..936cd1169e 100644 --- a/frontend/src/routes.ts +++ b/frontend/src/routes.ts @@ -13,9 +13,8 @@ export default [ route("accept-tos", "routes/accept-tos.tsx"), route("settings", "routes/settings.tsx", [ index("routes/llm-settings.tsx"), - route("security", "routes/security-settings.tsx"), route("condenser", "routes/condenser-settings.tsx"), - route("critic", "routes/critic-settings.tsx"), + route("verification", "routes/verification-settings.tsx"), route("mcp", "routes/mcp-settings.tsx"), route("user", "routes/user-settings.tsx"), route("integrations", "routes/git-settings.tsx"), diff --git a/frontend/src/routes/security-settings.tsx b/frontend/src/routes/security-settings.tsx deleted file mode 100644 index f37ee45135..0000000000 --- a/frontend/src/routes/security-settings.tsx +++ /dev/null @@ -1,12 +0,0 @@ -import { SdkSectionPage } from "#/components/features/settings/sdk-settings/sdk-section-page"; - -function SecuritySettingsScreen() { - return ( - - ); -} - -export default SecuritySettingsScreen; diff --git a/frontend/src/routes/critic-settings.tsx b/frontend/src/routes/verification-settings.tsx similarity index 57% rename from frontend/src/routes/critic-settings.tsx rename to frontend/src/routes/verification-settings.tsx index 4932fedaac..f37c848b22 100644 --- a/frontend/src/routes/critic-settings.tsx +++ b/frontend/src/routes/verification-settings.tsx @@ -1,12 +1,15 @@ import { SdkSectionPage } from "#/components/features/settings/sdk-settings/sdk-section-page"; import { createPermissionGuard } from "#/utils/org/permission-guard"; -function CriticSettingsScreen() { +function VerificationSettingsScreen() { return ( - + ); } export const clientLoader = createPermissionGuard("view_llm_settings"); -export default CriticSettingsScreen; +export default VerificationSettingsScreen; diff --git a/openhands/server/routes/settings.py b/openhands/server/routes/settings.py index 8ffad40cff..e312c5e7fa 100644 --- a/openhands/server/routes/settings.py +++ b/openhands/server/routes/settings.py @@ -6,6 +6,7 @@ # Unless you are working on deprecation, please avoid extending this legacy file and consult the V1 codepaths above. # Tag: Legacy-V0 # This module belongs to the old V0 web server. The V1 application server lives under openhands/app_server/. +import importlib import os from typing import Any @@ -40,17 +41,13 @@ LITE_LLM_API_URL = os.environ.get( def _get_sdk_settings_schema() -> dict[str, Any] | None: - """Return the SDK settings schema when the SDK package is installed. - - Uses ``OpenHandsAgentSettings`` which extends the base SDK - ``AgentSettings`` with OpenHands-specific sections (e.g. *security*). - """ + """Return the SDK settings schema when the SDK package is installed.""" try: - from openhands.storage.data_models.settings import OpenHandsAgentSettings - except Exception: + settings_module = importlib.import_module('openhands.sdk.settings') + except ModuleNotFoundError: return None - return OpenHandsAgentSettings.export_schema().model_dump(mode='json') + return settings_module.AgentSettings.export_schema().model_dump(mode='json') def _get_sdk_field_keys(schema: dict[str, Any] | None) -> set[str]: @@ -82,8 +79,8 @@ def _get_sdk_secret_field_keys(schema: dict[str, Any] | None) -> set[str]: # also written to the legacy flat field so existing consumers (session # init, security-analyzer setup, etc.) keep working. _SDK_TO_FLAT_SETTINGS: dict[str, str] = { - 'security.confirmation_mode': 'confirmation_mode', - 'security.security_analyzer': 'security_analyzer', + 'verification.confirmation_mode': 'confirmation_mode', + 'verification.security_analyzer': 'security_analyzer', } diff --git a/openhands/storage/data_models/settings.py b/openhands/storage/data_models/settings.py index 6f11dbd85a..cf0a685ea7 100644 --- a/openhands/storage/data_models/settings.py +++ b/openhands/storage/data_models/settings.py @@ -1,7 +1,7 @@ from __future__ import annotations from enum import Enum -from typing import Annotated, Any, Literal +from typing import Annotated, Any from pydantic import ( BaseModel, @@ -18,66 +18,8 @@ from openhands.core.config.llm_config import LLMConfig from openhands.core.config.mcp_config import MCPConfig from openhands.core.config.utils import load_openhands_config from openhands.sdk.settings import AgentSettings -from openhands.sdk.settings_metadata import ( - SETTINGS_METADATA_KEY, - SETTINGS_SECTION_METADATA_KEY, - SettingProminence, - SettingsFieldMetadata, - SettingsSectionMetadata, -) from openhands.storage.data_models.secrets import Secrets -# --------------------------------------------------------------------------- -# Extended settings sections that live in OpenHands (not yet in the SDK) -# --------------------------------------------------------------------------- - -SecurityAnalyzerType = Literal['none', 'llm'] - - -class SecuritySettings(BaseModel): - """Security-related agent settings.""" - - confirmation_mode: bool = Field( - default=False, - description='Require human confirmation before executing actions.', - json_schema_extra={ - SETTINGS_METADATA_KEY: SettingsFieldMetadata( - label='Confirmation mode', - prominence=SettingProminence.CRITICAL, - ).model_dump() - }, - ) - security_analyzer: SecurityAnalyzerType | None = Field( - default=None, - description='Security analyzer used to evaluate actions.', - json_schema_extra={ - SETTINGS_METADATA_KEY: SettingsFieldMetadata( - label='Security analyzer', - prominence=SettingProminence.MAJOR, - ).model_dump() - }, - ) - - -class OpenHandsAgentSettings(AgentSettings): - """``AgentSettings`` extended with OpenHands-specific sections. - - Sections defined here (e.g. *security*) follow the same metadata - conventions as the SDK so that ``export_schema()`` includes them - automatically. - """ - - security: SecuritySettings = Field( - default_factory=SecuritySettings, - description='Security settings for the agent.', - json_schema_extra={ - SETTINGS_SECTION_METADATA_KEY: SettingsSectionMetadata( - key='security', - label='Security', - ).model_dump() - }, - ) - def _assign_dotted_value(target: dict[str, Any], dotted_key: str, value: Any) -> None: current = target diff --git a/tests/unit/app_server/test_live_status_app_conversation_service.py b/tests/unit/app_server/test_live_status_app_conversation_service.py index 477824a919..1b10c8d682 100644 --- a/tests/unit/app_server/test_live_status_app_conversation_service.py +++ b/tests/unit/app_server/test_live_status_app_conversation_service.py @@ -970,11 +970,11 @@ class TestLiveStatusAppConversationService: 'api_key': 'test_api_key', }, 'condenser': {'enabled': False}, - 'critic': { - 'enabled': True, - 'mode': 'all_actions', + 'verification': { + 'critic_enabled': True, + 'critic_mode': 'all_actions', 'enable_iterative_refinement': True, - 'threshold': 0.75, + 'critic_threshold': 0.75, 'max_refinement_iterations': 2, }, } diff --git a/tests/unit/server/routes/test_settings_api.py b/tests/unit/server/routes/test_settings_api.py index 3d5c89c92b..3a2dd58a1a 100644 --- a/tests/unit/server/routes/test_settings_api.py +++ b/tests/unit/server/routes/test_settings_api.py @@ -88,15 +88,16 @@ def test_get_sdk_settings_schema_returns_none_when_sdk_missing(): assert mock_fn() is None -def test_get_sdk_settings_schema_includes_security_section(): +def test_get_sdk_settings_schema_includes_verification_section(): schema = settings_routes._get_sdk_settings_schema() assert schema is not None section_keys = [s['key'] for s in schema['sections']] - assert 'security' in section_keys - security_section = next(s for s in schema['sections'] if s['key'] == 'security') - field_keys = [f['key'] for f in security_section['fields']] - assert 'security.confirmation_mode' in field_keys - assert 'security.security_analyzer' in field_keys + assert 'verification' in section_keys + section = next(s for s in schema['sections'] if s['key'] == 'verification') + field_keys = [f['key'] for f in section['fields']] + assert 'verification.confirmation_mode' in field_keys + assert 'verification.security_analyzer' in field_keys + assert 'verification.critic_enabled' in field_keys @pytest.mark.asyncio @@ -138,34 +139,44 @@ async def test_settings_api_endpoints(test_client): ], }, { - 'key': 'critic', - 'label': 'Critic', + 'key': 'verification', + 'label': 'Verification', 'fields': [ { - 'key': 'critic.enabled', + 'key': 'verification.critic_enabled', 'value_type': 'boolean', 'prominence': 'critical', }, { - 'key': 'critic.mode', + 'key': 'verification.critic_mode', 'value_type': 'string', 'prominence': 'minor', }, { - 'key': 'critic.enable_iterative_refinement', + 'key': 'verification.enable_iterative_refinement', 'value_type': 'boolean', 'prominence': 'major', }, { - 'key': 'critic.threshold', + 'key': 'verification.critic_threshold', 'value_type': 'number', 'prominence': 'minor', }, { - 'key': 'critic.max_refinement_iterations', + 'key': 'verification.max_refinement_iterations', 'value_type': 'integer', 'prominence': 'minor', }, + { + 'key': 'verification.confirmation_mode', + 'value_type': 'boolean', + 'prominence': 'major', + }, + { + 'key': 'verification.security_analyzer', + 'value_type': 'string', + 'prominence': 'major', + }, ], }, ], @@ -184,11 +195,13 @@ async def test_settings_api_endpoints(test_client): 'llm.timeout': 123, 'llm.litellm_extra_body': {'metadata': {'tier': 'pro'}}, 'remote_runtime_resource_factor': 2, - 'critic.enabled': True, - 'critic.mode': 'all_actions', - 'critic.enable_iterative_refinement': True, - 'critic.threshold': 0.7, - 'critic.max_refinement_iterations': 4, + 'verification.critic_enabled': True, + 'verification.critic_mode': 'all_actions', + 'verification.enable_iterative_refinement': True, + 'verification.critic_threshold': 0.7, + 'verification.max_refinement_iterations': 4, + 'verification.confirmation_mode': True, + 'verification.security_analyzer': 'llm', } with patch( @@ -208,8 +221,11 @@ async def test_settings_api_endpoints(test_client): schema = response_data['sdk_settings_schema'] assert schema['model_name'] == 'AgentSettings' assert isinstance(schema['sections'], list) - assert [section['key'] for section in schema['sections']] == ['llm', 'critic'] - llm_section, critic_section = schema['sections'] + assert [section['key'] for section in schema['sections']] == [ + 'llm', + 'verification', + ] + llm_section, verification_section = schema['sections'] assert llm_section['label'] == 'LLM' assert [field['key'] for field in llm_section['fields']] == [ 'llm.model', @@ -221,31 +237,19 @@ async def test_settings_api_endpoints(test_client): assert llm_section['fields'][-1]['secret'] is True assert llm_section['fields'][2]['value_type'] == 'integer' assert llm_section['fields'][3]['value_type'] == 'object' - assert critic_section['label'] == 'Critic' - assert [field['key'] for field in critic_section['fields']] == [ - 'critic.enabled', - 'critic.mode', - 'critic.enable_iterative_refinement', - 'critic.threshold', - 'critic.max_refinement_iterations', - ] - assert response_data['sdk_settings_values']['llm.model'] == 'test-model' - assert response_data['sdk_settings_values']['llm.timeout'] == 123 - assert response_data['sdk_settings_values']['llm.litellm_extra_body'] == { - 'metadata': {'tier': 'pro'} - } - assert response_data['sdk_settings_values']['critic.enabled'] is True - assert response_data['sdk_settings_values']['critic.mode'] == 'all_actions' - assert ( - response_data['sdk_settings_values']['critic.enable_iterative_refinement'] - is True - ) - assert response_data['sdk_settings_values']['critic.threshold'] == 0.7 - assert ( - response_data['sdk_settings_values']['critic.max_refinement_iterations'] - == 4 - ) - assert response_data['sdk_settings_values']['llm.api_key'] is None + assert verification_section['label'] == 'Verification' + vals = response_data['sdk_settings_values'] + assert vals['llm.model'] == 'test-model' + assert vals['llm.timeout'] == 123 + assert vals['llm.litellm_extra_body'] == {'metadata': {'tier': 'pro'}} + assert vals['verification.critic_enabled'] is True + assert vals['verification.critic_mode'] == 'all_actions' + assert vals['verification.enable_iterative_refinement'] is True + assert vals['verification.critic_threshold'] == 0.7 + assert vals['verification.max_refinement_iterations'] == 4 + assert vals['verification.confirmation_mode'] is True + assert vals['verification.security_analyzer'] == 'llm' + assert vals['llm.api_key'] is None # Test updating with partial settings partial_settings = { diff --git a/tests/unit/storage/data_models/test_settings.py b/tests/unit/storage/data_models/test_settings.py index 2199b0013a..ab97336ed9 100644 --- a/tests/unit/storage/data_models/test_settings.py +++ b/tests/unit/storage/data_models/test_settings.py @@ -94,16 +94,16 @@ def test_settings_preserve_sdk_settings_values(): settings = Settings( llm_api_key='test-key', sdk_settings_values={ - 'critic.enabled': True, - 'critic.mode': 'all_actions', + 'verification.critic_enabled': True, + 'verification.critic_mode': 'all_actions', 'llm.litellm_extra_body': {'metadata': {'tier': 'pro'}}, }, ) assert settings.llm_api_key.get_secret_value() == 'test-key' assert settings.sdk_settings_values == { - 'critic.enabled': True, - 'critic.mode': 'all_actions', + 'verification.critic_enabled': True, + 'verification.critic_mode': 'all_actions', 'llm.litellm_extra_body': {'metadata': {'tier': 'pro'}}, } @@ -116,8 +116,8 @@ def test_settings_to_agent_settings_uses_sdk_values(): 'llm.litellm_extra_body': {'metadata': {'tier': 'enterprise'}}, 'condenser.enabled': False, 'condenser.max_size': 88, - 'critic.enabled': True, - 'critic.mode': 'all_actions', + 'verification.critic_enabled': True, + 'verification.critic_mode': 'all_actions', }, )