From bf06b7e3f3c99e9ec4d675c64ff665147d5fda2c Mon Sep 17 00:00:00 2001 From: Hiep Le <69354317+hieptl@users.noreply.github.com> Date: Mon, 8 Dec 2025 23:03:59 +0700 Subject: [PATCH] fix(frontend): hide api key input field in advanced settings when provider is openhands (#11949) --- .../__tests__/routes/llm-settings.test.tsx | 205 ++++++++++++++++++ frontend/src/routes/llm-settings.tsx | 22 +- 2 files changed, 223 insertions(+), 4 deletions(-) diff --git a/frontend/__tests__/routes/llm-settings.test.tsx b/frontend/__tests__/routes/llm-settings.test.tsx index 378df48506..adfca6b40a 100644 --- a/frontend/__tests__/routes/llm-settings.test.tsx +++ b/frontend/__tests__/routes/llm-settings.test.tsx @@ -10,6 +10,7 @@ import { } from "#/mocks/handlers"; import * as AdvancedSettingsUtlls from "#/utils/has-advanced-settings-set"; import * as ToastHandlers from "#/utils/custom-toast-handlers"; +import OptionService from "#/api/option-service/option-service.api"; // Mock react-router hooks const mockUseSearchParams = vi.fn(); @@ -255,6 +256,210 @@ describe("Content", () => { }); it.todo("should render an indicator if the llm api key is set"); + + describe("API key visibility in Basic Settings", () => { + it("should hide API key input when SaaS mode is enabled and OpenHands provider is selected", async () => { + const getConfigSpy = vi.spyOn(OptionService, "getConfig"); + // @ts-expect-error - only return APP_MODE for these tests + getConfigSpy.mockResolvedValue({ + APP_MODE: "saas", + }); + + renderLlmSettingsScreen(); + await screen.findByTestId("llm-settings-screen"); + + const basicForm = screen.getByTestId("llm-settings-form-basic"); + const provider = within(basicForm).getByTestId("llm-provider-input"); + + // Verify OpenHands is selected by default + await waitFor(() => { + expect(provider).toHaveValue("OpenHands"); + }); + + // API key input should not be visible when OpenHands provider is selected in SaaS mode + expect( + within(basicForm).queryByTestId("llm-api-key-input"), + ).not.toBeInTheDocument(); + expect( + within(basicForm).queryByTestId("llm-api-key-help-anchor"), + ).not.toBeInTheDocument(); + }); + + it("should show API key input when SaaS mode is enabled and non-OpenHands provider is selected", async () => { + const getConfigSpy = vi.spyOn(OptionService, "getConfig"); + // @ts-expect-error - only return APP_MODE for these tests + getConfigSpy.mockResolvedValue({ + APP_MODE: "saas", + }); + + renderLlmSettingsScreen(); + await screen.findByTestId("llm-settings-screen"); + + const basicForm = screen.getByTestId("llm-settings-form-basic"); + const provider = within(basicForm).getByTestId("llm-provider-input"); + + // Select OpenAI provider + await userEvent.click(provider); + const providerOption = screen.getByText("OpenAI"); + await userEvent.click(providerOption); + + await waitFor(() => { + expect(provider).toHaveValue("OpenAI"); + }); + + // API key input should be visible when non-OpenHands provider is selected in SaaS mode + expect( + within(basicForm).getByTestId("llm-api-key-input"), + ).toBeInTheDocument(); + expect( + within(basicForm).getByTestId("llm-api-key-help-anchor"), + ).toBeInTheDocument(); + }); + + it("should show API key input when OSS mode is enabled and OpenHands provider is selected", async () => { + const getConfigSpy = vi.spyOn(OptionService, "getConfig"); + // @ts-expect-error - only return APP_MODE for these tests + getConfigSpy.mockResolvedValue({ + APP_MODE: "oss", + }); + + renderLlmSettingsScreen(); + await screen.findByTestId("llm-settings-screen"); + + const basicForm = screen.getByTestId("llm-settings-form-basic"); + const provider = within(basicForm).getByTestId("llm-provider-input"); + + // Verify OpenHands is selected by default + await waitFor(() => { + expect(provider).toHaveValue("OpenHands"); + }); + + // API key input should be visible when OSS mode is enabled (even with OpenHands provider) + expect( + within(basicForm).getByTestId("llm-api-key-input"), + ).toBeInTheDocument(); + expect( + within(basicForm).getByTestId("llm-api-key-help-anchor"), + ).toBeInTheDocument(); + }); + + it("should show API key input when OSS mode is enabled and non-OpenHands provider is selected", async () => { + const getConfigSpy = vi.spyOn(OptionService, "getConfig"); + // @ts-expect-error - only return APP_MODE for these tests + getConfigSpy.mockResolvedValue({ + APP_MODE: "oss", + }); + + renderLlmSettingsScreen(); + await screen.findByTestId("llm-settings-screen"); + + const basicForm = screen.getByTestId("llm-settings-form-basic"); + const provider = within(basicForm).getByTestId("llm-provider-input"); + + // Select OpenAI provider + await userEvent.click(provider); + const providerOption = screen.getByText("OpenAI"); + await userEvent.click(providerOption); + + await waitFor(() => { + expect(provider).toHaveValue("OpenAI"); + }); + + // API key input should be visible when OSS mode is enabled + expect( + within(basicForm).getByTestId("llm-api-key-input"), + ).toBeInTheDocument(); + expect( + within(basicForm).getByTestId("llm-api-key-help-anchor"), + ).toBeInTheDocument(); + }); + + it("should hide API key input when switching from non-OpenHands to OpenHands provider in SaaS mode", async () => { + const getConfigSpy = vi.spyOn(OptionService, "getConfig"); + // @ts-expect-error - only return APP_MODE for these tests + getConfigSpy.mockResolvedValue({ + APP_MODE: "saas", + }); + + renderLlmSettingsScreen(); + await screen.findByTestId("llm-settings-screen"); + + const basicForm = screen.getByTestId("llm-settings-form-basic"); + const provider = within(basicForm).getByTestId("llm-provider-input"); + + // Start with OpenAI provider + await userEvent.click(provider); + const openAIOption = screen.getByText("OpenAI"); + await userEvent.click(openAIOption); + + await waitFor(() => { + expect(provider).toHaveValue("OpenAI"); + }); + + // API key input should be visible with OpenAI + expect( + within(basicForm).getByTestId("llm-api-key-input"), + ).toBeInTheDocument(); + + // Switch to OpenHands provider + await userEvent.click(provider); + const openHandsOption = screen.getByText("OpenHands"); + await userEvent.click(openHandsOption); + + await waitFor(() => { + expect(provider).toHaveValue("OpenHands"); + }); + + // API key input should now be hidden + expect( + within(basicForm).queryByTestId("llm-api-key-input"), + ).not.toBeInTheDocument(); + expect( + within(basicForm).queryByTestId("llm-api-key-help-anchor"), + ).not.toBeInTheDocument(); + }); + + it("should show API key input when switching from OpenHands to non-OpenHands provider in SaaS mode", async () => { + const getConfigSpy = vi.spyOn(OptionService, "getConfig"); + // @ts-expect-error - only return APP_MODE for these tests + getConfigSpy.mockResolvedValue({ + APP_MODE: "saas", + }); + + renderLlmSettingsScreen(); + await screen.findByTestId("llm-settings-screen"); + + const basicForm = screen.getByTestId("llm-settings-form-basic"); + const provider = within(basicForm).getByTestId("llm-provider-input"); + + // Verify OpenHands is selected by default + await waitFor(() => { + expect(provider).toHaveValue("OpenHands"); + }); + + // API key input should be hidden with OpenHands + expect( + within(basicForm).queryByTestId("llm-api-key-input"), + ).not.toBeInTheDocument(); + + // Switch to OpenAI provider + await userEvent.click(provider); + const openAIOption = screen.getByText("OpenAI"); + await userEvent.click(openAIOption); + + await waitFor(() => { + expect(provider).toHaveValue("OpenAI"); + }); + + // API key input should now be visible + expect( + within(basicForm).getByTestId("llm-api-key-input"), + ).toBeInTheDocument(); + expect( + within(basicForm).getByTestId("llm-api-key-help-anchor"), + ).toBeInTheDocument(); + }); + }); }); describe("Form submission", () => { diff --git a/frontend/src/routes/llm-settings.tsx b/frontend/src/routes/llm-settings.tsx index 81c864fc25..056bf28c2c 100644 --- a/frontend/src/routes/llm-settings.tsx +++ b/frontend/src/routes/llm-settings.tsx @@ -112,11 +112,25 @@ function LlmSettingsScreen() { // Determine if we should hide the API key input and use OpenHands-managed key (when using OpenHands provider in SaaS mode) const currentModel = currentSelectedModel || settings?.LLM_MODEL; - const isOpenHandsProvider = - (view === "basic" && selectedProvider === "openhands") || - (view === "advanced" && currentModel?.startsWith("openhands/")); + const isSaasMode = config?.APP_MODE === "saas"; - const shouldUseOpenHandsKey = isOpenHandsProvider && isSaasMode; + + const isOpenHandsProvider = () => { + if (view === "basic") { + return selectedProvider === "openhands"; + } + + if (view === "advanced") { + if (dirtyInputs.model) { + return currentModel?.startsWith("openhands/"); + } + return settings?.LLM_MODEL?.startsWith("openhands/"); + } + + return false; + }; + + const shouldUseOpenHandsKey = isOpenHandsProvider() && isSaasMode; // Determine if we should hide the agent dropdown when V1 conversation API is enabled const isV1Enabled = settings?.V1_ENABLED;