Skip to content

Commit

Permalink
Fix for settings update (All-Hands-AI#5858)
Browse files Browse the repository at this point in the history
  • Loading branch information
rbren authored Dec 27, 2024
1 parent 9bdc1df commit 97b1867
Show file tree
Hide file tree
Showing 9 changed files with 77 additions and 77 deletions.
18 changes: 15 additions & 3 deletions frontend/public/mockServiceWorker.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down Expand Up @@ -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 })
}
Expand Down
2 changes: 0 additions & 2 deletions frontend/src/api/open-hands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -250,14 +250,12 @@ class OpenHands {

static async newConversation(params: {
githubToken?: string;
args?: Record<string, unknown>;
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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", {
Expand All @@ -101,8 +101,8 @@ export function SettingsForm({
});
};

const handleConfirmResetSettings = () => {
saveSettings(getDefaultSettings());
const handleConfirmResetSettings = async () => {
await saveSettings(getDefaultSettings());
resetOngoingSession();
posthog.capture("settings_reset");

Expand Down
3 changes: 0 additions & 3 deletions frontend/src/components/shared/task-form.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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,
Expand All @@ -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 }) => {
Expand Down
8 changes: 4 additions & 4 deletions frontend/src/context/settings-context.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -33,8 +33,8 @@ function SettingsProvider({ children }: React.PropsWithChildren) {
);
const queryClient = useQueryClient();

const saveSettings = (newSettings: Partial<Settings>) => {
updateAndSaveSettingsToLocalStorage(newSettings);
const handleSaveSettings = async (newSettings: Partial<Settings>) => {
await saveSettings(newSettings);
queryClient.invalidateQueries({ queryKey: SETTINGS_QUERY_KEY });
setSettingsAreUpToDate(checkIfSettingsAreUpToDate());
};
Expand All @@ -49,7 +49,7 @@ function SettingsProvider({ children }: React.PropsWithChildren) {
() => ({
settings,
settingsAreUpToDate,
saveSettings,
saveSettings: handleSaveSettings,
}),
[settings, settingsAreUpToDate],
);
Expand Down
13 changes: 11 additions & 2 deletions openhands/server/listen_socket.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand All @@ -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')
Expand Down
12 changes: 5 additions & 7 deletions openhands/server/session/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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):
Expand All @@ -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:
Expand Down
66 changes: 20 additions & 46 deletions openhands/server/session/session.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import asyncio
import json
import time
from copy import deepcopy

Expand All @@ -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}'

Expand Down Expand Up @@ -65,70 +63,46 @@ 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)

llm = LLM(config=self.config.get_llm_config_from_agent(agent_cls))
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,
Expand All @@ -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}')
Expand Down
24 changes: 18 additions & 6 deletions tests/unit/test_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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'}
)
Expand Down Expand Up @@ -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'}
)
Expand Down

0 comments on commit 97b1867

Please sign in to comment.