Separate data extraction and convo creation logic (#6298)

openhands/server/listen_socket.py

@@ -44,7 +44,8 @@ async def connect(connection_id: str, environ, auth):
 
         conversation_store = await ConversationStoreImpl.get_instance(config, user_id)
         metadata = await conversation_store.get_metadata(conversation_id)
-        if metadata.github_user_id != user_id:
+
+        if metadata.github_user_id != str(user_id):
             logger.error(
                 f'User {user_id} is not allowed to join conversation {conversation_id}'
             )

openhands/server/routes/manage_conversations.py

@@ -12,6 +12,7 @@
 from openhands.server.routes.settings import ConversationStoreImpl, SettingsStoreImpl
 from openhands.server.session.conversation_init_data import ConversationInitData
 from openhands.server.shared import config, session_manager
+from openhands.server.types import LLMAuthenticationError, MissingSettingsError
 from openhands.storage.data_models.conversation_info import ConversationInfo
 from openhands.storage.data_models.conversation_info_result_set import (
     ConversationInfoResultSet,
@@ -33,16 +34,12 @@ class InitSessionRequest(BaseModel):
     selected_repository: str | None = None
 
 
-@app.post('/conversations')
-async def new_conversation(request: Request, data: InitSessionRequest):
-    """Initialize a new session or join an existing one.
-    After successful initialization, the client should connect to the WebSocket
-    using the returned conversation ID
-    """
-    logger.info('Initializing new conversation')
-
+async def _create_new_conversation(
+    user_id: str | None,
+    token: str | None,
+    selected_repository: str | None,
+):
     logger.info('Loading settings')
-    user_id = get_user_id(request)
     settings_store = await SettingsStoreImpl.get_instance(config, user_id)
     settings = await settings_store.load()
     logger.info('Settings loaded')
@@ -54,25 +51,16 @@ async def new_conversation(request: Request, data: InitSessionRequest):
         # but that would run a tiny inference.
         if not settings.llm_api_key or settings.llm_api_key.isspace():
             logger.warn(f'Missing api key for model {settings.llm_model}')
-            return JSONResponse(
-                content={
-                    'status': 'error',
-                    'message': 'Error authenticating with the LLM provider. Please check your API key',
-                    'msg_id': 'STATUS$ERROR_LLM_AUTHENTICATION',
-                }
+            raise LLMAuthenticationError(
+                'Error authenticating with the LLM provider. Please check your API key'
             )
+
     else:
         logger.warn('Settings not present, not starting conversation')
-        return JSONResponse(
-            content={
-                'status': 'error',
-                'message': 'Settings not found',
-                'msg_id': 'CONFIGURATION$SETTINGS_NOT_FOUND',
-            }
-        )
-    github_token = getattr(request.state, 'github_token', '')
-    session_init_args['github_token'] = github_token or data.github_token or ''
-    session_init_args['selected_repository'] = data.selected_repository
+        raise MissingSettingsError('Settings not found')
+
+    session_init_args['github_token'] = token or ''
+    session_init_args['selected_repository'] = selected_repository
     conversation_init_data = ConversationInitData(**session_init_args)
     logger.info('Loading conversation store')
     conversation_store = await ConversationStoreImpl.get_instance(config, user_id)
@@ -85,7 +73,7 @@ async def new_conversation(request: Request, data: InitSessionRequest):
     logger.info(f'New conversation ID: {conversation_id}')
 
     repository_title = (
-        data.selected_repository.split('/')[-1] if data.selected_repository else None
+        selected_repository.split('/')[-1] if selected_repository else None
     )
     conversation_title = f'{repository_title or "Conversation"} {conversation_id[:5]}'
 
@@ -95,7 +83,7 @@ async def new_conversation(request: Request, data: InitSessionRequest):
             conversation_id=conversation_id,
             title=conversation_title,
             github_user_id=user_id,
-            selected_repository=data.selected_repository,
+            selected_repository=selected_repository,
         )
     )
 
@@ -112,7 +100,47 @@ async def new_conversation(request: Request, data: InitSessionRequest):
     except ValueError:
         pass  # Already subscribed - take no action
     logger.info(f'Finished initializing conversation {conversation_id}')
-    return JSONResponse(content={'status': 'ok', 'conversation_id': conversation_id})
+
+    return conversation_id
+
+
+@app.post('/conversations')
+async def new_conversation(request: Request, data: InitSessionRequest):
+    """Initialize a new session or join an existing one.
+    After successful initialization, the client should connect to the WebSocket
+    using the returned conversation ID
+    """
+    logger.info('Initializing new conversation')
+    user_id = get_user_id(request)
+    github_token = getattr(request.state, 'github_token', '') or data.github_token
+    selected_repository = data.selected_repository
+
+    try:
+        conversation_id = await _create_new_conversation(
+            user_id, github_token, selected_repository
+        )
+
+        return JSONResponse(
+            content={'status': 'ok', 'conversation_id': conversation_id}
+        )
+    except MissingSettingsError as e:
+        return JSONResponse(
+            content={
+                'status': 'error',
+                'message': str(e),
+                'msg_id': 'CONFIGURATION$SETTINGS_NOT_FOUND',
+            },
+            status_code=400,
+        )
+
+    except LLMAuthenticationError as e:
+        return JSONResponse(
+            content={
+                'status': 'error',
+                'message': str(e),
+                'msg_id': 'STATUS$ERROR_LLM_AUTHENTICATION',
+            },
+        )
 
 
 @app.get('/conversations')

openhands/server/types.py

@@ -35,3 +35,15 @@ async def verify_github_repo_list(self, installation_id: int | None) -> None:
     async def get_config(self) -> dict[str, str]:
         """Configure attributes for frontend"""
         raise NotImplementedError
+
+
+class MissingSettingsError(ValueError):
+    """Raised when settings are missing or not found."""
+
+    pass
+
+
+class LLMAuthenticationError(ValueError):
+    """Raised when there is an issue with LLM authentication."""
+
+    pass