fix(lifecycle): run postAttachCommand on every attach per spec

cmd/agent/container/setup.go

@@ -476,7 +476,7 @@ func (cmd *SetupContainerCmd) startPostAttachHooks(sctx *setupContext) error {
 		return nil
 	}
 
-	return command.StartBackgroundOnce("devsy.post-attach", func() (*exec.Cmd, error) {
+	return command.StartBackground("devsy.post-attach", func() (*exec.Cmd, error) {
 		log.Debugf("starting postAttachCommand as background process")
 		binaryPath, err := os.Executable()
 		if err != nil {

e2e/tests/up/provider_docker.go

@@ -467,6 +467,45 @@ var _ = ginkgo.Describe(
 			)
 		}, ginkgo.SpecTimeout(framework.TimeoutShort()))
 
+		ginkgo.It("postAttachCommand runs on every attach", func(ctx context.Context) {
+			tempDir, err := setupWorkspace(
+				"tests/up/testdata/docker-post-attach-every-time",
+				dtc.initialDir,
+				dtc.f,
+			)
+			framework.ExpectNoError(err)
+
+			// First attach
+			err = dtc.f.DevsyUp(ctx, tempDir)
+			framework.ExpectNoError(err)
+
+			gomega.Eventually(func() string {
+				out, err := dtc.execSSH(ctx, tempDir, "cat $HOME/attach-count.out 2>/dev/null")
+				if err != nil {
+					return ""
+				}
+				return strings.TrimSpace(out)
+			}).WithTimeout(15*time.Second).WithPolling(1*time.Second).Should(
+				gomega.Equal("1"),
+				"postAttachCommand should run on first attach",
+			)
+
+			// Second attach (re-up the same workspace)
+			err = dtc.f.DevsyUp(ctx, tempDir)
+			framework.ExpectNoError(err)
+
+			gomega.Eventually(func() string {
+				out, err := dtc.execSSH(ctx, tempDir, "cat $HOME/attach-count.out 2>/dev/null")
+				if err != nil {
+					return ""
+				}
+				return strings.TrimSpace(out)
+			}).WithTimeout(15*time.Second).WithPolling(1*time.Second).Should(
+				gomega.Equal("2"),
+				"postAttachCommand should run again on second attach",
+			)
+		}, ginkgo.SpecTimeout(framework.TimeoutShort()))
+
 		ginkgo.It(
 			"initializeCommand with object syntax runs named sub-commands in parallel",
 			func(ctx context.Context) {

e2e/tests/up/testdata/docker-post-attach-every-time/.devcontainer.json

@@ -0,0 +1,4 @@
+{
+  "image": "ghcr.io/devsy-org/test-images/go:1",
+  "postAttachCommand": "count=$(cat $HOME/attach-count.out 2>/dev/null || echo 0); echo $((count+1)) > $HOME/attach-count.out"
+}

pkg/command/background.go

@@ -65,7 +65,28 @@ func StartBackgroundOnce(commandName string, createCommand CreateCommand) error
 	return startCommand(cmd, pidFile, streamsFile)
 }
 
+// StartBackground starts a background process unconditionally, without any
+// once-guard. Use this for commands that must run on every invocation (e.g.
+// postAttachCommand which runs on every attach per the devcontainer spec).
+func StartBackground(commandName string, createCommand CreateCommand) error {
+	streamsFile, err := config.DefaultPathManager().ProcessStreamsFile(commandName)
+	if err != nil {
+		return fmt.Errorf("process streams file: %w", err)
+	}
+
+	cmd, err := createCommand()
+	if err != nil {
+		return err
+	}
+
+	return startDetached(cmd, "", streamsFile)
+}
+
 func startCommand(cmd *exec.Cmd, pidFile, streamsFile string) error {
+	return startDetached(cmd, pidFile, streamsFile)
+}
+
+func startDetached(cmd *exec.Cmd, pidFile, streamsFile string) error {
 	streamsF, err := openStreamsFile(cmd, streamsFile)
 	if err != nil {
 		return err
@@ -75,19 +96,16 @@ func startCommand(cmd *exec.Cmd, pidFile, streamsFile string) error {
 		closeFile(streamsF)
 		return fmt.Errorf("start process: %w", err)
 	}
-	// Close the parent's copy of the streams fd. After Start() forks, the
-	// child has its own copy; the parent no longer needs it.
 	closeFile(streamsF)
 
-	if err := os.WriteFile(pidFile, []byte(strconv.Itoa(cmd.Process.Pid)), 0o600); err != nil {
-		// Process is running but untracked. Kill and reap it to prevent orphans/zombies.
-		_ = cmd.Process.Kill()
-		_ = cmd.Wait()
-		return fmt.Errorf("write pid file (process killed to prevent orphan): %w", err)
+	if pidFile != "" {
+		if err := os.WriteFile(pidFile, []byte(strconv.Itoa(cmd.Process.Pid)), 0o600); err != nil {
+			_ = cmd.Process.Kill()
+			_ = cmd.Wait()
+			return fmt.Errorf("write pid file (process killed to prevent orphan): %w", err)
+		}
 	}
 
-	// Release the process handle so the child runs independently of this
-	// parent process. After release, we cannot wait for or signal it.
 	_ = cmd.Process.Release()
 
 	return nil

pkg/command/background_test.go

@@ -0,0 +1,67 @@
+package command
+
+import (
+	"fmt"
+	"os/exec"
+	"testing"
+	"time"
+
+	"github.com/devsy-org/devsy/pkg/config"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+type testPathManager struct {
+	config.PathManager
+	runtimeDir string
+}
+
+func (t *testPathManager) RuntimeDir() (string, error) { return t.runtimeDir, nil }
+
+func TestStartBackgroundRunsEveryTime(t *testing.T) {
+	tmpDir := t.TempDir()
+	config.SetPathManager(
+		&testPathManager{PathManager: config.NewPathManager(), runtimeDir: tmpDir},
+	)
+	t.Cleanup(func() { config.ResetPathManager() })
+
+	commandName := fmt.Sprintf("test.start-bg.%d", time.Now().UnixNano())
+
+	callCount := 0
+	createCmd := func() (*exec.Cmd, error) {
+		callCount++
+		return exec.Command("true"), nil
+	}
+
+	err := StartBackground(commandName, createCmd)
+	require.NoError(t, err)
+	assert.Equal(t, 1, callCount)
+
+	err = StartBackground(commandName, createCmd)
+	require.NoError(t, err)
+	assert.Equal(t, 2, callCount, "StartBackground must invoke createCommand on every call")
+}
+
+func TestStartBackgroundOnceSkipsSecondCall(t *testing.T) {
+	tmpDir := t.TempDir()
+	config.SetPathManager(
+		&testPathManager{PathManager: config.NewPathManager(), runtimeDir: tmpDir},
+	)
+	t.Cleanup(func() { config.ResetPathManager() })
+
+	commandName := fmt.Sprintf("test.start-bg-once.%d", time.Now().UnixNano())
+
+	callCount := 0
+	createCmd := func() (*exec.Cmd, error) {
+		callCount++
+		return exec.Command("sleep", "5"), nil
+	}
+
+	err := StartBackgroundOnce(commandName, createCmd)
+	require.NoError(t, err)
+	assert.Equal(t, 1, callCount)
+
+	err = StartBackgroundOnce(commandName, createCmd)
+	require.NoError(t, err)
+	assert.Equal(t, 1, callCount, "StartBackgroundOnce must skip when process is already running")
+}

pkg/devcontainer/setup/lifecyclehooks_test.go

@@ -678,6 +678,109 @@ func (s *LifecycleHookTestSuite) TestMergeSecretsEnvValueWithEquals() {
 	assert.Equal(s.T(), "host=db port=5432", env["CONN"])
 }
 
+func (s *LifecycleHookTestSuite) TestPostAttachHooksRunEveryTime() {
+	t := s.T()
+	currentUser, err := user.Current()
+	assert.NoError(t, err)
+
+	dir := t.TempDir()
+	counterFile := filepath.Join(dir, "counter.txt")
+
+	result := &config.Result{
+		MergedConfig: &config.MergedDevContainerConfig{
+			DevContainerConfigBase: config.DevContainerConfigBase{
+				RemoteUser: currentUser.Username,
+			},
+			UpdatedConfigProperties: config.UpdatedConfigProperties{
+				PostAttachCommands: []types.LifecycleHook{
+					{"": {"sh", "-c", fmt.Sprintf(
+						`count=$(cat %s 2>/dev/null || echo 0); echo $((count+1)) > %s`,
+						counterFile, counterFile,
+					)}},
+				},
+			},
+		},
+		ContainerDetails: &config.ContainerDetails{
+			State: config.ContainerDetailsState{},
+		},
+		SubstitutionContext: &config.SubstitutionContext{
+			ContainerWorkspaceFolder: dir,
+		},
+	}
+
+	// Run postAttachCommand multiple times — it must execute every time.
+	for i := 1; i <= 3; i++ {
+		err := RunPostAttachHooks(context.Background(), result, nil)
+		assert.NoError(t, err)
+
+		content, readErr := os.ReadFile(counterFile) //nolint:gosec // test file from TempDir
+		assert.NoError(t, readErr)
+		assert.Equal(t, fmt.Sprintf("%d\n", i), string(content),
+			"postAttachCommand should run on call %d", i)
+	}
+}
+
+func (s *LifecycleHookTestSuite) TestPostCreateHookUsesOnceSemantics() {
+	t := s.T()
+
+	// postCreateCommand passes container.Created as content to shouldSkipHook.
+	// When content is non-empty, shouldSkipHook uses a marker file to ensure
+	// the hook runs only once per container creation.
+	skip, err := shouldSkipHook("test-postCreate", "")
+	assert.NoError(t, err)
+	assert.False(t, skip, "empty content should never skip")
+
+	// Verify that preAttachPhaseParams sets content for postCreate and postStart.
+	env := lifecycleEnv{remoteUser: "test", workspaceFolder: "/tmp"}
+	result := &config.Result{
+		MergedConfig: &config.MergedDevContainerConfig{
+			UpdatedConfigProperties: config.UpdatedConfigProperties{
+				PostCreateCommands: []types.LifecycleHook{{"": {"echo", "hi"}}},
+				PostStartCommands:  []types.LifecycleHook{{"": {"echo", "hi"}}},
+			},
+		},
+		ContainerDetails: &config.ContainerDetails{
+			Created: "2024-01-01T00:00:00Z",
+			State:   config.ContainerDetailsState{StartedAt: "2024-01-01T00:00:01Z"},
+		},
+		SubstitutionContext: &config.SubstitutionContext{ContainerWorkspaceFolder: "/tmp"},
+	}
+
+	hooks := preAttachPhaseParams(result, env, false)
+
+	// postCreateCommand should have content = Created (non-empty → once semantics).
+	var postCreate, postStart hookRunParams
+	for _, h := range hooks {
+		if h.phase == PhasePostCreate {
+			postCreate = h.params
+		}
+		if h.phase == PhasePostStart {
+			postStart = h.params
+		}
+	}
+	assert.NotEmpty(t, postCreate.content,
+		"postCreateCommand must have non-empty content for once-semantics")
+	assert.NotEmpty(t, postStart.content,
+		"postStartCommand must have non-empty content for once-semantics")
+}
+
+func (s *LifecycleHookTestSuite) TestPostAttachHookHasNoOnceGuard() {
+	t := s.T()
+
+	// RunPostAttachHooks passes content="" which means shouldSkipHook always
+	// returns false — the hook runs every time. Verify the contract.
+	skip, err := shouldSkipHook("postAttachCommands", "")
+	assert.NoError(t, err)
+	assert.False(t, skip, "postAttachCommand must never be skipped (content is always empty)")
+
+	// Call shouldSkipHook multiple times with empty content — must never skip.
+	for i := range 5 {
+		skip, err := shouldSkipHook("postAttachCommands", "")
+		assert.NoError(t, err)
+		assert.False(t, skip, "postAttachCommand must not skip on call %d", i)
+	}
+}
+
 func TestLifecycleHookTestSuite(t *testing.T) {
 	suite.Run(t, new(LifecycleHookTestSuite))
 }