github/OpenHands
1
0
Fork 0
mirror of https://github.com/OpenHands/OpenHands.git synced 2025-12-26 05:48:36 +08:00
Code Issues Packages Projects Releases Wiki Activity

feat: migrate settings storage from localStorage to server API (#5703)

Browse Source 
Co-authored-by: openhands <openhands@all-hands.dev>
This commit is contained in:
tofarr 2024-12-26 09:09:23 -07:00 committed by GitHub
parent 8975fcd714
commit c195e467ee
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
14 changed files with 116 additions and 235 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

124
frontend/__tests__/services/settings.test.ts
View File

@ -1,124 +0,0 @@
import { describe, expect, it, vi, Mock, afterEach } from "vitest";
import {
DEFAULT_SETTINGS,
Settings,
getSettings,
saveSettings,
} from "../../src/services/settings";
Storage.prototype.getItem = vi.fn();
Storage.prototype.setItem = vi.fn();
describe("getSettings", () => {
afterEach(() => {
vi.resetAllMocks();
});
it("should get the stored settings", () => {
(localStorage.getItem as Mock)
.mockReturnValueOnce("llm_value")
.mockReturnValueOnce("base_url")
.mockReturnValueOnce("agent_value")
.mockReturnValueOnce("language_value")
.mockReturnValueOnce("api_key")
.mockReturnValueOnce("true")
.mockReturnValueOnce("invariant");
const settings = getSettings();
expect(settings).toEqual({
LLM_MODEL: "llm_value",
LLM_BASE_URL: "base_url",
AGENT: "agent_value",
LANGUAGE: "language_value",
LLM_API_KEY: "api_key",
CONFIRMATION_MODE: true,
SECURITY_ANALYZER: "invariant",
});
});
it("should handle return defaults if localStorage key does not exist", () => {
(localStorage.getItem as Mock)
.mockReturnValueOnce(null)
.mockReturnValueOnce(null)
.mockReturnValueOnce(null)
.mockReturnValueOnce(null)
.mockReturnValueOnce(null)
.mockReturnValueOnce(null)
.mockReturnValueOnce(null)
.mockReturnValueOnce(null);
const settings = getSettings();
expect(settings).toEqual({
LLM_MODEL: DEFAULT_SETTINGS.LLM_MODEL,
AGENT: DEFAULT_SETTINGS.AGENT,
LANGUAGE: DEFAULT_SETTINGS.LANGUAGE,
LLM_API_KEY: "",
LLM_BASE_URL: DEFAULT_SETTINGS.LLM_BASE_URL,
CONFIRMATION_MODE: DEFAULT_SETTINGS.CONFIRMATION_MODE,
SECURITY_ANALYZER: DEFAULT_SETTINGS.SECURITY_ANALYZER,
});
});
});
describe("saveSettings", () => {
it("should save the settings", () => {
const settings: Settings = {
LLM_MODEL: "llm_value",
LLM_BASE_URL: "base_url",
AGENT: "agent_value",
LANGUAGE: "language_value",
LLM_API_KEY: "some_key",
CONFIRMATION_MODE: true,
SECURITY_ANALYZER: "invariant",
};
saveSettings(settings);
expect(localStorage.setItem).toHaveBeenCalledWith("LLM_MODEL", "llm_value");
expect(localStorage.setItem).toHaveBeenCalledWith("AGENT", "agent_value");
expect(localStorage.setItem).toHaveBeenCalledWith(
"LANGUAGE",
"language_value",
);
expect(localStorage.setItem).toHaveBeenCalledWith(
"LLM_API_KEY",
"some_key",
);
});
it.skip("should save partial settings", () => {
const settings = {
LLM_MODEL: "llm_value",
};
saveSettings(settings);
expect(localStorage.setItem).toHaveBeenCalledTimes(2);
expect(localStorage.setItem).toHaveBeenCalledWith("LLM_MODEL", "llm_value");
expect(localStorage.setItem).toHaveBeenCalledWith("SETTINGS_VERSION", "2");
});
it("should not save invalid settings", () => {
const settings = {
LLM_MODEL: "llm_value",
AGENT: "agent_value",
LANGUAGE: "language_value",
INVALID: "invalid_value",
};
saveSettings(settings);
expect(localStorage.setItem).toHaveBeenCalledWith("LLM_MODEL", "llm_value");
expect(localStorage.setItem).toHaveBeenCalledWith("AGENT", "agent_value");
expect(localStorage.setItem).toHaveBeenCalledWith(
"LANGUAGE",
"language_value",
);
expect(localStorage.setItem).not.toHaveBeenCalledWith(
"INVALID",
"invalid_value",
);
});
});

4
frontend/src/components/features/sidebar/sidebar.tsx
View File

@ -1,7 +1,7 @@
import React from "react";
import { useLocation } from "react-router";
import { useAuth } from "#/context/auth-context";
import { useUserPrefs } from "#/context/user-prefs-context";
import { useSettings } from "#/context/settings-context";
import { useGitHubUser } from "#/hooks/query/use-github-user";
import { useIsAuthed } from "#/hooks/query/use-is-authed";
import { UserActions } from "./user-actions";
@ -21,7 +21,7 @@ export function Sidebar() {
const { data: isAuthed } = useIsAuthed();
const { logout } = useAuth();
const { settingsAreUpToDate } = useUserPrefs();
const { settingsAreUpToDate } = useSettings();
const [accountSettingsModalOpen, setAccountSettingsModalOpen] =
React.useState(false);

4
frontend/src/components/shared/modals/account-settings/account-settings-form.tsx
View File

@ -8,7 +8,7 @@ import { ModalBody } from "../modal-body";
import { AvailableLanguages } from "#/i18n";
import { I18nKey } from "#/i18n/declaration";
import { useAuth } from "#/context/auth-context";
import { useUserPrefs } from "#/context/user-prefs-context";
import { useSettings } from "#/context/settings-context";
import { handleCaptureConsent } from "#/utils/handle-capture-consent";
import { ModalButton } from "../../buttons/modal-button";
import { CustomInput } from "../../custom-input";
@ -30,7 +30,7 @@ export function AccountSettingsForm({
}: AccountSettingsFormProps) {
const { gitHubToken, setGitHubToken, logout } = useAuth();
const { data: config } = useConfig();
const { saveSettings } = useUserPrefs();
const { saveSettings } = useSettings();
const { t } = useTranslation();
const handleSubmit = (event: React.FormEvent<HTMLFormElement>) => {

4
frontend/src/components/shared/modals/account-settings/account-settings-modal.tsx
View File

@ -1,4 +1,4 @@
import { useUserPrefs } from "#/context/user-prefs-context";
import { useSettings } from "#/context/settings-context";
import { useGitHubUser } from "#/hooks/query/use-github-user";
import { ModalBackdrop } from "../modal-backdrop";
import { AccountSettingsForm } from "./account-settings-form";
@ -9,7 +9,7 @@ interface AccountSettingsModalProps {
export function AccountSettingsModal({ onClose }: AccountSettingsModalProps) {
const user = useGitHubUser();
const { settings } = useUserPrefs();
const { settings } = useSettings();
// FIXME: Bad practice to use localStorage directly
const analyticsConsent = localStorage.getItem("analytics-consent");

52
frontend/src/components/shared/modals/settings/settings-form.tsx
View File

@ -13,7 +13,7 @@ import {
updateSettingsVersion,
} from "#/utils/settings-utils";
import { useEndSession } from "#/hooks/use-end-session";
import { useUserPrefs } from "#/context/user-prefs-context";
import { useSettings } from "#/context/settings-context";
import { ModalButton } from "../../buttons/modal-button";
import { AdvancedOptionSwitch } from "../../inputs/advanced-option-switch";
import { AgentInput } from "../../inputs/agent-input";
@ -43,7 +43,7 @@ export function SettingsForm({
securityAnalyzers,
onClose,
}: SettingsFormProps) {
const { saveSettings } = useUserPrefs();
const { saveSettings } = useSettings();
const endSession = useEndSession();
const { logout } = useAuth();
@ -84,7 +84,6 @@ export function SettingsForm({
React.useState(false);
const [confirmEndSessionModalOpen, setConfirmEndSessionModalOpen] =
React.useState(false);
const [showWarningModal, setShowWarningModal] = React.useState(false);
const resetOngoingSession = () => {
if (location.pathname.startsWith("/conversations/")) {
@ -125,11 +124,8 @@ export function SettingsForm({
const handleSubmit = (event: React.FormEvent<HTMLFormElement>) => {
event.preventDefault();
const formData = new FormData(event.currentTarget);
const apiKey = formData.get("api-key");
if (!apiKey) {
setShowWarningModal(true);
} else if (location.pathname.startsWith("/conversations/")) {
if (location.pathname.startsWith("/conversations/")) {
setConfirmEndSessionModalOpen(true);
} else {
handleFormSubmission(formData);
@ -137,26 +133,6 @@ export function SettingsForm({
}
};
const handleCloseClick = () => {
const formData = new FormData(formRef.current ?? undefined);
const apiKey = formData.get("api-key");
if (!apiKey) setShowWarningModal(true);
else onClose();
};
const handleWarningConfirm = () => {
setShowWarningModal(false);
const formData = new FormData(formRef.current ?? undefined);
formData.set("api-key", ""); // Set null value for API key
handleFormSubmission(formData);
onClose();
};
const handleWarningCancel = () => {
setShowWarningModal(false);
};
return (
<div>
<form
@ -196,7 +172,7 @@ export function SettingsForm({
<APIKeyInput
isDisabled={!!disabled}
defaultValue={settings.LLM_API_KEY}
defaultValue={settings.LLM_API_KEY || ""}
/>
{showAdvancedOptions && (
@ -234,7 +210,7 @@ export function SettingsForm({
<ModalButton
text={t(I18nKey.SETTINGS_FORM$CLOSE_LABEL)}
className="bg-[#737373] w-full"
onClick={handleCloseClick}
onClick={onClose}
/>
</div>
<ModalButton
@ -289,24 +265,6 @@ export function SettingsForm({
/>
</ModalBackdrop>
)}
{showWarningModal && (
<ModalBackdrop>
<DangerModal
title="Are you sure?"
description="You haven't set an API key. Without an API key, you won't be able to use the AI features. Are you sure you want to close the settings?"
buttons={{
danger: {
text: "Yes, close settings",
onClick: handleWarningConfirm,
},
cancel: {
text: "Cancel",
onClick: handleWarningCancel,
},
}}
/>
</ModalBackdrop>
)}
</div>
);
}

4
frontend/src/components/shared/modals/settings/settings-modal.tsx
View File

@ -1,4 +1,4 @@
import { useUserPrefs } from "#/context/user-prefs-context";
import { useSettings } from "#/context/settings-context";
import { useAIConfigOptions } from "#/hooks/query/use-ai-config-options";
import { LoadingSpinner } from "../../loading-spinner";
import { ModalBackdrop } from "../modal-backdrop";
@ -9,7 +9,7 @@ interface SettingsModalProps {
}
export function SettingsModal({ onClose }: SettingsModalProps) {
const { settings } = useUserPrefs();
const { settings } = useSettings();
const aiConfigOptions = useAIConfigOptions();
return (

4
frontend/src/components/shared/task-form.tsx
View File

@ -11,7 +11,7 @@ import {
} from "#/state/initial-query-slice";
import OpenHands from "#/api/open-hands";
import { useAuth } from "#/context/auth-context";
import { useUserPrefs } from "#/context/user-prefs-context";
import { useSettings } from "#/context/settings-context";
import { SuggestionBubble } from "#/components/features/suggestions/suggestion-bubble";
import { SUGGESTIONS } from "#/utils/suggestions";
@ -28,7 +28,7 @@ export const TaskForm = React.forwardRef<HTMLFormElement>((_, ref) => {
const navigation = useNavigation();
const navigate = useNavigate();
const { gitHubToken } = useAuth();
const { settings } = useUserPrefs();
const { settings } = useSettings();
const { selectedRepository, files } = useSelector(
(state: RootState) => state.initialQuery,

36
frontend/src/context/user-prefs-context.tsx → frontend/src/context/settings-context.tsx
View File

@ -1,39 +1,49 @@
import React from "react";
import posthog from "posthog-js";
import { useQuery, useQueryClient } from "@tanstack/react-query";
import {
getSettings,
Settings,
saveSettings as updateAndSaveSettingsToLocalStorage,
settingsAreUpToDate as checkIfSettingsAreUpToDate,
DEFAULT_SETTINGS,
} from "#/services/settings";
interface UserPrefsContextType {
interface SettingsContextType {
settings: Settings;
settingsAreUpToDate: boolean;
saveSettings: (settings: Partial<Settings>) => void;
}
const UserPrefsContext = React.createContext<UserPrefsContextType | undefined>(
const SettingsContext = React.createContext<SettingsContextType | undefined>(
undefined,
);
function UserPrefsProvider({ children }: React.PropsWithChildren) {
const [settings, setSettings] = React.useState(getSettings());
const SETTINGS_QUERY_KEY = ["settings"];
function SettingsProvider({ children }: React.PropsWithChildren) {
const { data: settings } = useQuery({
queryKey: SETTINGS_QUERY_KEY,
queryFn: getSettings,
initialData: DEFAULT_SETTINGS,
});
const [settingsAreUpToDate, setSettingsAreUpToDate] = React.useState(
checkIfSettingsAreUpToDate(),
);
const queryClient = useQueryClient();
const saveSettings = (newSettings: Partial<Settings>) => {
updateAndSaveSettingsToLocalStorage(newSettings);
setSettings(getSettings());
queryClient.invalidateQueries({ queryKey: SETTINGS_QUERY_KEY });
setSettingsAreUpToDate(checkIfSettingsAreUpToDate());
};
React.useEffect(() => {
if (settings.LLM_API_KEY) {
if (settings?.LLM_API_KEY) {
posthog.capture("user_activated");
}
}, [settings.LLM_API_KEY]);
}, [settings?.LLM_API_KEY]);
const value = React.useMemo(
() => ({
@ -45,18 +55,18 @@ function UserPrefsProvider({ children }: React.PropsWithChildren) {
);
return (
<UserPrefsContext.Provider value={value}>
<SettingsContext.Provider value={value}>
{children}
</UserPrefsContext.Provider>
</SettingsContext.Provider>
);
}
function useUserPrefs() {
const context = React.useContext(UserPrefsContext);
function useSettings() {
const context = React.useContext(SettingsContext);
if (context === undefined) {
throw new Error("useUserPrefs must be used within a UserPrefsProvider");
throw new Error("useSettings must be used within a SettingsProvider");
}
return context;
}
export { UserPrefsProvider, useUserPrefs };
export { SettingsProvider, useSettings };

14
frontend/src/entry.client.tsx
View File

@ -20,7 +20,7 @@ import toast from "react-hot-toast";
import store from "./store";
import { useConfig } from "./hooks/query/use-config";
import { AuthProvider } from "./context/auth-context";
import { UserPrefsProvider } from "./context/user-prefs-context";
import { SettingsProvider } from "./context/settings-context";
function PosthogInit() {
const { data: config } = useConfig();
@ -71,14 +71,14 @@ prepareApp().then(() =>
document,
<StrictMode>
<Provider store={store}>
<UserPrefsProvider>
<AuthProvider>
<QueryClientProvider client={queryClient}>
<QueryClientProvider client={queryClient}>
<SettingsProvider>
<AuthProvider>
<HydratedRouter />
<PosthogInit />
</QueryClientProvider>
</AuthProvider>
</UserPrefsProvider>
</AuthProvider>
</SettingsProvider>
</QueryClientProvider>
</Provider>
</StrictMode>,
);

4
frontend/src/routes/_oh.app/route.tsx
View File

@ -21,7 +21,7 @@ import { WsClientProvider } from "#/context/ws-client-provider";
import { EventHandler } from "./event-handler";
import { useLatestRepoCommit } from "#/hooks/query/use-latest-repo-commit";
import { useAuth } from "#/context/auth-context";
import { useUserPrefs } from "#/context/user-prefs-context";
import { useSettings } from "#/context/settings-context";
import { useConversationConfig } from "#/hooks/query/use-conversation-config";
import { Container } from "#/components/layout/container";
import Security from "#/components/shared/modals/security/security";
@ -30,7 +30,7 @@ import { TerminalStatusLabel } from "#/components/features/terminal/terminal-sta
function AppContent() {
const { gitHubToken } = useAuth();
const { settings } = useUserPrefs();
const { settings } = useSettings();
const { conversationId } = useConversation();
const dispatch = useDispatch();

4
frontend/src/routes/_oh/route.tsx
View File

@ -4,7 +4,7 @@ import i18n from "#/i18n";
import { useGitHubAuthUrl } from "#/hooks/use-github-auth-url";
import { useIsAuthed } from "#/hooks/query/use-is-authed";
import { useAuth } from "#/context/auth-context";
import { useUserPrefs } from "#/context/user-prefs-context";
import { useSettings } from "#/context/settings-context";
import { useConfig } from "#/hooks/query/use-config";
import { Sidebar } from "#/components/features/sidebar/sidebar";
import { WaitlistModal } from "#/components/features/waitlist/waitlist-modal";
@ -45,7 +45,7 @@ export function ErrorBoundary() {
export default function MainApp() {
const { gitHubToken } = useAuth();
const { settings, settingsAreUpToDate } = useUserPrefs();
const { settings, settingsAreUpToDate } = useSettings();
const [consentFormIsOpen, setConsentFormIsOpen] = React.useState(
!localStorage.getItem("analytics-consent"),

75
frontend/src/services/settings.ts
View File

@ -1,3 +1,5 @@
import { openHands } from "#/api/open-hands-axios";
export const LATEST_SETTINGS_VERSION = 4;
export type Settings = {
@ -5,23 +7,31 @@ export type Settings = {
LLM_BASE_URL: string;
AGENT: string;
LANGUAGE: string;
LLM_API_KEY: string;
LLM_API_KEY: string | null;
CONFIRMATION_MODE: boolean;
SECURITY_ANALYZER: string;
};
export type ApiSettings = {
llm_model: string;
llm_base_url: string;
agent: string;
language: string;
llm_api_key: string | null;
confirmation_mode: boolean;
security_analyzer: string;
};
export const DEFAULT_SETTINGS: Settings = {
LLM_MODEL: "anthropic/claude-3-5-sonnet-20241022",
LLM_BASE_URL: "",
AGENT: "CodeActAgent",
LANGUAGE: "en",
LLM_API_KEY: "",
LLM_API_KEY: null,
CONFIRMATION_MODE: false,
SECURITY_ANALYZER: "",
};
const validKeys = Object.keys(DEFAULT_SETTINGS) as (keyof Settings)[];
export const getCurrentSettingsVersion = () => {
const settingsVersion = localStorage.getItem("SETTINGS_VERSION");
if (!settingsVersion) return 0;
@ -66,39 +76,64 @@ export const maybeMigrateSettings = (logout: () => void) => {
export const getDefaultSettings = (): Settings => DEFAULT_SETTINGS;
/**
* Get the settings from local storage or use the default settings if not found
* Get the settings from the server or use the default settings if not found
*/
export const getSettings = (): Settings => {
const model = localStorage.getItem("LLM_MODEL");
export const getSettings = async (): Promise<Settings> => {
const { data: apiSettings } =
await openHands.get<ApiSettings>("/api/settings");
if (apiSettings != null) {
return {
LLM_MODEL: apiSettings.llm_model,
LLM_BASE_URL: apiSettings.llm_base_url,
AGENT: apiSettings.agent,
LANGUAGE: apiSettings.language,
CONFIRMATION_MODE: apiSettings.confirmation_mode,
SECURITY_ANALYZER: apiSettings.security_analyzer,
LLM_API_KEY: "",
};
}
const llmModel = localStorage.getItem("LLM_MODEL");
const baseUrl = localStorage.getItem("LLM_BASE_URL");
const agent = localStorage.getItem("AGENT");
const language = localStorage.getItem("LANGUAGE");
const apiKey = localStorage.getItem("LLM_API_KEY");
const llmApiKey = localStorage.getItem("LLM_API_KEY");
const confirmationMode = localStorage.getItem("CONFIRMATION_MODE") === "true";
const securityAnalyzer = localStorage.getItem("SECURITY_ANALYZER");
return {
LLM_MODEL: model || DEFAULT_SETTINGS.LLM_MODEL,
LLM_MODEL: llmModel || DEFAULT_SETTINGS.LLM_MODEL,
LLM_BASE_URL: baseUrl || DEFAULT_SETTINGS.LLM_BASE_URL,
AGENT: agent || DEFAULT_SETTINGS.AGENT,
LANGUAGE: language || DEFAULT_SETTINGS.LANGUAGE,
LLM_API_KEY: apiKey || DEFAULT_SETTINGS.LLM_API_KEY,
LLM_API_KEY: llmApiKey || DEFAULT_SETTINGS.LLM_API_KEY,
CONFIRMATION_MODE: confirmationMode || DEFAULT_SETTINGS.CONFIRMATION_MODE,
SECURITY_ANALYZER: securityAnalyzer || DEFAULT_SETTINGS.SECURITY_ANALYZER,
};
};
/**
* Save the settings to local storage. Only valid settings are saved.
* Save the settings to the server. Only valid settings are saved.
* @param settings - the settings to save
*/
export const saveSettings = (settings: Partial<Settings>) => {
Object.keys(settings).forEach((key) => {
const isValid = validKeys.includes(key as keyof Settings);
if (!isValid) return;
let value = settings[key as keyof Settings];
if (value === undefined || value === null) value = "";
localStorage.setItem(key, value.toString().trim());
});
localStorage.setItem("SETTINGS_VERSION", LATEST_SETTINGS_VERSION.toString());
export const saveSettings = async (
settings: Partial<Settings>,
): Promise<boolean> => {
try {
const apiSettings = {
llm_model: settings.LLM_MODEL || null,
llm_base_url: settings.LLM_BASE_URL || null,
agent: settings.AGENT || null,
language: settings.LANGUAGE || null,
confirmation_mode: settings.CONFIRMATION_MODE || null,
security_analyzer: settings.SECURITY_ANALYZER || null,
llm_api_key: settings.LLM_API_KEY || null,
};
const { data } = await openHands.post("/api/settings", apiSettings);
return data;
} catch (error) {
console.error("Error saving settings:", error);
return false;
}
};

18
frontend/test-utils.tsx
View File

@ -12,7 +12,7 @@ import { initReactI18next } from "react-i18next";
import { AppStore, RootState, rootReducer } from "./src/store";
import { vi } from "vitest";
import { AuthProvider } from "#/context/auth-context";
import { UserPrefsProvider } from "#/context/user-prefs-context";
import { SettingsProvider } from "#/context/settings-context";
import { ConversationProvider } from "#/context/conversation-context";
// Mock useParams before importing components
@ -70,17 +70,17 @@ export function renderWithProviders(
function Wrapper({ children }: PropsWithChildren<object>): JSX.Element {
return (
<Provider store={store}>
<UserPrefsProvider>
<AuthProvider>
<ConversationProvider>
<QueryClientProvider client={new QueryClient()}>
<QueryClientProvider client={new QueryClient()}>
<SettingsProvider>
<AuthProvider>
<ConversationProvider>
<I18nextProvider i18n={i18n}>
{children}
</I18nextProvider>
</QueryClientProvider>
</ConversationProvider>
</AuthProvider>
</UserPrefsProvider>
</ConversationProvider>
</AuthProvider>
</SettingsProvider>
</QueryClientProvider>
</Provider>
);
}

4
openhands/server/routes/settings.py
View File

@ -42,9 +42,11 @@ async def store_settings(
settings_store = await SettingsStoreImpl.get_instance(config, github_auth)
existing_settings = await settings_store.load()
if existing_settings:
settings = Settings(**{**existing_settings.__dict__, **settings.__dict__})
if settings.llm_api_key is None:
settings.llm_api_key = existing_settings.llm_api_key
return await settings_store.store(settings)
await settings_store.store(settings)
return True
except Exception as e:
logger.warning(f'Invalid token: {e}')
return JSONResponse(