-
Notifications
You must be signed in to change notification settings - Fork 0
fix: Attempt to fix dev container pnpm install #649
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
|
WalkthroughAdds a named Docker volume for node_modules in the devcontainer and updates the post-create script to chown the node_modules directory to vscode. No other logic or configuration changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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.
Actionable comments posted: 1
🧹 Nitpick comments (3)
.devcontainer/post-create.sh (2)
6-6: Prevent failure when node_modules is absent; ensure recursive ownership.
set -ewill exit ifnode_modulesdoesn't exist yet. Also, use-Rso previously cached contents are owned correctly.Apply:
-sudo chown vscode:vscode node_modules +sudo mkdir -p node_modules +sudo chown -R vscode:vscode node_modules
12-15: Sanity-check working directory before install.The script assumes it runs at the workspace root. Add a defensive
cdto avoid chowning the wrong path if the cwd changes.Example:
set -e +cd "$(dirname "$0")/..".devcontainer/devcontainer.json (1)
48-53: Use in-container postStartCommand to verify mounts and ownership via /proc/self/mounts
Switch to checking/proc/self/mounts(nomountbinary in the container), ensurenode_modulesexists before runningstat, and hook this intopostStartCommandto immediately catch misconfigurations.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.devcontainer/devcontainer.json(1 hunks).devcontainer/post-create.sh(1 hunks)
⏰ 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). (2)
- GitHub Check: Lint
- GitHub Check: Test E2E
|
|
||
| "mounts": [ | ||
| "source=${localWorkspaceFolderBasename}-pnpm,target=/home/vscode/.local/share/pnpm,type=volume", | ||
| "source=${localWorkspaceFolderBasename}-node_modules,target=${localWorkspaceFolder}/node_modules,type=volume", |
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.
Wrong target variable: mounts to a host-like path inside container.
target=${localWorkspaceFolder}/node_modules resolves to a host path string and will not match the workspace path inside the container. This creates a second, unexpected path and breaks the post-create.sh chown against ./node_modules.
Use the container path:
- "source=${localWorkspaceFolderBasename}-node_modules,target=${localWorkspaceFolder}/node_modules,type=volume",
+ "source=${localWorkspaceFolderBasename}-node_modules,target=${containerWorkspaceFolder}/node_modules,type=volume",Optional (macOS performance):
- "source=${localWorkspaceFolderBasename}-node_modules,target=${containerWorkspaceFolder}/node_modules,type=volume",
+ "source=${localWorkspaceFolderBasename}-node_modules,target=${containerWorkspaceFolder}/node_modules,type=volume,consistency=cached",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "source=${localWorkspaceFolderBasename}-node_modules,target=${localWorkspaceFolder}/node_modules,type=volume", | |
| "source=${localWorkspaceFolderBasename}-node_modules,target=${containerWorkspaceFolder}/node_modules,type=volume", |
🤖 Prompt for AI Agents
In .devcontainer/devcontainer.json around line 50, the mount uses
target=${localWorkspaceFolder}/node_modules which resolves to a host path and
creates a duplicate path inside the container; change the target to the
container workspace variable (e.g. ${containerWorkspaceFolder}/node_modules) so
the volume mounts to the workspace path inside the container and fixes
post-create chown; optionally add macOS performance options (delegated) to the
mount if needed.
Summary by CodeRabbit
New Features
Bug Fixes
Chores
Documentation