Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for when settings are unset #5921

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 9 additions & 8 deletions frontend/src/components/features/sidebar/sidebar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { useLocation } from "react-router";
import { useAuth } from "#/context/auth-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";
import { AllHandsLogoButton } from "#/components/shared/buttons/all-hands-logo-button";
import { DocsButton } from "#/components/shared/buttons/docs-button";
Expand All @@ -18,13 +17,12 @@ export function Sidebar() {
const location = useLocation();

const user = useGitHubUser();
const { data: isAuthed } = useIsAuthed();

const { logout } = useAuth();
const { settingsAreUpToDate } = useSettings();
const { settingsAreUpToDate, settings } = useSettings();

const [accountSettingsModalOpen, setAccountSettingsModalOpen] =
React.useState(false);

const [settingsModalIsOpen, setSettingsModalIsOpen] = React.useState(false);
const [startNewProjectModalIsOpen, setStartNewProjectModalIsOpen] =
React.useState(false);
Expand All @@ -36,6 +34,12 @@ export function Sidebar() {
}
}, [user.isError]);

React.useEffect(() => {
if (!settings || !settingsAreUpToDate) {
setSettingsModalIsOpen(true);
}
}, [settings, settingsAreUpToDate]);

const handleAccountSettingsModalClose = () => {
// If the user closes the modal without connecting to GitHub,
// we need to log them out to clear the invalid token from the
Expand All @@ -49,9 +53,6 @@ export function Sidebar() {
setStartNewProjectModalIsOpen(true);
};

const showSettingsModal =
isAuthed && (!settingsAreUpToDate || settingsModalIsOpen);

return (
<>
<aside className="h-[40px] md:h-auto px-1 flex flex-row md:flex-col gap-1">
Expand Down Expand Up @@ -79,7 +80,7 @@ export function Sidebar() {
{accountSettingsModalOpen && (
<AccountSettingsModal onClose={handleAccountSettingsModalClose} />
)}
{showSettingsModal && (
{!accountSettingsModalOpen && settingsModalIsOpen && (
<SettingsModal onClose={() => setSettingsModalIsOpen(false)} />
)}
{startNewProjectModalIsOpen && (
Expand Down
16 changes: 12 additions & 4 deletions frontend/src/services/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,9 @@ export const getDefaultSettings = (): Settings => DEFAULT_SETTINGS;
* Get the settings from the server or use the default settings if not found
*/
export const getSettings = async (): Promise<Settings> => {
const { data: apiSettings } =
await openHands.get<ApiSettings>("/api/settings");
if (apiSettings != null) {
try {
const { data: apiSettings } =
await openHands.get<ApiSettings>("/api/settings");
return {
LLM_MODEL: apiSettings.llm_model,
LLM_BASE_URL: apiSettings.llm_base_url,
Expand All @@ -143,6 +143,14 @@ export const getSettings = async (): Promise<Settings> => {
SECURITY_ANALYZER: apiSettings.security_analyzer,
LLM_API_KEY: "",
};
} catch (error) {
// @ts-expect-error we don't have a type annotation for the response
if (error.response?.status !== 404) {
throw error;
}
}
return getLocalStorageSettings();
// FIXME: remove local storage settings after 1/31/2025
const localSettings = getLocalStorageSettings();
await saveSettings(localSettings);
return localSettings;
};
13 changes: 9 additions & 4 deletions openhands/server/routes/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@

SettingsStoreImpl = get_impl(SettingsStore, openhands_config.settings_store_class) # type: ignore
ConversationStoreImpl = get_impl(
ConversationStore, openhands_config.conversation_store_class # type: ignore
ConversationStore, # type: ignore
openhands_config.conversation_store_class, # type: ignore
)


Expand All @@ -26,9 +27,13 @@ async def load_settings(
try:
settings_store = await SettingsStoreImpl.get_instance(config, github_token)
settings = await settings_store.load()
if settings:
# For security reasons we don't ever send the api key to the client
settings.llm_api_key = None
if not settings:
return JSONResponse(
status_code=status.HTTP_404_NOT_FOUND,
content={'error': 'Settings not found'},
)
# For security reasons we don't ever send the api key to the client
settings.llm_api_key = None
return settings
except Exception as e:
logger.warning(f'Invalid token: {e}')
Expand Down
Loading