Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions cmd/root/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,12 @@ type runExecFlags struct {
outputJSON bool

// Run only
hideToolResults bool
lean bool
appName string
listenAddr string
onEventSpecs []string
hideToolResults bool
lean bool
appName string
listenAddr string
onEventSpecs []string
disabledCommands []string

// globalPermissions holds the user-level global permission checker built
// from user config settings. Nil when no global permissions are configured.
Expand Down Expand Up @@ -140,6 +141,7 @@ func addRunOrExecFlags(cmd *cobra.Command, flags *runExecFlags) {
_ = cmd.PersistentFlags().MarkHidden("force-tui")
cmd.PersistentFlags().BoolVar(&flags.lean, "lean", false, "Use a simplified TUI with minimal chrome")
cmd.PersistentFlags().StringVar(&flags.appName, "app-name", "", "Application name shown in the TUI in place of \"docker agent\"")
cmd.PersistentFlags().StringSliceVar(&flags.disabledCommands, "disable-commands", nil, "Comma-separated list of slash commands to hide and disable in the TUI (e.g. /cost,/eval,/model)")
cmd.PersistentFlags().BoolVar(&flags.sandbox, "sandbox", false, "Run the agent inside a Docker sandbox (requires Docker Desktop with sandbox support)")
cmd.PersistentFlags().StringVar(&flags.sandboxTemplate, "template", "docker/sandbox-templates:docker-agent", "Template image for the sandbox (passed to docker sandbox create -t)")
cmd.PersistentFlags().BoolVar(&flags.sbx, "sbx", true, "Prefer the sbx CLI backend when available (set --sbx=false to force docker sandbox)")
Expand Down Expand Up @@ -503,6 +505,9 @@ func (f *runExecFlags) tuiOpts() []tui.Option {
if f.appName != "" {
opts = append(opts, tui.WithAppName(f.appName))
}
if len(f.disabledCommands) > 0 {
opts = append(opts, tui.WithDisabledCommands(f.disabledCommands))
}
return opts
}

Expand Down
51 changes: 50 additions & 1 deletion pkg/tui/tui.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,10 @@ type appModel struct {

appName string
appVersion string

// disabledCommands holds slash commands to hide and disable.
// Normalized to start with "/".
disabledCommands map[string]bool
}

// Transcriber is the speech-to-text interface used by the TUI. It is an
Expand Down Expand Up @@ -230,6 +234,32 @@ func WithVersion(v string) Option {
}
}

// WithDisabledCommands hides and disables the given slash commands so they
// are stripped from the command palette, the slash-command parser, and
// completion. Each entry is normalized to start with "/" (so "cost" and
// "/cost" are equivalent) and lower-cased to match the registered slash
// command names (so "/Cost" and "/cost" are equivalent).
func WithDisabledCommands(slashCommands []string) Option {
return func(m *appModel) {
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 !strings.HasPrefix(c, "/") {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[LOW] WithDisabledCommands is case-sensitive — /Cost silently fails to disable /cost

The function normalizes leading slashes (e.g. "cost""/cost") and trims whitespace, but does not fold to lowercase. If a programmatic caller passes "/Cost" or "/COST", the map lookup m.disabledCommands[item.SlashCommand] will not match the actual slash command (which is stored in lowercase). The command will silently remain enabled.

The doc comment currently only mentions the slash-prefix normalization. In practice, all defined slash commands use lowercase, so interactive users of the CLI flag are unlikely to encounter this. But the WithDisabledCommands public API gives no indication that casing matters.

Suggestion: either add a note to the doc comment (// Case-sensitive: entries must match the exact casing of the slash command.) or normalize with strings.ToLower.

c = "/" + c
}
m.disabledCommands[c] = true
}
}
}

// WithCommandBuilder builds the command categories shown in the command
// palette from the given function. It overrides the default command category
// builder. To include the default commands, the given function should call
Expand Down Expand Up @@ -392,7 +422,26 @@ func (m *appModel) reapplyKeyboardEnhancements() {
}

func (m *appModel) commandCategories() []commands.Category {
return m.buildCommandCategories(context.Background(), m)
categories := m.buildCommandCategories(context.Background(), m)
if len(m.disabledCommands) == 0 {
return categories
}
filtered := make([]commands.Category, 0, len(categories))
for _, cat := range categories {
items := make([]commands.Item, 0, len(cat.Commands))
for _, item := range cat.Commands {
if m.disabledCommands[item.SlashCommand] {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MEDIUM] Case-insensitive filtering silently fails for dynamic commands with uppercase names

WithDisabledCommands normalises user input to lowercase before storing in the disabledCommands map (e.g. "MyTool""/mytool"), but the lookup in commandCategories() uses item.SlashCommand verbatim — with no strings.ToLower applied to the registered value.

For built-in commands this is harmless (they're hardcoded as lowercase strings like "/cost", "/eval"). However, dynamically-registered entries — agent commands and skill commands — are built as "/" + name where name comes directly from the runtime registry:

// pkg/tui/commands/commands.go (existing, not in diff)
SlashCommand: "/" + name,      // agent command — name not lowercased
SlashCommand: "/" + skillName, // skill command — skillName not lowercased

If any such name contains uppercase letters (e.g. "MySkill""/MySkill"), passing --disable-commands=/myskill adds "/myskill" to the map, but the lookup m.disabledCommands["/MySkill"] misses it. The PR's claim of case-insensitive matching only holds for the user-input side, not for the registered value side.

Fix: lowercase item.SlashCommand at the point of lookup:

if m.disabledCommands[strings.ToLower(item.SlashCommand)] {

continue
}
items = append(items, item)
}
if len(items) == 0 {
continue
}
cat.Commands = items
filtered = append(filtered, cat)
}
return filtered
}

// chatPageOpts returns the chat.PageOption slice derived from the current
Expand Down
77 changes: 77 additions & 0 deletions pkg/tui/tui_helpers_test.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
package tui

import (
"context"
"strings"
"testing"

tea "charm.land/bubbletea/v2"

"github.com/docker/docker-agent/pkg/tui/commands"
"github.com/docker/docker-agent/pkg/tui/components/statusbar"
"github.com/docker/docker-agent/pkg/tui/components/tabbar"
)
Expand Down Expand Up @@ -263,3 +265,78 @@ func TestFormatWindowTitle(t *testing.T) {
})
}
}

