-
Notifications
You must be signed in to change notification settings - Fork 492
feat: add support for external server mode with Docker integration #376
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
Conversation
- Introduced a new `docker-compose.dev-server.yml` for running the backend API in a container, enabling local Electron to connect to it. - Updated `dev.mjs` to include a new option for launching the Docker server container. - Enhanced the UI application to support external server mode, allowing session-based authentication and adjusting routing logic accordingly. - Added utility functions to check and cache the external server mode status for improved performance. - Updated various components to handle authentication and routing based on the server mode.
📝 WalkthroughWalkthroughThis pull request introduces external Docker server mode support to the Electron application, enabling it to connect to a backend API running in a Docker container instead of using an embedded server. The changes include detection mechanisms via IPC, conditional control flow to skip embedded server startup, modified authentication for session-based auth, and developer tooling to run the Docker container locally. Changes
Sequence Diagram(s)sequenceDiagram
participant Electron Main
participant Renderer
participant HTTP Client
participant External Server
participant IPC Bridge
Electron Main->>Electron Main: Check SKIP_EMBEDDED_SERVER
alt External Server Mode
Electron Main->>External Server: Poll /api/health
External Server-->>Electron Main: 503 (not ready)
Note over Electron Main: Retry with backoff
External Server-->>Electron Main: 200 (healthy)
Electron Main->>Renderer: Create window<br/>Set PORT/VITE_SERVER_URL env vars
Renderer->>IPC Bridge: window.electronAPI.isExternalServerMode()
IPC Bridge->>Electron Main: ipcMain.invoke('auth:isExternalServerMode')
Electron Main-->>IPC Bridge: true
IPC Bridge-->>Renderer: Promise resolves true
Renderer->>HTTP Client: checkExternalServerMode()
HTTP Client->>IPC Bridge: window.electronAPI.isExternalServerMode()
IPC Bridge-->>HTTP Client: true
HTTP Client->>HTTP Client: Cache result
HTTP Client-->>Renderer: true
Renderer->>Renderer: Set needsSessionAuth = true
Renderer->>Renderer: Skip to /login<br/>(session-based auth)
Renderer->>External Server: API requests to localhost:3008
External Server-->>Renderer: Responses
else Default Mode (Embedded Server)
Electron Main->>Electron Main: Generate/Load API key
Electron Main->>Electron Main: Find available port
Electron Main->>Electron Main: Start embedded server
Electron Main->>Renderer: Create window
Renderer->>Renderer: Set needsSessionAuth = false
Renderer->>Renderer: Assume API-key auth ready
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (7)
🧰 Additional context used📓 Path-based instructions (2)**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (1)📚 Learning: 2025-12-30T01:02:07.114ZApplied to files:
🧬 Code graph analysis (4)apps/ui/src/main.ts (2)
apps/ui/src/routes/__root.tsx (3)
dev.mjs (1)
scripts/launcher-utils.mjs (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (14)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @webdevcody, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the development workflow by introducing an "external server mode" for the Electron application. This mode allows developers to run the backend API within a Docker container while the Electron UI runs locally, connecting to the containerized backend. This setup facilitates a more isolated and consistent development environment for the backend, while still leveraging the rapid iteration capabilities of local Electron development. The changes include new Docker Compose configurations, updates to the Electron application's authentication and routing logic to accommodate this new mode, and modifications to development scripts for easy launching. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a new external server mode, allowing the Electron app to connect to a backend running in a Docker container. The changes are well-structured, including a new Docker Compose file for the server, updates to the development launcher scripts, and modifications to the Electron main and renderer processes to handle the new authentication and routing logic.
My review focuses on improving maintainability and type safety. I've pointed out a few areas for improvement:
- In
apps/ui/src/routes/__root.tsx, there's some duplicated logic that can be refactored. - In
scripts/launcher-utils.mjs, a hardcoded port number could be extracted into a constant. - In
apps/ui/src/lib/http-api-client.ts, a type assertionas anyis used, which could be avoided by updating the relevant type definition.
Overall, this is a solid implementation of a useful new development feature.
| const api = window.electronAPI as any; | ||
| if (api?.isExternalServerMode) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The use of as any to access isExternalServerMode indicates that the type definition for window.electronAPI (likely ElectronAPI) is missing this new property. While this works, it bypasses TypeScript's type safety. To improve maintainability and type safety, the ElectronAPI interface should be updated to include the optional isExternalServerMode method. This would remove the need for the type assertion.
| // Check if we need session-based auth (web mode OR external server mode) | ||
| const needsSessionAuth = !isElectronMode() || isExternalServerMode() === true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
|
||
| // Wait for the server to become healthy | ||
| log('Waiting for server to be ready...', 'yellow'); | ||
| const serverPort = 3008; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The port number 3008 is hardcoded here. This value is also used when launching Electron on lines 987-988. Using a hardcoded "magic number" in multiple places makes the code harder to maintain.
To improve maintainability, please define a constant for this port at the top of the launchDockerDevServerContainer function and use it in all relevant places.
|
I'll keep an eye on this cve |
docker-compose.dev-server.ymlfor running the backend API in a container, enabling local Electron to connect to it.dev.mjsto include a new option for launching the Docker server container.Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.