From ea1c06cc5aac8d87759f4b1f78b5664eea267fea Mon Sep 17 00:00:00 2001
From: Samuel K <skevetter@pm.me>
Date: Mon, 17 Aug 2026 18:19:47 +0000
Subject: [PATCH] refactor(devcontainer): flatten shell commands to single-line
 scripts

Build entrypoint scripts by joining statements with '; ' instead of
embedded newlines. Single-line scripts survive log lines and YAML
string scalars intact (no more block-scalar '|' with blank lines).

Preserves the '$$@' compose-YAML escaping required by docker
compose's variable interpolation on the entrypoint round-trip; the
prior draft of this refactor had regressed that to a single '$',
which would break exec passthrough under compose.

Signed-off-by: Samuel K <skevetter@pm.me>
---
 pkg/devcontainer/compose_test.go | 59 ++++++++++++++++++++++++++++
 pkg/devcontainer/compose_up.go   | 15 +++++---
 pkg/devcontainer/single.go       | 40 ++++++++++++-------
 pkg/devcontainer/single_test.go  | 66 ++++++++++++++++++++++++++++++++
 4 files changed, 161 insertions(+), 19 deletions(-)

diff --git a/pkg/devcontainer/compose_test.go b/pkg/devcontainer/compose_test.go
index 4a848398c1..516096cad9 100644
--- a/pkg/devcontainer/compose_test.go
+++ b/pkg/devcontainer/compose_test.go
@@ -727,6 +727,65 @@ func TestResolveServiceEntrypoint(t *testing.T) {
 	})
 }
 
+func TestBuildOverrideEntrypointSingleLine(t *testing.T) {
+	script := buildOverrideEntrypoint(
+		&config.MergedDevContainerConfig{
+			UpdatedConfigProperties: config.UpdatedConfigProperties{
+				Entrypoints: []string{"echo setup"},
+			},
+		},
+		nil,
+	)
+	if len(script) < 3 {
+		t.Fatalf("expected at least [%s -c <script>], got %v", shShellPath, script)
+	}
+	if script[0] != shShellPath || script[1] != "-c" {
+		t.Fatalf("expected %s -c prefix, got %v", shShellPath, script)
+	}
+	body := script[2]
+	if strings.Contains(body, "\n") {
+		t.Fatalf("buildOverrideEntrypoint() script must be single-line, got %q", body)
+	}
+	if !strings.Contains(body, "echo setup") {
+		t.Errorf("expected custom entrypoint in script, got %q", body)
+	}
+}
+
+func TestBuildOverrideEntrypointComposeEscapesExecPassthrough(t *testing.T) {
+	script := buildOverrideEntrypoint(&config.MergedDevContainerConfig{}, nil)
+	body := script[2]
+	if !strings.Contains(body, `exec "$$@"`) {
+		t.Fatalf(`expected literal exec "$$@" (compose-escaped), got %q`, body)
+	}
+	if strings.Contains(body, `exec "$@"`) {
+		t.Fatalf("exec \"$@\" must be compose-escaped to \"$$@\", got %q", body)
+	}
+}
+
+func TestBuildOverrideEntrypointAppendsUserEntrypoint(t *testing.T) {
+	script := buildOverrideEntrypoint(
+		&config.MergedDevContainerConfig{},
+		[]string{"user-cmd", "arg"},
+	)
+	want := []string{shShellPath, "-c", script[2], "-", "user-cmd", "arg"}
+	if len(script) != len(want) {
+		t.Fatalf("script = %v, want %v", script, want)
+	}
+	for i := range want {
+		if script[i] != want[i] {
+			t.Fatalf("script[%d] = %q, want %q", i, script[i], want[i])
+		}
+	}
+}
+
+func TestBuildOverrideEntrypointKeepsDefaultEntrypointReachable(t *testing.T) {
+	script := buildOverrideEntrypoint(&config.MergedDevContainerConfig{}, nil)
+	body := script[2]
+	if !strings.Contains(body, "devsy internal agent container daemon") {
+		t.Errorf("expected default entrypoint invocation in script, got %q", body)
+	}
+}
+
 func TestNamedVolumesFromMounts(t *testing.T) {
 	t.Run("nil when no volume mounts", func(t *testing.T) {
 		if got := namedVolumesFromMounts([]*config.Mount{{Type: mountTypeBind}}); got != nil {
diff --git a/pkg/devcontainer/compose_up.go b/pkg/devcontainer/compose_up.go
index 5b75139817..4db9a6ba27 100644
--- a/pkg/devcontainer/compose_up.go
+++ b/pkg/devcontainer/compose_up.go
@@ -142,14 +142,17 @@ func buildOverrideEntrypoint(
 	mergedConfig *config.MergedDevContainerConfig,
 	userEntrypoint []string,
 ) composetypes.ShellCommand {
+	statements := []string{
+		startScriptEchoStatement,
+		startScriptTrapStatement,
+	}
+	statements = append(statements, mergedConfig.Entrypoints...)
+	// "$$@" keeps a literal "$@" after compose's variable interpolation; see composeLabelEscaper.
+	statements = append(statements, `exec "$$@"`, DefaultEntrypoint)
 	entrypoint := composetypes.ShellCommand{
-		"/bin/sh",
+		shShellPath,
 		"-c",
-		`echo Container started
-trap "exit 0" 15
-` + strings.Join(mergedConfig.Entrypoints, "\n") + `
-exec "$$@"
-` + DefaultEntrypoint,
+		joinShellStatements(statements...),
 		"-",
 	}
 	return append(entrypoint, userEntrypoint...)
diff --git a/pkg/devcontainer/single.go b/pkg/devcontainer/single.go
index 84993849ea..26936b8253 100644
--- a/pkg/devcontainer/single.go
+++ b/pkg/devcontainer/single.go
@@ -30,13 +30,24 @@ const (
 	DevsyExtraEnvVar            = "DEVSY"
 	RemoteContainersExtraEnvVar = "REMOTE_CONTAINERS"
 
-	DefaultEntrypoint = `
-while ! command -v /usr/local/bin/devsy >/dev/null 2>&1; do
-  echo "waiting for devsy agent to be available"
-  sleep 1
-done
-exec /usr/local/bin/devsy internal agent container daemon
-`
+	shShellPath              = "/bin/sh"
+	startScriptEchoStatement = "echo Container started"
+	startScriptTrapStatement = `trap "exit 0" 15`
+)
+
+// joinShellStatements joins statements into a single-line script, avoiding
+// embedded newlines that get mangled by YAML/log formatting.
+func joinShellStatements(statements ...string) string {
+	return strings.Join(statements, "; ")
+}
+
+// DefaultEntrypoint waits for the devsy agent binary to become available
+// before handing off to the container daemon.
+var DefaultEntrypoint = joinShellStatements(
+	`while ! command -v /usr/local/bin/devsy >/dev/null 2>&1; do echo "waiting for devsy agent to be available"`,
+	"sleep 1",
+	"done",
+	"exec /usr/local/bin/devsy internal agent container daemon",
 )
 
 // resolvedContainer holds the outputs that every code path through
@@ -938,11 +949,14 @@ func (r *runner) addWorkspaceEnvVars(env map[string]string) {
 }
 
 func GetStartScript(mergedConfig *config.MergedDevContainerConfig) string {
-	customEntrypoints := mergedConfig.Entrypoints
-	return `echo Container started
-trap "exit 0" 15
-exec "$@"
-` + strings.Join(customEntrypoints, "\n") + DefaultEntrypoint
+	statements := []string{
+		startScriptEchoStatement,
+		startScriptTrapStatement,
+		`exec "$@"`,
+	}
+	statements = append(statements, mergedConfig.Entrypoints...)
+	statements = append(statements, DefaultEntrypoint)
+	return joinShellStatements(statements...)
 }
 
 func GetContainerEntrypointAndArgs(
@@ -959,5 +973,5 @@ func GetContainerEntrypointAndArgs(
 		cmd = append(cmd, imageDetails.Config.Entrypoint...)
 		cmd = append(cmd, imageDetails.Config.Cmd...)
 	}
-	return "/bin/sh", cmd
+	return shShellPath, cmd
 }
diff --git a/pkg/devcontainer/single_test.go b/pkg/devcontainer/single_test.go
index bbff6e8213..ec425b6177 100644
--- a/pkg/devcontainer/single_test.go
+++ b/pkg/devcontainer/single_test.go
@@ -1,6 +1,7 @@
 package devcontainer
 
 import (
+	"strings"
 	"testing"
 
 	pkgconfig "github.com/devsy-org/devsy/pkg/config"
@@ -204,6 +205,71 @@ func TestRecoveryDevContainerConfigDockerfile(t *testing.T) {
 	}
 }
 
+func TestDefaultEntrypointSingleLine(t *testing.T) {
+	if strings.Contains(DefaultEntrypoint, "\n") {
+		t.Fatalf("DefaultEntrypoint must be single-line, got %q", DefaultEntrypoint)
+	}
+	if !strings.Contains(DefaultEntrypoint, "devsy internal agent container daemon") {
+		t.Errorf("DefaultEntrypoint must invoke the agent daemon, got %q", DefaultEntrypoint)
+	}
+}
+
+func TestGetStartScriptSingleLine(t *testing.T) {
+	merged := &config.MergedDevContainerConfig{
+		UpdatedConfigProperties: config.UpdatedConfigProperties{
+			Entrypoints: []string{"echo setup"},
+		},
+	}
+	got := GetStartScript(merged)
+	if strings.Contains(got, "\n") {
+		t.Fatalf("GetStartScript() must be single-line, got %q", got)
+	}
+	if !strings.Contains(got, `exec "$@"`) {
+		t.Fatalf("GetStartScript() must keep the shell exec passthrough, got %q", got)
+	}
+	if !strings.Contains(got, "devsy internal agent container daemon") {
+		t.Fatalf("GetStartScript() must invoke the agent, got %q", got)
+	}
+}
+
+func TestGetStartScriptPreservesStatementOrder(t *testing.T) {
+	merged := &config.MergedDevContainerConfig{
+		UpdatedConfigProperties: config.UpdatedConfigProperties{
+			Entrypoints: []string{"first-entrypoint", "second-entrypoint"},
+		},
+	}
+	got := GetStartScript(merged)
+	wantOrder := []string{
+		startScriptEchoStatement,
+		startScriptTrapStatement,
+		`exec "$@"`,
+		"first-entrypoint",
+		"second-entrypoint",
+		"devsy internal agent container daemon",
+	}
+	lastIdx := -1
+	for _, want := range wantOrder {
+		idx := strings.Index(got, want)
+		if idx == -1 {
+			t.Fatalf("expected %q in script, got %q", want, got)
+		}
+		if idx <= lastIdx {
+			t.Fatalf("expected %q to appear after previous statement in %q", want, got)
+		}
+		lastIdx = idx
+	}
+}
+
+func TestGetStartScriptOmitsEmptyStatementForNoCustomEntrypoints(t *testing.T) {
+	got := GetStartScript(&config.MergedDevContainerConfig{})
+	if strings.Contains(got, ";;") || strings.Contains(got, "; ;") {
+		t.Errorf(
+			"expected no empty statement between built-ins and default entrypoint, got %q",
+			got,
+		)
+	}
+}
+
 func TestIsRecoveryContainer(t *testing.T) {
 	if isRecoveryContainer(nil) {
 		t.Error("nil details must not be a recovery container")