From fa067edbac7c458d7f0b69dfeb0c4915a0c08ea2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20B=C3=A4uerle?= Date: Mon, 29 Apr 2024 13:18:00 -0700 Subject: [PATCH] feat: set API key from settings modal (#1319) * feat: set API key from settings modal * feat: init with api key * test * fix * fixes * fix api key reference * test * minor fixes * fix settings update * combine settings call --------- Co-authored-by: Robert Brennan Co-authored-by: sp.wack <83104063+amanape@users.noreply.github.com> --- .../components/file-explorer/FileExplorer.tsx | 2 +- .../modals/settings/SettingsForm.test.tsx | 24 +++++++++++-- .../modals/settings/SettingsForm.tsx | 34 +++++++++++++++++- .../modals/settings/SettingsModal.test.tsx | 11 +++--- .../modals/settings/SettingsModal.tsx | 36 +++++++++++++------ frontend/src/i18n/translation.json | 4 +++ frontend/src/services/agent.test.ts | 13 +++---- frontend/src/services/settings.ts | 6 ++++ frontend/src/types/ConfigType.tsx | 2 ++ opendevin/server/agent/agent.py | 4 +-- 10 files changed, 109 insertions(+), 27 deletions(-) diff --git a/frontend/src/components/file-explorer/FileExplorer.tsx b/frontend/src/components/file-explorer/FileExplorer.tsx index f66f1c72a9..89bc6c716f 100644 --- a/frontend/src/components/file-explorer/FileExplorer.tsx +++ b/frontend/src/components/file-explorer/FileExplorer.tsx @@ -1,4 +1,3 @@ -import { WorkspaceFile, getWorkspace } from "#/services/fileService"; import React from "react"; import { IoIosArrowBack, @@ -6,6 +5,7 @@ import { IoIosRefresh, } from "react-icons/io"; import { twMerge } from "tailwind-merge"; +import { WorkspaceFile, getWorkspace } from "#/services/fileService"; import IconButton from "../IconButton"; import ExplorerTree from "./ExplorerTree"; import { removeEmptyNodes } from "./utils"; diff --git a/frontend/src/components/modals/settings/SettingsForm.test.tsx b/frontend/src/components/modals/settings/SettingsForm.test.tsx index 023571a78f..35c0a4c011 100644 --- a/frontend/src/components/modals/settings/SettingsForm.test.tsx +++ b/frontend/src/components/modals/settings/SettingsForm.test.tsx @@ -1,14 +1,15 @@ +import { Settings } from "#/services/settings"; +import AgentTaskState from "#/types/AgentTaskState"; import { act, screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import React from "react"; import { renderWithProviders } from "test-utils"; -import AgentTaskState from "#/types/AgentTaskState"; import SettingsForm from "./SettingsForm"; -import { Settings } from "#/services/settings"; const onModelChangeMock = vi.fn(); const onAgentChangeMock = vi.fn(); const onLanguageChangeMock = vi.fn(); +const onAPIKeyChangeMock = vi.fn(); const renderSettingsForm = (settings?: Settings) => { renderWithProviders( @@ -18,6 +19,7 @@ const renderSettingsForm = (settings?: Settings) => { LLM_MODEL: "model1", AGENT: "agent1", LANGUAGE: "en", + LLM_API_KEY: "sk-...", } } models={["model1", "model2", "model3"]} @@ -25,6 +27,7 @@ const renderSettingsForm = (settings?: Settings) => { onModelChange={onModelChangeMock} onAgentChange={onAgentChangeMock} onLanguageChange={onLanguageChangeMock} + onAPIKeyChange={onAPIKeyChangeMock} />, ); }; @@ -36,10 +39,12 @@ describe("SettingsForm", () => { const modelInput = screen.getByRole("combobox", { name: "model" }); const agentInput = screen.getByRole("combobox", { name: "agent" }); const languageInput = screen.getByRole("combobox", { name: "language" }); + const apiKeyInput = screen.getByTestId("apikey"); expect(modelInput).toHaveValue("model1"); expect(agentInput).toHaveValue("agent1"); expect(languageInput).toHaveValue("English"); + expect(apiKeyInput).toHaveValue("sk-..."); }); it("should display the existing values if it they are present", () => { @@ -47,6 +52,7 @@ describe("SettingsForm", () => { LLM_MODEL: "model2", AGENT: "agent2", LANGUAGE: "es", + LLM_API_KEY: "sk-...", }); const modelInput = screen.getByRole("combobox", { name: "model" }); @@ -65,12 +71,14 @@ describe("SettingsForm", () => { LLM_MODEL: "model1", AGENT: "agent1", LANGUAGE: "en", + LLM_API_KEY: "sk-...", }} models={["model1", "model2", "model3"]} agents={["agent1", "agent2", "agent3"]} onModelChange={onModelChangeMock} onAgentChange={onAgentChangeMock} onLanguageChange={onLanguageChangeMock} + onAPIKeyChange={onAPIKeyChangeMock} />, { preloadedState: { agent: { curTaskState: AgentTaskState.RUNNING } } }, ); @@ -98,6 +106,7 @@ describe("SettingsForm", () => { }); expect(onModelChangeMock).toHaveBeenCalledWith("model3"); + expect(onAPIKeyChangeMock).toHaveBeenCalledWith(""); }); it("should call the onAgentChange handler when the agent changes", () => { @@ -131,5 +140,16 @@ describe("SettingsForm", () => { expect(onLanguageChangeMock).toHaveBeenCalledWith("Français"); }); + + it("should call the onAPIKeyChange handler when the API key changes", () => { + renderSettingsForm(); + + const apiKeyInput = screen.getByTestId("apikey"); + act(() => { + userEvent.type(apiKeyInput, "x"); + }); + + expect(onAPIKeyChangeMock).toHaveBeenCalledWith("sk-...x"); + }); }); }); diff --git a/frontend/src/components/modals/settings/SettingsForm.tsx b/frontend/src/components/modals/settings/SettingsForm.tsx index caa2d34cd1..21042fbc22 100644 --- a/frontend/src/components/modals/settings/SettingsForm.tsx +++ b/frontend/src/components/modals/settings/SettingsForm.tsx @@ -1,5 +1,7 @@ +import { Input, useDisclosure } from "@nextui-org/react"; import React, { useEffect } from "react"; import { useTranslation } from "react-i18next"; +import { FaEye, FaEyeSlash } from "react-icons/fa"; import { useSelector } from "react-redux"; import { AvailableLanguages } from "../../../i18n"; import { I18nKey } from "../../../i18n/declaration"; @@ -14,6 +16,7 @@ interface SettingsFormProps { agents: string[]; onModelChange: (model: string) => void; + onAPIKeyChange: (apiKey: string) => void; onAgentChange: (agent: string) => void; onLanguageChange: (language: string) => void; } @@ -23,12 +26,14 @@ function SettingsForm({ models, agents, onModelChange, + onAPIKeyChange, onAgentChange, onLanguageChange, }: SettingsFormProps) { const { t } = useTranslation(); const { curTaskState } = useSelector((state: RootState) => state.agent); const [disabled, setDisabled] = React.useState(false); + const { isOpen: isVisible, onOpenChange: onVisibleChange } = useDisclosure(); useEffect(() => { if ( @@ -47,11 +52,38 @@ function SettingsForm({ ariaLabel="model" items={models.map((model) => ({ value: model, label: model }))} defaultKey={settings.LLM_MODEL || models[0]} - onChange={onModelChange} + onChange={(e) => { + onModelChange(e); + }} tooltip={t(I18nKey.SETTINGS$MODEL_TOOLTIP)} allowCustomValue // user can type in a custom LLM model that is not in the list disabled={disabled} /> + { + onAPIKeyChange(e.target.value); + }} + endContent={ + + } + /> ({ value: agent, label: agent }))} diff --git a/frontend/src/components/modals/settings/SettingsModal.test.tsx b/frontend/src/components/modals/settings/SettingsModal.test.tsx index a37502cefe..8530f185f0 100644 --- a/frontend/src/components/modals/settings/SettingsModal.test.tsx +++ b/frontend/src/components/modals/settings/SettingsModal.test.tsx @@ -1,14 +1,14 @@ +import { fetchAgents, fetchModels } from "#/api"; +import { initializeAgent } from "#/services/agent"; +import { Settings, getSettings, saveSettings } from "#/services/settings"; +import toast from "#/utils/toast"; import { act, screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; +import i18next from "i18next"; import React from "react"; import { renderWithProviders } from "test-utils"; import { Mock } from "vitest"; -import i18next from "i18next"; import SettingsModal from "./SettingsModal"; -import { Settings, getSettings, saveSettings } from "#/services/settings"; -import { initializeAgent } from "#/services/agent"; -import toast from "#/utils/toast"; -import { fetchAgents, fetchModels } from "#/api"; const toastSpy = vi.spyOn(toast, "settingsChanged"); const i18nSpy = vi.spyOn(i18next, "changeLanguage"); @@ -98,6 +98,7 @@ describe("SettingsModal", () => { LLM_MODEL: "gpt-3.5-turbo", AGENT: "MonologueAgent", LANGUAGE: "en", + LLM_API_KEY: "sk-...", }; it("should save the settings", async () => { diff --git a/frontend/src/components/modals/settings/SettingsModal.tsx b/frontend/src/components/modals/settings/SettingsModal.tsx index 62456ef217..58e1660a28 100644 --- a/frontend/src/components/modals/settings/SettingsModal.tsx +++ b/frontend/src/components/modals/settings/SettingsModal.tsx @@ -1,20 +1,20 @@ -import { Spinner } from "@nextui-org/react"; -import React from "react"; -import { useTranslation } from "react-i18next"; -import i18next from "i18next"; +import { fetchAgents, fetchModels } from "#/api"; import { AvailableLanguages } from "#/i18n"; import { I18nKey } from "#/i18n/declaration"; -import { fetchAgents, fetchModels } from "#/api"; -import BaseModal from "../base-modal/BaseModal"; -import SettingsForm from "./SettingsForm"; +import { initializeAgent } from "#/services/agent"; import { Settings, - saveSettings, getSettings, getSettingsDifference, + saveSettings, } from "#/services/settings"; import toast from "#/utils/toast"; -import { initializeAgent } from "#/services/agent"; +import { Spinner } from "@nextui-org/react"; +import i18next from "i18next"; +import React from "react"; +import { useTranslation } from "react-i18next"; +import BaseModal from "../base-modal/BaseModal"; +import SettingsForm from "./SettingsForm"; interface SettingsProps { isOpen: boolean; @@ -45,7 +45,13 @@ function SettingsModal({ isOpen, onOpenChange }: SettingsProps) { }, []); const handleModelChange = (model: string) => { - setSettings((prev) => ({ ...prev, LLM_MODEL: model })); + // Needs to also reset the API key. + const key = localStorage.getItem(`API_KEY_${model}`); + setSettings((prev) => ({ + ...prev, + LLM_MODEL: model, + LLM_API_KEY: key || "", + })); }; const handleAgentChange = (agent: string) => { @@ -60,6 +66,10 @@ function SettingsModal({ isOpen, onOpenChange }: SettingsProps) { if (key) setSettings((prev) => ({ ...prev, LANGUAGE: key })); }; + const handleAPIKeyChange = (key: string) => { + setSettings((prev) => ({ ...prev, LLM_API_KEY: key })); + }; + const handleSaveSettings = () => { const updatedSettings = getSettingsDifference(settings); saveSettings(settings); @@ -69,6 +79,11 @@ function SettingsModal({ isOpen, onOpenChange }: SettingsProps) { Object.entries(updatedSettings).forEach(([key, value]) => { toast.settingsChanged(`${key} set to "${value}"`); }); + + localStorage.setItem( + `API_KEY_${settings.LLM_MODEL || models[0]}`, + settings.LLM_API_KEY, + ); }; return ( @@ -106,6 +121,7 @@ function SettingsModal({ isOpen, onOpenChange }: SettingsProps) { onModelChange={handleModelChange} onAgentChange={handleAgentChange} onLanguageChange={handleLanguageChange} + onAPIKeyChange={handleAPIKeyChange} /> )} diff --git a/frontend/src/i18n/translation.json b/frontend/src/i18n/translation.json index 6967d26aa2..953c14d8cf 100644 --- a/frontend/src/i18n/translation.json +++ b/frontend/src/i18n/translation.json @@ -324,5 +324,9 @@ "SETTINGS$DISABLED_RUNNING": { "en": "Cannot be changed while the agent is running.", "de": "Kann nicht geändert werden während ein Task ausgeführt wird." + }, + "SETTINGS$API_KEY_PLACEHOLDER": { + "en": "Enter your API key.", + "de": "Model API key." } } diff --git a/frontend/src/services/agent.test.ts b/frontend/src/services/agent.test.ts index 39afa86fef..3e86dff12b 100644 --- a/frontend/src/services/agent.test.ts +++ b/frontend/src/services/agent.test.ts @@ -1,11 +1,11 @@ -import { describe, it, expect, vi } from "vitest"; +import { describe, expect, it, vi } from "vitest"; -import ActionType from "#/types/ActionType"; -import { Settings } from "./settings"; -import { initializeAgent } from "./agent"; -import Socket from "./socket"; -import store from "#/store"; import { setInitialized } from "#/state/taskSlice"; +import store from "#/store"; +import ActionType from "#/types/ActionType"; +import { initializeAgent } from "./agent"; +import { Settings } from "./settings"; +import Socket from "./socket"; const sendSpy = vi.spyOn(Socket, "send"); const dispatchSpy = vi.spyOn(store, "dispatch"); @@ -16,6 +16,7 @@ describe("initializeAgent", () => { LLM_MODEL: "llm_value", AGENT: "agent_value", LANGUAGE: "language_value", + LLM_API_KEY: "sk-...", }; const event = { diff --git a/frontend/src/services/settings.ts b/frontend/src/services/settings.ts index 651e2defde..29129b13f3 100644 --- a/frontend/src/services/settings.ts +++ b/frontend/src/services/settings.ts @@ -2,12 +2,14 @@ export type Settings = { LLM_MODEL: string; AGENT: string; LANGUAGE: string; + LLM_API_KEY: string; }; export const DEFAULT_SETTINGS: Settings = { LLM_MODEL: "gpt-3.5-turbo", AGENT: "MonologueAgent", LANGUAGE: "en", + LLM_API_KEY: "", }; const validKeys = Object.keys(DEFAULT_SETTINGS) as (keyof Settings)[]; @@ -19,6 +21,10 @@ export const getSettings = (): Settings => ({ LLM_MODEL: localStorage.getItem("LLM_MODEL") || DEFAULT_SETTINGS.LLM_MODEL, AGENT: localStorage.getItem("AGENT") || DEFAULT_SETTINGS.AGENT, LANGUAGE: localStorage.getItem("LANGUAGE") || DEFAULT_SETTINGS.LANGUAGE, + LLM_API_KEY: + localStorage.getItem( + `API_KEY_${localStorage.getItem("LLM_MODEL") || DEFAULT_SETTINGS.LLM_MODEL}`, + ) || DEFAULT_SETTINGS.LLM_API_KEY, }); /** diff --git a/frontend/src/types/ConfigType.tsx b/frontend/src/types/ConfigType.tsx index d72bd7cf55..526558a028 100644 --- a/frontend/src/types/ConfigType.tsx +++ b/frontend/src/types/ConfigType.tsx @@ -2,12 +2,14 @@ enum ArgConfigType { LLM_MODEL = "LLM_MODEL", AGENT = "AGENT", LANGUAGE = "LANGUAGE", + LLM_API_KEY = "LLM_API_KEY", } const SupportedSettings: string[] = [ ArgConfigType.LLM_MODEL, ArgConfigType.AGENT, ArgConfigType.LANGUAGE, + ArgConfigType.LLM_API_KEY, ]; export { ArgConfigType, SupportedSettings }; diff --git a/opendevin/server/agent/agent.py b/opendevin/server/agent/agent.py index 05ec0d67e7..2290fda407 100644 --- a/opendevin/server/agent/agent.py +++ b/opendevin/server/agent/agent.py @@ -1,5 +1,5 @@ import asyncio -from typing import Optional, Dict, List +from typing import Dict, List, Optional from opendevin import config from opendevin.action import ( @@ -136,7 +136,7 @@ class AgentUnit: } # remove empty values, prevent FE from sending empty strings agent_cls = self.get_arg_or_default(args, ConfigType.AGENT) model = self.get_arg_or_default(args, ConfigType.LLM_MODEL) - api_key = config.get(ConfigType.LLM_API_KEY) + api_key = self.get_arg_or_default(args, ConfigType.LLM_API_KEY) api_base = config.get(ConfigType.LLM_BASE_URL) max_iterations = self.get_arg_or_default(args, ConfigType.MAX_ITERATIONS) max_chars = self.get_arg_or_default(args, ConfigType.MAX_CHARS)