Skip to content

fix(agent): disable dev container integration inside sub agents #18781

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Jul 8, 2025
+28 −23
Merged

fix(agent): disable dev container integration inside sub agents #18781

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
801585a
fix(agent): ensure dev container integration disabled inside of sub a…
DanielleMaywood Jul 7, 2025
12f3dc7
test: ensure devcontainers = true for better testing
DanielleMaywood Jul 7, 2025
36c276f
chore: listen to copilot
DanielleMaywood Jul 7, 2025
15b38be
Merge branch 'main' into danielle/stop-devcontainer-in-devcontainer
DanielleMaywood Jul 8, 2025
bb48815
fix: update error check
DanielleMaywood Jul 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 13 additions & 17 deletions agent/agent.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,18 +336,16 @@ func (a *agent) init() {
// will not report anywhere.
a.scriptRunner.RegisterMetrics(a.prometheusRegistry)

if a.devcontainers {
containerAPIOpts := []agentcontainers.Option{
agentcontainers.WithExecer(a.execer),
agentcontainers.WithCommandEnv(a.sshServer.CommandEnv),
agentcontainers.WithScriptLogger(func(logSourceID uuid.UUID) agentcontainers.ScriptLogger {
return a.logSender.GetScriptLogger(logSourceID)
}),
}
containerAPIOpts = append(containerAPIOpts, a.containerAPIOptions...)

a.containerAPI = agentcontainers.NewAPI(a.logger.Named("containers"), containerAPIOpts...)
containerAPIOpts := []agentcontainers.Option{
agentcontainers.WithExecer(a.execer),
agentcontainers.WithCommandEnv(a.sshServer.CommandEnv),
agentcontainers.WithScriptLogger(func(logSourceID uuid.UUID) agentcontainers.ScriptLogger {
return a.logSender.GetScriptLogger(logSourceID)
}),
}
containerAPIOpts = append(containerAPIOpts, a.containerAPIOptions...)

a.containerAPI = agentcontainers.NewAPI(a.logger.Named("containers"), containerAPIOpts...)
Comment on lines +339 to +348
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're going to create the API by default. This is fine as we only Init and then Start at a later stage. This allows us to assume containerAPI will always be a non-nil value.


a.reconnectingPTYServer = reconnectingpty.NewServer(
a.logger.Named("reconnecting-pty"),
Expand Down Expand Up @@ -1162,7 +1160,7 @@ func (a *agent) handleManifest(manifestOK *checkpoint) func(ctx context.Context,
scripts = manifest.Scripts
devcontainerScripts map[uuid.UUID]codersdk.WorkspaceAgentScript
)
if a.containerAPI != nil {
if a.devcontainers {
// Init the container API with the manifest and client so that
// we can start accepting requests. The final start of the API
// happens after the startup scripts have been executed to
Expand Down Expand Up @@ -1197,7 +1195,7 @@ func (a *agent) handleManifest(manifestOK *checkpoint) func(ctx context.Context,
// autostarted devcontainer will be included in this time.
err := a.scriptRunner.Execute(a.gracefulCtx, agentscripts.ExecuteStartScripts)

if a.containerAPI != nil {
if a.devcontainers {
// Start the container API after the startup scripts have
// been executed to ensure that the required tools can be
// installed.
Expand Down Expand Up @@ -1928,10 +1926,8 @@ func (a *agent) Close() error {
a.logger.Error(a.hardCtx, "script runner close", slog.Error(err))
}

if a.containerAPI != nil {
if err := a.containerAPI.Close(); err != nil {
a.logger.Error(a.hardCtx, "container API close", slog.Error(err))
}
if err := a.containerAPI.Close(); err != nil {
a.logger.Error(a.hardCtx, "container API close", slog.Error(err))
}

// Wait for the graceful shutdown to complete, but don't wait forever so
Expand Down
7 changes: 4 additions & 3 deletions agent/agent_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2441,7 +2441,8 @@ func TestAgent_DevcontainersDisabledForSubAgent(t *testing.T) {

// Setup the agent with devcontainers enabled initially.
//nolint:dogsled
conn, _, _, _, _ := setupAgent(t, manifest, 0, func(*agenttest.Client, *agent.Options) {
conn, _, _, _, _ := setupAgent(t, manifest, 0, func(_ *agenttest.Client, o *agent.Options) {
o.Devcontainers = true
Copy link
Contributor Author

@DanielleMaywood DanielleMaywood Jul 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It appears the test allowed the regression to occur as it had Dev Containers disabled by default. This means it didn't properly test the flow we'd expect.

})

// Query the containers API endpoint. This should fail because
Expand All @@ -2453,8 +2454,8 @@ func TestAgent_DevcontainersDisabledForSubAgent(t *testing.T) {
require.Error(t, err)

// Verify the error message contains the expected text.
require.Contains(t, err.Error(), "The agent dev containers feature is experimental and not enabled by default.")
require.Contains(t, err.Error(), "To enable this feature, set CODER_AGENT_DEVCONTAINERS_ENABLE=true in your template.")
require.Contains(t, err.Error(), "Dev Container feature not supported.")
require.Contains(t, err.Error(), "Dev Container integration inside other Dev Containers is explicitly not supported.")
Comment on lines +2457 to +2458
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reminder to run CODER_TEST_USE_DOCKER=1 go test ./coderd ./agent ./agent/agentcontainers if you want to verify the "integration" tests.

}

func TestAgent_Dial(t *testing.T) {
Expand Down
12 changes: 10 additions & 2 deletions agent/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"time"

"github.com/go-chi/chi/v5"
"github.com/google/uuid"

"github.com/coder/coder/v2/coderd/httpapi"
"github.com/coder/coder/v2/codersdk"
Expand Down Expand Up @@ -36,12 +37,19 @@ func (a *agent) apiHandler() http.Handler {
cacheDuration: cacheDuration,
}

if a.containerAPI != nil {
if a.devcontainers {
r.Mount("/api/v0/containers", a.containerAPI.Routes())
} else if manifest := a.manifest.Load(); manifest != nil && manifest.ParentID != uuid.Nil {
r.HandleFunc("/api/v0/containers", func(w http.ResponseWriter, r *http.Request) {
httpapi.Write(r.Context(), w, http.StatusForbidden, codersdk.Response{
Message: "Dev Container feature not supported.",
Detail: "Dev Container integration inside other Dev Containers is explicitly not supported.",
})
})
} else {
r.HandleFunc("/api/v0/containers", func(w http.ResponseWriter, r *http.Request) {
httpapi.Write(r.Context(), w, http.StatusForbidden, codersdk.Response{
Message: "The agent dev containers feature is experimental and not enabled by default.",
Message: "Dev Container feature not enabled.",
Detail: "To enable this feature, set CODER_AGENT_DEVCONTAINERS_ENABLE=true in your template.",
})
})
Expand Down
2 changes: 1 addition & 1 deletion cli/ssh_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2104,7 +2104,7 @@ func TestSSH_Container(t *testing.T) {
clitest.SetupConfig(t, client, root)

err := inv.WithContext(ctx).Run()
require.ErrorContains(t, err, "The agent dev containers feature is experimental and not enabled by default.")
require.ErrorContains(t, err, "Dev Container feature not enabled.")
})
}

Expand Down
Loading