feat(devcontainer): import external --devcontainer configs into the workspace#717
Conversation
…orkspace Passing --devcontainer with a path to a config outside the repo previously flattened it to a single synthesized file at the repo root, which broke self-contained configs: a relative Dockerfile/feature/compose path resolves against the config's directory, so the sibling assets were lost. Import the external config into the workspace as a native named profile (.devcontainer/<binary>/) instead: - A self-contained "<folder>/devcontainer.json" is copied whole so its sibling assets travel with it; a bare file is copied on its own. - The profile is discovered natively (.devcontainer/<name>/devcontainer.json), so config-relative paths resolve. - The imported dir is added to .git/info/exclude so it does not dirty git status, and carries a marker file so teardown can identify it. A collision with a project-owned .devcontainer/<binary>/ falls back to a suffixed dir. - runner.Delete removes the imported profile for local-folder sources (alongside cleanupDeliveryVolume), so both up-delete and ci teardown clean up through one path.
✅ Deploy Preview for images-devsy-sh canceled.
|
✅ Deploy Preview for devsydev canceled.
|
|
Warning Review limit reached
Next review available in: 35 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAbsolute external devcontainer paths now pass through workspace resolution. External configurations are copied into marked workspace-local profiles, optionally excluded from Git, parsed, and cleaned up during local workspace deletion. ChangesExternal Devcontainer Lifecycle
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant SourcePath
participant DevcontainerRunner
participant WorkspaceFilesystem
participant ConfigParser
SourcePath->>DevcontainerRunner: provide external devcontainer path
DevcontainerRunner->>WorkspaceFilesystem: copy files into imported profile
DevcontainerRunner->>WorkspaceFilesystem: write marker and Git exclude entry
DevcontainerRunner->>ConfigParser: parse copied devcontainer.json
ConfigParser-->>DevcontainerRunner: return imported configuration
DevcontainerRunner->>WorkspaceFilesystem: remove marked profiles during Delete
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
pkg/devcontainer/config.go (1)
247-252: 🔒 Security & Privacy | 🔵 Trivial | 🏗️ Heavy lift"Self-contained" detection copies the entire sibling directory unfiltered.
Any single sibling entry marks the config as self-contained and triggers a full recursive copy of
filepath.Dir(srcPath)viacopypkg.Directory. If an external--devcontainerpath lives outside a dedicated.devcontainer/folder (e.g. at a project root), this can pull in large trees, VCS metadata, or credentials into the workspace — andexcludeFromGitis best-effort only, so nothing guarantees these stay untracked. Consider narrowing the heuristic (e.g. only copy.devcontainer-adjacent files/known devcontainer asset patterns) or documenting the risk.Also applies to: 268-281
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/devcontainer/config.go` around lines 247 - 252, Restrict isSelfContainedDevContainer and the surrounding copy flow so a self-contained configuration is recognized only within a dedicated devcontainer directory or approved devcontainer asset set. Do not recursively copy filepath.Dir(srcPath) via copypkg.Directory for external paths such as a project-root configuration; copy only the required configuration and related assets instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/devcontainer/config.go`:
- Around line 206-209: Preserve the original srcPath value before the
filepath.Abs call in the surrounding devcontainer path-resolution logic, then
use that preserved value in the error returned by the Abs failure branch. Keep
the successful resolved path assigned to srcPath and continue wrapping the
original error.
---
Nitpick comments:
In `@pkg/devcontainer/config.go`:
- Around line 247-252: Restrict isSelfContainedDevContainer and the surrounding
copy flow so a self-contained configuration is recognized only within a
dedicated devcontainer directory or approved devcontainer asset set. Do not
recursively copy filepath.Dir(srcPath) via copypkg.Directory for external paths
such as a project-root configuration; copy only the required configuration and
related assets instead.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dd1e0828-8b72-4019-8937-af395fdb5cd6
📒 Files selected for processing (6)
cmd/workspace/up/up_devcontainer_source_test.gocmd/workspace/up/up_validate.gopkg/devcontainer/config.gopkg/devcontainer/delete.gopkg/devcontainer/delete_test.gopkg/devcontainer/import_external_test.go
Address review feedback: - Only treat an external config as self-contained (folder copy) when it lives in a dedicated .devcontainer directory or a .devcontainer/<name> profile, matching the spec's layout. A config at a project root now copies just the file, so it can't pull in arbitrary sibling trees (secrets, .git, ...). - Preserve the original srcPath in the filepath.Abs error message instead of the empty string Abs returns on failure.
1 participant
Summary
When
--devcontainer <path>points at a devcontainer config outside the repo, devsy previously flattened it to a single synthesized file at the repo root. That broke self-contained configs: a relativedockerfile/features/dockerComposeFileresolves against the config's own directory, so those sibling assets were lost, and the write also left the checkout with a dirtygit status.This imports the external config into the workspace as a native named devcontainer profile (
.devcontainer/<binary>/) instead:"<folder>/devcontainer.json"is copied whole, so its sibling assets (Dockerfile, features, compose) travel with it and config-relative paths resolve. A bare file is copied on its own..devcontainer/<name>/devcontainer.jsonmechanism — no specialOriginplumbing..git/info/excludeso it doesn't dirtygit status(local, reversible, never touches the external source repo)..devcontainer/<binary>/falls back to a suffixed dir (<binary>_<rand>).runner.Deleteremoves the imported profile for local-folder sources (alongsidecleanupDeliveryVolume), so bothup-delete and (future)citeardown clean up through one lifecycle path.Behavior
--devcontainerexternal-path routing: an in-repo relative path stays a config path (DevContainerPath); an external absolute path is left inDevContainerSourceso the runner imports it.none/image:<ref>are unchanged.Tests
pkg/devcontainer: self-contained folder (Dockerfile resolves), bare file, marker written, collision→suffix, re-import reuses own dir, git-exclude added + non-git-repo skip,runner.Deleteremoves the imported profile / no-ops for non-local sources, and cleanup preserves project-owned profiles. Plus an external-path pass-through case in theupsource-resolution test.Notes
devsy ciPR (feat(ci): add devsy ci command for devcontainer CI #603); that branch rebases on top.maindirectly and touches onlypkg/devcontainer+upsource resolution — noci-specific code.Summary by CodeRabbit
New Features
Bug Fixes
Cleanup