refactor: move session initialization from WebSocket to REST API #5493
+427
−254
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Add POST /api/conversation endpoint for session initialization - Update frontend to use new endpoint instead of WebSocket INIT event - Remove WebSocket INIT event handling from backend
… and rename middleware
- Rename AttachSessionMiddleware to AttachConversationMiddleware
- Move protected endpoints under /api/conversation/{convo_id}/...
- Keep POST /api/conversation endpoint separate (without middleware)
- Add conversation context and update frontend to handle new URL structure
- Add session initialization in ChatInterface when sending first message - Add loading state while initializing session - Disable chat input during session initialization
- Move session initialization to TaskForm where users start new conversations - Add loading state while initializing session - Revert ChatInterface changes as it shouldn't handle session initialization
- Create new session.py router for POST /api/conversation endpoint - Move session initialization code to new router - Fix middleware application to use protected_router instead of main router - Add comment to clarify middleware application
- Remove session initialization from WsClientProvider - Add conversation ID to websocket path - Update backend to validate conversation ID in websocket path - Store conversation ID in socket session data
- Move ConversationProvider to root component to fix useConversation hook - Remove ConversationProvider from App component - Fix TypeScript errors in build
- Update route path to match API structure - Update TaskForm to navigate to new URL - Update App component to get conversation ID from URL - Update all components that referenced /app path
- Generate conversation ID using UUID in backend - Return conversation_id in session init response - Update frontend to use conversation_id from response - Remove JWT token parsing in frontend
- Use existing session ID logic from main branch - Add auth data to websocket connection - Add token and GitHub token validation in websocket connect handler - Verify conversation ID matches token in websocket connection
amanape
reviewed
Dec 11, 2024
Comment on lines
+76
to
+97
| try { | ||
| setIsInitializing(true); | ||
| // Initialize the session before navigating | ||
| const { conversation_id: conversationId } = await OpenHands.initSession({ | ||
| githubToken: gitHubToken || undefined, | ||
| selectedRepository: selectedRepository || undefined, | ||
| args: settings || undefined, | ||
| }); | ||
|
|
||
| navigate("/app"); | ||
| posthog.capture("initial_query_submitted", { | ||
| entry_point: "task_form", | ||
| query_character_length: q.length, | ||
| has_repository: !!selectedRepository, | ||
| has_files: files.length > 0, | ||
| }); | ||
|
|
||
| navigate(`/conversation/${conversationId}`); | ||
| } catch (error) { | ||
| // TODO: Show error toast | ||
| } finally { | ||
| setIsInitializing(false); | ||
| } |
There was a problem hiding this comment.
We could move this into a useMutation hook: https://tanstack.com/query/latest/docs/framework/react/guides/mutations
| @@ -173,7 +150,7 @@ export function WsClientProvider({ | |||
| sio.off("connect_failed", handleError); | |||
| sio.off("disconnect", handleDisconnect); | |||
| }; | |||
| }, [enabled, token, ghToken, selectedRepository]); | |||
| }, [enabled, ghToken, selectedRepository]); | |||
There was a problem hiding this comment.
Wouldn't we want the effect to trigger when conversationId changes instead of token?
There was a problem hiding this comment.
This looks like it is misplaced
| if (token) navigate("/app"); | ||
| // If we have a token but no conversation ID, we need to initialize a new session | ||
| // This will be handled by the TaskForm | ||
| }, [location.pathname]); |
There was a problem hiding this comment.
Might as well remove the entire useEffect statement
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently the frontend starts a session by connecting to the websocket and then sending an INIT event with initialization data. This PR changes that to use a REST API endpoint instead.
Changes
/api/conversation/{id}/...Resolves #4281
To run this PR locally, use the following command: