Merge pull request #3724 from docker/fix/lean-mode-exclude-noop-commands

fix(tui): hide no-op slash commands (/settings) in lean mode

Author: Sayt-0

pkg/tui/tui.go

@@ -8,7 +8,9 @@ import (
 	"log/slog"
 	"os"
 	"os/exec"
+	"reflect"
 	goruntime "runtime"
+	"slices"
 	"strings"
 	"sync"
 	"time"
@@ -288,9 +290,23 @@ type Option func(*appModel)
 func WithLeanMode() Option {
 	return func(m *appModel) {
 		m.leanMode = true
+		m.addDisabledCommands(leanModeDisabledSlashCommands)
 	}
 }
 
+// leanModeDisabledSlashCommands lists the built-in slash commands that are
+// no-ops in lean mode: their Execute produces a message type that
+// leanModeDroppedMessageTypes (see appModel.update) silently drops there, so
+// running the command would do nothing. WithLeanMode seeds disabledCommands
+// from this list so they are also hidden from completion and the palette.
+//
+// TestLeanModeDisabledSlashCommandsMatchDroppedMessages cross-checks this
+// list against leanModeDroppedMessageTypes so the two can't silently drift
+// apart.
+var leanModeDisabledSlashCommands = []string{
+	"/settings", // OpenSettingsDialogMsg
+}
+
 // WithHideSidebar hides the chat sidebar. Unlike lean mode, the rest of
 // the chrome (tab bar, status bar, dialogs) remains visible. The user
 // cannot bring the sidebar back via the TUI.
@@ -360,22 +376,28 @@ func WithVersion(v string) Option {
 // command names (so "/Cost" and "/cost" are equivalent).
 func WithDisabledCommands(slashCommands []string) Option {
 	return func(m *appModel) {
-		if len(slashCommands) == 0 {
-			return
+		m.addDisabledCommands(slashCommands)
+	}
+}
+
+// addDisabledCommands normalizes slashCommands (lower-cased, "/"-prefixed)
+// and merges them into m.disabledCommands.
+func (m *appModel) addDisabledCommands(slashCommands []string) {
+	if len(slashCommands) == 0 {
+		return
+	}
+	if m.disabledCommands == nil {
+		m.disabledCommands = make(map[string]bool, len(slashCommands))
+	}
+	for _, c := range slashCommands {
+		c = strings.ToLower(strings.TrimSpace(c))
+		if c == "" {
+			continue
 		}
-		if m.disabledCommands == nil {
-			m.disabledCommands = make(map[string]bool, len(slashCommands))
-		}
-		for _, c := range slashCommands {
-			c = strings.ToLower(strings.TrimSpace(c))
-			if c == "" {
-				continue
-			}
-			if !strings.HasPrefix(c, "/") {
-				c = "/" + c
-			}
-			m.disabledCommands[c] = true
+		if !strings.HasPrefix(c, "/") {
+			c = "/" + c
 		}
+		m.disabledCommands[c] = true
 	}
 }
 
@@ -778,15 +800,33 @@ func (m *appModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 	return model, cmd
 }
 
+// leanModeDroppedMessageTypes lists the message types that appModel.update
+// silently drops when leanMode is set, because the corresponding feature
+// (multi-tab sessions, sidebar, settings dialog) doesn't exist in lean mode.
+// It is the single source of truth for isLeanModeNoOp below, which in turn
+// TestLeanModeDisabledSlashCommandsMatchDroppedMessages uses to keep
+// leanModeDisabledSlashCommands (see WithLeanMode) from drifting out of sync.
+var leanModeDroppedMessageTypes = []reflect.Type{
+	reflect.TypeFor[messages.SpawnSessionMsg](),
+	reflect.TypeFor[messages.SwitchTabMsg](),
+	reflect.TypeFor[messages.CloseTabMsg](),
+	reflect.TypeFor[messages.ReorderTabMsg](),
+	reflect.TypeFor[messages.ToggleSidebarMsg](),
+	reflect.TypeFor[messages.OpenSettingsDialogMsg](),
+}
+
+// isLeanModeNoOp reports whether msg is one of leanModeDroppedMessageTypes.
+func isLeanModeNoOp(msg tea.Msg) bool {
+	return slices.Contains(leanModeDroppedMessageTypes, reflect.TypeOf(msg))
+}
+
 func (m *appModel) update(msg tea.Msg) (tea.Model, tea.Cmd) {
 	// In lean mode, silently drop messages for features that don't exist.
-	if m.leanMode {
-		switch msg.(type) {
-		case messages.SpawnSessionMsg, messages.SwitchTabMsg,
-			messages.CloseTabMsg, messages.ReorderTabMsg,
-			messages.ToggleSidebarMsg, messages.OpenSettingsDialogMsg:
-			return m, nil
-		}
+	// leanModeDroppedMessageTypes is the single source of truth for what's
+	// dropped here; leanModeDisabledSlashCommands (see WithLeanMode) hides
+	// the built-in slash commands that would otherwise do nothing.
+	if m.leanMode && isLeanModeNoOp(msg) {
+		return m, nil
 	}
 
 	switch msg := msg.(type) {

pkg/tui/tui_leanmode_test.go

@@ -0,0 +1,126 @@
+package tui
+
+import (
+	"context"
+	"testing"
+
+	tea "charm.land/bubbletea/v2"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/docker/docker-agent/pkg/app"
+	"github.com/docker/docker-agent/pkg/session"
+	"github.com/docker/docker-agent/pkg/tui/commands"
+	"github.com/docker/docker-agent/pkg/tui/components/editor/completions"
+)
+
+// leanModeTestCategories mirrors the real "Session"/"Settings" categories
+// closely enough to exercise lean-mode filtering: /settings is a no-op in
+// lean mode (OpenSettingsDialogMsg is dropped), /exit is not.
+func leanModeTestCategories(context.Context, tea.Model) []commands.Category {
+	noop := func(string) tea.Cmd { return func() tea.Msg { return nil } }
+	return []commands.Category{
+		{
+			Name: "Session",
+			Commands: []commands.Item{
+				{ID: "session.exit", Label: "Exit", SlashCommand: "/exit", Immediate: true, Execute: noop},
+			},
+		},
+		{
+			Name: "Settings",
+			Commands: []commands.Item{
+				{ID: "settings.open", Label: "Preferences", SlashCommand: "/settings", Immediate: true, Execute: noop},
+			},
+		},
+	}
+}
+
+func TestCommandCategories_LeanModeExcludesSettings(t *testing.T) {
+	t.Parallel()
+
+	t.Run("lean mode drops /settings from categories, completion, and the parser", func(t *testing.T) {
+		t.Parallel()
+
+		m := &appModel{ctx: t.Context, buildCommandCategories: leanModeTestCategories}
+		WithLeanMode()(m)
+
+		categories := m.commandCategories()
+		require.Len(t, categories, 1, "the now-empty Settings category should be dropped")
+		assert.Equal(t, "Session", categories[0].Name)
+
+		items := completions.NewCommandCompletion(categories).Items()
+		for _, item := range items {
+			assert.NotEqual(t, "/settings", item.Value, "inline completion should not offer /settings in lean mode")
+		}
+
+		parser := commands.NewParser(categories...)
+		assert.Nil(t, parser.Parse("/settings"), "the palette parser should not run /settings in lean mode")
+	})
+
+	t.Run("classic mode keeps /settings in categories, completion, and the parser", func(t *testing.T) {
+		t.Parallel()
+
+		m := &appModel{ctx: t.Context, buildCommandCategories: leanModeTestCategories}
+
+		categories := m.commandCategories()
+		require.Len(t, categories, 2)
+
+		items := completions.NewCommandCompletion(categories).Items()
+		var sawSettings bool
+		for _, item := range items {
+			if item.Value == "/settings" {
+				sawSettings = true
+			}
+		}
+		assert.True(t, sawSettings, "inline completion should offer /settings in classic mode")
+
+		parser := commands.NewParser(categories...)
+		cmd := parser.Parse("/settings")
+		require.NotNil(t, cmd, "the palette parser should run /settings in classic mode")
+	})
+}
+
+// TestLeanModeDisabledSlashCommandsMatchDroppedMessages guards
+// leanModeDisabledSlashCommands against drifting out of sync with
+// leanModeDroppedMessageTypes (the lean-mode message-drop switch in
+// appModel.update): every built-in slash command whose Execute produces a
+// dropped message type must be listed, and every listed command must
+// actually produce one of those types.
+func TestLeanModeDisabledSlashCommandsMatchDroppedMessages(t *testing.T) {
+	t.Parallel()
+
+	application := app.New(t.Context(), stubRuntime{}, session.New())
+	categories := commands.BuildCommandCategories(t.Context(), application)
+
+	disabled := make(map[string]bool, len(leanModeDisabledSlashCommands))
+	for _, c := range leanModeDisabledSlashCommands {
+		disabled[c] = true
+	}
+
+	matched := make(map[string]bool, len(leanModeDisabledSlashCommands))
+	for _, category := range categories {
+		for _, item := range category.Commands {
+			if item.SlashCommand == "" || !item.Immediate || item.Execute == nil {
+				continue
+			}
+			cmd := item.Execute("")
+			if cmd == nil {
+				continue
+			}
+			msg := cmd()
+
+			if isLeanModeNoOp(msg) {
+				matched[item.SlashCommand] = true
+				assert.True(t, disabled[item.SlashCommand],
+					"%s produces %T, which lean mode drops, but is missing from leanModeDisabledSlashCommands", item.SlashCommand, msg)
+			} else {
+				assert.False(t, disabled[item.SlashCommand],
+					"%s is listed in leanModeDisabledSlashCommands but produces %T, which lean mode does not drop", item.SlashCommand, msg)
+			}
+		}
+	}
+
+	for _, c := range leanModeDisabledSlashCommands {
+		assert.True(t, matched[c], "%s is listed in leanModeDisabledSlashCommands but no built-in command produces a dropped message type for it", c)
+	}
+}