chore: Prevent user from modifying basic LLM settings in saas mode (#7137)

frontend/__tests__/routes/home.test.tsx

@@ -8,6 +8,7 @@ import MainApp from "#/routes/_oh/route";
 import SettingsScreen from "#/routes/settings";
 import Home from "#/routes/_oh._index/route";
 import OpenHands from "#/api/open-hands";
+import * as FeatureFlags from "#/utils/feature-flags";
 
 const createAxiosNotFoundErrorObject = () =>
   new AxiosError(
@@ -27,6 +28,7 @@ const createAxiosNotFoundErrorObject = () =>
 
 describe("Home Screen", () => {
   const getSettingsSpy = vi.spyOn(OpenHands, "getSettings");
+  const getConfigSpy = vi.spyOn(OpenHands, "getConfig");
 
   const RouterStub = createRoutesStub([
     {
@@ -113,5 +115,21 @@ describe("Home Screen", () => {
       const settingsModalAfter = screen.queryByTestId("ai-config-modal");
       expect(settingsModalAfter).not.toBeInTheDocument();
     });
+
+    it("should not open the settings modal if GET /settings fails but is SaaS mode", async () => {
+      // TODO: Remove HIDE_LLM_SETTINGS check once released
+      vi.spyOn(FeatureFlags, "HIDE_LLM_SETTINGS").mockReturnValue(true);
+      // @ts-expect-error - we only need APP_MODE for this test
+      getConfigSpy.mockResolvedValue({ APP_MODE: "saas" });
+      const error = createAxiosNotFoundErrorObject();
+      getSettingsSpy.mockRejectedValue(error);
+
+      renderWithProviders(<RouterStub initialEntries={["/"]} />);
+
+      // small hack to wait for the modal to not appear
+      await expect(
+        screen.findByTestId("ai-config-modal", {}, { timeout: 1000 }),
+      ).rejects.toThrow();
+    });
   });
 });

frontend/__tests__/routes/settings.test.tsx

@@ -1,6 +1,15 @@
 import { render, screen, waitFor, within } from "@testing-library/react";
 import { createRoutesStub } from "react-router";
-import { afterEach, describe, expect, it, test, vi } from "vitest";
+import {
+  afterEach,
+  beforeAll,
+  beforeEach,
+  describe,
+  expect,
+  it,
+  test,
+  vi,
+} from "vitest";
 import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
 import userEvent, { UserEvent } from "@testing-library/user-event";
 import OpenHands from "#/api/open-hands";
@@ -11,6 +20,7 @@ import { MOCK_DEFAULT_USER_SETTINGS } from "#/mocks/handlers";
 import { PostApiSettings } from "#/types/settings";
 import * as ConsentHandlers from "#/utils/handle-capture-consent";
 import AccountSettings from "#/routes/account-settings";
+import * as FeatureFlags from "#/utils/feature-flags";
 
 const toggleAdvancedSettings = async (user: UserEvent) => {
   const advancedSwitch = await screen.findByTestId("advanced-settings-switch");
@@ -29,6 +39,11 @@ describe("Settings Screen", () => {
     useAppLogout: vi.fn().mockReturnValue({ handleLogout: handleLogoutMock }),
   }));
 
+  beforeAll(() => {
+    // TODO: Remove this once we release
+    vi.spyOn(FeatureFlags, "HIDE_LLM_SETTINGS").mockReturnValue(true);
+  });
+
   afterEach(() => {
     vi.clearAllMocks();
   });
@@ -67,6 +82,14 @@ describe("Settings Screen", () => {
   });
 
   describe("Account Settings", () => {
+    beforeEach(() => {
+      getConfigSpy.mockResolvedValue({
+        APP_MODE: "oss",
+        GITHUB_CLIENT_ID: "123",
+        POSTHOG_CLIENT_KEY: "456",
+      });
+    });
+
     it("should render the account settings", async () => {
       renderSettingsScreen();
 
@@ -280,6 +303,14 @@ describe("Settings Screen", () => {
   });
 
   describe("LLM Settings", () => {
+    beforeEach(() => {
+      getConfigSpy.mockResolvedValue({
+        APP_MODE: "oss",
+        GITHUB_CLIENT_ID: "123",
+        POSTHOG_CLIENT_KEY: "456",
+      });
+    });
+
     it("should render the basic LLM settings by default", async () => {
       renderSettingsScreen();
 
@@ -427,8 +458,7 @@ describe("Settings Screen", () => {
         expect(input).not.toBeInTheDocument();
       });
 
-      it("should render the runtime settings input if SaaS mode", async () => {
-        const user = userEvent.setup();
+      it.skip("should render the runtime settings input if SaaS mode", async () => {
         getConfigSpy.mockResolvedValue({
           APP_MODE: "saas",
           GITHUB_CLIENT_ID: "123",
@@ -436,12 +466,10 @@ describe("Settings Screen", () => {
         });
 
         renderSettingsScreen();
-
-        await toggleAdvancedSettings(user);
-        screen.getByTestId("runtime-settings-input");
+        await screen.findByTestId("runtime-settings-input");
       });
 
-      it("should set the default runtime setting set", async () => {
+      it.skip("should set the default runtime setting set", async () => {
         getConfigSpy.mockResolvedValue({
           APP_MODE: "saas",
           GITHUB_CLIENT_ID: "123",
@@ -455,13 +483,11 @@ describe("Settings Screen", () => {
 
         renderSettingsScreen();
 
-        await toggleAdvancedSettings(userEvent.setup());
-
         const input = await screen.findByTestId("runtime-settings-input");
         expect(input).toHaveValue("1x (2 core, 8G)");
       });
 
-      it("should always have the runtime input disabled", async () => {
+      it.skip("should always have the runtime input disabled", async () => {
         getConfigSpy.mockResolvedValue({
           APP_MODE: "saas",
           GITHUB_CLIENT_ID: "123",
@@ -470,8 +496,6 @@ describe("Settings Screen", () => {
 
         renderSettingsScreen();
 
-        await toggleAdvancedSettings(userEvent.setup());
-
         const input = await screen.findByTestId("runtime-settings-input");
         expect(input).toBeDisabled();
       });
@@ -490,8 +514,6 @@ describe("Settings Screen", () => {
 
         renderSettingsScreen();
 
-        await toggleAdvancedSettings(user);
-
         const input = await screen.findByTestId("runtime-settings-input");
         await user.click(input);
 
@@ -953,4 +975,104 @@ describe("Settings Screen", () => {
       );
     });
   });
+
+  describe("SaaS mode", () => {
+    beforeEach(() => {
+      getConfigSpy.mockResolvedValue({
+        APP_MODE: "saas",
+        GITHUB_CLIENT_ID: "123",
+        POSTHOG_CLIENT_KEY: "456",
+      });
+    });
+
+    it("should hide the entire LLM section", async () => {
+      renderSettingsScreen();
+
+      await waitFor(() => {
+        screen.getByTestId("account-settings-form"); // wait for the account settings form to render
+
+        const llmSection = screen.queryByTestId("llm-settings-section");
+        expect(llmSection).not.toBeInTheDocument();
+      });
+    });
+
+    it("should not render LLM Provider, LLM Model, and LLM API Key inputs", async () => {
+      renderSettingsScreen();
+
+      await waitFor(() => {
+        screen.getByTestId("account-settings-form"); // wait for the account settings form to render
+
+        const providerInput = screen.queryByTestId("llm-provider-input");
+        const modelInput = screen.queryByTestId("llm-model-input");
+        const apiKeyInput = screen.queryByTestId("llm-api-key-input");
+        const llmApiKeyHelpAnchor = screen.queryByTestId(
+          "llm-api-key-help-anchor",
+        );
+        const advancedSettingsSwitch = screen.queryByTestId(
+          "advanced-settings-switch",
+        );
+
+        expect(providerInput).not.toBeInTheDocument();
+        expect(modelInput).not.toBeInTheDocument();
+        expect(apiKeyInput).not.toBeInTheDocument();
+        expect(llmApiKeyHelpAnchor).not.toBeInTheDocument();
+        expect(advancedSettingsSwitch).not.toBeInTheDocument();
+      });
+    });
+
+    // We might want to bring this back if users wish to set advanced settings
+    it.skip("should render advanced settings by default", async () => {
+      renderSettingsScreen();
+
+      await waitFor(() => {
+        screen.getByTestId("account-settings-form"); // wait for the account settings form to render
+
+        const agentInput = screen.getByTestId("agent-input"); // this is only rendered in advanced mode
+        expect(agentInput).toBeInTheDocument();
+
+        // we should not allow the user to change these fields
+        const customModelInput = screen.queryByTestId("llm-custom-model-input");
+        const baseUrlInput = screen.queryByTestId("base-url-input");
+
+        expect(customModelInput).not.toBeInTheDocument();
+        expect(baseUrlInput).not.toBeInTheDocument();
+      });
+    });
+
+    it("should not submit the unwanted fields when saving", async () => {
+      const user = userEvent.setup();
+      renderSettingsScreen();
+
+      const saveButton = await screen.findByText("Save Changes");
+      await user.click(saveButton);
+
+      expect(saveSettingsSpy).toHaveBeenCalledWith(
+        expect.objectContaining({
+          llm_api_key: undefined,
+          llm_base_url: undefined,
+          llm_model: undefined,
+        }),
+      );
+    });
+
+    it("should not submit the unwanted fields when resetting", async () => {
+      const user = userEvent.setup();
+      renderSettingsScreen();
+
+      const resetButton = await screen.findByText("Reset to defaults");
+      await user.click(resetButton);
+
+      const modal = await screen.findByTestId("reset-modal");
+      const confirmButton = within(modal).getByText("Reset");
+      await user.click(confirmButton);
+
+      expect(saveSettingsSpy).toHaveBeenCalledWith(
+        expect.objectContaining({
+          llm_api_key: undefined,
+          llm_base_url: undefined,
+          llm_model: undefined,
+        }),
+      );
+    });
+  });
 });

frontend/src/components/features/sidebar/sidebar.tsx

@@ -22,6 +22,7 @@ import { ConversationPanelWrapper } from "../conversation-panel/conversation-pan
 import { useLogout } from "#/hooks/mutation/use-logout";
 import { useConfig } from "#/hooks/query/use-config";
 import { cn } from "#/utils/utils";
+import { HIDE_LLM_SETTINGS } from "#/utils/feature-flags";
 
 export function Sidebar() {
   const location = useLocation();
@@ -42,7 +43,12 @@ export function Sidebar() {
   const [conversationPanelIsOpen, setConversationPanelIsOpen] =
     React.useState(false);
 
+  // TODO: Remove HIDE_LLM_SETTINGS check once released
+  const isSaas = HIDE_LLM_SETTINGS() && config?.APP_MODE === "saas";
+
   React.useEffect(() => {
+    if (isSaas) return;
+
     if (location.pathname === "/settings") {
       setSettingsModalIsOpen(false);
     } else if (

frontend/src/routes/account-settings.tsx

@@ -25,6 +25,8 @@ import {
   displayErrorToast,
   displaySuccessToast,
 } from "#/utils/custom-toast-handlers";
+import { PostSettings } from "#/types/settings";
+import { HIDE_LLM_SETTINGS } from "#/utils/feature-flags";
 
 const REMOTE_RUNTIME_OPTIONS = [
   { key: 1, label: "1x (2 core, 8G)" },
@@ -50,7 +52,12 @@ function AccountSettings() {
   const isFetching = isFetchingSettings || isFetchingResources;
   const isSuccess = isSuccessfulSettings && isSuccessfulResources;
 
+  const isSaas = config?.APP_MODE === "saas";
+  const shouldHandleSpecialSaasCase = HIDE_LLM_SETTINGS() && isSaas;
+
   const determineWhetherToToggleAdvancedSettings = () => {
+    if (shouldHandleSpecialSaasCase) return true;
+
     if (isSuccess) {
       return (
         isCustomModel(resources.models, settings.LLM_MODEL) ||
@@ -61,7 +68,6 @@ function AccountSettings() {
     return false;
   };
 
-  const isSaas = config?.APP_MODE === "saas";
   const hasAppSlug = !!config?.APP_SLUG;
   const isGitHubTokenSet = settings?.GITHUB_TOKEN_IS_SET;
   const isLLMKeySet = settings?.LLM_API_KEY === "**********";
@@ -106,6 +112,21 @@ function AccountSettings() {
       formData.get("enable-memory-condenser-switch")?.toString() === "on";
     const enableSoundNotifications =
       formData.get("enable-sound-notifications-switch")?.toString() === "on";
+    const llmBaseUrl = formData.get("base-url-input")?.toString() || "";
+    const llmApiKey =
+      formData.get("llm-api-key-input")?.toString() ||
+      (isLLMKeySet
+        ? undefined // don't update if it's already set
+        : ""); // reset if it's first time save to avoid 500 error
+
+    // we don't want the user to be able to modify these settings in SaaS
+    const finalLlmModel = shouldHandleSpecialSaasCase
+      ? undefined
+      : customLlmModel || fullLlmModel;
+    const finalLlmBaseUrl = shouldHandleSpecialSaasCase
+      ? undefined
+      : llmBaseUrl;
+    const finalLlmApiKey = shouldHandleSpecialSaasCase ? undefined : llmApiKey;
 
     saveSettings(
       {
@@ -115,13 +136,9 @@ function AccountSettings() {
         user_consents_to_analytics: userConsentsToAnalytics,
         ENABLE_DEFAULT_CONDENSER: enableMemoryCondenser,
         ENABLE_SOUND_NOTIFICATIONS: enableSoundNotifications,
-        LLM_MODEL: customLlmModel || fullLlmModel,
-        LLM_BASE_URL: formData.get("base-url-input")?.toString() || "",
-        LLM_API_KEY:
-          formData.get("llm-api-key-input")?.toString() ||
-          (isLLMKeySet
-            ? undefined // don't update if it's already set
-            : ""), // reset if it's first time save to avoid 500 error
+        LLM_MODEL: finalLlmModel,
+        LLM_BASE_URL: finalLlmBaseUrl,
+        LLM_API_KEY: finalLlmApiKey,
         AGENT: formData.get("agent-input")?.toString(),
         SECURITY_ANALYZER:
           formData.get("security-analyzer-input")?.toString() || "",
@@ -145,19 +162,26 @@ function AccountSettings() {
   };
 
   const handleReset = () => {
-    saveSettings(
-      {
-        ...DEFAULT_SETTINGS,
-        LLM_API_KEY: "", // reset LLM API key
+    const newSettings: Partial<PostSettings> = {
+      ...DEFAULT_SETTINGS,
+      LLM_API_KEY: "", // reset LLM API key
+    };
+
+    // we don't want the user to be able to modify these settings in SaaS
+    // and we should make sure they aren't included in the reset
+    if (shouldHandleSpecialSaasCase) {
+      delete newSettings.LLM_API_KEY;
+      delete newSettings.LLM_BASE_URL;
+      delete newSettings.LLM_MODEL;
+    }
+
+    saveSettings(newSettings, {
+      onSuccess: () => {
+        displaySuccessToast("Settings reset");
+        setResetSettingsModalIsOpen(false);
+        setLlmConfigMode(isAdvancedSettingsSet ? "advanced" : "basic");
       },
-      {
-        onSuccess: () => {
-          displaySuccessToast("Settings reset");
-          setResetSettingsModalIsOpen(false);
-          setLlmConfigMode(isAdvancedSettingsSet ? "advanced" : "basic");
-        },
-      },
-    );
+    });
   };
 
   React.useEffect(() => {
@@ -190,150 +214,162 @@ function AccountSettings() {
   return (
     <>
       <form
+        data-testid="account-settings-form"
         ref={formRef}
         action={onSubmit}
         className="flex flex-col grow overflow-auto"
       >
         <div className="flex flex-col gap-12 px-11 py-9">
-          <section className="flex flex-col gap-6">
-            <div className="flex items-center gap-7">
-              <h2 className="text-[28px] leading-8 tracking-[-0.02em] font-bold">
-                LLM Settings
-              </h2>
-              <SettingsSwitch
-                testId="advanced-settings-switch"
-                defaultIsToggled={isAdvancedSettingsSet}
-                onToggle={onToggleAdvancedMode}
-              >
-                Advanced
-              </SettingsSwitch>
-            </div>
+          {!shouldHandleSpecialSaasCase && (
+            <section
+              data-testid="llm-settings-section"
+              className="flex flex-col gap-6"
+            >
+              <div className="flex items-center gap-7">
+                <h2 className="text-[28px] leading-8 tracking-[-0.02em] font-bold">
+                  LLM Settings
+                </h2>
+                {!shouldHandleSpecialSaasCase && (
+                  <SettingsSwitch
+                    testId="advanced-settings-switch"
+                    defaultIsToggled={isAdvancedSettingsSet}
+                    onToggle={onToggleAdvancedMode}
+                  >
+                    Advanced
+                  </SettingsSwitch>
+                )}
+              </div>
 
-            {llmConfigMode === "basic" && (
-              <ModelSelector
-                models={modelsAndProviders}
-                currentModel={settings.LLM_MODEL}
-              />
-            )}
+              {llmConfigMode === "basic" && !shouldHandleSpecialSaasCase && (
+                <ModelSelector
+                  models={modelsAndProviders}
+                  currentModel={settings.LLM_MODEL}
+                />
+              )}
 
-            {llmConfigMode === "advanced" && (
-              <SettingsInput
-                testId="llm-custom-model-input"
-                name="llm-custom-model-input"
-                label="Custom Model"
-                defaultValue={settings.LLM_MODEL}
-                placeholder="anthropic/claude-3-5-sonnet-20241022"
-                type="text"
-                className="w-[680px]"
-              />
-            )}
-            {llmConfigMode === "advanced" && (
-              <SettingsInput
-                testId="base-url-input"
-                name="base-url-input"
-                label="Base URL"
-                defaultValue={settings.LLM_BASE_URL}
-                placeholder="https://api.openai.com"
-                type="text"
-                className="w-[680px]"
-              />
-            )}
+              {llmConfigMode === "advanced" && !shouldHandleSpecialSaasCase && (
+                <SettingsInput
+                  testId="llm-custom-model-input"
+                  name="llm-custom-model-input"
+                  label="Custom Model"
+                  defaultValue={settings.LLM_MODEL}
+                  placeholder="anthropic/claude-3-5-sonnet-20241022"
+                  type="text"
+                  className="w-[680px]"
+                />
+              )}
+              {llmConfigMode === "advanced" && !shouldHandleSpecialSaasCase && (
+                <SettingsInput
+                  testId="base-url-input"
+                  name="base-url-input"
+                  label="Base URL"
+                  defaultValue={settings.LLM_BASE_URL}
+                  placeholder="https://api.openai.com"
+                  type="text"
+                  className="w-[680px]"
+                />
+              )}
 
-            <SettingsInput
-              testId="llm-api-key-input"
-              name="llm-api-key-input"
-              label="API Key"
-              type="password"
-              className="w-[680px]"
-              startContent={
-                isLLMKeySet && <KeyStatusIcon isSet={isLLMKeySet} />
-              }
-              placeholder={isLLMKeySet ? "**********" : ""}
-            />
+              {!shouldHandleSpecialSaasCase && (
+                <SettingsInput
+                  testId="llm-api-key-input"
+                  name="llm-api-key-input"
+                  label="API Key"
+                  type="password"
+                  className="w-[680px]"
+                  startContent={
+                    isLLMKeySet && <KeyStatusIcon isSet={isLLMKeySet} />
+                  }
+                  placeholder={isLLMKeySet ? "**********" : ""}
+                />
+              )}
 
-            <HelpLink
-              testId="llm-api-key-help-anchor"
-              text="Don't know your API key?"
-              linkText="Click here for instructions"
-              href="https://docs.all-hands.dev/modules/usage/llms"
-            />
+              {!shouldHandleSpecialSaasCase && (
+                <HelpLink
+                  testId="llm-api-key-help-anchor"
+                  text="Don't know your API key?"
+                  linkText="Click here for instructions"
+                  href="https://docs.all-hands.dev/modules/usage/llms"
+                />
+              )}
 
-            {llmConfigMode === "advanced" && (
-              <SettingsDropdownInput
-                testId="agent-input"
-                name="agent-input"
-                label="Agent"
-                items={
-                  resources?.agents.map((agent) => ({
-                    key: agent,
-                    label: agent,
-                  })) || []
-                }
-                defaultSelectedKey={settings.AGENT}
-                isClearable={false}
-              />
-            )}
-
-            {isSaas && llmConfigMode === "advanced" && (
-              <SettingsDropdownInput
-                testId="runtime-settings-input"
-                name="runtime-settings-input"
-                label={
-                  <>
-                    Runtime Settings (
-                    <a href="mailto:contact@all-hands.dev">
-                      get in touch for access
-                    </a>
-                    )
-                  </>
-                }
-                items={REMOTE_RUNTIME_OPTIONS}
-                defaultSelectedKey={settings.REMOTE_RUNTIME_RESOURCE_FACTOR?.toString()}
-                isDisabled
-                isClearable={false}
-              />
-            )}
-
-            {llmConfigMode === "advanced" && (
-              <SettingsSwitch
-                testId="enable-confirmation-mode-switch"
-                onToggle={setConfirmationModeIsEnabled}
-                defaultIsToggled={!!settings.CONFIRMATION_MODE}
-                isBeta
-              >
-                Enable confirmation mode
-              </SettingsSwitch>
-            )}
-
-            {llmConfigMode === "advanced" && (
-              <SettingsSwitch
-                testId="enable-memory-condenser-switch"
-                name="enable-memory-condenser-switch"
-                defaultIsToggled={!!settings.ENABLE_DEFAULT_CONDENSER}
-              >
-                Enable memory condensation
-              </SettingsSwitch>
-            )}
-
-            {llmConfigMode === "advanced" && confirmationModeIsEnabled && (
-              <div>
+              {llmConfigMode === "advanced" && (
                 <SettingsDropdownInput
-                  testId="security-analyzer-input"
-                  name="security-analyzer-input"
-                  label="Security Analyzer"
+                  testId="agent-input"
+                  name="agent-input"
+                  label="Agent"
                   items={
-                    resources?.securityAnalyzers.map((analyzer) => ({
-                      key: analyzer,
-                      label: analyzer,
+                    resources?.agents.map((agent) => ({
+                      key: agent,
+                      label: agent,
                     })) || []
                   }
-                  defaultSelectedKey={settings.SECURITY_ANALYZER}
-                  isClearable
-                  showOptionalTag
+                  defaultSelectedKey={settings.AGENT}
+                  isClearable={false}
                 />
-              </div>
-            )}
-          </section>
+              )}
+
+              {isSaas && llmConfigMode === "advanced" && (
+                <SettingsDropdownInput
+                  testId="runtime-settings-input"
+                  name="runtime-settings-input"
+                  label={
+                    <>
+                      Runtime Settings (
+                      <a href="mailto:contact@all-hands.dev">
+                        get in touch for access
+                      </a>
+                      )
+                    </>
+                  }
+                  items={REMOTE_RUNTIME_OPTIONS}
+                  defaultSelectedKey={settings.REMOTE_RUNTIME_RESOURCE_FACTOR?.toString()}
+                  isDisabled
+                  isClearable={false}
+                />
+              )}
+
+              {llmConfigMode === "advanced" && (
+                <SettingsSwitch
+                  testId="enable-confirmation-mode-switch"
+                  onToggle={setConfirmationModeIsEnabled}
+                  defaultIsToggled={!!settings.CONFIRMATION_MODE}
+                  isBeta
+                >
+                  Enable confirmation mode
+                </SettingsSwitch>
+              )}
+
+              {llmConfigMode === "advanced" && (
+                <SettingsSwitch
+                  testId="enable-memory-condenser-switch"
+                  name="enable-memory-condenser-switch"
+                  defaultIsToggled={!!settings.ENABLE_DEFAULT_CONDENSER}
+                >
+                  Enable memory condensation
+                </SettingsSwitch>
+              )}
+
+              {llmConfigMode === "advanced" && confirmationModeIsEnabled && (
+                <div>
+                  <SettingsDropdownInput
+                    testId="security-analyzer-input"
+                    name="security-analyzer-input"
+                    label="Security Analyzer"
+                    items={
+                      resources?.securityAnalyzers.map((analyzer) => ({
+                        key: analyzer,
+                        label: analyzer,
+                      })) || []
+                    }
+                    defaultSelectedKey={settings.SECURITY_ANALYZER}
+                    isClearable
+                    showOptionalTag
+                  />
+                </div>
+              )}
+            </section>
+          )}
 
           <section className="flex flex-col gap-6">
             <h2 className="text-[28px] leading-8 tracking-[-0.02em] font-bold">

frontend/src/utils/feature-flags.ts

@@ -13,3 +13,4 @@ function loadFeatureFlag(
 }
 
 export const BILLING_SETTINGS = () => loadFeatureFlag("BILLING_SETTINGS");
+export const HIDE_LLM_SETTINGS = () => loadFeatureFlag("HIDE_LLM_SETTINGS");

openhands/server/routes/settings.py

@@ -83,6 +83,12 @@ async def store_settings(
                     existing_settings.user_consents_to_analytics
                 )
 
+            if settings.llm_model is None:
+                settings.llm_model = existing_settings.llm_model
+
+            if settings.llm_base_url is None:
+                settings.llm_base_url = existing_settings.llm_base_url
+
         response = JSONResponse(
             status_code=status.HTTP_200_OK,
             content={'message': 'Settings stored'},

tests/unit/test_settings_api.py

@@ -208,3 +208,60 @@ async def test_settings_unset_github_token(
     response = test_client.get('/api/settings')
     assert response.status_code == 200
     assert response.json()['github_token_is_set'] is False
+
+
+@pytest.mark.asyncio
+async def test_settings_preserve_llm_fields_when_none(test_client, mock_settings_store):
+    # Setup initial settings with LLM fields populated
+    initial_settings = Settings(
+        language='en',
+        agent='test-agent',
+        max_iterations=100,
+        security_analyzer='default',
+        confirmation_mode=True,
+        llm_model='existing-model',
+        llm_api_key=SecretStr('existing-key'),
+        llm_base_url='https://existing.com',
+    )
+
+    # Mock the settings store to return our initial settings
+    mock_settings_store.load.return_value = initial_settings
+
+    # Test data with None values for LLM fields
+    settings_update = {
+        'language': 'fr',  # Change something else to verify the update happens
+        'llm_model': None,
+        'llm_api_key': None,
+        'llm_base_url': None,
+    }
+
+    # Make the POST request to update settings
+    response = test_client.post('/api/settings', json=settings_update)
+    assert response.status_code == 200
+
+    # Verify that the settings were stored with preserved LLM values
+    stored_settings = mock_settings_store.store.call_args[0][0]
+
+    # Check that language was updated
+    assert stored_settings.language == 'fr'
+
+    # Check that LLM fields were preserved and not cleared
+    assert stored_settings.llm_model == 'existing-model'
+    assert isinstance(stored_settings.llm_api_key, SecretStr)
+    assert stored_settings.llm_api_key.get_secret_value() == 'existing-key'
+    assert stored_settings.llm_base_url == 'https://existing.com'
+
+    # Update the mock to return our new settings for the GET request
+    mock_settings_store.load.return_value = stored_settings
+
+    # Make a GET request to verify the updated settings
+    response = test_client.get('/api/settings')
+    assert response.status_code == 200
+    data = response.json()
+
+    # Verify fields in the response
+    assert data['language'] == 'fr'
+    assert data['llm_model'] == 'existing-model'
+    assert data['llm_base_url'] == 'https://existing.com'
+    # We expect the API key not to be included in the response
+    assert 'test-key' not in str(response.content)