From e28defb77c94237256d6f213de174319719e36b3 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Fri, 24 Apr 2026 12:21:44 -0500 Subject: [PATCH 1/5] fix(lifecycle): run initializeCommand named sub-commands in parallel The devcontainer spec requires that when initializeCommand uses object syntax with multiple named keys, sub-commands run concurrently. Container-side hooks (onCreateCommand etc.) already had this behavior via executeLifecycleHook, but initializeCommand used a separate code path in runInitializeCommand that iterated sequentially. Refactor runInitializeCommand to use goroutines + sync.WaitGroup for multi-key object syntax, matching the executeLifecycleHook pattern. Single-key and string/array formats continue to run directly without goroutine overhead. --- e2e/tests/up/provider_docker.go | 19 +++ .../.devcontainer.json | 7 + pkg/devcontainer/run.go | 120 +++++++++++++----- pkg/devcontainer/run_test.go | 97 ++++++++++++++ 4 files changed, 213 insertions(+), 30 deletions(-) create mode 100644 e2e/tests/up/testdata/docker-initcmd-parallel/.devcontainer.json create mode 100644 pkg/devcontainer/run_test.go diff --git a/e2e/tests/up/provider_docker.go b/e2e/tests/up/provider_docker.go index 55e583817..a0abe6070 100644 --- a/e2e/tests/up/provider_docker.go +++ b/e2e/tests/up/provider_docker.go @@ -476,6 +476,25 @@ var _ = ginkgo.Describe( ginkgo.SpecTimeout(framework.GetTimeout()), ) + ginkgo.It( + "initializeCommand with object syntax runs named sub-commands in parallel", + func(ctx context.Context) { + tempDir, err := dtc.setupAndUp(ctx, "tests/up/testdata/docker-initcmd-parallel") + framework.ExpectNoError(err) + + // Both initializeCommand sub-commands run on the host and write + // marker files into the workspace folder. + one, err := os.ReadFile(filepath.Join(tempDir, "init-cmd-one.out")) + framework.ExpectNoError(err) + gomega.Expect(string(one)).To(gomega.Equal("initCmdOne")) + + two, err := os.ReadFile(filepath.Join(tempDir, "init-cmd-two.out")) + framework.ExpectNoError(err) + gomega.Expect(string(two)).To(gomega.Equal("initCmdTwo")) + }, + ginkgo.SpecTimeout(framework.GetTimeout()), + ) + ginkgo.It("IDE accessible before postAttachCommand completes", func(ctx context.Context) { tempDir, err := setupWorkspace( "tests/up/testdata/docker-post-attach-nonblocking", diff --git a/e2e/tests/up/testdata/docker-initcmd-parallel/.devcontainer.json b/e2e/tests/up/testdata/docker-initcmd-parallel/.devcontainer.json new file mode 100644 index 000000000..3ca4f59fa --- /dev/null +++ b/e2e/tests/up/testdata/docker-initcmd-parallel/.devcontainer.json @@ -0,0 +1,7 @@ +{ + "image": "ghcr.io/devsy-org/test-images/go:1", + "initializeCommand": { + "write-one": "echo -n initCmdOne > ./init-cmd-one.out", + "write-two": ["sh", "-c", "echo -n initCmdTwo > ./init-cmd-two.out"] + } +} diff --git a/pkg/devcontainer/run.go b/pkg/devcontainer/run.go index b962e9d67..a001a90e0 100644 --- a/pkg/devcontainer/run.go +++ b/pkg/devcontainer/run.go @@ -2,12 +2,14 @@ package devcontainer import ( "context" + "errors" "fmt" "io" "os" "os/exec" "runtime" "strings" + "sync" "time" "github.com/devsy-org/devsy/pkg/devcontainer/config" @@ -201,6 +203,14 @@ func isDockerFileConfig(config *config.DevContainerConfig) bool { return config.GetDockerfile() != "" } +// initCmdContext groups the shared state for running initializeCommand +// sub-commands. +type initCmdContext struct { + shellArgs []string + workspaceFolder string + extraEnvVars []string +} + func runInitializeCommand( workspaceFolder string, config *config.DevContainerConfig, @@ -210,51 +220,101 @@ func runInitializeCommand( return nil } - shellArgs := []string{"sh", "-c"} + ctx := initCmdContext{ + shellArgs: []string{"sh", "-c"}, + workspaceFolder: workspaceFolder, + extraEnvVars: extraEnvVars, + } // According to the devcontainer spec, `initializeCommand` needs to be run on the host. // On Windows we can't assume everyone has `sh` added to their PATH so we need to use Windows default shell (usually cmd.exe) if runtime.GOOS == "windows" { comSpec := os.Getenv("COMSPEC") if comSpec != "" { - shellArgs = []string{comSpec, "/c"} + ctx.shellArgs = []string{comSpec, "/c"} } else { - shellArgs = []string{"cmd.exe", "/c"} + ctx.shellArgs = []string{"cmd.exe", "/c"} } } - for _, cmd := range config.InitializeCommand { - // should run in shell? - var args []string - if len(cmd) == 1 { - args = []string{shellArgs[0], shellArgs[1], cmd[0]} - } else { - args = cmd - } + // When the hook has multiple named keys (object syntax), run + // sub-commands concurrently per the devcontainer spec, matching + // executeLifecycleHook in lifecyclehooks.go. + if len(config.InitializeCommand) > 1 { + return ctx.runParallel(config.InitializeCommand) + } - // run the command - log.Infof("Running initializeCommand from devcontainer.json: '%s'", strings.Join(args, " ")) - writer := log.Writer(log.LevelInfo) - errwriter := log.Writer(log.LevelError) - defer func() { _ = writer.Close() }() - defer func() { _ = errwriter.Close() }() - - cmd := exec.Command(args[0], args[1:]...) - env := cmd.Environ() - env = append(env, extraEnvVars...) - - cmd.Stdout = writer - cmd.Stderr = errwriter - cmd.Dir = workspaceFolder - cmd.Env = env - err := cmd.Run() - if err != nil { - return err - } + for name, cmd := range config.InitializeCommand { + return ctx.runSingle(name, cmd) } return nil } +func (c *initCmdContext) runParallel( + commands map[string][]string, +) error { + var ( + wg sync.WaitGroup + mu sync.Mutex + errs []error + ) + + wg.Add(len(commands)) + for name, cmd := range commands { + go func() { + defer wg.Done() + if err := c.runSingle(name, cmd); err != nil { + mu.Lock() + errs = append( + errs, + fmt.Errorf("named command %q failed: %w", name, err), + ) + mu.Unlock() + } + }() + } + + wg.Wait() + return errors.Join(errs...) +} + +func (c *initCmdContext) runSingle( + name string, + cmd []string, +) error { + var args []string + if len(cmd) == 1 { + args = []string{ + c.shellArgs[0], + c.shellArgs[1], + cmd[0], + } + } else { + args = cmd + } + + log.Infof( + "Running initializeCommand from devcontainer.json: %s '%s'", + name, + strings.Join(args, " "), + ) + writer := log.Writer(log.LevelInfo) + errwriter := log.Writer(log.LevelError) + defer func() { _ = writer.Close() }() + defer func() { _ = errwriter.Close() }() + + execCmd := exec.Command(args[0], args[1:]...) + env := execCmd.Environ() + env = append(env, c.extraEnvVars...) + + execCmd.Stdout = writer + execCmd.Stderr = errwriter + execCmd.Dir = c.workspaceFolder + execCmd.Env = env + + return execCmd.Run() +} + func getWorkspace( workspaceFolder, workspaceID string, conf *config.DevContainerConfig, diff --git a/pkg/devcontainer/run_test.go b/pkg/devcontainer/run_test.go new file mode 100644 index 000000000..72af184c0 --- /dev/null +++ b/pkg/devcontainer/run_test.go @@ -0,0 +1,97 @@ +package devcontainer + +import ( + "fmt" + "os" + "path/filepath" + "testing" + "time" + + "github.com/devsy-org/devsy/pkg/devcontainer/config" + "github.com/devsy-org/devsy/pkg/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func newInitCmdConfig(hook types.LifecycleHook) *config.DevContainerConfig { + return &config.DevContainerConfig{ + DevContainerConfigBase: config.DevContainerConfigBase{ + InitializeCommand: hook, + }, + } +} + +func TestRunInitializeCommand_ParallelTiming(t *testing.T) { + // Two "sleep 0.5" commands that, if run in parallel, complete in ~0.5s. + cfg := newInitCmdConfig(types.LifecycleHook{ + "sleep-a": {"sleep", "0.5"}, + "sleep-b": {"sleep", "0.5"}, + }) + + start := time.Now() + err := runInitializeCommand(t.TempDir(), cfg, nil) + elapsed := time.Since(start) + + assert.NoError(t, err) + assert.Less(t, elapsed, 900*time.Millisecond, + "two 0.5s commands should complete in ~0.5s when parallel, not ~1s") +} + +func TestRunInitializeCommand_ParallelErrorCollection(t *testing.T) { + dir := t.TempDir() + markerFile := filepath.Join(dir, "ran.txt") + + cfg := newInitCmdConfig(types.LifecycleHook{ + "fail": {"sh", "-c", "exit 1"}, + "succeed": {"sh", "-c", fmt.Sprintf("sleep 0.1 && echo done > %s", markerFile)}, + }) + + err := runInitializeCommand(dir, cfg, nil) + + // The combined error should mention which named command failed. + assert.Error(t, err) + assert.Contains(t, err.Error(), `named command "fail" failed`) + + // The succeed command should have run to completion despite the failure. + _, statErr := os.Stat(markerFile) + assert.NoError(t, statErr, "succeed command should run even when fail command errors") +} + +func TestRunInitializeCommand_SingleKey(t *testing.T) { + dir := t.TempDir() + outFile := filepath.Join(dir, "out.txt") + + cfg := newInitCmdConfig(types.LifecycleHook{ + "": {"sh", "-c", fmt.Sprintf("echo hello > %s", outFile)}, + }) + + err := runInitializeCommand(dir, cfg, nil) + require.NoError(t, err) + + data, err := os.ReadFile(outFile) + require.NoError(t, err) + assert.Contains(t, string(data), "hello") +} + +func TestRunInitializeCommand_StringFormat(t *testing.T) { + dir := t.TempDir() + outFile := filepath.Join(dir, "out.txt") + + // String format results in a single key with a shell command string. + cfg := newInitCmdConfig(types.LifecycleHook{ + "": {fmt.Sprintf("echo hello > %s", outFile)}, + }) + + err := runInitializeCommand(dir, cfg, nil) + require.NoError(t, err) + + data, err := os.ReadFile(outFile) + require.NoError(t, err) + assert.Contains(t, string(data), "hello") +} + +func TestRunInitializeCommand_Empty(t *testing.T) { + cfg := &config.DevContainerConfig{} + err := runInitializeCommand(t.TempDir(), cfg, nil) + assert.NoError(t, err) +} From cfbc5b6266f4462a920bd283831130e804fa0379 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Fri, 24 Apr 2026 12:29:37 -0500 Subject: [PATCH 2/5] fix: add gosec nosec annotations for lint compliance --- e2e/tests/up/provider_docker.go | 4 ++-- pkg/devcontainer/run.go | 2 +- pkg/devcontainer/run_test.go | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/e2e/tests/up/provider_docker.go b/e2e/tests/up/provider_docker.go index a0abe6070..3a0950f66 100644 --- a/e2e/tests/up/provider_docker.go +++ b/e2e/tests/up/provider_docker.go @@ -484,11 +484,11 @@ var _ = ginkgo.Describe( // Both initializeCommand sub-commands run on the host and write // marker files into the workspace folder. - one, err := os.ReadFile(filepath.Join(tempDir, "init-cmd-one.out")) + one, err := os.ReadFile(filepath.Join(tempDir, "init-cmd-one.out")) // #nosec G304 -- test path from setupAndUp framework.ExpectNoError(err) gomega.Expect(string(one)).To(gomega.Equal("initCmdOne")) - two, err := os.ReadFile(filepath.Join(tempDir, "init-cmd-two.out")) + two, err := os.ReadFile(filepath.Join(tempDir, "init-cmd-two.out")) // #nosec G304 -- test path from setupAndUp framework.ExpectNoError(err) gomega.Expect(string(two)).To(gomega.Equal("initCmdTwo")) }, diff --git a/pkg/devcontainer/run.go b/pkg/devcontainer/run.go index a001a90e0..27ca6e068 100644 --- a/pkg/devcontainer/run.go +++ b/pkg/devcontainer/run.go @@ -303,7 +303,7 @@ func (c *initCmdContext) runSingle( defer func() { _ = writer.Close() }() defer func() { _ = errwriter.Close() }() - execCmd := exec.Command(args[0], args[1:]...) + execCmd := exec.Command(args[0], args[1:]...) // #nosec G204 -- args come from devcontainer.json initializeCommand env := execCmd.Environ() env = append(env, c.extraEnvVars...) diff --git a/pkg/devcontainer/run_test.go b/pkg/devcontainer/run_test.go index 72af184c0..f980d07a9 100644 --- a/pkg/devcontainer/run_test.go +++ b/pkg/devcontainer/run_test.go @@ -68,7 +68,7 @@ func TestRunInitializeCommand_SingleKey(t *testing.T) { err := runInitializeCommand(dir, cfg, nil) require.NoError(t, err) - data, err := os.ReadFile(outFile) + data, err := os.ReadFile(outFile) // #nosec G304 -- test path from t.TempDir require.NoError(t, err) assert.Contains(t, string(data), "hello") } @@ -85,7 +85,7 @@ func TestRunInitializeCommand_StringFormat(t *testing.T) { err := runInitializeCommand(dir, cfg, nil) require.NoError(t, err) - data, err := os.ReadFile(outFile) + data, err := os.ReadFile(outFile) // #nosec G304 -- test path from t.TempDir require.NoError(t, err) assert.Contains(t, string(data), "hello") } From 6322fba8ebaafaa850c60d0cedbf3eae5a4420bd Mon Sep 17 00:00:00 2001 From: Samuel K Date: Fri, 24 Apr 2026 12:33:59 -0500 Subject: [PATCH 3/5] fix: move nosec annotations to line above to satisfy golines --- e2e/tests/up/provider_docker.go | 6 ++++-- pkg/devcontainer/run.go | 3 ++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/e2e/tests/up/provider_docker.go b/e2e/tests/up/provider_docker.go index 3a0950f66..1ab507f38 100644 --- a/e2e/tests/up/provider_docker.go +++ b/e2e/tests/up/provider_docker.go @@ -484,11 +484,13 @@ var _ = ginkgo.Describe( // Both initializeCommand sub-commands run on the host and write // marker files into the workspace folder. - one, err := os.ReadFile(filepath.Join(tempDir, "init-cmd-one.out")) // #nosec G304 -- test path from setupAndUp + // #nosec G304 -- test path from setupAndUp + one, err := os.ReadFile(filepath.Join(tempDir, "init-cmd-one.out")) framework.ExpectNoError(err) gomega.Expect(string(one)).To(gomega.Equal("initCmdOne")) - two, err := os.ReadFile(filepath.Join(tempDir, "init-cmd-two.out")) // #nosec G304 -- test path from setupAndUp + // #nosec G304 -- test path from setupAndUp + two, err := os.ReadFile(filepath.Join(tempDir, "init-cmd-two.out")) framework.ExpectNoError(err) gomega.Expect(string(two)).To(gomega.Equal("initCmdTwo")) }, diff --git a/pkg/devcontainer/run.go b/pkg/devcontainer/run.go index 27ca6e068..60dd1e76f 100644 --- a/pkg/devcontainer/run.go +++ b/pkg/devcontainer/run.go @@ -303,7 +303,8 @@ func (c *initCmdContext) runSingle( defer func() { _ = writer.Close() }() defer func() { _ = errwriter.Close() }() - execCmd := exec.Command(args[0], args[1:]...) // #nosec G204 -- args come from devcontainer.json initializeCommand + // #nosec G204 -- args come from devcontainer.json initializeCommand + execCmd := exec.Command(args[0], args[1:]...) env := execCmd.Environ() env = append(env, c.extraEnvVars...) From 6bc299784bef32a3e975110095cea90d3fafb531 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Fri, 24 Apr 2026 13:07:08 -0500 Subject: [PATCH 4/5] fix(inject): drain second goroutine in pipe() to eliminate data race The pipe() function returned after the first io.Copy goroutine completed, but the second goroutine could still be writing to toStdout. Callers reading toStdout after pipe() returns would race with that write. Draining the second errChan result ensures both goroutines finish before pipe() returns. --- pkg/inject/inject.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/inject/inject.go b/pkg/inject/inject.go index a9d76b851..ede5bec26 100644 --- a/pkg/inject/inject.go +++ b/pkg/inject/inject.go @@ -329,5 +329,9 @@ func pipe( _ = toStdin.Close() _ = fromStdout.Close() + // Drain the second goroutine so callers can safely read toStdout after + // pipe returns without a data race. + <-errChan + return first } From 3041906d6815cbbf5313fa11485dbac3d9b1cdb2 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Fri, 24 Apr 2026 13:28:00 -0500 Subject: [PATCH 5/5] fix(inject): close fromStdin in pipe() to prevent goroutine deadlock When the first copy direction finishes with an error, the remaining goroutine may be blocked reading from fromStdin. If fromStdin is not closed, the goroutine hangs forever since nothing signals EOF on the read side. Close fromStdin (when it implements io.Closer) alongside the other endpoints so the drain of the second goroutine completes. --- pkg/inject/inject.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/inject/inject.go b/pkg/inject/inject.go index ede5bec26..85460c711 100644 --- a/pkg/inject/inject.go +++ b/pkg/inject/inject.go @@ -325,9 +325,13 @@ func pipe( // Wait for the first copy to complete — its error is the meaningful one. first := <-errChan - // Close both endpoints so the remaining goroutine unblocks promptly. + // Close all closable endpoints so the remaining goroutine unblocks + // promptly instead of leaking. _ = toStdin.Close() _ = fromStdout.Close() + if c, ok := fromStdin.(io.Closer); ok { + _ = c.Close() + } // Drain the second goroutine so callers can safely read toStdout after // pipe returns without a data race.