From b3137e7ae854d30edfc4e47dbb0fe27b861dcd74 Mon Sep 17 00:00:00 2001 From: Graham Neubig Date: Wed, 21 Jan 2026 08:50:49 -0500 Subject: [PATCH] fix: remove frozen=True from Field() to fix Pydantic warning (#12518) Co-authored-by: openhands --- .../server/session/conversation_init_data.py | 5 ++-- .../session/test_conversation_init_data.py | 29 +++++++++++++++++++ .../unit/storage/data_models/test_settings.py | 29 +++++++++++++++++++ 3 files changed, 61 insertions(+), 2 deletions(-) diff --git a/openhands/server/session/conversation_init_data.py b/openhands/server/session/conversation_init_data.py index efdbef5524..ce4af1ba50 100644 --- a/openhands/server/session/conversation_init_data.py +++ b/openhands/server/session/conversation_init_data.py @@ -19,8 +19,8 @@ from openhands.storage.data_models.settings import Settings class ConversationInitData(Settings): """Session initialization data for the web environment - a deep copy of the global config is made and then overridden with this data.""" - git_provider_tokens: PROVIDER_TOKEN_TYPE | None = Field(default=None, frozen=True) - custom_secrets: CUSTOM_SECRETS_TYPE | None = Field(default=None, frozen=True) + git_provider_tokens: PROVIDER_TOKEN_TYPE | None = Field(default=None) + custom_secrets: CUSTOM_SECRETS_TYPE | None = Field(default=None) selected_repository: str | None = Field(default=None) replay_json: str | None = Field(default=None) selected_branch: str | None = Field(default=None) @@ -29,6 +29,7 @@ class ConversationInitData(Settings): model_config = ConfigDict( arbitrary_types_allowed=True, + frozen=True, ) @field_validator('git_provider_tokens', 'custom_secrets') diff --git a/tests/unit/server/session/test_conversation_init_data.py b/tests/unit/server/session/test_conversation_init_data.py index 3c5d7d97f7..739729ca2d 100644 --- a/tests/unit/server/session/test_conversation_init_data.py +++ b/tests/unit/server/session/test_conversation_init_data.py @@ -4,6 +4,7 @@ These tests verify that the immutable_validator correctly converts dict to Mappi for git_provider_tokens and custom_secrets fields, ensuring type safety. """ +import warnings from types import MappingProxyType import pytest @@ -270,3 +271,31 @@ class TestConversationInitDataValidator: init_data.custom_secrets['SECRET'].secret.get_secret_value() == 'secret_proxy' ) + + +def test_conversation_init_data_no_pydantic_frozen_field_warning(): + """Test that ConversationInitData model does not trigger Pydantic UnsupportedFieldAttributeWarning. + + This test ensures that the 'frozen' parameter is not incorrectly used in Field() + definitions, which would cause warnings in Pydantic v2 for union types. + See: https://github.com/All-Hands-AI/infra/issues/860 + """ + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter('always') + + # Re-import to trigger any warnings during model definition + import importlib + + import openhands.server.session.conversation_init_data + + importlib.reload(openhands.server.session.conversation_init_data) + + # Check for warnings containing 'frozen' which would indicate + # incorrect usage of frozen=True in Field() + frozen_warnings = [ + warning for warning in w if 'frozen' in str(warning.message).lower() + ] + + assert len(frozen_warnings) == 0, ( + f'Pydantic frozen field warnings found: {[str(w.message) for w in frozen_warnings]}' + ) diff --git a/tests/unit/storage/data_models/test_settings.py b/tests/unit/storage/data_models/test_settings.py index a9db7c7547..621baf1b2c 100644 --- a/tests/unit/storage/data_models/test_settings.py +++ b/tests/unit/storage/data_models/test_settings.py @@ -1,3 +1,4 @@ +import warnings from unittest.mock import patch from pydantic import SecretStr @@ -98,3 +99,31 @@ def test_convert_to_settings(): settings = convert_to_settings(settings_with_token_data) assert settings.llm_api_key.get_secret_value() == 'test-key' + + +def test_settings_no_pydantic_frozen_field_warning(): + """Test that Settings model does not trigger Pydantic UnsupportedFieldAttributeWarning. + + This test ensures that the 'frozen' parameter is not incorrectly used in Field() + definitions, which would cause warnings in Pydantic v2 for union types. + See: https://github.com/All-Hands-AI/infra/issues/860 + """ + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter('always') + + # Re-import to trigger any warnings during model definition + import importlib + + import openhands.storage.data_models.settings + + importlib.reload(openhands.storage.data_models.settings) + + # Check for warnings containing 'frozen' which would indicate + # incorrect usage of frozen=True in Field() + frozen_warnings = [ + warning for warning in w if 'frozen' in str(warning.message).lower() + ] + + assert len(frozen_warnings) == 0, ( + f'Pydantic frozen field warnings found: {[str(w.message) for w in frozen_warnings]}' + )