mirror of
https://github.com/OpenHands/OpenHands.git
synced 2026-03-22 13:47:19 +08:00
settings: merge Critic+Security into Verification, remove OpenHandsAgentSettings
- SDK: combined CriticSettings + SecuritySettings into VerificationSettings with backward-compat property accessors and type aliases - Removed OpenHandsAgentSettings subclass — use AgentSettings directly - Nav order: LLM → Condenser → Verification (was separate Security + Critic) - Single verification-settings route replaces critic-settings + security-settings - Updated _SDK_TO_FLAT_SETTINGS keys to verification.* namespace - All 119 backend tests pass, frontend builds, lint clean Co-authored-by: openhands <openhands@all-hands.dev>
This commit is contained in:
@@ -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: <LockIcon width={22} height={22} />,
|
||||
to: "/settings/security",
|
||||
text: "SETTINGS$NAV_SECURITY",
|
||||
},
|
||||
{
|
||||
icon: <MemoryIcon width={22} height={22} />,
|
||||
to: "/settings/condenser",
|
||||
text: "SETTINGS$NAV_CONDENSER",
|
||||
},
|
||||
{
|
||||
icon: <LightbulbIcon width={22} height={22} />,
|
||||
to: "/settings/critic",
|
||||
text: "SETTINGS$NAV_CRITIC",
|
||||
icon: <LockIcon width={22} height={22} />,
|
||||
to: "/settings/verification",
|
||||
text: "SETTINGS$NAV_VERIFICATION",
|
||||
},
|
||||
{
|
||||
icon: <CreditCardIcon width={22} height={22} />,
|
||||
@@ -90,20 +84,15 @@ export const OSS_NAV_ITEMS: SettingsNavItem[] = [
|
||||
to: "/settings",
|
||||
text: "SETTINGS$NAV_LLM",
|
||||
},
|
||||
{
|
||||
icon: <LockIcon width={22} height={22} />,
|
||||
to: "/settings/security",
|
||||
text: "SETTINGS$NAV_SECURITY",
|
||||
},
|
||||
{
|
||||
icon: <MemoryIcon width={22} height={22} />,
|
||||
to: "/settings/condenser",
|
||||
text: "SETTINGS$NAV_CONDENSER",
|
||||
},
|
||||
{
|
||||
icon: <LightbulbIcon width={22} height={22} />,
|
||||
to: "/settings/critic",
|
||||
text: "SETTINGS$NAV_CRITIC",
|
||||
icon: <LockIcon width={22} height={22} />,
|
||||
to: "/settings/verification",
|
||||
text: "SETTINGS$NAV_VERIFICATION",
|
||||
},
|
||||
{
|
||||
icon: <ServerProcessIcon width={22} height={22} />,
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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"),
|
||||
|
||||
@@ -1,12 +0,0 @@
|
||||
import { SdkSectionPage } from "#/components/features/settings/sdk-settings/sdk-section-page";
|
||||
|
||||
function SecuritySettingsScreen() {
|
||||
return (
|
||||
<SdkSectionPage
|
||||
sectionKeys={["security"]}
|
||||
testId="security-settings-screen"
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
||||
export default SecuritySettingsScreen;
|
||||
@@ -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 (
|
||||
<SdkSectionPage sectionKeys={["critic"]} testId="critic-settings-screen" />
|
||||
<SdkSectionPage
|
||||
sectionKeys={["verification"]}
|
||||
testId="verification-settings-screen"
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
||||
export const clientLoader = createPermissionGuard("view_llm_settings");
|
||||
|
||||
export default CriticSettingsScreen;
|
||||
export default VerificationSettingsScreen;
|
||||
@@ -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',
|
||||
}
|
||||
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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,
|
||||
},
|
||||
}
|
||||
|
||||
@@ -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 = {
|
||||
|
||||
@@ -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',
|
||||
},
|
||||
)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user