fix: filter completion candidates to aliases and agent YAMLs only#3008
fix: filter completion candidates to aliases and agent YAMLs only#3008hamza-jeddad wants to merge 5 commits into
Conversation
- Replace completeAgentFilename fallthrough in completeAlias with a new completeAgentYAMLInCwd helper that only returns non-dotfile *.yaml/yml files from the current directory (no directories, no dotfiles) - Sort alias names alphabetically using slices.Sorted(maps.Keys(...)) - Add tests: no directories in plain-prefix completion, no dotfile YAMLs, path drill-down still works with ./ prefix, aliases are sorted
docker-agent
left a comment
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟢 APPROVE
The fix correctly narrows tab-completion candidates to aliases and agent YAML files, excluding directories and dotfiles. The sorting via slices.Sorted(maps.Keys(...)) is a clean improvement. Test coverage is solid — four new tests cover the key scenarios. One minor edge case noted inline.
| } | ||
| var out []string | ||
| for _, e := range entries { | ||
| if e.IsDir() { |
There was a problem hiding this comment.
[LOW] Symlinks-to-directories with .yaml/.yml names pass the IsDir() guard
os.ReadDir on Linux returns DirEntry entries where IsDir() returns false for symlinks — even when the symlink target is a directory. This means a symlink like ln -s /some/dir my-agent.yaml in the working directory would pass all guards in completeAgentYAMLInCwd and appear as a valid YAML completion candidate, even though it is not a file.
A more robust check would be:
if !e.Type().IsRegular() {
continue
}This correctly excludes directories, symlinks, pipes, devices, and other non-regular entries.
The practical impact is low (unusual edge case for a completion helper), but using IsRegular() would make the intent explicit and correct.
…MLInCwd Replace e.IsDir() with !e.Type().IsRegular() so that symlinks-to-directories and other non-regular entries (pipes, devices) are also excluded.
|
Addressed in the latest commit: replaced |
…in completion Add TestCompleteAlias_PlainNonAgentYAMLStillAppears to make the current contract explicit: regular non-dotfile .yml/.yaml files (e.g. Taskfile.yml) are included in plain-prefix completions. Filtering those is a known follow-up, not covered by this PR.
…ker CLI When Docker CLI delegates tab-completion to the plugin it calls: docker-agent __complete agent <subcommand> <args...> RunningStandalone() returns true here (os.Args[1] is '__complete', not the metadata subcommand), so the cobra root command received 'agent' as the first argument to __complete and could not resolve the subcommand tree. The result was zsh falling back to default filesystem completion, showing every file and directory in cwd. Fix: strip the redundant 'agent' token in stripPluginNameFromCompletionArgs() before passing args to cobra, so 'docker agent run <TAB>' correctly delegates to completeRunExec and returns only aliases and agent YAMLs.
aheritier
added
area/cli
- Reorder isManagementInvocation and stripPluginNameFromCompletionArgs so each function has its own doc comment (previously the merged block was attributed entirely to stripPluginNameFromCompletionArgs) - Add test case for stripPluginNameFromCompletionArgs with len==2 input ([__complete, agent]) to document the no-trailing-args edge case
3 participants
Previously,
docker agent run <TAB>included all directories from cwd in completions.What this PR fixes:
.golangci.yml) are excludedKnown limitation / follow-up: Non-dotfile, non-agent YAMLs like
Taskfile.ymlanddocker-compose.yamlare regular.ymlfiles and still appear — filtering those is out of scope for this PR.Implementation:
completeAliasnow calls a newcompleteAgentYAMLInCwdhelper instead ofcompleteAgentFilenamefor plain-prefix input.completeAgentFilenameis unchanged —./and/path drill-down still works.Also sorts alias names alphabetically.