From 97b1867ea14aec18a090bf15b4115b3c005cfcfb Mon Sep 17 00:00:00 2001 From: Robert Brennan Date: Fri, 27 Dec 2024 11:28:11 -0500 Subject: [PATCH] Fix for settings update (#5858) --- frontend/public/mockServiceWorker.js | 18 ++++- frontend/src/api/open-hands.ts | 2 - .../shared/modals/settings/settings-form.tsx | 8 +-- frontend/src/components/shared/task-form.tsx | 3 - frontend/src/context/settings-context.tsx | 8 +-- openhands/server/listen_socket.py | 13 +++- openhands/server/session/manager.py | 12 ++-- openhands/server/session/session.py | 66 ++++++------------- tests/unit/test_manager.py | 24 +++++-- 9 files changed, 77 insertions(+), 77 deletions(-) diff --git a/frontend/public/mockServiceWorker.js b/frontend/public/mockServiceWorker.js index fead0b3ff91c..ec47a9a50a24 100644 --- a/frontend/public/mockServiceWorker.js +++ b/frontend/public/mockServiceWorker.js @@ -8,8 +8,8 @@ * - Please do NOT serve this file on production. */ -const PACKAGE_VERSION = '2.6.6' -const INTEGRITY_CHECKSUM = 'ca7800994cc8bfb5eb961e037c877074' +const PACKAGE_VERSION = '2.7.0' +const INTEGRITY_CHECKSUM = '00729d72e3b82faf54ca8b9621dbb96f' const IS_MOCKED_RESPONSE = Symbol('isMockedResponse') const activeClientIds = new Set() @@ -199,7 +199,19 @@ async function getResponse(event, client, requestId) { // Remove the "accept" header value that marked this request as passthrough. // This prevents request alteration and also keeps it compliant with the // user-defined CORS policies. - headers.delete('accept', 'msw/passthrough') + const acceptHeader = headers.get('accept') + if (acceptHeader) { + const values = acceptHeader.split(',').map((value) => value.trim()) + const filteredValues = values.filter( + (value) => value !== 'msw/passthrough', + ) + + if (filteredValues.length > 0) { + headers.set('accept', filteredValues.join(', ')) + } else { + headers.delete('accept') + } + } return fetch(requestClone, { headers }) } diff --git a/frontend/src/api/open-hands.ts b/frontend/src/api/open-hands.ts index 9093514a6357..84d254102c08 100644 --- a/frontend/src/api/open-hands.ts +++ b/frontend/src/api/open-hands.ts @@ -250,14 +250,12 @@ class OpenHands { static async newConversation(params: { githubToken?: string; - args?: Record; selectedRepository?: string; }): Promise<{ conversation_id: string }> { const { data } = await openHands.post<{ conversation_id: string; }>("/api/conversations", { github_token: params.githubToken, - args: params.args, selected_repository: params.selectedRepository, }); // TODO: remove this once we have a multi-conversation UI diff --git a/frontend/src/components/shared/modals/settings/settings-form.tsx b/frontend/src/components/shared/modals/settings/settings-form.tsx index e22d845846da..918046178f6b 100644 --- a/frontend/src/components/shared/modals/settings/settings-form.tsx +++ b/frontend/src/components/shared/modals/settings/settings-form.tsx @@ -86,13 +86,13 @@ export function SettingsForm({ } }; - const handleFormSubmission = (formData: FormData) => { + const handleFormSubmission = async (formData: FormData) => { const keys = Array.from(formData.keys()); const isUsingAdvancedOptions = keys.includes("use-advanced-options"); const newSettings = extractSettings(formData); saveSettingsView(isUsingAdvancedOptions ? "advanced" : "basic"); - saveSettings(newSettings); + await saveSettings(newSettings); resetOngoingSession(); posthog.capture("settings_saved", { @@ -101,8 +101,8 @@ export function SettingsForm({ }); }; - const handleConfirmResetSettings = () => { - saveSettings(getDefaultSettings()); + const handleConfirmResetSettings = async () => { + await saveSettings(getDefaultSettings()); resetOngoingSession(); posthog.capture("settings_reset"); diff --git a/frontend/src/components/shared/task-form.tsx b/frontend/src/components/shared/task-form.tsx index 4f95262059c9..3f71dadca9c5 100644 --- a/frontend/src/components/shared/task-form.tsx +++ b/frontend/src/components/shared/task-form.tsx @@ -11,7 +11,6 @@ import { } from "#/state/initial-query-slice"; import OpenHands from "#/api/open-hands"; import { useAuth } from "#/context/auth-context"; -import { useSettings } from "#/context/settings-context"; import { SuggestionBubble } from "#/components/features/suggestions/suggestion-bubble"; import { SUGGESTIONS } from "#/utils/suggestions"; @@ -33,7 +32,6 @@ export function TaskForm({ ref }: TaskFormProps) { const navigation = useNavigation(); const navigate = useNavigate(); const { gitHubToken } = useAuth(); - const { settings } = useSettings(); const { selectedRepository, files } = useSelector( (state: RootState) => state.initialQuery, @@ -50,7 +48,6 @@ export function TaskForm({ ref }: TaskFormProps) { return OpenHands.newConversation({ githubToken: gitHubToken || undefined, selectedRepository: selectedRepository || undefined, - args: settings || undefined, }); }, onSuccess: ({ conversation_id: conversationId }, { q }) => { diff --git a/frontend/src/context/settings-context.tsx b/frontend/src/context/settings-context.tsx index ee41d46538c4..3d2c426dc095 100644 --- a/frontend/src/context/settings-context.tsx +++ b/frontend/src/context/settings-context.tsx @@ -4,7 +4,7 @@ import { useQuery, useQueryClient } from "@tanstack/react-query"; import { getSettings, Settings, - saveSettings as updateAndSaveSettingsToLocalStorage, + saveSettings, settingsAreUpToDate as checkIfSettingsAreUpToDate, DEFAULT_SETTINGS, } from "#/services/settings"; @@ -33,8 +33,8 @@ function SettingsProvider({ children }: React.PropsWithChildren) { ); const queryClient = useQueryClient(); - const saveSettings = (newSettings: Partial) => { - updateAndSaveSettingsToLocalStorage(newSettings); + const handleSaveSettings = async (newSettings: Partial) => { + await saveSettings(newSettings); queryClient.invalidateQueries({ queryKey: SETTINGS_QUERY_KEY }); setSettingsAreUpToDate(checkIfSettingsAreUpToDate()); }; @@ -49,7 +49,7 @@ function SettingsProvider({ children }: React.PropsWithChildren) { () => ({ settings, settingsAreUpToDate, - saveSettings, + saveSettings: handleSaveSettings, }), [settings, settingsAreUpToDate], ); diff --git a/openhands/server/listen_socket.py b/openhands/server/listen_socket.py index c9fba18ed511..5a5e82757e51 100644 --- a/openhands/server/listen_socket.py +++ b/openhands/server/listen_socket.py @@ -13,6 +13,7 @@ from openhands.events.observation.agent import AgentStateChangedObservation from openhands.events.serialization import event_to_dict from openhands.events.stream import AsyncEventStreamWrapper +from openhands.server.routes.settings import SettingsStoreImpl from openhands.server.session.manager import ConversationDoesNotExistError from openhands.server.shared import config, openhands_config, session_manager, sio from openhands.server.types import AppMode @@ -32,10 +33,12 @@ async def connect(connection_id: str, environ, auth): logger.error('No conversation_id in query params') raise ConnectionRefusedError('No conversation_id in query params') + github_token = '' if openhands_config.app_mode != AppMode.OSS: user_id = '' if auth and 'github_token' in auth: - with Github(auth['github_token']) as g: + github_token = auth['github_token'] + with Github(github_token) as g: gh_user = await call_sync_from_async(g.get_user) user_id = gh_user.id @@ -51,9 +54,15 @@ async def connect(connection_id: str, environ, auth): f'User {user_id} is not allowed to join conversation {conversation_id}' ) + settings_store = await SettingsStoreImpl.get_instance(config, github_token) + settings = await settings_store.load() + + if not settings: + raise ConnectionRefusedError('Settings not found') + try: event_stream = await session_manager.join_conversation( - conversation_id, connection_id + conversation_id, connection_id, settings ) except ConversationDoesNotExistError: logger.error(f'Conversation {conversation_id} does not exist') diff --git a/openhands/server/session/manager.py b/openhands/server/session/manager.py index a2f88eea93ab..fcb7153ac55c 100644 --- a/openhands/server/session/manager.py +++ b/openhands/server/session/manager.py @@ -11,8 +11,8 @@ from openhands.core.logger import openhands_logger as logger from openhands.events.stream import EventStream, session_exists from openhands.server.session.conversation import Conversation -from openhands.server.session.conversation_init_data import ConversationInitData from openhands.server.session.session import ROOM_KEY, Session +from openhands.server.settings import Settings from openhands.storage.files import FileStore from openhands.utils.async_utils import call_sync_from_async from openhands.utils.shutdown_listener import should_continue @@ -205,13 +205,13 @@ async def attach_to_conversation(self, sid: str) -> Conversation | None: self._active_conversations[sid] = (c, 1) return c - async def join_conversation(self, sid: str, connection_id: str) -> EventStream: + async def join_conversation(self, sid: str, connection_id: str, settings: Settings): logger.info(f'join_conversation:{sid}:{connection_id}') await self.sio.enter_room(connection_id, ROOM_KEY.format(sid=sid)) self.local_connection_id_to_session_id[connection_id] = sid event_stream = await self._get_event_stream(sid) if not event_stream: - return await self.maybe_start_agent_loop(sid) + return await self.maybe_start_agent_loop(sid, settings) return event_stream async def detach_from_conversation(self, conversation: Conversation): @@ -342,9 +342,7 @@ async def _has_remote_connections(self, sid: str) -> bool: finally: self._has_remote_connections_flags.pop(sid, None) - async def maybe_start_agent_loop( - self, sid: str, conversation_init_data: ConversationInitData | None = None - ) -> EventStream: + async def maybe_start_agent_loop(self, sid: str, settings: Settings) -> EventStream: logger.info(f'maybe_start_agent_loop:{sid}') session: Session | None = None if not await self.is_agent_loop_running(sid): @@ -353,7 +351,7 @@ async def maybe_start_agent_loop( sid=sid, file_store=self.file_store, config=self.config, sio=self.sio ) self._local_agent_loops_by_sid[sid] = session - await session.initialize_agent(conversation_init_data) + await session.initialize_agent(settings) event_stream = await self._get_event_stream(sid) if not event_stream: diff --git a/openhands/server/session/session.py b/openhands/server/session/session.py index 22bf2ec7a6bd..da412a435c40 100644 --- a/openhands/server/session/session.py +++ b/openhands/server/session/session.py @@ -1,5 +1,4 @@ import asyncio -import json import time from copy import deepcopy @@ -23,9 +22,8 @@ from openhands.llm.llm import LLM from openhands.server.session.agent_session import AgentSession from openhands.server.session.conversation_init_data import ConversationInitData +from openhands.server.settings import Settings from openhands.storage.files import FileStore -from openhands.storage.locations import get_conversation_init_data_filename -from openhands.utils.async_utils import call_sync_from_async ROOM_KEY = 'room:{sid}' @@ -65,63 +63,33 @@ def close(self): self.is_alive = False self.agent_session.close() - async def _restore_init_data(self, sid: str) -> ConversationInitData: - # FIXME: we should not store/restore this data once we have server-side - # LLM configs. Should be done by 1/1/2025 - json_str = await call_sync_from_async( - self.file_store.read, get_conversation_init_data_filename(sid) - ) - data = json.loads(json_str) - return ConversationInitData(**data) - - async def _save_init_data(self, sid: str, init_data: ConversationInitData): - # FIXME: we should not store/restore this data once we have server-side - # LLM configs. Should be done by 1/1/2025 - json_str = json.dumps(init_data.__dict__) - await call_sync_from_async( - self.file_store.write, get_conversation_init_data_filename(sid), json_str - ) - async def initialize_agent( - self, conversation_init_data: ConversationInitData | None = None + self, + settings: Settings, ): self.agent_session.event_stream.add_event( AgentStateChangedObservation('', AgentState.LOADING), EventSource.ENVIRONMENT, ) - if conversation_init_data is None: - try: - conversation_init_data = await self._restore_init_data(self.sid) - except FileNotFoundError: - logger.error(f'User settings not found for session {self.sid}') - raise RuntimeError('User settings not found') - - agent_cls = conversation_init_data.agent or self.config.default_agent + + agent_cls = settings.agent or self.config.default_agent self.config.security.confirmation_mode = ( self.config.security.confirmation_mode - if conversation_init_data.confirmation_mode is None - else conversation_init_data.confirmation_mode + if settings.confirmation_mode is None + else settings.confirmation_mode ) self.config.security.security_analyzer = ( - conversation_init_data.security_analyzer - or self.config.security.security_analyzer - ) - max_iterations = ( - conversation_init_data.max_iterations or self.config.max_iterations + settings.security_analyzer or self.config.security.security_analyzer ) + max_iterations = settings.max_iterations or self.config.max_iterations # override default LLM config default_llm_config = self.config.get_llm_config() - default_llm_config.model = ( - conversation_init_data.llm_model or default_llm_config.model - ) - default_llm_config.api_key = ( - conversation_init_data.llm_api_key or default_llm_config.api_key - ) + default_llm_config.model = settings.llm_model or default_llm_config.model + default_llm_config.api_key = settings.llm_api_key or default_llm_config.api_key default_llm_config.base_url = ( - conversation_init_data.llm_base_url or default_llm_config.base_url + settings.llm_base_url or default_llm_config.base_url ) - await self._save_init_data(self.sid, conversation_init_data) # TODO: override other LLM config & agent config groups (#2075) @@ -129,6 +97,12 @@ async def initialize_agent( agent_config = self.config.get_agent_config(agent_cls) agent = Agent.get_cls(agent_cls)(llm, agent_config) + github_token = None + selected_repository = None + if isinstance(settings, ConversationInitData): + github_token = settings.github_token + selected_repository = settings.selected_repository + try: await self.agent_session.start( runtime_name=self.config.runtime, @@ -138,8 +112,8 @@ async def initialize_agent( max_budget_per_task=self.config.max_budget_per_task, agent_to_llm_config=self.config.get_agent_to_llm_config_map(), agent_configs=self.config.get_agent_configs(), - github_token=conversation_init_data.github_token, - selected_repository=conversation_init_data.selected_repository, + github_token=github_token, + selected_repository=selected_repository, ) except Exception as e: logger.exception(f'Error creating controller: {e}') diff --git a/tests/unit/test_manager.py b/tests/unit/test_manager.py index b653f15ac143..c2a61104a864 100644 --- a/tests/unit/test_manager.py +++ b/tests/unit/test_manager.py @@ -116,7 +116,9 @@ async def test_init_new_local_session(): await session_manager.maybe_start_agent_loop( 'new-session-id', ConversationInitData() ) - await session_manager.join_conversation('new-session-id', 'new-session-id') + await session_manager.join_conversation( + 'new-session-id', 'new-session-id', ConversationInitData() + ) assert session_instance.initialize_agent.call_count == 1 assert sio.enter_room.await_count == 1 @@ -148,8 +150,12 @@ async def test_join_local_session(): await session_manager.maybe_start_agent_loop( 'new-session-id', ConversationInitData() ) - await session_manager.join_conversation('new-session-id', 'new-session-id') - await session_manager.join_conversation('new-session-id', 'new-session-id') + await session_manager.join_conversation( + 'new-session-id', 'new-session-id', ConversationInitData() + ) + await session_manager.join_conversation( + 'new-session-id', 'new-session-id', ConversationInitData() + ) assert session_instance.initialize_agent.call_count == 1 assert sio.enter_room.await_count == 2 @@ -178,7 +184,9 @@ async def test_join_cluster_session(): async with SessionManager( sio, AppConfig(), InMemoryFileStore() ) as session_manager: - await session_manager.join_conversation('new-session-id', 'new-session-id') + await session_manager.join_conversation( + 'new-session-id', 'new-session-id', ConversationInitData() + ) assert session_instance.initialize_agent.call_count == 0 assert sio.enter_room.await_count == 1 @@ -210,7 +218,9 @@ async def test_add_to_local_event_stream(): await session_manager.maybe_start_agent_loop( 'new-session-id', ConversationInitData() ) - await session_manager.join_conversation('new-session-id', 'connection-id') + await session_manager.join_conversation( + 'new-session-id', 'connection-id', ConversationInitData() + ) await session_manager.send_to_event_stream( 'connection-id', {'event_type': 'some_event'} ) @@ -241,7 +251,9 @@ async def test_add_to_cluster_event_stream(): async with SessionManager( sio, AppConfig(), InMemoryFileStore() ) as session_manager: - await session_manager.join_conversation('new-session-id', 'connection-id') + await session_manager.join_conversation( + 'new-session-id', 'connection-id', ConversationInitData() + ) await session_manager.send_to_event_stream( 'connection-id', {'event_type': 'some_event'} )