Repository navigation
Conversation
Add a global --account persistent flag that overrides the active GitHub account for a single command invocation, eliminating the error-prone pattern of switching accounts globally with gh auth switch. Changes: - Add SetActiveUser method to AuthConfig interface and implementation - Modify ActiveUser to check activeUserOverride before config lookup - Modify ActiveToken to use overridden user's token via TokenForUser - Add --account persistent flag to root command - Wire flag in PersistentPreRunE before auth check Fixes cli#13088 Co-authored-by: Copilot <[email protected]>
github-actions
Bot
added
external
|
Thanks for your pull request! Unfortunately, it doesn't meet the minimum requirements for review:
Please update your PR to address the above. Requirements:
This PR will be automatically closed in 7 days if these requirements are not met. |
There was a problem hiding this comment.
Pull request overview
Adds a global --account flag to allow selecting a specific authenticated GitHub user for a single gh command invocation, avoiding the need to gh auth switch back and forth.
Changes:
- Added
SetActiveUser(user string)to thegh.AuthConfiginterface and implemented it ininternal/config.AuthConfig. - Updated auth resolution to prefer an in-memory active-user override for
ActiveUser()andActiveToken(). - Added a root persistent
--accountflag and applied it duringPersistentPreRunEbefore auth checks; added unit tests for override behavior and token lookup.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/cmd/root/root.go | Introduces --account persistent flag and applies the override during root pre-run. |
| internal/gh/gh.go | Extends the AuthConfig interface with SetActiveUser. |
| internal/config/config.go | Stores an active-user override and uses it for ActiveUser/ActiveToken resolution. |
| internal/config/auth_config_test.go | Adds unit tests covering active-user override and token resolution paths. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // apply per-invocation user override if --account flag is set | ||
| if account, err := cmd.Flags().GetString("account"); err == nil && account != "" { | ||
| cfg.Authentication().SetActiveUser(account) | ||
| } |
There was a problem hiding this comment.
--account currently gets applied without any validation, and errors from GetString("account") are silently ignored. Because cmdutil.CheckAuth only checks HasEnvToken() or len(Hosts()) > 0, a typo/unknown account will still pass the auth gate and commands will proceed with an empty token, leading to later authorization failures that are harder to diagnose. Consider: (1) returning the flag lookup error, and (2) when --account is set, verifying that the specified user has a token on at least one known host (or at minimum on the default host) and returning a clear error if not.
| // The following methods are only for testing and that is a design smell we should consider fixing. | ||
|
|
||
| // SetActiveToken will override any token resolution and return the given token and source for all calls to | ||
| // ActiveToken. | ||
| // Use for testing purposes only. | ||
| SetActiveToken(token, source string) | ||
|
|
||
| // SetHosts will override any hosts resolution and return the given hosts for all calls to Hosts. | ||
| // Use for testing purposes only. | ||
| SetHosts(hosts []string) | ||
|
|
||
| // SetDefaultHost will override any host resolution and return the given host and source for all calls to | ||
| // DefaultHost. | ||
| // Use for testing purposes only. | ||
| SetDefaultHost(host, source string) | ||
|
|
||
| // SetActiveUser will override the active user resolution for all hosts, returning the given user | ||
| // for all calls to ActiveUser and using that user's token for all calls to ActiveToken. | ||
| // This is used by the --user flag to override the active account per-invocation. | ||
| SetActiveUser(user string) |
There was a problem hiding this comment.
The doc comment for SetActiveUser says it is used by a --user flag, but this PR introduces --account. Also, SetActiveUser is placed after the “only for testing” section header even though it is used in production code. Please update the comment to reference --account and either move SetActiveUser above the test-only section or adjust the section comment so it’s not misleading.
Summary
Adds a global
--accountpersistent flag that overrides the active GitHub account for a single command invocation, eliminating the error-proneswitch -> command -> switch backpattern.Usage
Changes
internal/gh/gh.go: AddedSetActiveUser(user string)to theAuthConfiginterfaceinternal/config/config.go:activeUserOverridefield toAuthConfigstructActiveUser()to check override before config lookupActiveToken()to use overridden user's token viaTokenForUserSetActiveUser()methodpkg/cmd/root/root.go: Added--accountpersistent flag, wired inPersistentPreRunEbefore auth checkinternal/config/auth_config_test.go: 5 new unit tests covering:Fixes #13088