feat: add --disable-commands flag to hide and disable slash commands in TUI#2913
feat: add --disable-commands flag to hide and disable slash commands in TUI#2913dgageot wants to merge 3 commits into
Conversation
| } | ||
| isDisabled := func(item commands.Item) bool { return m.disabledCommands[item.SlashCommand] } | ||
| for i := range categories { | ||
| categories[i].Commands = slices.DeleteFunc(categories[i].Commands, isDisabled) |
There was a problem hiding this comment.
[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)| if c == "" { | ||
| continue | ||
| } | ||
| if !strings.HasPrefix(c, "/") { |
There was a problem hiding this comment.
[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.
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-commandsflag todocker agent runanddocker agent execthat 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 singlecommandCategories()chokepoint, so all three surfaces inherit the behavior consistently.This PR also includes a pre-existing test flakiness fix for
pkg/snapshoton macOS, wheret.TempDir()returns a symlink that needs canonicalization to match what production code returns, and a strictly behavior-preserving refactor that simplifies the command filter usingslices.DeleteFunc.