Skip to content

Commit 088c873

Browse files
refactor(sdk): remove legacy verification shims
Co-authored-by: openhands <openhands@all-hands.dev>
1 parent 47d65dc commit 088c873

File tree

6 files changed

+24
-206
lines changed

6 files changed

+24
-206
lines changed

AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ When reviewing code, provide constructive feedback:
107107
- `AgentSettings.tools` is part of the exported settings schema so the schema stays aligned with the settings payload that round-trips through `AgentSettings` and drives `create_agent()`.
108108
- `AgentSettings.mcp_config` now uses FastMCP's typed `MCPConfig` at runtime. When serializing settings back to plain data (e.g. `model_dump()` or `create_agent()`), keep the output compact with `exclude_none=True, exclude_defaults=True` so callers still see the familiar `.mcp.json`-style dict shape.
109109
- Persisted `AgentSettings` now have an SDK-owned canonical migration contract: legacy v1 payloads are the raw unversioned settings mapping, while the current canonical format is v2 `{\"version\": 2, \"settings\": ...}`. Use `AgentSettings.load_persisted()`/`dump_persisted()` so consumers share the same upgrade path and preserve sparse overlays with `exclude_unset=True`.
110-
- `ConversationSettings` is now the canonical owner of conversation-scoped confirmation controls (`confirmation_mode`, `security_analyzer`). Keep `ConversationVerificationSettings` only as a compatibility shim for older payloads/API callers, and hide the legacy wrapper from exported settings schemas rather than reintroducing it into the agent schema.
110+
- `ConversationSettings` owns the conversation-scoped confirmation controls directly (`confirmation_mode`, `security_analyzer`); keep those fields top-level on the model and grouped into the exported `verification` section via schema metadata rather than nested helper models.
111111
- Anthropic malformed tool-use/tool-result history errors (for example, missing or duplicated ``tool_result`` blocks) are intentionally mapped to a dedicated `LLMMalformedConversationHistoryError` and caught separately in `Agent.step()`, so recovery can still use condensation while logs preserve that this was malformed history rather than a true context-window overflow.
112112
- AgentSkills progressive disclosure goes through `AgentContext.get_system_message_suffix()` into `<available_skills>`, and `openhands.sdk.context.skills.to_prompt()` truncates each prompt description to 1024 characters because the AgentSkills specification caps `description` at 1-1024 characters.
113113
- Workspace-wide uv resolver guardrails belong in the repository root `[tool.uv]` table. When `exclude-newer` is configured there, `uv lock` persists it into the root `uv.lock` `[options]` section as both an absolute cutoff and `exclude-newer-span`, and `uv sync --frozen` continues to use that locked workspace state.

openhands-sdk/openhands/sdk/__init__.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@
5353
AgentSettings,
5454
CondenserSettings,
5555
ConversationSettings,
56-
ConversationVerificationSettings,
5756
SettingsChoice,
5857
SettingsFieldSchema,
5958
SettingsSchema,
@@ -140,7 +139,6 @@
140139
"LLMSummarizingCondenser",
141140
"CondenserSettings",
142141
"ConversationSettings",
143-
"ConversationVerificationSettings",
144142
"VerificationSettings",
145143
"AgentSettings",
146144
"SettingsChoice",

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
AgentSettings,
1818
CondenserSettings,
1919
ConversationSettings,
20-
ConversationVerificationSettings,
2120
SettingsChoice,
2221
SettingsFieldSchema,
2322
SettingsSchema,
@@ -30,7 +29,6 @@
3029
"AgentSettings",
3130
"CondenserSettings",
3231
"ConversationSettings",
33-
"ConversationVerificationSettings",
3432
"SettingsChoice",
3533
"SettingsFieldSchema",
3634
"SettingsSchema",
@@ -43,7 +41,6 @@
4341
"AgentSettings",
4442
"CondenserSettings",
4543
"ConversationSettings",
46-
"ConversationVerificationSettings",
4744
"SETTINGS_METADATA_KEY",
4845
"SETTINGS_SECTION_METADATA_KEY",
4946
"SettingProminence",

openhands-sdk/openhands/sdk/settings/model.py

Lines changed: 3 additions & 130 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
SecretStr,
1414
field_serializer,
1515
field_validator,
16-
model_validator,
1716
)
1817
from pydantic.fields import FieldInfo
1918

@@ -199,61 +198,6 @@ class VerificationSettings(BaseModel):
199198
},
200199
)
201200

202-
# Keep these legacy fields on the public SDK model for backward compatibility,
203-
# but hide them from the exported AgentSettings schema now that conversation-
204-
# level verification lives on ConversationSettings.
205-
_SCHEMA_EXCLUDED_FIELDS: ClassVar[frozenset[str]] = frozenset(
206-
{"confirmation_mode", "security_analyzer"}
207-
)
208-
209-
confirmation_mode: bool = Field(
210-
default=False,
211-
description="Require user confirmation before executing risky actions.",
212-
json_schema_extra={
213-
SETTINGS_METADATA_KEY: SettingsFieldMetadata(
214-
label="Confirmation mode",
215-
prominence=SettingProminence.MAJOR,
216-
).model_dump()
217-
},
218-
)
219-
security_analyzer: SecurityAnalyzerType | None = Field(
220-
default=None,
221-
description="Security analyzer that evaluates actions before execution.",
222-
json_schema_extra={
223-
SETTINGS_METADATA_KEY: SettingsFieldMetadata(
224-
label="Security analyzer",
225-
prominence=SettingProminence.MAJOR,
226-
depends_on=("confirmation_mode",),
227-
).model_dump()
228-
},
229-
)
230-
231-
232-
class ConversationVerificationSettings(BaseModel):
233-
"""Conversation-level confirmation and security settings."""
234-
235-
confirmation_mode: bool = Field(
236-
default=False,
237-
description="Require user confirmation before executing risky actions.",
238-
json_schema_extra={
239-
SETTINGS_METADATA_KEY: SettingsFieldMetadata(
240-
label="Confirmation mode",
241-
prominence=SettingProminence.MAJOR,
242-
).model_dump()
243-
},
244-
)
245-
security_analyzer: SecurityAnalyzerType | None = Field(
246-
default="llm",
247-
description="Security analyzer that evaluates actions before execution.",
248-
json_schema_extra={
249-
SETTINGS_METADATA_KEY: SettingsFieldMetadata(
250-
label="Security analyzer",
251-
prominence=SettingProminence.MAJOR,
252-
depends_on=("confirmation_mode",),
253-
).model_dump()
254-
},
255-
)
256-
257201

258202
def _default_llm_settings() -> LLM:
259203
model = LLM.model_fields["model"].get_default()
@@ -516,54 +460,6 @@ class ConversationSettings(BaseModel):
516460
).model_dump(),
517461
},
518462
)
519-
verification: ConversationVerificationSettings = Field(
520-
default_factory=ConversationVerificationSettings,
521-
description="Conversation confirmation and security settings.",
522-
json_schema_extra={
523-
SETTINGS_SECTION_METADATA_KEY: SettingsSectionMetadata(
524-
key="verification",
525-
label="Verification",
526-
).model_dump(),
527-
"openhands_settings_schema_hidden": True,
528-
},
529-
)
530-
531-
@model_validator(mode="before")
532-
@classmethod
533-
def _flatten_verification_fields(cls, data: Any) -> Any:
534-
if not isinstance(data, Mapping):
535-
return data
536-
537-
normalized = dict(data)
538-
verification = normalized.get("verification")
539-
if isinstance(verification, BaseModel):
540-
verification = verification.model_dump(mode="python")
541-
if not isinstance(verification, Mapping):
542-
return normalized
543-
544-
if (
545-
"confirmation_mode" not in normalized
546-
and "confirmation_mode" in verification
547-
):
548-
normalized["confirmation_mode"] = deepcopy(
549-
verification["confirmation_mode"]
550-
)
551-
if (
552-
"security_analyzer" not in normalized
553-
and "security_analyzer" in verification
554-
):
555-
normalized["security_analyzer"] = deepcopy(
556-
verification["security_analyzer"]
557-
)
558-
return normalized
559-
560-
@model_validator(mode="after")
561-
def _sync_verification_compatibility_view(self) -> ConversationSettings:
562-
self.verification = ConversationVerificationSettings(
563-
confirmation_mode=self.confirmation_mode,
564-
security_analyzer=self.security_analyzer,
565-
)
566-
return self
567463

568464
@classmethod
569465
def export_schema(cls) -> SettingsSchema:
@@ -784,18 +680,10 @@ def settings_metadata(field: FieldInfo) -> SettingsFieldMetadata | None:
784680
return SettingsFieldMetadata.model_validate(metadata)
785681

786682

787-
_SETTINGS_SCHEMA_HIDDEN_KEY = "openhands_settings_schema_hidden"
788683
_GENERAL_SECTION_KEY = "general"
789684
_GENERAL_SECTION_LABEL = "General"
790685

791686

792-
def settings_schema_hidden(field: FieldInfo) -> bool:
793-
extra = field.json_schema_extra
794-
if not isinstance(extra, dict):
795-
return False
796-
return bool(extra.get(_SETTINGS_SCHEMA_HIDDEN_KEY, False))
797-
798-
799687
def export_settings_schema(model: type[BaseModel]) -> SettingsSchema:
800688
"""Export a structured settings schema for a Pydantic settings model.
801689
@@ -817,9 +705,6 @@ def ensure_section(key: str, label: str) -> SettingsSectionSchema:
817705
return section
818706

819707
for field_name, field in model.model_fields.items():
820-
if settings_schema_hidden(field):
821-
continue
822-
823708
section_metadata = settings_section_metadata(field)
824709
nested_model = _nested_model_type(field.annotation)
825710

@@ -830,15 +715,8 @@ def ensure_section(key: str, label: str) -> SettingsSectionSchema:
830715
section_metadata.key
831716
)
832717
section = ensure_section(section_metadata.key, section_label)
833-
schema_excluded_fields = getattr(
834-
nested_model, "_SCHEMA_EXCLUDED_FIELDS", frozenset()
835-
)
836718
for nested_key, nested_field in nested_model.model_fields.items():
837-
if (
838-
nested_field.exclude
839-
or nested_key in schema_excluded_fields
840-
or settings_schema_hidden(nested_field)
841-
):
719+
if nested_field.exclude:
842720
continue
843721
metadata = settings_metadata(nested_field)
844722
default_value = None
@@ -905,7 +783,7 @@ def ensure_section(key: str, label: str) -> SettingsSectionSchema:
905783
section = ensure_section(section_metadata.key, section_label)
906784
section.fields.append(
907785
SettingsFieldSchema(
908-
key=f"{section_metadata.key}.{field_name}",
786+
key=field_name,
909787
label=(
910788
metadata.label
911789
if metadata.label is not None
@@ -917,12 +795,7 @@ def ensure_section(key: str, label: str) -> SettingsSectionSchema:
917795
value_type=_infer_value_type(field.annotation),
918796
default=_normalize_default(default_value),
919797
prominence=metadata.prominence,
920-
depends_on=[
921-
dependency
922-
if "." in dependency
923-
else f"{section_metadata.key}.{dependency}"
924-
for dependency in metadata.depends_on
925-
],
798+
depends_on=list(metadata.depends_on),
926799
secret=_contains_secret(field.annotation),
927800
choices=_extract_choices(field.annotation),
928801
)

tests/agent_server/test_settings_router.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ def test_get_agent_settings_schema():
2323
)
2424
verification_field_keys = {field["key"] for field in verification_section["fields"]}
2525
assert "verification.critic_enabled" in verification_field_keys
26-
assert "verification.confirmation_mode" not in verification_field_keys
27-
assert "verification.security_analyzer" not in verification_field_keys
26+
assert "confirmation_mode" not in verification_field_keys
27+
assert "security_analyzer" not in verification_field_keys
2828

2929

3030
def test_get_conversation_settings_schema():
@@ -43,5 +43,5 @@ def test_get_conversation_settings_schema():
4343
section for section in body["sections"] if section["key"] == "verification"
4444
)
4545
verification_field_keys = {field["key"] for field in verification_section["fields"]}
46-
assert "verification.confirmation_mode" in verification_field_keys
47-
assert "verification.security_analyzer" in verification_field_keys
46+
assert "confirmation_mode" in verification_field_keys
47+
assert "security_analyzer" in verification_field_keys

tests/sdk/test_settings.py

Lines changed: 16 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,7 @@
1414
from openhands.sdk.critic.impl.api import APIBasedCritic
1515
from openhands.sdk.security.confirmation_policy import AlwaysConfirm, ConfirmRisky
1616
from openhands.sdk.security.llm_analyzer import LLMSecurityAnalyzer
17-
from openhands.sdk.settings import (
18-
CondenserSettings,
19-
ConversationVerificationSettings,
20-
VerificationSettings,
21-
)
17+
from openhands.sdk.settings import CondenserSettings, VerificationSettings
2218

2319

2420
# Fields on LLM that have ``exclude=True`` and should not appear in the schema.
@@ -110,8 +106,8 @@ def test_agent_settings_export_schema_groups_sections() -> None:
110106
assert (
111107
v_fields["verification.critic_threshold"].prominence is SettingProminence.MINOR
112108
)
113-
assert "verification.confirmation_mode" not in v_fields
114-
assert "verification.security_analyzer" not in v_fields
109+
assert "confirmation_mode" not in v_fields
110+
assert "security_analyzer" not in v_fields
115111

116112

117113
def test_conversation_settings_export_schema_groups_sections() -> None:
@@ -130,65 +126,23 @@ def test_conversation_settings_export_schema_groups_sections() -> None:
130126
assert len(sections["verification"].fields) == 2
131127
verification_fields = {f.key: f for f in sections["verification"].fields}
132128
assert set(verification_fields) == {
133-
"verification.confirmation_mode",
134-
"verification.security_analyzer",
129+
"confirmation_mode",
130+
"security_analyzer",
135131
}
136-
assert verification_fields["verification.confirmation_mode"].default is False
137-
assert (
138-
verification_fields["verification.confirmation_mode"].prominence
139-
is SettingProminence.MAJOR
140-
)
141-
assert verification_fields["verification.security_analyzer"].default == "llm"
132+
assert verification_fields["confirmation_mode"].default is False
142133
assert (
143-
verification_fields["verification.security_analyzer"].choices[0].value == "llm"
134+
verification_fields["confirmation_mode"].prominence is SettingProminence.MAJOR
144135
)
145-
assert verification_fields["verification.security_analyzer"].depends_on == [
146-
"verification.confirmation_mode"
147-
]
148-
149-
150-
def test_verification_settings_accepts_legacy_conversation_fields() -> None:
151-
settings = VerificationSettings.model_validate(
152-
{
153-
"critic_enabled": True,
154-
"confirmation_mode": True,
155-
"security_analyzer": "none",
156-
}
157-
)
158-
159-
assert settings.critic_enabled is True
160-
assert settings.confirmation_mode is True
161-
assert settings.security_analyzer == "none"
162-
163-
dumped = settings.model_dump()
164-
assert dumped["critic_enabled"] is True
165-
assert dumped["confirmation_mode"] is True
166-
assert dumped["security_analyzer"] == "none"
167-
168-
169-
def test_conversation_settings_flattens_verification_fields() -> None:
170-
settings = ConversationSettings.model_validate(
171-
{
172-
"verification": {
173-
"confirmation_mode": True,
174-
"security_analyzer": "none",
175-
}
176-
}
177-
)
178-
179-
assert settings.confirmation_mode is True
180-
assert settings.security_analyzer == "none"
181-
assert settings.verification.confirmation_mode is True
182-
assert settings.verification.security_analyzer == "none"
136+
assert verification_fields["security_analyzer"].default == "llm"
137+
assert verification_fields["security_analyzer"].choices[0].value == "llm"
138+
assert verification_fields["security_analyzer"].depends_on == ["confirmation_mode"]
183139

184140

185141
def test_conversation_settings_model_dump_roundtrip() -> None:
186142
settings = ConversationSettings(
187143
max_iterations=42,
188-
verification=ConversationVerificationSettings(
189-
confirmation_mode=True,
190-
security_analyzer="none",
191-
),
144+
confirmation_mode=True,
145+
security_analyzer="none",
192146
)
193147

194148
restored = ConversationSettings.model_validate(settings.model_dump(mode="json"))
@@ -199,10 +153,8 @@ def test_conversation_settings_model_dump_roundtrip() -> None:
199153
def test_conversation_settings_builds_runtime_helpers() -> None:
200154
settings = ConversationSettings(
201155
max_iterations=77,
202-
verification=ConversationVerificationSettings(
203-
confirmation_mode=True,
204-
security_analyzer="llm",
205-
),
156+
confirmation_mode=True,
157+
security_analyzer="llm",
206158
)
207159

208160
request_kwargs = settings.to_start_request_kwargs()
@@ -213,10 +165,8 @@ def test_conversation_settings_builds_runtime_helpers() -> None:
213165

214166
fallback_settings = ConversationSettings(
215167
max_iterations=settings.max_iterations,
216-
verification=ConversationVerificationSettings(
217-
confirmation_mode=True,
218-
security_analyzer="none",
219-
),
168+
confirmation_mode=True,
169+
security_analyzer="none",
220170
)
221171
fallback_request_kwargs = fallback_settings.to_start_request_kwargs()
222172

0 commit comments

Comments
 (0)