Merge pull request #3505 from docker/fix/3501-theme-watcher-rewire

fix(tui): rewire theme watcher lost in tab-view rewrite

Author: dgageot

e2e/tui/testdata/cassettes/TestTheme_HotReload.yaml

@@ -0,0 +1,3 @@
+---
+version: 2
+interactions: []

e2e/tui/theme_test.go

@@ -0,0 +1,54 @@
+package tui_test
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/docker/docker-agent/pkg/tui/messages"
+	"github.com/docker/docker-agent/pkg/tui/styles"
+	"github.com/docker/docker-agent/pkg/tui/tuitest"
+)
+
+// TestTheme_HotReload is the regression net for the theme watcher wiring
+// (issue #3501): editing the active custom theme file while the TUI runs
+// must hot-reload it without a restart. It drives the full loop — switch to
+// a user theme, edit its file on disk, and wait for the watcher to emit
+// ThemeFileChangedMsg, which surfaces as the "Theme hot-reloaded"
+// notification. No LLM call is made, so the cassette is empty.
+func TestTheme_HotReload(t *testing.T) {
+	// The theme registry is process-global; restore the default so other
+	// tests never see this test's theme. Registered before newTUI so that
+	// (LIFO) it runs only after the program stopped rendering.
+	t.Cleanup(func() { styles.ApplyThemeRef(styles.DefaultThemeRef) })
+	d := newTUI(t, "testdata/basic.yaml", 120, 40)
+
+	// isolateState (inside newTUI) redirected the data dir to a temp dir, so
+	// this writes the user theme where the running TUI expects it.
+	themesDir := styles.ThemesDir()
+	require.NoError(t, os.MkdirAll(themesDir, 0o755))
+	themePath := filepath.Join(themesDir, "hotreload.yaml")
+	require.NoError(t, os.WriteFile(themePath, []byte("version: 1\nname: Hot Reload Before\n"), 0o644))
+
+	// Switching themes re-targets the watcher onto the new theme's file.
+	d.Send(messages.ChangeThemeMsg{ThemeRef: "user:hotreload"}).
+		WaitFor(tuitest.Contains("Theme changed to Hot Reload Before"))
+
+	// The watcher arms asynchronously (ThemeChangedMsg is sequenced after the
+	// notification above) and debounces events for 500ms, so a single write
+	// could slip in before fsnotify is attached. Re-edit the file on every
+	// poll — like a user tweaking colors — until the reload lands. The 700ms
+	// interval exceeds the debounce so rewrites cannot starve the timer.
+	require.Eventually(t, func() bool {
+		require.NoError(t, os.WriteFile(themePath, []byte("version: 1\nname: Hot Reload After\n"), 0o644))
+		return strings.Contains(d.Frame(), "Theme hot-reloaded")
+	}, 15*time.Second, 700*time.Millisecond, "editing the theme file should hot-reload it")
+
+	require.Eventually(t, func() bool {
+		return styles.CurrentTheme().Name == "Hot Reload After"
+	}, 3*time.Second, 50*time.Millisecond, "styles.CurrentTheme() should pick up the edited theme")
+}

pkg/tui/handlers.go

