From a75b576f1cf087650227c8d4df805f9f45e6758e Mon Sep 17 00:00:00 2001 From: Abi Date: Fri, 20 Mar 2026 15:44:15 +0530 Subject: [PATCH] fix: treat llm_base_url="" as explicit clear in store_llm_settings (#13471) Co-authored-by: Claude Sonnet 4.6 --- openhands/server/routes/settings.py | 10 ++++--- .../routes/test_settings_store_functions.py | 29 +++++++++++++++++-- 2 files changed, 32 insertions(+), 7 deletions(-) diff --git a/openhands/server/routes/settings.py b/openhands/server/routes/settings.py index 62944d11ce..4affad1d61 100644 --- a/openhands/server/routes/settings.py +++ b/openhands/server/routes/settings.py @@ -123,10 +123,9 @@ async def store_llm_settings( settings.llm_api_key = existing_settings.llm_api_key if settings.llm_model is None: settings.llm_model = existing_settings.llm_model - # if llm_base_url is missing or empty, try to preserve existing or determine appropriate URL - if not settings.llm_base_url: - if settings.llm_base_url is None and existing_settings.llm_base_url: - # Not provided at all (e.g. MCP config save) - preserve existing + if settings.llm_base_url is None: + # Not provided at all (e.g. MCP config save) - preserve existing or auto-detect + if existing_settings.llm_base_url: settings.llm_base_url = existing_settings.llm_base_url elif is_openhands_model(settings.llm_model): # OpenHands models use the LiteLLM proxy @@ -145,6 +144,9 @@ async def store_llm_settings( logger.error( f'Failed to get api_base from litellm for model {settings.llm_model}: {e}' ) + elif settings.llm_base_url == '': + # Explicitly cleared by the user (basic view save or advanced view clear) + settings.llm_base_url = None # Keep search API key if missing or empty if not settings.search_api_key: settings.search_api_key = existing_settings.search_api_key diff --git a/tests/unit/server/routes/test_settings_store_functions.py b/tests/unit/server/routes/test_settings_store_functions.py index f51a5b506a..48bc79a280 100644 --- a/tests/unit/server/routes/test_settings_store_functions.py +++ b/tests/unit/server/routes/test_settings_store_functions.py @@ -211,9 +211,32 @@ async def test_store_llm_settings_partial_update(): assert result.llm_model == 'gpt-4' # For SecretStr objects, we need to compare the secret value assert result.llm_api_key.get_secret_value() == 'existing-api-key' - # llm_base_url was explicitly cleared (""), so auto-detection runs - # OpenAI models: litellm.get_api_base() returns https://api.openai.com - assert result.llm_base_url == 'https://api.openai.com' + # llm_base_url="" is an explicit clear — must not be repopulated via auto-detection + assert result.llm_base_url is None + + +@pytest.mark.asyncio +async def test_store_llm_settings_advanced_view_clear_removes_base_url(): + """Regression test for #13420: clearing Base URL in Advanced view must persist. + + Before the fix, llm_base_url="" was treated identically to llm_base_url=None, + causing the backend to re-run auto-detection and overwrite the user's intent. + """ + settings = Settings( + llm_model='gpt-4', + llm_base_url='', # User deleted the field in Advanced view + ) + + existing_settings = Settings( + llm_model='gpt-4', + llm_api_key=SecretStr('my-api-key'), + llm_base_url='https://my-custom-proxy.example.com', + ) + + result = await store_llm_settings(settings, existing_settings) + + # The old custom URL must not come back + assert result.llm_base_url is None @pytest.mark.asyncio