[Repo Assist] refactor(cmd): cobra UX improvements — NoArgs, MarkFlagFilename, preRun test fix#4395
Open
github-actions[bot] wants to merge 1 commit intomainfrom
Open
Conversation
…un test fix Implements three quick-win improvements identified in issue #4379 (cobra module review): 1. Add Args: cobra.NoArgs to root command: positional arguments are not supported by awmg; without this, 'awmg config.toml' silently falls through to a confusing 'required flag(s) not set' error instead of a clear 'unknown command' message. 2. Replace verbose RegisterFlagCompletionFunc callbacks with idiomatic cobra helpers: MarkFlagFilename("config", "toml"), MarkFlagDirname("log-dir"), MarkFlagDirname("payload-dir"), MarkFlagFilename("env", "env"). Reduces ~20 lines of boilerplate to 4 lines with identical shell-completion behaviour. 3. Fix fragile preRun(nil, nil) in root_test.go: replace all 9 occurrences with preRun(&cobra.Command{}, nil). The current code is safe because preRun does not dereference cmd, but any future use of cmd.Context(), cmd.Flags(), etc. would cause a nil pointer panic. Using a real (empty) Command value prevents that footgun. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors parts of the internal/cmd cobra CLI setup to improve UX and reduce completion boilerplate for the awmg command, aligning with recommendations from issue #4379.
Changes:
- Add
Args: cobra.NoArgsto the root command to reject accidental positional args with clearer cobra diagnostics. - Replace custom flag completion lambdas with
MarkFlagFilename/MarkFlagDirnamehelpers. - Update tests to avoid calling
preRun(nil, nil)by passing an empty*cobra.Command.
Show a summary per file
| File | Description |
|---|---|
internal/cmd/root.go |
Enforces no positional args on the root command for improved UX. |
internal/cmd/flags.go |
Simplifies file/dir flag completion registration using cobra helper APIs. |
internal/cmd/root_test.go |
Makes preRun tests more robust by avoiding nil *cobra.Command. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
internal/cmd/flags.go:70
MarkFlagFilename/MarkFlagDirname(andRegisterFlagCompletionFuncbelow) return an error in cobra. Calling them without handling the return value will fail to compile and also hides cases where the flag name is wrong or not registered yet. Please capture and handle the errors (e.g., fail fast during init, or intentionally ignore via_ = ...with a comment explaining why).
// File and directory completions using idiomatic cobra helpers
cmd.MarkFlagFilename("config", "toml")
cmd.MarkFlagDirname("log-dir")
cmd.MarkFlagDirname("payload-dir")
cmd.MarkFlagFilename("env", "env")
// Enum completions for DIFC flags
cmd.RegisterFlagCompletionFunc("guards-mode", cobra.FixedCompletions(
[]string{"strict", "filter", "propagate"}, cobra.ShellCompDirectiveNoFileComp))
cmd.RegisterFlagCompletionFunc("allowonly-min-integrity", cobra.FixedCompletions(
[]string{"none", "unapproved", "approved", "merged"}, cobra.ShellCompDirectiveNoFileComp))
- Files reviewed: 3/3 changed files
- Comments generated: 0
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 This PR was created by Repo Assist, an automated AI assistant.
Relates to #4379 (cobra module review — items P1, P2, P3)
Changes
1.
Args: cobra.NoArgson root command (root.go)Without this, typing
awmg config.toml(a common mistake) produces:With
cobra.NoArgs, cobra gives a clear diagnostic before any validation runs:This is a one-line change; it cannot affect normal usage (the root command runs with
RunE: run, which takes no positional arguments).2. Simplify file/directory completions (
flags.go)Replace four verbose
RegisterFlagCompletionFunclambdas with idiomatic cobra shorthands:RegisterFlagCompletionFunc("config", func(...) { return []string{"toml"}, FilterFileExt })MarkFlagFilename("config", "toml")RegisterFlagCompletionFunc("log-dir", func(...) { return nil, FilterDirs })MarkFlagDirname("log-dir")RegisterFlagCompletionFunc("payload-dir", func(...) { return nil, FilterDirs })MarkFlagDirname("payload-dir")RegisterFlagCompletionFunc("env", func(...) { return []string{"env"}, FilterFileExt })MarkFlagFilename("env", "env")Shell-completion behaviour is identical; the delta is −20 lines of boilerplate.
MarkFlagFilename/MarkFlagDirnamehave been available since cobra v1.5; this project uses v1.10.2.3. Fix fragile
preRun(nil, nil)in tests (root_test.go)Nine test cases called
preRun(nil, nil). This works today becausepreRunnever dereferencescmd. Any future addition that callscmd.Context(),cmd.Flags(), or any other method will panic at nil. Replacing withpreRun(&cobra.Command{}, nil)eliminates the footgun with zero test-logic change.Test Status
The changes are:
root.go: one struct-field addition (Args: cobra.NoArgs)flags.go: method call substitutions with identical semanticsroot_test.go:nil→&cobra.Command{}in 9 call sites (no logic change)All three are syntactically straightforward refactors with no logic changes.
Warning
The following domain was blocked by the firewall during workflow execution:
proxy.golang.orgTo allow these domains, add them to the
network.allowedlist in your workflow frontmatter:See Network Configuration for more information.