@@ -771,6 +771,9 @@ func (m *appModel) invalidateCachesForThemeChange() {
 
 func (m *appModel) applyThemeChanged() (tea.Model, tea.Cmd) {
 	m.invalidateCachesForThemeChange()
+	// Re-target the file watcher: theme changes (picker selection, preview,
+	// hot reload) can move the active theme to a different backing file.
+	m.watchCurrentTheme()
 	return m, tea.Batch(
 		m.updateDialogCmd(messages.ThemeChangedMsg{}),
 		m.updateChatCmd(messages.ThemeChangedMsg{}),

pkg/tui/theme_watcher_wiring_test.go

@@ -0,0 +1,63 @@
+package tui
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/docker/docker-agent/pkg/tui/components/statusbar"
+	"github.com/docker/docker-agent/pkg/tui/styles"
+)
+
+// fakeThemeWatcher records Watch/Stop calls so the wiring between the model
+// lifecycle and the theme watcher can be asserted without touching the
+// filesystem.
+type fakeThemeWatcher struct {
+	watched []string
+	stopped bool
+}
+
+func (f *fakeThemeWatcher) Watch(themeRef string) error {
+	f.watched = append(f.watched, themeRef)
+	return nil
+}
+
+func (f *fakeThemeWatcher) Stop() { f.stopped = true }
+
+func TestWatchCurrentTheme_TargetsAppliedTheme(t *testing.T) {
+	m, _ := newTestModel(t)
+	fw := &fakeThemeWatcher{}
+	m.themeWatcher = fw
+
+	m.watchCurrentTheme()
+
+	assert.Equal(t, []string{styles.CurrentTheme().Ref}, fw.watched)
+}
+
+func TestWatchCurrentTheme_NoWatcherIsNoop(t *testing.T) {
+	m, _ := newTestModel(t)
+
+	assert.NotPanics(t, func() { m.watchCurrentTheme() })
+}
+
+func TestApplyThemeChanged_RetargetsWatcher(t *testing.T) {
+	m, _ := newTestModel(t)
+	m.statusBar = statusbar.New(m)
+	fw := &fakeThemeWatcher{}
+	m.themeWatcher = fw
+
+	_, _ = m.applyThemeChanged()
+
+	assert.Equal(t, []string{styles.CurrentTheme().Ref}, fw.watched,
+		"theme changes must re-target the watcher onto the active theme")
+}
+
+func TestCleanupManagedResources_StopsThemeWatcher(t *testing.T) {
+	m, _ := newTestModel(t)
+	fw := &fakeThemeWatcher{}
+	m.themeWatcher = fw
+
+	m.cleanupManagedResources()
+
+	assert.True(t, fw.stopped, "TUI shutdown must stop the theme watcher")
+}

pkg/tui/tui.go

@@ -154,6 +154,13 @@ type appModel struct {
 	// perform a full terminal release/restore cycle on focus events.
 	program *tea.Program
 
+	// themeWatcher hot-reloads the active custom theme: it watches the
+	// current theme's backing file and reports edits as ThemeFileChangedMsg.
+	// Created in SetProgram (the callback needs the program to inject
+	// messages into the event loop) and re-targeted after every theme
+	// change via watchCurrentTheme.
+	themeWatcher themeFileWatcher
+
 	// dockerDesktop is true when running inside Docker Desktop's terminal
 	// (TERM_PROGRAM=docker_desktop). Focus reporting and the terminal
 	// release/restore cycle on tab switch are only enabled in this
@@ -243,6 +250,13 @@ type appModel struct {
 	disabledCommands map[string]bool
 }
 
+// themeFileWatcher is the subset of *styles.ThemeWatcher the model drives.
+// It is an interface so tests can record Watch/Stop calls.
+type themeFileWatcher interface {
+	Watch(themeRef string) error
+	Stop()
+}
+
 // Transcriber is the speech-to-text interface used by the TUI. It is an
 // interface (rather than the concrete *transcribe.Transcriber) so that tests
 // can inject a fake implementation via WithTranscriber and so that the TUI
@@ -500,9 +514,35 @@ func (m *appModel) Resolve(v any) any {
 }
 
 // SetProgram sets the tea.Program for the supervisor to send routed messages.
+// It also starts the theme file watcher, whose callback needs the program to
+// inject hot-reload messages into the event loop.
 func (m *appModel) SetProgram(p *tea.Program) {
 	m.program = p
 	m.supervisor.SetProgram(p)
+
+	if m.themeWatcher != nil {
+		m.themeWatcher.Stop()
+	}
+	m.themeWatcher = styles.NewThemeWatcher(func(themeRef string) {
+		p.Send(messages.ThemeFileChangedMsg{ThemeRef: themeRef})
+	})
+	m.watchCurrentTheme()
+}
+
+// watchCurrentTheme points the theme watcher at the currently applied theme
+// so edits to its backing file hot-reload it. The watcher no-ops for themes
+// without a user theme file (e.g. built-ins).
+func (m *appModel) watchCurrentTheme() {
+	if m.themeWatcher == nil {
+		return
+	}
+	theme := styles.CurrentTheme()
+	if theme.Ref == "" {
+		return
+	}
+	if err := m.themeWatcher.Watch(theme.Ref); err != nil {
+		slog.Warn("Failed to watch theme file", "theme", theme.Ref, "error", err)
+	}
 }
 
 // reapplyKeyboardEnhancements forwards the cached keyboard enhancements message
@@ -2766,6 +2806,9 @@ func (m *appModel) shutdownTimeoutOrDefault() time.Duration {
 // exit) and the context watcher (external cancellation).
 func (m *appModel) cleanupManagedResources() {
 	m.cleanupOnce.Do(func() {
+		if m.themeWatcher != nil {
+			m.themeWatcher.Stop()
+		}
 		if m.tuiStore != nil {
 			_ = m.tuiStore.Close()
 		}