From 41d8bd28e9fa6837a89dc6901a86e7adb08a60fa Mon Sep 17 00:00:00 2001 From: Chris Bagwell Date: Thu, 5 Mar 2026 19:39:58 -0600 Subject: [PATCH] fix: preserve llm_base_url when saving MCP server config (#13225) --- openhands/server/routes/settings.py | 7 +- .../routes/test_settings_store_functions.py | 66 ++++++++++++++++++- 2 files changed, 70 insertions(+), 3 deletions(-) diff --git a/openhands/server/routes/settings.py b/openhands/server/routes/settings.py index eda0cb938f..cc431f2e4d 100644 --- a/openhands/server/routes/settings.py +++ b/openhands/server/routes/settings.py @@ -141,9 +141,12 @@ 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 determine appropriate URL + # if llm_base_url is missing or empty, try to preserve existing or determine appropriate URL if not settings.llm_base_url: - if is_openhands_model(settings.llm_model): + if settings.llm_base_url is None and 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 + elif is_openhands_model(settings.llm_model): # OpenHands models use the LiteLLM proxy settings.llm_base_url = LITE_LLM_API_URL elif settings.llm_model: diff --git a/tests/unit/server/routes/test_settings_store_functions.py b/tests/unit/server/routes/test_settings_store_functions.py index 366f9e145f..f51a5b506a 100644 --- a/tests/unit/server/routes/test_settings_store_functions.py +++ b/tests/unit/server/routes/test_settings_store_functions.py @@ -6,6 +6,7 @@ from fastapi import FastAPI from fastapi.testclient import TestClient from pydantic import SecretStr +from openhands.core.config.mcp_config import MCPConfig, MCPStdioServerConfig from openhands.integrations.provider import ProviderToken from openhands.integrations.service_types import ProviderType from openhands.server.routes.secrets import ( @@ -193,7 +194,8 @@ async def test_store_llm_settings_partial_update(): For OpenAI models, this returns https://api.openai.com. """ settings = Settings( - llm_model='gpt-4' # Only updating model (not an openhands model) + llm_model='gpt-4', # Only updating model (not an openhands model) + llm_base_url='', # Explicitly cleared (e.g. basic mode save) ) # Create existing settings @@ -209,10 +211,72 @@ 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' +@pytest.mark.asyncio +async def test_store_llm_settings_mcp_update_preserves_base_url(): + """Test that saving MCP config (without LLM fields) preserves existing base URL. + + Regression test: When adding an MCP server, the frontend sends only mcp_config + and v1_enabled. This should not wipe out the existing llm_base_url. + """ + # Simulate what the MCP add/update/delete mutations send: mcp_config but no LLM fields + settings = Settings( + mcp_config=MCPConfig( + stdio_servers=[ + MCPStdioServerConfig( + name='my-server', + command='npx', + args=['-y', '@my/mcp-server'], + env={'API_TOKEN': 'secret123', 'ENDPOINT': 'https://example.com'}, + ) + ], + ), + ) + + # Create existing settings with a custom base URL + existing_settings = Settings( + llm_model='anthropic/claude-sonnet-4-5-20250929', + llm_api_key=SecretStr('existing-api-key'), + llm_base_url='https://my-custom-proxy.example.com', + ) + + result = await store_llm_settings(settings, existing_settings) + + # All existing LLM settings should be preserved + assert result.llm_model == 'anthropic/claude-sonnet-4-5-20250929' + assert result.llm_api_key.get_secret_value() == 'existing-api-key' + assert result.llm_base_url == 'https://my-custom-proxy.example.com' + + +@pytest.mark.asyncio +async def test_store_llm_settings_no_existing_base_url_uses_auto_detection(): + """Test auto-detection kicks in only when there is no existing base URL. + + When neither the incoming settings nor existing settings have a base URL, + auto-detection from litellm should be used. + """ + settings = Settings( + llm_model='gpt-4' # Not an openhands model + ) + + # Existing settings without a base URL + existing_settings = Settings( + llm_model='gpt-3.5', + llm_api_key=SecretStr('existing-api-key'), + ) + + result = await store_llm_settings(settings, existing_settings) + + assert result.llm_model == 'gpt-4' + assert result.llm_api_key.get_secret_value() == 'existing-api-key' + # No existing base URL, so auto-detection should set it + assert result.llm_base_url == 'https://api.openai.com' + + @pytest.mark.asyncio async def test_store_llm_settings_anthropic_model_gets_api_base(): """Test store_llm_settings with an Anthropic model.