From 13dba59bb8dec12e12c91c46ad45f4f5c70bf793 Mon Sep 17 00:00:00 2001 From: Graham Neubig Date: Thu, 23 Apr 2026 12:47:37 -0400 Subject: [PATCH] Fix enterprise migration 108 settings mapping (#14088) Co-authored-by: openhands --- ...d_agent_settings_to_enterprise_settings.py | 494 +++++++++++++----- .../test_migration_108_add_agent_settings.py | 135 +++++ 2 files changed, 512 insertions(+), 117 deletions(-) create mode 100644 enterprise/tests/unit/test_migration_108_add_agent_settings.py diff --git a/enterprise/migrations/versions/108_add_agent_settings_to_enterprise_settings.py b/enterprise/migrations/versions/108_add_agent_settings_to_enterprise_settings.py index 419bb92e8f..209818da76 100644 --- a/enterprise/migrations/versions/108_add_agent_settings_to_enterprise_settings.py +++ b/enterprise/migrations/versions/108_add_agent_settings_to_enterprise_settings.py @@ -6,7 +6,8 @@ Create Date: 2026-03-22 00:00:00.000000 """ -from typing import Sequence, Union +from collections.abc import Mapping +from typing import Any, Sequence, Union import sqlalchemy as sa from alembic import op @@ -21,6 +22,186 @@ depends_on: Union[str, Sequence[str], None] = None _EMPTY_JSON = sa.text("'{}'::json") +def _deep_merge( + base: dict[str, Any], overrides: Mapping[str, Any] | None +) -> dict[str, Any]: + merged = dict(base) + for key, value in (overrides or {}).items(): + existing = merged.get(key) + if isinstance(existing, dict) and isinstance(value, Mapping): + merged[key] = _deep_merge(existing, value) + else: + merged[key] = value + return merged + + +def _strip_none_and_empty(value: Any) -> Any: + if isinstance(value, Mapping): + cleaned: dict[str, Any] = {} + for key, item in value.items(): + cleaned_item = _strip_none_and_empty(item) + if cleaned_item is None: + continue + if isinstance(cleaned_item, dict) and not cleaned_item: + continue + cleaned[key] = cleaned_item + return cleaned + return value + + +def _build_user_agent_settings(row: Mapping[str, Any]) -> dict[str, Any]: + generated = _strip_none_and_empty( + { + 'schema_version': 1, + 'agent': row['agent'], + 'llm': { + 'model': row['llm_model'], + 'base_url': row['llm_base_url'], + }, + 'condenser': { + 'enabled': row['enable_default_condenser'], + 'max_size': row['condenser_max_size'], + }, + } + ) + return _deep_merge(generated, row.get('agent_settings') or {}) + + +def _build_user_conversation_settings(row: Mapping[str, Any]) -> dict[str, Any]: + generated = _strip_none_and_empty( + { + 'max_iterations': row['max_iterations'], + 'confirmation_mode': row['confirmation_mode'], + 'security_analyzer': row['security_analyzer'], + } + ) + return _deep_merge(generated, row.get('conversation_settings') or {}) + + +def _build_org_member_agent_settings_diff(row: Mapping[str, Any]) -> dict[str, Any]: + generated = _strip_none_and_empty( + { + 'schema_version': 1, + 'llm': { + 'model': row['llm_model'], + 'base_url': row['llm_base_url'], + }, + 'mcp_config': row['mcp_config'], + } + ) + return _deep_merge(generated, row.get('agent_settings_diff') or {}) + + +def _build_org_member_conversation_settings_diff( + row: Mapping[str, Any], +) -> dict[str, Any]: + generated = _strip_none_and_empty({'max_iterations': row['max_iterations']}) + return _deep_merge(generated, row.get('conversation_settings_diff') or {}) + + +def _build_org_agent_settings(row: Mapping[str, Any]) -> dict[str, Any]: + generated = _strip_none_and_empty( + { + 'schema_version': 1, + 'agent': row['agent'], + 'llm': { + 'model': row['default_llm_model'], + 'base_url': row['default_llm_base_url'], + }, + 'condenser': { + 'enabled': row['enable_default_condenser'], + 'max_size': row['condenser_max_size'], + }, + 'mcp_config': row['mcp_config'], + } + ) + return _deep_merge(generated, row.get('agent_settings') or {}) + + +def _build_org_conversation_settings(row: Mapping[str, Any]) -> dict[str, Any]: + generated = _strip_none_and_empty( + { + 'max_iterations': row['default_max_iterations'], + 'confirmation_mode': row['confirmation_mode'], + 'security_analyzer': row['security_analyzer'], + } + ) + return _deep_merge(generated, row.get('conversation_settings') or {}) + + +def _get_nested_value(data: Mapping[str, Any] | None, *path: str) -> Any: + current: Any = data or {} + for key in path: + if not isinstance(current, Mapping) or key not in current: + return None + current = current[key] + return current + + +def _legacy_user_settings_values(row: Mapping[str, Any]) -> dict[str, Any]: + agent_settings = row.get('agent_settings') or {} + conversation_settings = row.get('conversation_settings') or {} + condenser_enabled = _get_nested_value(agent_settings, 'condenser', 'enabled') + return { + 'agent': _get_nested_value(agent_settings, 'agent'), + 'max_iterations': _get_nested_value(conversation_settings, 'max_iterations'), + 'security_analyzer': _get_nested_value( + conversation_settings, 'security_analyzer' + ), + 'confirmation_mode': _get_nested_value( + conversation_settings, 'confirmation_mode' + ), + 'llm_model': _get_nested_value(agent_settings, 'llm', 'model'), + 'llm_base_url': _get_nested_value(agent_settings, 'llm', 'base_url'), + 'enable_default_condenser': ( + True if condenser_enabled is None else condenser_enabled + ), + 'condenser_max_size': _get_nested_value( + agent_settings, 'condenser', 'max_size' + ), + } + + +def _legacy_org_member_values(row: Mapping[str, Any]) -> dict[str, Any]: + agent_settings_diff = row.get('agent_settings_diff') or {} + conversation_settings_diff = row.get('conversation_settings_diff') or {} + return { + 'llm_model': _get_nested_value(agent_settings_diff, 'llm', 'model'), + 'llm_base_url': _get_nested_value(agent_settings_diff, 'llm', 'base_url'), + 'max_iterations': _get_nested_value( + conversation_settings_diff, 'max_iterations' + ), + 'mcp_config': _get_nested_value(agent_settings_diff, 'mcp_config'), + } + + +def _legacy_org_values(row: Mapping[str, Any]) -> dict[str, Any]: + agent_settings = row.get('agent_settings') or {} + conversation_settings = row.get('conversation_settings') or {} + condenser_enabled = _get_nested_value(agent_settings, 'condenser', 'enabled') + return { + 'agent': _get_nested_value(agent_settings, 'agent'), + 'default_max_iterations': _get_nested_value( + conversation_settings, 'max_iterations' + ), + 'security_analyzer': _get_nested_value( + conversation_settings, 'security_analyzer' + ), + 'confirmation_mode': _get_nested_value( + conversation_settings, 'confirmation_mode' + ), + 'default_llm_model': _get_nested_value(agent_settings, 'llm', 'model'), + 'default_llm_base_url': _get_nested_value(agent_settings, 'llm', 'base_url'), + 'enable_default_condenser': ( + True if condenser_enabled is None else condenser_enabled + ), + 'mcp_config': _get_nested_value(agent_settings, 'mcp_config'), + 'condenser_max_size': _get_nested_value( + agent_settings, 'condenser', 'max_size' + ), + } + + def upgrade() -> None: op.add_column( 'user_settings', @@ -82,63 +263,123 @@ def upgrade() -> None: ), ) - op.execute( - sa.text( - """ - UPDATE user_settings - SET agent_settings = jsonb_strip_nulls( - jsonb_build_object( - 'schema_version', 1, - 'agent', agent, - 'llm.model', llm_model, - 'llm.base_url', llm_base_url, - 'verification.confirmation_mode', confirmation_mode, - 'verification.security_analyzer', security_analyzer, - 'condenser.enabled', enable_default_condenser, - 'condenser.max_size', condenser_max_size, - 'max_iterations', max_iterations - ) || COALESCE(agent_settings::jsonb, '{}'::jsonb) - )::json - """ - ) + bind = op.get_bind() + + user_settings_table = sa.table( + 'user_settings', + sa.column('id', sa.Integer()), + sa.column('agent', sa.String()), + sa.column('max_iterations', sa.Integer()), + sa.column('security_analyzer', sa.String()), + sa.column('confirmation_mode', sa.Boolean()), + sa.column('llm_model', sa.String()), + sa.column('llm_base_url', sa.String()), + sa.column('enable_default_condenser', sa.Boolean()), + sa.column('condenser_max_size', sa.Integer()), + sa.column('agent_settings', sa.JSON()), + sa.column('conversation_settings', sa.JSON()), ) - op.execute( - sa.text( - """ - UPDATE org_member - SET agent_settings_diff = jsonb_strip_nulls( - jsonb_build_object( - 'schema_version', 1, - 'llm.model', llm_model, - 'llm.base_url', llm_base_url, - 'max_iterations', max_iterations, - 'mcp_config', mcp_config - ) || COALESCE(agent_settings_diff::jsonb, '{}'::jsonb) - )::json - """ + user_settings_rows = bind.execute( + sa.select( + user_settings_table.c.id, + user_settings_table.c.agent, + user_settings_table.c.max_iterations, + user_settings_table.c.security_analyzer, + user_settings_table.c.confirmation_mode, + user_settings_table.c.llm_model, + user_settings_table.c.llm_base_url, + user_settings_table.c.enable_default_condenser, + user_settings_table.c.condenser_max_size, + user_settings_table.c.agent_settings, + user_settings_table.c.conversation_settings, ) - ) - op.execute( - sa.text( - """ - UPDATE org - SET agent_settings = jsonb_strip_nulls( - jsonb_build_object( - 'schema_version', 1, - 'agent', agent, - 'llm.model', default_llm_model, - 'llm.base_url', default_llm_base_url, - 'verification.confirmation_mode', confirmation_mode, - 'verification.security_analyzer', security_analyzer, - 'condenser.enabled', enable_default_condenser, - 'condenser.max_size', condenser_max_size, - 'max_iterations', default_max_iterations, - 'mcp_config', mcp_config - ) || COALESCE(agent_settings::jsonb, '{}'::jsonb) - )::json - """ + ).mappings() + for row in user_settings_rows: + bind.execute( + user_settings_table.update() + .where(user_settings_table.c.id == row['id']) + .values( + agent_settings=_build_user_agent_settings(row), + conversation_settings=_build_user_conversation_settings(row), + ) ) + + org_member_table = sa.table( + 'org_member', + sa.column('org_id', sa.Uuid()), + sa.column('user_id', sa.Uuid()), + sa.column('max_iterations', sa.Integer()), + sa.column('llm_model', sa.String()), + sa.column('llm_base_url', sa.String()), + sa.column('mcp_config', sa.JSON()), + sa.column('agent_settings_diff', sa.JSON()), + sa.column('conversation_settings_diff', sa.JSON()), ) + org_member_rows = bind.execute( + sa.select( + org_member_table.c.org_id, + org_member_table.c.user_id, + org_member_table.c.max_iterations, + org_member_table.c.llm_model, + org_member_table.c.llm_base_url, + org_member_table.c.mcp_config, + org_member_table.c.agent_settings_diff, + org_member_table.c.conversation_settings_diff, + ) + ).mappings() + for row in org_member_rows: + bind.execute( + org_member_table.update() + .where(org_member_table.c.org_id == row['org_id']) + .where(org_member_table.c.user_id == row['user_id']) + .values( + agent_settings_diff=_build_org_member_agent_settings_diff(row), + conversation_settings_diff=_build_org_member_conversation_settings_diff( + row + ), + ) + ) + + org_table = sa.table( + 'org', + sa.column('id', sa.Uuid()), + sa.column('agent', sa.String()), + sa.column('default_max_iterations', sa.Integer()), + sa.column('security_analyzer', sa.String()), + sa.column('confirmation_mode', sa.Boolean()), + sa.column('default_llm_model', sa.String()), + sa.column('default_llm_base_url', sa.String()), + sa.column('enable_default_condenser', sa.Boolean()), + sa.column('mcp_config', sa.JSON()), + sa.column('condenser_max_size', sa.Integer()), + sa.column('agent_settings', sa.JSON()), + sa.column('conversation_settings', sa.JSON()), + ) + org_rows = bind.execute( + sa.select( + org_table.c.id, + org_table.c.agent, + org_table.c.default_max_iterations, + org_table.c.security_analyzer, + org_table.c.confirmation_mode, + org_table.c.default_llm_model, + org_table.c.default_llm_base_url, + org_table.c.enable_default_condenser, + org_table.c.mcp_config, + org_table.c.condenser_max_size, + org_table.c.agent_settings, + org_table.c.conversation_settings, + ) + ).mappings() + for row in org_rows: + bind.execute( + org_table.update() + .where(org_table.c.id == row['id']) + .values( + agent_settings=_build_org_agent_settings(row), + conversation_settings=_build_org_conversation_settings(row), + ) + ) op.alter_column('user_settings', 'agent_settings', server_default=None) op.alter_column('user_settings', 'conversation_settings', server_default=None) @@ -223,73 +464,92 @@ def downgrade() -> None: op.add_column('org', sa.Column('mcp_config', sa.JSON(), nullable=True)) op.add_column('org', sa.Column('condenser_max_size', sa.Integer(), nullable=True)) - op.execute( - sa.text( - """ - UPDATE user_settings - SET - agent = agent_settings ->> 'agent', - max_iterations = NULLIF(agent_settings ->> 'max_iterations', '')::integer, - security_analyzer = - agent_settings ->> 'verification.security_analyzer', - confirmation_mode = CASE - WHEN agent_settings::jsonb ? 'verification.confirmation_mode' - THEN (agent_settings ->> 'verification.confirmation_mode')::boolean - ELSE NULL - END, - llm_model = agent_settings ->> 'llm.model', - llm_base_url = agent_settings ->> 'llm.base_url', - enable_default_condenser = CASE - WHEN agent_settings::jsonb ? 'condenser.enabled' - THEN (agent_settings ->> 'condenser.enabled')::boolean - ELSE TRUE - END, - condenser_max_size = - NULLIF(agent_settings ->> 'condenser.max_size', '')::integer - """ - ) + bind = op.get_bind() + + user_settings_table = sa.table( + 'user_settings', + sa.column('id', sa.Integer()), + sa.column('agent_settings', sa.JSON()), + sa.column('conversation_settings', sa.JSON()), + sa.column('agent', sa.String()), + sa.column('max_iterations', sa.Integer()), + sa.column('security_analyzer', sa.String()), + sa.column('confirmation_mode', sa.Boolean()), + sa.column('llm_model', sa.String()), + sa.column('llm_base_url', sa.String()), + sa.column('enable_default_condenser', sa.Boolean()), + sa.column('condenser_max_size', sa.Integer()), ) - op.execute( - sa.text( - """ - UPDATE org_member - SET - llm_model = agent_settings_diff ->> 'llm.model', - llm_base_url = agent_settings_diff ->> 'llm.base_url', - max_iterations = - NULLIF(agent_settings_diff ->> 'max_iterations', '')::integer, - mcp_config = agent_settings_diff -> 'mcp_config' - """ + user_settings_rows = bind.execute( + sa.select( + user_settings_table.c.id, + user_settings_table.c.agent_settings, + user_settings_table.c.conversation_settings, ) - ) - op.execute( - sa.text( - """ - UPDATE org - SET - agent = agent_settings ->> 'agent', - default_max_iterations = - NULLIF(agent_settings ->> 'max_iterations', '')::integer, - security_analyzer = - agent_settings ->> 'verification.security_analyzer', - confirmation_mode = CASE - WHEN agent_settings::jsonb ? 'verification.confirmation_mode' - THEN (agent_settings ->> 'verification.confirmation_mode')::boolean - ELSE NULL - END, - default_llm_model = agent_settings ->> 'llm.model', - default_llm_base_url = agent_settings ->> 'llm.base_url', - enable_default_condenser = CASE - WHEN agent_settings::jsonb ? 'condenser.enabled' - THEN (agent_settings ->> 'condenser.enabled')::boolean - ELSE TRUE - END, - mcp_config = agent_settings -> 'mcp_config', - condenser_max_size = - NULLIF(agent_settings ->> 'condenser.max_size', '')::integer - """ + ).mappings() + for row in user_settings_rows: + bind.execute( + user_settings_table.update() + .where(user_settings_table.c.id == row['id']) + .values(**_legacy_user_settings_values(row)) ) + + org_member_table = sa.table( + 'org_member', + sa.column('org_id', sa.Uuid()), + sa.column('user_id', sa.Uuid()), + sa.column('agent_settings_diff', sa.JSON()), + sa.column('conversation_settings_diff', sa.JSON()), + sa.column('llm_model', sa.String()), + sa.column('llm_base_url', sa.String()), + sa.column('max_iterations', sa.Integer()), + sa.column('mcp_config', sa.JSON()), ) + org_member_rows = bind.execute( + sa.select( + org_member_table.c.org_id, + org_member_table.c.user_id, + org_member_table.c.agent_settings_diff, + org_member_table.c.conversation_settings_diff, + ) + ).mappings() + for row in org_member_rows: + bind.execute( + org_member_table.update() + .where(org_member_table.c.org_id == row['org_id']) + .where(org_member_table.c.user_id == row['user_id']) + .values(**_legacy_org_member_values(row)) + ) + + org_table = sa.table( + 'org', + sa.column('id', sa.Uuid()), + sa.column('agent_settings', sa.JSON()), + sa.column('conversation_settings', sa.JSON()), + sa.column('agent', sa.String()), + sa.column('default_max_iterations', sa.Integer()), + sa.column('security_analyzer', sa.String()), + sa.column('confirmation_mode', sa.Boolean()), + sa.column('default_llm_model', sa.String()), + sa.column('default_llm_base_url', sa.String()), + sa.column('enable_default_condenser', sa.Boolean()), + sa.column('mcp_config', sa.JSON()), + sa.column('condenser_max_size', sa.Integer()), + ) + org_rows = bind.execute( + sa.select( + org_table.c.id, + org_table.c.agent_settings, + org_table.c.conversation_settings, + ) + ).mappings() + for row in org_rows: + bind.execute( + org_table.update() + .where(org_table.c.id == row['id']) + .values(**_legacy_org_values(row)) + ) + op.drop_column('org', 'agent_settings') op.drop_column('org', 'conversation_settings') op.drop_column('org', '_llm_api_key') diff --git a/enterprise/tests/unit/test_migration_108_add_agent_settings.py b/enterprise/tests/unit/test_migration_108_add_agent_settings.py new file mode 100644 index 0000000000..21efc8c2df --- /dev/null +++ b/enterprise/tests/unit/test_migration_108_add_agent_settings.py @@ -0,0 +1,135 @@ +from importlib.util import module_from_spec, spec_from_file_location +from pathlib import Path + +from storage.user_settings import UserSettings + +MIGRATION_PATH = ( + Path(__file__).resolve().parents[2] + / 'migrations' + / 'versions' + / '108_add_agent_settings_to_enterprise_settings.py' +) +spec = spec_from_file_location('migration_108', MIGRATION_PATH) +assert spec is not None and spec.loader is not None +migration_108 = module_from_spec(spec) +spec.loader.exec_module(migration_108) + + +def test_user_settings_are_split_into_agent_and_conversation_buckets(): + row = { + 'agent': 'CodeActAgent', + 'max_iterations': 42, + 'security_analyzer': 'llm', + 'confirmation_mode': True, + 'llm_model': 'anthropic/claude-sonnet-4-5-20250929', + 'llm_base_url': 'https://api.example.com', + 'enable_default_condenser': False, + 'condenser_max_size': 128, + 'agent_settings': {}, + 'conversation_settings': {}, + } + + agent_settings = migration_108._build_user_agent_settings(row) + conversation_settings = migration_108._build_user_conversation_settings(row) + + assert agent_settings == { + 'schema_version': 1, + 'agent': 'CodeActAgent', + 'llm': { + 'model': 'anthropic/claude-sonnet-4-5-20250929', + 'base_url': 'https://api.example.com', + }, + 'condenser': {'enabled': False, 'max_size': 128}, + } + assert conversation_settings == { + 'max_iterations': 42, + 'confirmation_mode': True, + 'security_analyzer': 'llm', + } + + +def test_org_member_diffs_use_nested_llm_and_conversation_settings(): + row = { + 'max_iterations': 50, + 'llm_model': 'openhands/claude-3', + 'llm_base_url': 'https://proxy.example.com', + 'mcp_config': {'mcpServers': {'admin': {'url': 'https://mcp.example.com'}}}, + 'agent_settings_diff': {}, + 'conversation_settings_diff': {}, + } + + agent_settings_diff = migration_108._build_org_member_agent_settings_diff(row) + conversation_settings_diff = ( + migration_108._build_org_member_conversation_settings_diff(row) + ) + + assert agent_settings_diff == { + 'schema_version': 1, + 'llm': { + 'model': 'openhands/claude-3', + 'base_url': 'https://proxy.example.com', + }, + 'mcp_config': {'mcpServers': {'admin': {'url': 'https://mcp.example.com'}}}, + } + assert conversation_settings_diff == {'max_iterations': 50} + + +def test_downgrade_extracts_legacy_values_from_nested_settings(): + row = { + 'agent_settings': { + 'schema_version': 1, + 'agent': 'CodeActAgent', + 'llm': { + 'model': 'anthropic/claude-sonnet-4-5-20250929', + 'base_url': 'https://api.example.com', + }, + 'condenser': {'enabled': False, 'max_size': 128}, + }, + 'conversation_settings': { + 'max_iterations': 42, + 'confirmation_mode': True, + 'security_analyzer': 'llm', + }, + } + + assert migration_108._legacy_user_settings_values(row) == { + 'agent': 'CodeActAgent', + 'max_iterations': 42, + 'security_analyzer': 'llm', + 'confirmation_mode': True, + 'llm_model': 'anthropic/claude-sonnet-4-5-20250929', + 'llm_base_url': 'https://api.example.com', + 'enable_default_condenser': False, + 'condenser_max_size': 128, + } + + +def test_migrated_payload_loads_via_user_settings_to_settings(): + row = { + 'agent': 'CodeActAgent', + 'max_iterations': 42, + 'security_analyzer': 'llm', + 'confirmation_mode': True, + 'llm_model': 'anthropic/claude-sonnet-4-5-20250929', + 'llm_base_url': 'https://api.example.com', + 'enable_default_condenser': False, + 'condenser_max_size': 128, + 'agent_settings': {}, + 'conversation_settings': {}, + } + + user_settings = UserSettings( + agent_settings=migration_108._build_user_agent_settings(row), + conversation_settings=migration_108._build_user_conversation_settings(row), + ) + + settings = user_settings.to_settings() + + assert settings.agent_settings.agent == 'CodeActAgent' + assert settings.agent_settings.llm.model == 'anthropic/claude-sonnet-4-5-20250929' + assert settings.agent_settings.llm.base_url == 'https://api.example.com' + assert settings.agent_settings.condenser.enabled is False + assert settings.agent_settings.condenser.max_size == 128 + assert settings.conversation_settings.max_iterations == 42 + assert settings.conversation_settings.confirmation_mode is True + assert settings.conversation_settings.security_analyzer == 'llm'