func TestCommandCategories_DisabledCommandsFilter(t *testing.T) {
t.Parallel()

build := func(context.Context, tea.Model) []commands.Category {
return []commands.Category{
{
Name: "Session",
Commands: []commands.Item{
{ID: "a", SlashCommand: "/cost"},
{ID: "b", SlashCommand: "/eval"},
{ID: "c", SlashCommand: "/exit"},
},
},
{
Name: "Settings",
Commands: []commands.Item{
{ID: "d", SlashCommand: "/theme"},
},
},
}
}

t.Run("no filter keeps everything", func(t *testing.T) {
t.Parallel()
m := &appModel{buildCommandCategories: build}
got := m.commandCategories()
if len(got) != 2 {
t.Fatalf("len(categories) = %d, want 2", len(got))
}
})

t.Run("filters slash commands and drops empty categories", func(t *testing.T) {
t.Parallel()
m := &appModel{buildCommandCategories: build}
WithDisabledCommands([]string{"/cost", "eval", "/theme"})(m)

got := m.commandCategories()
if len(got) != 1 {
t.Fatalf("len(categories) = %d, want 1 (Settings dropped, Session kept)", len(got))
}
if got[0].Name != "Session" {
t.Fatalf("category = %q, want Session", got[0].Name)
}
if len(got[0].Commands) != 1 || got[0].Commands[0].SlashCommand != "/exit" {
t.Fatalf("session commands = %+v, want only /exit", got[0].Commands)
}
})

t.Run("blank entries are ignored", func(t *testing.T) {
t.Parallel()
m := &appModel{buildCommandCategories: build}
WithDisabledCommands([]string{"", " "})(m)
got := m.commandCategories()
if len(got) != 2 {
t.Fatalf("len(categories) = %d, want 2", len(got))
}
})

t.Run("matching is case-insensitive", func(t *testing.T) {
t.Parallel()
m := &appModel{buildCommandCategories: build}
WithDisabledCommands([]string{"/Cost", "EVAL", "/Theme"})(m)
got := m.commandCategories()
if len(got) != 1 {
t.Fatalf("len(categories) = %d, want 1 (Settings dropped, Session kept)", len(got))
}
if got[0].Name != "Session" {
t.Fatalf("category = %q, want Session", got[0].Name)
}
if len(got[0].Commands) != 1 || got[0].Commands[0].SlashCommand != "/exit" {
t.Fatalf("session commands = %+v, want only /exit", got[0].Commands)
}
})
}
Loading