- Notifications
You must be signed in to change notification settings - Fork1k
feat(agent/agentcontainers): auto detect dev containers#18950
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
coderabbitaibot commentedJul 21, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
WalkthroughThe changes introduce automatic discovery of devcontainer projects within agent directories, update the API initialization to support this, and expand manifest information handling. Tests are added for the discovery feature. UI adjustments include new Storybook scenarios, fallback display logic, conditional button labeling, debug logging, and refined control of app section visibility based on devcontainer status. A new CLI flag controls enabling or disabling devcontainer project discovery. Changes
Sequence Diagram(s)sequenceDiagram participant Agent as Agent participant ContainerAPI as Container API participant FS as Filesystem Agent->>ContainerAPI: Init(WithManifestInfo(..., agentDirectory)) ContainerAPI->>FS: On start, discoverDevcontainerProjects() FS-->>ContainerAPI: List directories, check for .git and devcontainer configs ContainerAPI->>ContainerAPI: Update knownDevcontainers mapEstimated code review effort4 (~90 minutes) Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
📓 Path-based instructions (1)**/*.go📄 CodeRabbit Inference Engine (.cursorrules)
Files:
✅ Files skipped from review due to trivial changes (2)
🧰 Additional context used📓 Path-based instructions (1)**/*.go📄 CodeRabbit Inference Engine (.cursorrules)
Files:
⏰ 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). (10)
🔇 Additional comments (3)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat withCodeRabbit:
SupportNeed help? Create a ticket on oursupport page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File ( |
@coderabbitai review |
✅ Actions performedReview triggered.
|
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: 3
🧹 Nitpick comments (3)
agent/agentcontainers/api.go (3)
393-398:Consider tracking discovery completion state.The discovery runs asynchronously without tracking completion. This could result in incomplete devcontainer lists being returned before discovery finishes.
Consider adding a discovery completion channel or atomic boolean to track when initial discovery is complete, similar to how
initDoneis used.
403-407:Improve error message clarity.The error message is misleading - it describes what was being checked rather than what failed.
Apply this diff to improve the error message:
-isGitProject, err := afero.DirExists(api.fs, filepath.Join(api.agentDirectory, "/.git"))-if err != nil {-return xerrors.Errorf(".git dir exists: %w", err)-}+isGitProject, err := afero.DirExists(api.fs, filepath.Join(api.agentDirectory, "/.git"))+if err != nil {+return xerrors.Errorf("check .git directory: %w", err)+}
429-432:Improve error message clarity.Same issue with misleading error message.
Apply this diff:
-isGitProject, err = afero.DirExists(api.fs, filepath.Join(api.agentDirectory, entry.Name(), ".git"))-if err != nil {-return xerrors.Errorf(".git dir exists: %w", err)-}+isGitProject, err = afero.DirExists(api.fs, filepath.Join(api.agentDirectory, entry.Name(), ".git"))+if err != nil {+return xerrors.Errorf("check .git directory in %s: %w", entry.Name(), err)+}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
agent/agent.go(1 hunks)agent/agentcontainers/api.go(8 hunks)agent/agentcontainers/api_test.go(4 hunks)site/src/modules/resources/AgentDevcontainerCard.stories.tsx(1 hunks)site/src/modules/resources/AgentDevcontainerCard.tsx(3 hunks)site/src/modules/resources/AgentRow.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
site/src/modules/resources/AgentRow.tsx (1)
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.tsx : Use functional updates (setX(prev ⇒ …)) whenever next state depends on previous state.
site/src/modules/resources/AgentDevcontainerCard.tsx (4)
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.tsx : Group related Tailwind classes
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.tsx : Responsive design - use Tailwind's responsive prefixes (sm:, md:, lg:, xl:)
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.tsx : Use className with clsx for conditional styling
Learnt from: CR
PR: coder/coder#0
File: site/CLAUDE.md:0-0
Timestamp: 2025-07-21T14:33:50.919Z
Learning: Applies to site/src/**/*.tsx : Use Tailwind classes for all new styling
agent/agentcontainers/api.go (4)
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.035Z
Learning: Applies to **/*.go : Coder emphasizes clear error handling, with specific patterns required: Concise error messages that avoid phrases like "failed to"; Wrapping errors with%w to maintain error chains; Using sentinel errors with the "err" prefix (e.g.,errNotFound).
Learnt from: CR
PR: coder/coder#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:32:56.474Z
Learning: Applies to **/*.go : OAuth2-compliant error responses must use writeOAuth2Error in Go code
Learnt from: CR
PR: coder/coder#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:32:56.474Z
Learning: Applies to **/*.go : Follow Uber Go Style Guide
Learnt from: CR
PR: coder/coder#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:32:56.474Z
Learning: Applies to **/*_test.go : Use unique identifiers in concurrent Go tests to prevent race conditions (e.g., fmt.Sprintf with t.Name() and time.Now().UnixNano())
agent/agentcontainers/api_test.go (6)
Learnt from: CR
PR: coder/coder#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:32:56.474Z
Learning: Applies to **/*_test.go : Never use hardcoded names in concurrent Go tests
Learnt from: CR
PR: coder/coder#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:32:56.474Z
Learning: Applies to **/*_test.go : Use unique identifiers in concurrent Go tests to prevent race conditions (e.g., fmt.Sprintf with t.Name() and time.Now().UnixNano())
Learnt from: CR
PR: coder/coder#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:32:56.474Z
Learning: Applies to **/*.go : Follow Uber Go Style Guide
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.034Z
Learning: Applies to coderd/coderd.go : The REST API is defined incoderd/coderd.go and uses Chi for HTTP routing.
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.035Z
Learning: Applies to **/*_test.go : All tests must uset.Parallel() to run concurrently, which improves test suite performance and helps identify race conditions.
Learnt from: CR
PR: coder/coder#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T14:32:43.035Z
Learning: Applies to **/*_test.go : All tests should run in parallel usingt.Parallel() to ensure efficient testing and expose potential race conditions.
🪛 GitHub Check: lint
agent/agentcontainers/api.go
[failure] 270-270:
import-shadowing: The name 'fs' shadows an import name (revive)
[failure] 452-452:
unused-parameter: parameter 'info' seems to be unused, consider removing or renaming it as _ (revive)
⏰ 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). (8)
- GitHub Check: test-e2e
- GitHub Check: chromatic
- GitHub Check: test-go-pg (windows-2022)
- GitHub Check: test-go-pg (macos-latest)
- GitHub Check: test-go-race-pg
- GitHub Check: test-go-pg-17
- GitHub Check: test-go-pg (ubuntu-latest)
- GitHub Check: gen
🔇 Additional comments (11)
site/src/modules/resources/AgentDevcontainerCard.stories.tsx (1)
94-103:LGTM! Good addition for edge case testing.The new Storybook story appropriately tests the UI behavior when both container and agent are undefined, which is valuable for comprehensive component testing.
site/src/modules/resources/AgentDevcontainerCard.tsx (2)
223-224:Good fallback logic for devcontainer display name.The updated logic properly falls back to
config_pathwhen bothsubAgent?.nameanddevcontainer.nameare unavailable, providing better UX for auto-detected devcontainers.
259-259:Excellent conditional button labeling.The conditional button text ("Start" vs "Rebuild") based on container presence provides clear user intent and aligns well with the devcontainer lifecycle.
agent/agent.go (1)
1171-1171:Appropriate extension of manifest information.Adding
manifest.Directoryto theWithManifestInfocall properly supports the devcontainer auto-detection feature by providing the necessary filesystem context for discovery.site/src/modules/resources/AgentRow.tsx (1)
140-151:Excellent refinement of app visibility logic.The updated condition properly narrows when to hide parent apps - only when devcontainers are actively "running" or "starting". This provides better UX by showing parent apps when all devcontainers are stopped, while still reducing clutter when devcontainers are active. The inline comments clearly explain the reasoning.
agent/agentcontainers/api_test.go (3)
23-23:LGTM: Appropriate import for filesystem testingThe afero package import is necessary for the new
TestDevcontainerDiscoverytest function which uses in-memory filesystem operations.
1682-1682:LGTM: Consistent updates to WithManifestInfo callsAll three calls to
WithManifestInfohave been correctly updated with the new agent directory parameter ("/parent-agent"), aligning with the updated function signature.Also applies to: 1666-1666, 3367-3367
3194-3423:Well-structured test for devcontainer discovery functionalityThe test comprehensively validates the automatic devcontainer discovery feature with:
- Table-driven test cases covering various directory structures
- Proper use of
t.Parallel()as per codebase conventions- In-memory filesystem for isolated testing
- Deterministic result sorting for reliable assertions
- Appropriate use of
Eventuallypattern for async operationsThe test effectively exercises the new discovery logic that scans agent directories for Git repositories containing devcontainer configurations.
agent/agentcontainers/api.go (3)
346-348:LGTM!Good defensive initialization ensuring the filesystem is always set.
462-478:No issues found with handling incomplete devcontainer entries in the update loop.The
processUpdatedContainersLockedmethod explicitly resets and then populates each devcontainer’sNameandStatus—defaulting to “Stopped” (or “Running” when appropriate) and generating a valid name based on the workspace folder or container friendly name when a container appears. This ensures there’s no race on uninitialized entries.
199-206:AllWithManifestInfoCallers UpdatedAll usages of
WithManifestInfoinagent/agent.goandagent/agentcontainers/api_test.goinclude the newagentDirectoryargument. No further changes are required.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Project discovery doesn't work well with these tests do not seteverything up requried for this feature to work.
We make sure to only run project discovery when the agent directory isnot empty. If we attempt to do project discovery when the agentdirectory is empty we will just get an error.
This makes the experience nicer when starting the workspace. It makesfound devcontainers become visible on the website sooner and at a moreconsistent speed (as it no longer relies on the updaterLoop).
@coderabbitai review |
✅ Actions performedReview triggered.
|
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.
Pull Request Overview
This PR implements automatic detection of devcontainer projects within agent directories by adding filesystem-based discovery capabilities. The system searches for Git repositories and recursively scans them for.devcontainer/devcontainer.json files to automatically configure devcontainer environments.
- Added project discovery functionality that searches for Git repositories and devcontainer configurations in agent directories
- Enhanced UI logic to better handle devcontainer states and display appropriate actions
- Updated tests to disable project discovery where needed to maintain existing behavior
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
agent/agentcontainers/api.go | Core implementation of project discovery with filesystem scanning and Git repository detection |
agent/agentcontainers/api_test.go | Comprehensive tests for devcontainer discovery using simulated filesystems |
agent/agent.go | Integration of project discovery into the main agent initialization |
site/src/modules/resources/AgentRow.tsx | Improved logic for showing/hiding parent apps based on devcontainer status |
site/src/modules/resources/AgentDevcontainerCard.tsx | Enhanced UI to show config path as fallback name and dynamic button text |
site/src/modules/resources/AgentDevcontainerCard.stories.tsx | New Storybook scenario for devcontainers without containers or agents |
| Multiple test files | Updated to disable project discovery to maintain test isolation |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
agent/agent.go Outdated
| containerAPIOpts:= []agentcontainers.Option{ | ||
| agentcontainers.WithExecer(a.execer), | ||
| agentcontainers.WithCommandEnv(a.sshServer.CommandEnv), | ||
| agentcontainers.WithProjectDiscovery(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.
suggestion: It might be good to have an 'off switch' for this. Could be a separate PR though.
DanielleMaywoodJul 22, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
I'm happy to include it in this PR. If you're happy with this, I'll make it an environment variable set on the agent similar toCODER_AGENT_DEVCONTAINERS_ENABLE?
So it would beCODER_AGENT_DEVCONTAINERS_PROJECT_DISCOVERY_ENABLE
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.
Yeah that sounds fine to me!
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.
Have added this ina5ec3c4 (and verified it works)
f41275e intomainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Relates tocoder/internal#711
This PR implements a project discovery mechanism that searches for any dev container projects and makes them visible in the UI so that they can be started. To make the wording on the site more clear, "Rebuild" has been changed to "Start" when there is no container associated with a known dev container configuration. I've also made it so that site will show the dev container config path when there is no other name available.
Design decisions
Just want to ensure my explanation for a few design decisions are noted down:
This limited approach is to reduce the amount of files we ultimately walk when trying to find these projects. It makes sense to limit it to only the agent directory, although I'm open to expanding how deep we search.
Follow up work
To close the linked issue, we should make it possible to auto start these found projects if their
customization.coderstanza has"autostart": true. I felt that change was separate enough that it should be in a different PR.Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation