fix(devcontainer): fixed devcontainers#1614
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
There was a problem hiding this comment.
Greptile Overview
Summary
This PR refactors the devcontainer configuration to improve portability, modularity, and documentation.
Key changes:
- Migrated from Debian-based to Alpine Linux base image (smaller footprint)
- Pinned Bun version to 1.2.22 for consistency
- Separated project commands into
sim-commands.shfile, removed monolithic.bashrc - Added memory limits to services (8GB for app, 4GB for realtime)
- Removed service health checks and inter-service dependencies (services now start independently)
- Added additional environment variables with sensible defaults
- Improved documentation with clearer instructions for running services separately or together
- Added guidance for personal shell customization using VS Code dotfiles feature
Architecture improvements:
- Services can now run independently using
sim-apporsim-sockets, or together usingsim-start - Better separation between project-specific commands and personal shell preferences
- Runtime installation of global packages instead of baking into image
Confidence Score: 3/5
- This PR is moderately safe to merge but requires testing in the devcontainer environment to ensure all changes work correctly
- Score of 3 reflects infrastructure changes that improve the codebase but have identified issues: (1) Dockerfile user creation logic has a potential edge case bug that could fail if USERNAME doesn't match existing user, (2) removed service health checks and dependencies could cause race conditions if not carefully managed, (3) significant architectural changes to how services start need validation. The changes are well-intentioned improvements (Alpine migration, better modularity, clearer docs) but the devcontainer is critical infrastructure that needs testing.
- Pay close attention to
.devcontainer/Dockerfile(user creation logic) and.devcontainer/docker-compose.yml(removed dependencies and health checks)
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| .devcontainer/Dockerfile | 4/5 | Migrated from Debian-based to Alpine Linux, pinned Bun version to 1.2.22, simplified user creation with Alpine's adduser/addgroup commands |
| .devcontainer/devcontainer.json | 5/5 | Removed terminal profile configuration, removed postStartCommand and features section, added port 3002 forwarding for socket server |
| .devcontainer/docker-compose.yml | 3/5 | Added memory limits, environment variables for configuration, removed healthchecks from app/realtime services, made realtime service independent |
| .devcontainer/post-create.sh | 4/5 | Refactored to source separate sim-commands.sh file, moved global package installation to runtime, improved shell configuration setup for both bash and zsh |
Sequence Diagram
sequenceDiagram
participant User
participant VSCode
participant Docker
participant AppContainer
participant RealtimeContainer
participant DBContainer
participant MigrationsContainer
User->>VSCode: Open in Dev Container
VSCode->>Docker: Build Dockerfile (Alpine + Bun 1.2.22)
Docker-->>VSCode: Image built
VSCode->>Docker: Start docker-compose services
Docker->>DBContainer: Start PostgreSQL (pgvector)
DBContainer->>DBContainer: Health check (pg_isready)
Docker->>MigrationsContainer: Start migrations
MigrationsContainer->>DBContainer: Wait for healthy
DBContainer-->>MigrationsContainer: Ready
MigrationsContainer->>DBContainer: Run bun run db:migrate
MigrationsContainer-->>Docker: Complete
Docker->>AppContainer: Start app container (sleep infinity)
AppContainer->>DBContainer: Wait for healthy
AppContainer->>MigrationsContainer: Wait for completion
Docker->>RealtimeContainer: Start realtime container (sleep infinity)
RealtimeContainer->>DBContainer: Wait for healthy
AppContainer->>AppContainer: Run post-create.sh
AppContainer->>AppContainer: Install global packages (turbo, drizzle-kit)
AppContainer->>AppContainer: Setup sim-commands.sh in shell rc
AppContainer->>AppContainer: Run bun install
AppContainer->>AppContainer: Generate Drizzle schema
AppContainer->>DBContainer: Push schema changes
AppContainer-->>VSCode: Setup complete
VSCode-->>User: Container ready
User->>AppContainer: Run sim-start
AppContainer->>AppContainer: Start Next.js app (port 3000)
AppContainer->>RealtimeContainer: Start socket server (port 3002)
AppContainer-->>User: Services running
7 files reviewed, 2 comments
Comment on lines
26
to
30
| depends_on: | ||
| db: | ||
| condition: service_healthy | ||
| realtime: | ||
| condition: service_healthy | ||
| migrations: | ||
| condition: service_completed_successfully |
Contributor
There was a problem hiding this comment.
logic: Removed dependency on realtime service. If the app tries to connect to the socket server at startup, this could cause connection failures since there's no guarantee the realtime service is ready.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .devcontainer/docker-compose.yml
Line: 26:30
Comment:
**logic:** Removed dependency on `realtime` service. If the app tries to connect to the socket server at startup, this could cause connection failures since there's no guarantee the realtime service is ready.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+31
to
+33
| RUN if ! getent passwd $USER_UID >/dev/null; then \ | ||
| adduser -D -u $USER_UID -G $(getent group $USER_GID | cut -d: -f1) $USERNAME; \ | ||
| fi |
Contributor
There was a problem hiding this comment.
logic: User creation may fail if group GID 1000 exists but with a different name. The $(getent group $USER_GID | cut -d: -f1) will return the existing group's name, not $USERNAME. Consider using -G $USERNAME or creating the group with a unique name first.
Suggested change
| RUN if ! getent passwd $USER_UID >/dev/null; then \ | |
| adduser -D -u $USER_UID -G $(getent group $USER_GID | cut -d: -f1) $USERNAME; \ | |
| fi | |
| RUN if ! getent passwd $USER_UID >/dev/null; then \ | |
| adduser -D -u $USER_UID -G $USERNAME $USERNAME; \ | |
| fi |
Prompt To Fix With AI
This is a comment left during a code review.
Path: .devcontainer/Dockerfile
Line: 31:33
Comment:
**logic:** User creation may fail if group GID 1000 exists but with a different name. The `$(getent group $USER_GID | cut -d: -f1)` will return the existing group's name, not `$USERNAME`. Consider using `-G $USERNAME` or creating the group with a unique name first.
```suggestion
RUN if ! getent passwd $USER_UID >/dev/null; then \
adduser -D -u $USER_UID -G $USERNAME $USERNAME; \
fi
```
How can I resolve this? If you propose a fix, please make it concise.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
fixed devcontainers, updated documentation
Fixes #1557
Type of Change
Testing
Tested building manually
Checklist