cobra: fix test failures, add Examples/AddGroup, improve comments#4414
Merged
cobra: fix test failures, add Examples/AddGroup, improve comments#4414
Conversation
… fixes Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/ea20cecf-d23b-471c-aa2d-51a42136e45d Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Review usage of spf13/cobra module in gh-aw
cobra: fix test failures, add Examples/AddGroup, improve comments
Apr 23, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the Cobra CLI layer to fix failing completion tests (now that some completions are annotation-based), and improves --help output by adding command examples and grouping subcommands.
Changes:
- Fixes
TestRegisterFlagCompletionsto assert on Cobra flag annotations for filename/dirname completion helpers. - Adds
Examplehelp sections and introduces command groups (modes,utils) withGroupIDassignments. - Updates comments and test helpers to explain/handle Cobra
PersistentPreRunEoverride behavior and GroupID requirements.
Show a summary per file
| File | Description |
|---|---|
| internal/cmd/root.go | Adds root-level Example and registers Cobra command groups (modes, utils). |
| internal/cmd/proxy.go | Adds proxy Example, assigns GroupID: modes, and clarifies why some flags are redeclared. |
| internal/cmd/flags_test.go | Updates completion tests to check annotation-based completions for file/dir flags. |
| internal/cmd/completion_test.go | Ensures test roots define the utils group so completion’s GroupID is valid in isolation. |
| internal/cmd/completion.go | Assigns GroupID: utils to the completion command and expands the PersistentPreRunE override comment. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 5/5 changed files
- Comments generated: 3
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This was referenced Apr 23, 2026
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.
The
TestRegisterFlagCompletionstests were failing becauseMarkFlagFilename/MarkFlagDirname(already adopted) use flag annotations rather thanRegisterFlagCompletionFunc, soGetFlagCompletionFuncreturned nothing. This PR fixes those tests and completes the remaining cobra improvement items from the Go Fan report.Changes
Bug fix: test assertions for annotation-based completions (
flags_test.go)GetFlagCompletionFuncchecks with direct annotation checks (BashCompFilenameExt/BashCompSubdirsInDir) forconfig,log-dir,payload-dir, andenvflagsHelp output improvements (
root.go,proxy.go)Examplefield torootCmdandproxycommand — gives the dedicated "Examples:" section in--helpoutputAddGroupinrootCmd.init()with"Operation Modes:"and"Utilities:"groups; assignedGroupIDtoproxy(modes) andcompletion(utils)Comments (
proxy.go,completion.go)--listen/--log-dirare re-declared onproxyrather than inherited as persistent flags (different defaults, distinct runtime purpose)PersistentPreRunEoverride comment to explain cobra does not chainPersistentPreRunEhooks automatically — child's hook fully replaces the parent'sTest helpers (
completion_test.go)newTestRootWithCompletion()andTestNewCompletionCmd_OverridesParentPersistentPreRunEnow callAddGroupfor"utils"so theGroupIDon the completion command doesn't panic whenExecute()is called in isolationWarning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
example.com/tmp/go-build2287418252/b509/launcher.test /tmp/go-build2287418252/b509/launcher.test -test.testlogfile=/tmp/go-build2287418252/b509/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true 2/compile.go 2/equals.go x_amd64/compile(dns block)/tmp/go-build299607628/b513/launcher.test /tmp/go-build299607628/b513/launcher.test -test.testlogfile=/tmp/go-build299607628/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build299607628/b500/vet.cfg g_.a -trimpath x_amd64/link -p google.golang.or/tmp/go-build556243166/b360/vet.cfg -lang=go1.24 x_amd64/link -ato�� khb6c8mIR -buildtags x_amd64/vet -errorsas -ifaceassert -nilfunc x_amd64/vet(dns block)invalid-host-that-does-not-exist-12345.com/tmp/go-build2287418252/b491/config.test /tmp/go-build2287418252/b491/config.test -test.testlogfile=/tmp/go-build2287418252/b491/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true ktype/networktype.go 64/src/encoding/google.golang.org/grpc/internal x_amd64/compile(dns block)/tmp/go-build299607628/b495/config.test /tmp/go-build299607628/b495/config.test -test.testlogfile=/tmp/go-build299607628/b495/testlog.txt -test.paniconexit0 -test.timeout=10m0s -o pkg/mod/google.golang.org/protobuf@v1.36.11/type-s -trimpath 64/pkg/tool/linux_amd64/vet -p google.golang.or/tmp/go-build556243166/b288/vet.cfg -lang=go1.23 64/pkg/tool/linux_amd64/vet -o /tmp/go-build2287418252/b434/_pk--log-target -trimpath 64/bin/go -p google.golang.or/tmp/go-build556243166/b396/vet.cfg -lang=go1.24 ker/cli-plugins/docker-compose(dns block)nonexistent.local/tmp/go-build2287418252/b509/launcher.test /tmp/go-build2287418252/b509/launcher.test -test.testlogfile=/tmp/go-build2287418252/b509/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true 2/compile.go 2/equals.go x_amd64/compile(dns block)/tmp/go-build299607628/b513/launcher.test /tmp/go-build299607628/b513/launcher.test -test.testlogfile=/tmp/go-build299607628/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build299607628/b500/vet.cfg g_.a -trimpath x_amd64/link -p google.golang.or/tmp/go-build556243166/b360/vet.cfg -lang=go1.24 x_amd64/link -ato�� khb6c8mIR -buildtags x_amd64/vet -errorsas -ifaceassert -nilfunc x_amd64/vet(dns block)slow.example.com/tmp/go-build2287418252/b509/launcher.test /tmp/go-build2287418252/b509/launcher.test -test.testlogfile=/tmp/go-build2287418252/b509/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true 2/compile.go 2/equals.go x_amd64/compile(dns block)/tmp/go-build299607628/b513/launcher.test /tmp/go-build299607628/b513/launcher.test -test.testlogfile=/tmp/go-build299607628/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build299607628/b500/vet.cfg g_.a -trimpath x_amd64/link -p google.golang.or/tmp/go-build556243166/b360/vet.cfg -lang=go1.24 x_amd64/link -ato�� khb6c8mIR -buildtags x_amd64/vet -errorsas -ifaceassert -nilfunc x_amd64/vet(dns block)this-host-does-not-exist-12345.com/tmp/go-build2287418252/b518/mcp.test /tmp/go-build2287418252/b518/mcp.test -test.testlogfile=/tmp/go-build2287418252/b518/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true 7418252/b157/_pk-errorsas .cfg ache/go/1.25.9/x-nilfunc -p errors -lang=go1.25 ache/go/1.25.9/x-importcfg(dns block)/tmp/go-build299607628/b522/mcp.test /tmp/go-build299607628/b522/mcp.test -test.testlogfile=/tmp/go-build299607628/b522/testlog.txt -test.paniconexit0 -test.timeout=10m0s -ato�� -bool -buildtags x_amd64/vet -errorsas -ifaceassert -nilfunc x_amd64/vet e=/t�� t0 m0s x_amd64/vet ache/go/1.25.9/xbash .cfg 64/pkg/tool/linu--version LP5OZdq/DvfQTWCuBUFu78y3ZbmN(dns block)If you need me to access, download, or install something from one of these locations, you can either: