From 3e25d9d6ec40a730e88fe49b06a554ba0ddfde68 Mon Sep 17 00:00:00 2001 From: openhands Date: Sat, 13 Jun 2026 02:00:45 +0000 Subject: [PATCH] Fix keyless active LLM profile recovery Detect local active LLM profiles whose API key is missing and guide users directly to the affected profile editor. Add unit and mock-LLM E2E coverage for the recovery path. Co-authored-by: openhands --- .../home/llm-not-configured-banner.test.tsx | 33 +++++ .../home/llm-not-configured-banner.tsx | 27 +++- .../llm-profiles/llm-settings-local-view.tsx | 22 ++- src/constants/llm-settings.ts | 11 ++ src/hooks/use-llm-configured.ts | 30 +++- src/i18n/translation.json | 34 +++++ src/routes/llm-settings.tsx | 9 +- .../mock-llm-profile-management.spec.ts | 133 +++++++++++++++++- 8 files changed, 288 insertions(+), 11 deletions(-) create mode 100644 src/constants/llm-settings.ts diff --git a/__tests__/components/home/llm-not-configured-banner.test.tsx b/__tests__/components/home/llm-not-configured-banner.test.tsx index 6258b03ad..b19f19538 100644 --- a/__tests__/components/home/llm-not-configured-banner.test.tsx +++ b/__tests__/components/home/llm-not-configured-banner.test.tsx @@ -149,6 +149,39 @@ describe("LlmNotConfiguredBanner", () => { ).not.toBeInTheDocument(); }); + it("shows a specific recovery message when the active profile has no API key", async () => { + vi.spyOn(SettingsService, "getSettings").mockResolvedValue( + buildSettings({ llm_api_key_set: false }), + ); + vi.spyOn(ProfilesService, "listProfiles").mockResolvedValue({ + profiles: [ + { + name: "active-profile", + model: "openai/gpt-4.1", + base_url: null, + api_key_set: false, + }, + ], + active_profile: "active-profile", + }); + const user = userEvent.setup(); + const { navigate } = renderBanner(); + + const banner = await screen.findByTestId("home-llm-not-configured-banner"); + expect(banner).toHaveTextContent( + "HOME$LLM_PROFILE_MISSING_API_KEY_MESSAGE", + ); + expect( + screen.getByTestId("home-llm-not-configured-action"), + ).toHaveTextContent("HOME$LLM_PROFILE_MISSING_API_KEY_ACTION"); + + await user.click(screen.getByTestId("home-llm-not-configured-action")); + + expect(navigate).toHaveBeenCalledWith( + "/settings/llm?profile=active-profile", + ); + }); + it("stays hidden for ACP agents, which own their LLM and need no key", async () => { // Arrange: ACP agent, no key — must not be nagged. vi.spyOn(SettingsService, "getSettings").mockResolvedValue( diff --git a/src/components/features/home/llm-not-configured-banner.tsx b/src/components/features/home/llm-not-configured-banner.tsx index 75634ab3a..0a27e6472 100644 --- a/src/components/features/home/llm-not-configured-banner.tsx +++ b/src/components/features/home/llm-not-configured-banner.tsx @@ -2,9 +2,13 @@ import { useTranslation } from "react-i18next"; import { FaTriangleExclamation } from "react-icons/fa6"; import { I18nKey } from "#/i18n/declaration"; import { useNavigation } from "#/context/navigation-context"; -import { useLlmConfigured } from "#/hooks/use-llm-configured"; +import { + LLM_CONFIGURATION_ISSUES, + useLlmConfigured, +} from "#/hooks/use-llm-configured"; import { BrandButton } from "#/components/features/settings/brand-button"; import { Typography } from "#/ui/typography"; +import { buildLlmSettingsRoute } from "#/constants/llm-settings"; /** * Warns the user on the home screen when the active agent has no usable LLM — @@ -19,12 +23,25 @@ import { Typography } from "#/ui/typography"; export function LlmNotConfiguredBanner() { const { t } = useTranslation("openhands"); const { navigate } = useNavigation(); - const { isConfigured, isLoading } = useLlmConfigured(); + const { isConfigured, isLoading, issue, activeProfileName } = + useLlmConfigured(); if (isLoading || isConfigured) { return null; } + const isActiveProfileMissingApiKey = + issue === LLM_CONFIGURATION_ISSUES.ACTIVE_PROFILE_MISSING_API_KEY; + const messageKey = isActiveProfileMissingApiKey + ? I18nKey.HOME$LLM_PROFILE_MISSING_API_KEY_MESSAGE + : I18nKey.HOME$LLM_NOT_CONFIGURED_MESSAGE; + const actionKey = isActiveProfileMissingApiKey + ? I18nKey.HOME$LLM_PROFILE_MISSING_API_KEY_ACTION + : I18nKey.HOME$LLM_NOT_CONFIGURED_ACTION; + const route = buildLlmSettingsRoute( + isActiveProfileMissingApiKey ? activeProfileName : null, + ); + return (
- {t(I18nKey.HOME$LLM_NOT_CONFIGURED_MESSAGE)} + {t(messageKey, { name: activeProfileName })} @@ -44,9 +61,9 @@ export function LlmNotConfiguredBanner() { testId="home-llm-not-configured-action" type="button" variant="primary" - onClick={() => navigate("/settings/llm")} + onClick={() => navigate(route)} > - {t(I18nKey.HOME$LLM_NOT_CONFIGURED_ACTION)} + {t(actionKey)} ); diff --git a/src/components/features/settings/llm-profiles/llm-settings-local-view.tsx b/src/components/features/settings/llm-profiles/llm-settings-local-view.tsx index caeaa3872..09b41ecdf 100644 --- a/src/components/features/settings/llm-profiles/llm-settings-local-view.tsx +++ b/src/components/features/settings/llm-profiles/llm-settings-local-view.tsx @@ -83,7 +83,13 @@ export function shouldReapplyProfileAfterSave({ * See PR review feedback for details. */ -export function LlmSettingsLocalView() { +interface LlmSettingsLocalViewProps { + editProfileName?: string | null; +} + +export function LlmSettingsLocalView({ + editProfileName = null, +}: LlmSettingsLocalViewProps = {}) { const { t } = useTranslation("openhands"); const { setHideSectionHeader } = useSettingsSectionHeader(); const saveProfile = useSaveLlmProfile(); @@ -112,6 +118,7 @@ export function LlmSettingsLocalView() { null, ); const [isSaving, setIsSaving] = useState(false); + const requestedEditProfileNameRef = useRef(null); useEffect(() => { setHideSectionHeader(viewMode !== "list"); @@ -206,6 +213,19 @@ export function LlmSettingsLocalView() { [t], ); + useEffect(() => { + if (!editProfileName) return; + if (requestedEditProfileNameRef.current === editProfileName) return; + + const profile = profilesData?.profiles.find( + (candidate) => candidate.name === editProfileName, + ); + if (!profile) return; + + requestedEditProfileNameRef.current = editProfileName; + void handleEditProfile(profile); + }, [editProfileName, handleEditProfile, profilesData?.profiles]); + const handleBackToList = useCallback(() => { setViewMode("list"); setEditingProfile(null); diff --git a/src/constants/llm-settings.ts b/src/constants/llm-settings.ts new file mode 100644 index 000000000..b0a3b97ef --- /dev/null +++ b/src/constants/llm-settings.ts @@ -0,0 +1,11 @@ +export const LLM_SETTINGS_ROUTE = "/settings/llm"; +export const LLM_SETTINGS_EDIT_PROFILE_QUERY_PARAM = "profile"; + +export function buildLlmSettingsRoute(profileName?: string | null) { + if (!profileName) return LLM_SETTINGS_ROUTE; + + const params = new URLSearchParams({ + [LLM_SETTINGS_EDIT_PROFILE_QUERY_PARAM]: profileName, + }); + return `${LLM_SETTINGS_ROUTE}?${params.toString()}`; +} diff --git a/src/hooks/use-llm-configured.ts b/src/hooks/use-llm-configured.ts index 7ece4dea0..03dbeef57 100644 --- a/src/hooks/use-llm-configured.ts +++ b/src/hooks/use-llm-configured.ts @@ -3,6 +3,15 @@ import { useConfig } from "#/hooks/query/use-config"; import { useLlmProfiles } from "#/hooks/query/use-llm-profiles"; import { useActiveBackend } from "#/contexts/active-backend-context"; import { isSettingsPageHidden } from "#/utils/settings-utils"; +import { LLM_SETTINGS_ROUTE } from "#/constants/llm-settings"; + +export const LLM_CONFIGURATION_ISSUES = { + NOT_CONFIGURED: "not-configured", + ACTIVE_PROFILE_MISSING_API_KEY: "active-profile-missing-api-key", +} as const; + +type LlmConfigurationIssue = + (typeof LLM_CONFIGURATION_ISSUES)[keyof typeof LLM_CONFIGURATION_ISSUES]; interface LlmConfiguredResult { /** @@ -21,6 +30,13 @@ interface LlmConfiguredResult { * warning doesn't flash before data loads or on a transient network error. */ isLoading: boolean; + /** + * Specific recovery issue for the unconfigured state. Used by the UI to + * distinguish a fresh setup gap from an active saved profile whose key is no + * longer usable. + */ + issue: LlmConfigurationIssue | null; + activeProfileName: string | null; } /** @@ -53,8 +69,10 @@ export function useLlmConfigured(): LlmConfiguredResult { (profile) => profile.name === profilesData.active_profile, ); const hasActiveProfileApiKey = activeProfile?.api_key_set === true; + const hasActiveProfileMissingApiKey = + isLocal && Boolean(activeProfile) && activeProfile?.api_key_set === false; const llmSettingsHidden = isSettingsPageHidden( - "/settings/llm", + LLM_SETTINGS_ROUTE, config?.feature_flags, ); @@ -76,10 +94,18 @@ export function useLlmConfigured(): LlmConfiguredResult { const configIndeterminate = configLoading || (configError && !config); const profilesIndeterminate = profilesLoading || (profilesError && !profilesData); + const isConfigured = isAcpAgent || llmSettingsHidden || hasUsableLlm; + const issue = isConfigured + ? null + : hasActiveProfileMissingApiKey + ? LLM_CONFIGURATION_ISSUES.ACTIVE_PROFILE_MISSING_API_KEY + : LLM_CONFIGURATION_ISSUES.NOT_CONFIGURED; return { - isConfigured: isAcpAgent || llmSettingsHidden || hasUsableLlm, + isConfigured, isLoading: settingsIndeterminate || configIndeterminate || profilesIndeterminate, + issue, + activeProfileName: activeProfile?.name ?? null, }; } diff --git a/src/i18n/translation.json b/src/i18n/translation.json index e8ebc2d9d..eb877abee 100644 --- a/src/i18n/translation.json +++ b/src/i18n/translation.json @@ -866,6 +866,40 @@ "uk": "Налаштувати LLM", "ca": "Configura el LLM" }, + "HOME$LLM_PROFILE_MISSING_API_KEY_MESSAGE": { + "en": "Your active LLM profile \"{{name}}\" appears to be missing its API key. Edit the profile and re-enter the key before starting a conversation.", + "ja": "有効な LLM プロファイル「{{name}}」の API キーが見つからないようです。会話を開始する前にプロファイルを編集してキーを再入力してください。", + "zh-CN": "您的活动 LLM 配置文件“{{name}}”似乎缺少 API 密钥。请先编辑该配置文件并重新输入密钥，然后再开始对话。", + "zh-TW": "您的作用中 LLM 設定檔「{{name}}」似乎缺少 API 金鑰。請先編輯該設定檔並重新輸入金鑰，再開始對話。", + "ko-KR": "활성 LLM 프로필 \"{{name}}\"에 API 키가 없는 것 같습니다. 대화를 시작하기 전에 프로필을 편집하고 키를 다시 입력하세요.", + "no": "Den aktive LLM-profilen «{{name}}» ser ut til å mangle API-nøkkelen. Rediger profilen og skriv inn nøkkelen på nytt før du starter en samtale.", + "it": "Il profilo LLM attivo \"{{name}}\" sembra non avere la chiave API. Modifica il profilo e reinserisci la chiave prima di avviare una conversazione.", + "pt": "O perfil LLM ativo \"{{name}}\" parece estar sem a chave de API. Edite o perfil e insira a chave novamente antes de iniciar uma conversa.", + "es": "El perfil LLM activo \"{{name}}\" parece no tener su clave de API. Edita el perfil y vuelve a introducir la clave antes de iniciar una conversación.", + "ar": "يبدو أن ملف تعريف LLM النشط \"{{name}}\" يفتقد مفتاح API. عدّل الملف وأعد إدخال المفتاح قبل بدء محادثة.", + "fr": "Le profil LLM actif « {{name}} » semble ne plus avoir de clé API. Modifiez le profil et saisissez à nouveau la clé avant de démarrer une conversation.", + "tr": "Etkin LLM profili \"{{name}}\" API anahtarını kaybetmiş görünüyor. Bir konuşma başlatmadan önce profili düzenleyip anahtarı yeniden girin.", + "de": "Beim aktiven LLM-Profil „{{name}}“ scheint der API-Schlüssel zu fehlen. Bearbeite das Profil und gib den Schlüssel erneut ein, bevor du eine Konversation startest.", + "uk": "Схоже, в активному профілі LLM «{{name}}» відсутній API-ключ. Відредагуйте профіль і повторно введіть ключ, перш ніж починати розмову.", + "ca": "Sembla que al perfil LLM actiu \"{{name}}\" li falta la clau d'API. Edita el perfil i torna a introduir la clau abans d'iniciar una conversa." + }, + "HOME$LLM_PROFILE_MISSING_API_KEY_ACTION": { + "en": "Edit profile", + "ja": "プロファイルを編集", + "zh-CN": "编辑配置文件", + "zh-TW": "編輯設定檔", + "ko-KR": "프로필 편집", + "no": "Rediger profil", + "it": "Modifica profilo", + "pt": "Editar perfil", + "es": "Editar perfil", + "ar": "تعديل الملف التعريفي", + "fr": "Modifier le profil", + "tr": "Profili düzenle", + "de": "Profil bearbeiten", + "uk": "Редагувати профіль", + "ca": "Edita el perfil" + }, "HOME$READ_THIS": { "en": "Read this", "ja": "詳細はこちら", diff --git a/src/routes/llm-settings.tsx b/src/routes/llm-settings.tsx index 37b96582e..48ced60ed 100644 --- a/src/routes/llm-settings.tsx +++ b/src/routes/llm-settings.tsx @@ -1,4 +1,5 @@ import React from "react"; +import { useSearchParams } from "react-router"; import { useTranslation } from "react-i18next"; import { ModelSelector } from "#/components/shared/modals/settings/model-selector"; import { useAgentSettingsSchema } from "#/hooks/query/use-agent-settings-schema"; @@ -32,6 +33,7 @@ import { OPENAI_SUBSCRIPTION_VENDOR, resolveLlmAuthType, } from "#/constants/llm-subscription"; +import { LLM_SETTINGS_EDIT_PROFILE_QUERY_PARAM } from "#/constants/llm-settings"; import { useOpenAISubscriptionModels } from "#/hooks/query/use-llm-subscription-models"; import { OPENHANDS_LLM_PROXY_BASE_URL } from "#/utils/openhands-llm"; @@ -523,6 +525,7 @@ export function LlmSettingsScreen({ */ export default function LlmSettingsRoute() { const { backend } = useActiveBackend(); + const [searchParams] = useSearchParams(); const isCloud = backend.kind === "cloud"; // Cloud backends use the standard LLM settings form (no profiles support) @@ -531,5 +534,9 @@ export default function LlmSettingsRoute() { } // Local backends use the profile management view - return ; + return ( + + ); } diff --git a/tests/e2e/mock-llm/mock-llm-profile-management.spec.ts b/tests/e2e/mock-llm/mock-llm-profile-management.spec.ts index e18e7fe20..93c6f8c43 100644 --- a/tests/e2e/mock-llm/mock-llm-profile-management.spec.ts +++ b/tests/e2e/mock-llm/mock-llm-profile-management.spec.ts @@ -1,7 +1,7 @@ /** * Mock-LLM E2E tests: LLM profile management regressions. * - * Covers two scenarios that previously had no end-to-end guard: + * Covers LLM profile scenarios that previously had no end-to-end guard: * * 1. Active profile deletion + reconciliation: * The active LLM profile IS deletable (the PR #1127 disable-guard was @@ -16,9 +16,16 @@ * user selected, not the first alphabetical match. The fix * stamps the active profile name on client-side conversation * metadata at creation and on per-conversation switches. + * + * 3. Active profile missing API key (issue #1344): + * When the active profile exists but has `api_key_set: false`, the home + * page should explain that the selected profile needs its key re-entered + * and link directly to the profile editor instead of showing generic setup + * copy. + */ -import { test, expect } from "@playwright/test"; +import { test, expect, type APIRequestContext } from "@playwright/test"; import { seedLocalStorage, routeSessionApiKey, @@ -35,10 +42,62 @@ import { createProfileViaUI, deleteProfileIfExists, activateProfileViaUI, + BACKEND_URL, + SESSION_API_KEY, + MOCK_LLM_AGENT_URL, } from "./utils/mock-llm-helpers"; const MOCK_MODEL = "openai/mock-test-model"; +async function deleteProfileViaAPI(request: APIRequestContext, name: string) { + await request.delete( + `${BACKEND_URL}/api/profiles/${encodeURIComponent(name)}`, + { + headers: { "X-Session-API-Key": SESSION_API_KEY }, + }, + ); +} + +async function saveProfileViaAPI( + request: APIRequestContext, + { name, apiKey }: { name: string; apiKey?: string }, +) { + const llm: Record = { + model: MOCK_MODEL, + base_url: MOCK_LLM_AGENT_URL, + }; + if (apiKey) { + llm.api_key = apiKey; + } + + const response = await request.post( + `${BACKEND_URL}/api/profiles/${encodeURIComponent(name)}`, + { + headers: { + "X-Session-API-Key": SESSION_API_KEY, + "Content-Type": "application/json", + }, + data: { + llm, + include_secrets: true, + }, + }, + ); + expect(response.ok(), `Save profile ${name}: ${response.status()}`).toBe( + true, + ); +} + +async function activateProfileViaAPI(request: APIRequestContext, name: string) { + const response = await request.post( + `${BACKEND_URL}/api/profiles/${encodeURIComponent(name)}/activate`, + { headers: { "X-Session-API-Key": SESSION_API_KEY } }, + ); + expect(response.ok(), `Activate profile ${name}: ${response.status()}`).toBe( + true, + ); +} + test.describe.configure({ mode: "serial" }); // ═══════════════════════════════════════════════════════════════════════ @@ -328,3 +387,73 @@ test.describe("same-model profile identity", () => { }); }); }); + +// ═══════════════════════════════════════════════════════════════════════ +// Test 3 — Active profile missing API key recovery (issue #1344) +// ═══════════════════════════════════════════════════════════════════════ + +test.describe("active profile missing API key recovery", () => { + const MISSING_KEY_PROFILE = "missing-key-recovery"; + const RESTORED_PROFILE = "mock-llm"; + + test.beforeEach(async ({ page }) => { + await seedLocalStorage(page); + }); + + test.afterAll(async ({ request }) => { + try { + await saveProfileViaAPI(request, { + name: RESTORED_PROFILE, + apiKey: "mock-api-key-for-testing", + }); + await activateProfileViaAPI(request, RESTORED_PROFILE); + await deleteProfileViaAPI(request, MISSING_KEY_PROFILE); + } catch { + // best-effort cleanup + } + }); + + test("home page explains and links directly to a keyless active profile", async ({ + page, + request, + }) => { + await deleteProfileViaAPI(request, MISSING_KEY_PROFILE); + await saveProfileViaAPI(request, { name: MISSING_KEY_PROFILE }); + await activateProfileViaAPI(request, MISSING_KEY_PROFILE); + + const profilesResponse = await request.get(`${BACKEND_URL}/api/profiles`, { + headers: { "X-Session-API-Key": SESSION_API_KEY }, + }); + expect( + profilesResponse.ok(), + `List profiles: ${profilesResponse.status()}`, + ).toBe(true); + const profilesBody = await profilesResponse.json(); + const activeProfile = profilesBody.profiles.find( + (profile: { name: string }) => profile.name === MISSING_KEY_PROFILE, + ); + expect(profilesBody.active_profile).toBe(MISSING_KEY_PROFILE); + expect(activeProfile?.api_key_set).toBe(false); + + await routeSessionApiKey(page); + await page.goto("/", { waitUntil: "domcontentloaded" }); + await dismissAnalyticsModal(page); + await waitForTestId(page, "home-chat-launcher"); + + const banner = page.getByTestId("home-llm-not-configured-banner"); + await expect(banner).toBeVisible({ timeout: 10_000 }); + await expect(banner).toContainText(MISSING_KEY_PROFILE); + await expect(banner).toContainText(/API key/i); + await expect(page.getByTestId("submit-button")).toBeDisabled(); + + await page.getByTestId("home-llm-not-configured-action").click(); + await expect(page).toHaveURL( + new RegExp(`/settings/llm\\?profile=${MISSING_KEY_PROFILE}$`), + { timeout: 10_000 }, + ); + await waitForTestId(page, "profile-editor-title"); + await expect(page.getByTestId("profile-name-input")).toHaveValue( + MISSING_KEY_PROFILE, + ); + }); +});