mirror of
https://github.com/OpenHands/OpenHands.git
synced 2026-03-22 05:37:20 +08:00
fix: treat llm_base_url="" as explicit clear in store_llm_settings (#13471)
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -123,10 +123,9 @@ async def store_llm_settings(
|
|||||||
settings.llm_api_key = existing_settings.llm_api_key
|
settings.llm_api_key = existing_settings.llm_api_key
|
||||||
if settings.llm_model is None:
|
if settings.llm_model is None:
|
||||||
settings.llm_model = existing_settings.llm_model
|
settings.llm_model = existing_settings.llm_model
|
||||||
# if llm_base_url is missing or empty, try to preserve existing or determine appropriate URL
|
if settings.llm_base_url is None:
|
||||||
if not settings.llm_base_url:
|
# Not provided at all (e.g. MCP config save) - preserve existing or auto-detect
|
||||||
if settings.llm_base_url is None and existing_settings.llm_base_url:
|
if existing_settings.llm_base_url:
|
||||||
# Not provided at all (e.g. MCP config save) - preserve existing
|
|
||||||
settings.llm_base_url = existing_settings.llm_base_url
|
settings.llm_base_url = existing_settings.llm_base_url
|
||||||
elif is_openhands_model(settings.llm_model):
|
elif is_openhands_model(settings.llm_model):
|
||||||
# OpenHands models use the LiteLLM proxy
|
# OpenHands models use the LiteLLM proxy
|
||||||
@@ -145,6 +144,9 @@ async def store_llm_settings(
|
|||||||
logger.error(
|
logger.error(
|
||||||
f'Failed to get api_base from litellm for model {settings.llm_model}: {e}'
|
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
|
# Keep search API key if missing or empty
|
||||||
if not settings.search_api_key:
|
if not settings.search_api_key:
|
||||||
settings.search_api_key = existing_settings.search_api_key
|
settings.search_api_key = existing_settings.search_api_key
|
||||||
|
|||||||
@@ -211,9 +211,32 @@ async def test_store_llm_settings_partial_update():
|
|||||||
assert result.llm_model == 'gpt-4'
|
assert result.llm_model == 'gpt-4'
|
||||||
# For SecretStr objects, we need to compare the secret value
|
# For SecretStr objects, we need to compare the secret value
|
||||||
assert result.llm_api_key.get_secret_value() == 'existing-api-key'
|
assert result.llm_api_key.get_secret_value() == 'existing-api-key'
|
||||||
# llm_base_url was explicitly cleared (""), so auto-detection runs
|
# llm_base_url="" is an explicit clear — must not be repopulated via auto-detection
|
||||||
# OpenAI models: litellm.get_api_base() returns https://api.openai.com
|
assert result.llm_base_url is None
|
||||||
assert result.llm_base_url == 'https://api.openai.com'
|
|
||||||
|
|
||||||
|
@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
|
@pytest.mark.asyncio
|
||||||
|
|||||||
Reference in New Issue
Block a user