fix(agent): fix unexpanded devcontainer paths for agentcontainers (#17736)

mafredri · web-flow · commit 7af188bfc102 · 2025-05-12T14:03:40.000+03:00
Devcontainers were duplicated in the API because paths weren't absolute, we now normalize them early on to keep it simple. Updates #16424
diff --git a/agent/agent.go b/agent/agent.go
@@ -1085,6 +1085,8 @@ func (a *agent) handleManifest(manifestOK *checkpoint) func(ctx context.Context,
 		if err != nil {
 			return xerrors.Errorf("expand directory: %w", err)
 		}
+		// Normalize all devcontainer paths by making them absolute.
+		manifest.Devcontainers = agentcontainers.ExpandAllDevcontainerPaths(a.logger, expandPathToAbs, manifest.Devcontainers)
 		subsys, err := agentsdk.ProtoFromSubsystems(a.subsystems)
 		if err != nil {
 			a.logger.Critical(ctx, "failed to convert subsystems", slog.Error(err))
@@ -1127,7 +1129,7 @@ func (a *agent) handleManifest(manifestOK *checkpoint) func(ctx context.Context,
 			)
 			if a.experimentalDevcontainersEnabled {
 				var dcScripts []codersdk.WorkspaceAgentScript
-				scripts, dcScripts = agentcontainers.ExtractAndInitializeDevcontainerScripts(a.logger, expandPathToAbs, manifest.Devcontainers, scripts)
+				scripts, dcScripts = agentcontainers.ExtractAndInitializeDevcontainerScripts(manifest.Devcontainers, scripts)
 				// See ExtractAndInitializeDevcontainerScripts for motivation
 				// behind running dcScripts as post start scripts.
 				scriptRunnerOpts = append(scriptRunnerOpts, agentscripts.WithPostStartScripts(dcScripts...))
diff --git a/agent/agent_test.go b/agent/agent_test.go
@@ -1998,8 +1998,9 @@ func TestAgent_ReconnectingPTYContainer(t *testing.T) {
 // You can run it manually as follows:
 //
 // CODER_TEST_USE_DOCKER=1 go test -count=1 ./agent -run TestAgent_DevcontainerAutostart
+//
+//nolint:paralleltest // This test sets an environment variable.
 func TestAgent_DevcontainerAutostart(t *testing.T) {
-	t.Parallel()
 	if os.Getenv("CODER_TEST_USE_DOCKER") != "1" {
 		t.Skip("Set CODER_TEST_USE_DOCKER=1 to run this test")
 	}
@@ -2012,9 +2013,12 @@ func TestAgent_DevcontainerAutostart(t *testing.T) {
 
 	// Prepare temporary devcontainer for test (mywork).
 	devcontainerID := uuid.New()
-	tempWorkspaceFolder := t.TempDir()
-	tempWorkspaceFolder = filepath.Join(tempWorkspaceFolder, "mywork")
+	tmpdir := t.TempDir()
+	t.Setenv("HOME", tmpdir)
+	tempWorkspaceFolder := filepath.Join(tmpdir, "mywork")
+	unexpandedWorkspaceFolder := filepath.Join("~", "mywork")
 	t.Logf("Workspace folder: %s", tempWorkspaceFolder)
+	t.Logf("Unexpanded workspace folder: %s", unexpandedWorkspaceFolder)
 	devcontainerPath := filepath.Join(tempWorkspaceFolder, ".devcontainer")
 	err = os.MkdirAll(devcontainerPath, 0o755)
 	require.NoError(t, err, "create devcontainer directory")
@@ -2031,9 +2035,10 @@ func TestAgent_DevcontainerAutostart(t *testing.T) {
 		// is expected to be prepared by the provisioner normally.
 		Devcontainers: []codersdk.WorkspaceAgentDevcontainer{
 			{
-				ID:              devcontainerID,
-				Name:            "test",
-				WorkspaceFolder: tempWorkspaceFolder,
+				ID:   devcontainerID,
+				Name: "test",
+				// Use an unexpanded path to test the expansion.
+				WorkspaceFolder: unexpandedWorkspaceFolder,
 			},
 		},
 		Scripts: []codersdk.WorkspaceAgentScript{
diff --git a/agent/agentcontainers/devcontainer.go b/agent/agentcontainers/devcontainer.go
@@ -36,8 +36,6 @@ devcontainer up %s
 // initialize the workspace (e.g. git clone, npm install, etc). This is
 // important if e.g. a Coder module to install @devcontainer/cli is used.
 func ExtractAndInitializeDevcontainerScripts(
-	logger slog.Logger,
-	expandPath func(string) (string, error),
 	devcontainers []codersdk.WorkspaceAgentDevcontainer,
 	scripts []codersdk.WorkspaceAgentScript,
 ) (filteredScripts []codersdk.WorkspaceAgentScript, devcontainerScripts []codersdk.WorkspaceAgentScript) {
@@ -47,7 +45,6 @@ ScriptLoop:
 			// The devcontainer scripts match the devcontainer ID for
 			// identification.
 			if script.ID == dc.ID {
-				dc = expandDevcontainerPaths(logger, expandPath, dc)
 				devcontainerScripts = append(devcontainerScripts, devcontainerStartupScript(dc, script))
 				continue ScriptLoop
 			}
@@ -75,6 +72,17 @@ func devcontainerStartupScript(dc codersdk.WorkspaceAgentDevcontainer, script co
 	return script
 }
 
+// ExpandAllDevcontainerPaths expands all devcontainer paths in the given
+// devcontainers. This is required by the devcontainer CLI, which requires
+// absolute paths for the workspace folder and config path.
+func ExpandAllDevcontainerPaths(logger slog.Logger, expandPath func(string) (string, error), devcontainers []codersdk.WorkspaceAgentDevcontainer) []codersdk.WorkspaceAgentDevcontainer {
+	expanded := make([]codersdk.WorkspaceAgentDevcontainer, 0, len(devcontainers))
+	for _, dc := range devcontainers {
+		expanded = append(expanded, expandDevcontainerPaths(logger, expandPath, dc))
+	}
+	return expanded
+}
+
 func expandDevcontainerPaths(logger slog.Logger, expandPath func(string) (string, error), dc codersdk.WorkspaceAgentDevcontainer) codersdk.WorkspaceAgentDevcontainer {
 	logger = logger.With(slog.F("devcontainer", dc.Name), slog.F("workspace_folder", dc.WorkspaceFolder), slog.F("config_path", dc.ConfigPath))
 
diff --git a/agent/agentcontainers/devcontainer_test.go b/agent/agentcontainers/devcontainer_test.go
@@ -242,9 +242,7 @@ func TestExtractAndInitializeDevcontainerScripts(t *testing.T) {
 				}
 			}
 			gotFilteredScripts, gotDevcontainerScripts := agentcontainers.ExtractAndInitializeDevcontainerScripts(
-				logger,
-				tt.args.expandPath,
-				tt.args.devcontainers,
+				agentcontainers.ExpandAllDevcontainerPaths(logger, tt.args.expandPath, tt.args.devcontainers),
 				tt.args.scripts,
 			)
 

Original file line number	Diff line number	Diff line change
`@@ -242,9 +242,7 @@ func TestExtractAndInitializeDevcontainerScripts(t *testing.T) {`
`242`	`242`	`}`
`243`	`243`	`}`
`244`	`244`	`gotFilteredScripts, gotDevcontainerScripts := agentcontainers.ExtractAndInitializeDevcontainerScripts(`
`245`		`- logger,`
`246`		`- tt.args.expandPath,`
`247`		`- tt.args.devcontainers,`
	`245`	`+ agentcontainers.ExpandAllDevcontainerPaths(logger, tt.args.expandPath, tt.args.devcontainers),`
`248`	`246`	`tt.args.scripts,`
`249`	`247`	`)`
`250`	`248`