From ff0c8b1dc7292ce830a9586be5bf8ded853dc9ef Mon Sep 17 00:00:00 2001
From: Samuel K <skevetter@pm.me>
Date: Sat, 25 Apr 2026 10:26:58 -0500
Subject: [PATCH] fix(lifecycle): warn when waitFor references phase with no
 commands

---
 pkg/devcontainer/setup/lifecyclehooks.go      | 22 +++++++++
 pkg/devcontainer/setup/lifecyclehooks_test.go | 47 +++++++++++++++++++
 pkg/log/testing.go                            | 15 ++++++
 3 files changed, 84 insertions(+)

diff --git a/pkg/devcontainer/setup/lifecyclehooks.go b/pkg/devcontainer/setup/lifecyclehooks.go
index 95a0b19d3..2aadfaaa7 100644
--- a/pkg/devcontainer/setup/lifecyclehooks.go
+++ b/pkg/devcontainer/setup/lifecyclehooks.go
@@ -52,6 +52,20 @@ func validWaitForPhase(phase LifecyclePhase) bool {
 	return phase == PhaseInitializeCommand || slices.Contains(phaseOrder, phase)
 }
 
+// phaseHasCommands reports whether any phaseHook for the given phase
+// carries actual work (either a runFunc or non-empty commands list).
+func phaseHasCommands(all []phaseHook, phase LifecyclePhase) bool {
+	for _, ph := range all {
+		if ph.phase != phase {
+			continue
+		}
+		if ph.runFunc != nil || len(ph.params.commands) > 0 {
+			return true
+		}
+	}
+	return false
+}
+
 // resolveWaitFor normalises the raw waitFor string from the config,
 // falling back to the spec default for empty or invalid values.
 func resolveWaitFor(raw string) LifecyclePhase {
@@ -222,6 +236,14 @@ func RunPreAttachHooks(
 
 	waitFor := resolveWaitFor(setupInfo.MergedConfig.WaitFor)
 	waitFor = promoteDotfilesWaitFor(waitFor, dotfiles)
+
+	if !phaseHasCommands(all, waitFor) {
+		log.Debugf(
+			"waitFor phase %q has no commands configured; the split point is a no-op",
+			waitFor,
+		)
+	}
+
 	deferred, err := runWithWaitFor(all, waitFor)
 	return DeferredHooks{hooks: deferred}, err
 }
diff --git a/pkg/devcontainer/setup/lifecyclehooks_test.go b/pkg/devcontainer/setup/lifecyclehooks_test.go
index d0ad0e713..014ba0d5c 100644
--- a/pkg/devcontainer/setup/lifecyclehooks_test.go
+++ b/pkg/devcontainer/setup/lifecyclehooks_test.go
@@ -11,9 +11,11 @@ import (
 	"time"
 
 	"github.com/devsy-org/devsy/pkg/devcontainer/config"
+	"github.com/devsy-org/devsy/pkg/log"
 	"github.com/devsy-org/devsy/pkg/types"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/suite"
+	"go.uber.org/zap/zapcore"
 )
 
 type LifecycleHookTestSuite struct {
@@ -596,6 +598,51 @@ func (s *LifecycleHookTestSuite) TestPromoteDotfilesWaitForNoDotfiles() {
 	assert.Equal(t, DefaultWaitFor, result)
 }
 
+func (s *LifecycleHookTestSuite) TestPhaseHasCommandsTrue() {
+	all := []phaseHook{
+		{
+			phase:  PhaseOnCreate,
+			params: hookRunParams{commands: []types.LifecycleHook{{"": {"echo", "hi"}}}},
+		},
+	}
+	assert.True(s.T(), phaseHasCommands(all, PhaseOnCreate))
+}
+
+func (s *LifecycleHookTestSuite) TestPhaseHasCommandsFalseEmpty() {
+	all := makeTestPhaseHooks()
+	assert.False(s.T(), phaseHasCommands(all, PhaseOnCreate))
+}
+
+func (s *LifecycleHookTestSuite) TestPhaseHasCommandsTrueRunFunc() {
+	all := []phaseHook{
+		{
+			phase:   PhasePostCreate,
+			runFunc: func() error { return nil },
+		},
+	}
+	assert.True(s.T(), phaseHasCommands(all, PhasePostCreate))
+}
+
+func (s *LifecycleHookTestSuite) TestWaitForEmptyPhaseLogsWarning() {
+	t := s.T()
+	logs := log.InitTestObserved(t, zapcore.DebugLevel)
+
+	all := makeTestPhaseHooks()
+	phase := PhaseUpdateContent
+
+	if !phaseHasCommands(all, phase) {
+		log.Debugf(
+			"waitFor phase %q has no commands configured; the split point is a no-op",
+			phase,
+		)
+	}
+
+	entries := logs.All()
+	assert.NotEmpty(t, entries, "expected at least one log entry")
+	assert.Contains(t, entries[0].Message,
+		`waitFor phase "updateContentCommand" has no commands configured`)
+}
+
 func TestLifecycleHookTestSuite(t *testing.T) {
 	suite.Run(t, new(LifecycleHookTestSuite))
 }
diff --git a/pkg/log/testing.go b/pkg/log/testing.go
index d66c005d9..f96a52bbc 100644
--- a/pkg/log/testing.go
+++ b/pkg/log/testing.go
@@ -3,7 +3,10 @@ package log
 import (
 	"testing"
 
+	"go.uber.org/zap"
+	"go.uber.org/zap/zapcore"
 	"go.uber.org/zap/zaptest"
+	"go.uber.org/zap/zaptest/observer"
 )
 
 // InitTest replaces the package-level logger with a test logger.
@@ -15,3 +18,15 @@ func InitTest(t testing.TB) {
 	sugar = logger.Sugar()
 	t.Cleanup(func() { sugar = prev })
 }
+
+// InitTestObserved replaces the package-level logger with an observable
+// logger at the given level. The returned ObservedLogs can be used to
+// assert that specific log messages were emitted.
+func InitTestObserved(t testing.TB, level zapcore.Level) *observer.ObservedLogs {
+	t.Helper()
+	prev := sugar
+	core, logs := observer.New(level)
+	sugar = zap.New(core).Sugar()
+	t.Cleanup(func() { sugar = prev })
+	return logs
+}