Skip to content

feat: add --disable-commands flag to hide and disable slash commands in TUI#2913

Open
dgageot wants to merge 3 commits into
docker:mainfrom
dgageot:board/a7b0512f057b0683
Open

feat: add --disable-commands flag to hide and disable slash commands in TUI#2913
dgageot wants to merge 3 commits into
docker:mainfrom
dgageot:board/a7b0512f057b0683

Conversation

@dgageot
Copy link
Copy Markdown
Member

@dgageot dgageot commented May 28, 2026

Users sometimes want to limit which slash commands are available in the TUI—for example, to hide commands that shouldn't be used in a particular environment or workflow. This change adds a --disable-commands flag to docker agent run and docker agent exec that lets you specify which commands to hide.

The flag accepts a comma-separated list of slash command names, like --disable-commands="/cost,/eval,/model". Leading slashes are optional. When set, the specified commands are completely hidden and disabled: they're stripped from the command palette (Ctrl+K), the slash-command parser, and the editor completion popup. The filtering happens in a single commandCategories() chokepoint, so all three surfaces inherit the behavior consistently.

This PR also includes a pre-existing test flakiness fix for pkg/snapshot on macOS, where t.TempDir() returns a symlink that needs canonicalization to match what production code returns, and a strictly behavior-preserving refactor that simplifies the command filter using slices.DeleteFunc.

@dgageot dgageot requested a review from a team as a code owner May 28, 2026 12:36
Copy link
Copy Markdown

@docker-agent docker-agent left a comment

Choose a reason for hiding this comment

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

Assessment: 🟡 NEEDS ATTENTION

Comment thread pkg/tui/tui.go
}
isDisabled := func(item commands.Item) bool { return m.disabledCommands[item.SlashCommand] }
for i := range categories {
categories[i].Commands = slices.DeleteFunc(categories[i].Commands, isDisabled)
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] slices.DeleteFunc mutates the Commands slice in-place — could corrupt shared/cached slices from custom builders

commandCategories() calls slices.DeleteFunc(categories[i].Commands, isDisabled) in-place on the slice returned by buildCommandCategories. In Go, slices.DeleteFunc modifies the underlying array and zeroes out the removed tail — it does not copy. If a caller provides a custom builder via WithCommandBuilder that returns a cached or shared []commands.Category, the first call to commandCategories() with any disabledCommands will permanently zero out the tail entries of that shared slice. Every subsequent call will see corrupted (nil/zero) items.

The default builder (commands.BuildCommandCategories) allocates a fresh slice each call and is safe. However, the WithCommandBuilder doc comment only warns about not accessing tea.Model during the build — it does not warn callers against returning shared slices, leaving a silent corruption trap for custom builders.

Fix: copy categories[i].Commands before filtering, e.g.:

cmds := make([]commands.Item, len(categories[i].Commands))
copy(cmds, categories[i].Commands)
categories[i].Commands = slices.DeleteFunc(cmds, isDisabled)

Comment thread pkg/tui/tui.go
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.

@aheritier aheritier added area/cli CLI commands, flags, output formatting area/tui For features/issues/fixes related to the TUI kind/feat PR adds a new feature (maps to feat: commit prefix) labels May 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/cli CLI commands, flags, output formatting area/tui For features/issues/fixes related to the TUI kind/feat PR adds a new feature (maps to feat: commit prefix)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants