Skip to content

Retire multi-account migration - #14430

Merged
williammartin merged 2 commits into
trunkfrom
williammartin-fix-keyring-user-lookup
Sep 11, 2026
Merged

williammartin merged 2 commits into
trunkfrom
williammartin-fix-keyring-user-lookup

Conversation

@williammartin

@williammartin williammartin commented Sep 11, 2026 •

Copy link
Copy Markdown
Member

Description

When initial multi account support was added to gh in v2.40.0 (December 2023, so almost 3 years ago at this point), we had to perform some data migrations. The code for this, with lots of nice explanations can be found in multi-account.go, and it has been running on every invocation of gh for 3 years:

cli/internal/ghcmd/cmd.go

Lines 136 to 142 in 7f0590c

if cfgErr == nil {
var m migration.MultiAccount
if err := cfg.Migrate(m); err != nil {
fmt.Fprintln(stderr, err)
return exitError
}
}

The TL;DR was that we needed to take host level user/token data and namespace it by username. For example, a hosts entry of:

//	github.localhost:
//	  user: monalisa
//	  git_protocol: https
//    oauth_token: xyz

would become:

// github.localhost:
//	 user: monalisa
//	 git_protocol: https
//   oauth_token: xyz
//   users:
//	   monalisa:
//	     oauth_token: xyz

A similar operation would occur for the keyring with a key of:

"gh:" + hostname

being copied to:

"gh:" + hostname + ":" + username

This set the stage for gh auth switch to pick the user and token from a list and "activate" them in the config/keyring entry. The data format was both backwards and forwards compatible, to allow users to move back and forward between versions without breaking anything. However, this global keyring entry has been the source of a number of bugs such as:

These bugs occur because people use GH_CONFIG_DIR to swap config files (and therefore active users). This can cause a split brain in some cases where the active user in the config, and the user owning the token in the keyring aren't the same. We can fix the code paths, but we're now 3 years on and I don't think we should give much concern to compatability pre v2.40.0, so I want to remove that keyring entry altogether.

This PR doesn't do that work, but it does remove the migration that circles the area and just sits there as debt.

How did you test this change?

➜  williammartin-probable-robot git:(williammartin-fix-keyring-user-lookup) ./bin/gh version       
gh version 2.100.0-103-g5fe0f89e1 (2026-09-11)
https://github.com/cli/cli/releases/latest
➜  williammartin-probable-robot git:(williammartin-fix-keyring-user-lookup) ./bin/gh api /zen | cat
Encourage flow.% 

Authorship and follow-up

Who wrote this:

  • A human wrote it.
  • An agent wrote it under close human direction.
  • An agent wrote it independently, and no human has guided the implementation beyond the initial prompt.

Who answers review comments:

  • @williammartin will read and reply directly. Name the account.
  • An agent will draft replies and @username will read them before they are posted.
  • Nobody has explicitly committed to replying.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings September 11, 2026 12:53

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Removing the fallback migration can leave legacy configurations inaccessible or cause existing credentials to be lost.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Lite
Findings: 2 High severity · 1 Low severity

New issues introduced by this change (3)
Severity Finding
High severity internal/​config/​migration/​multi_account.go — Retaining legacy configs can lose the existing account token
High severity internal/​ghcmd/​cmd.go — Versionless legacy configs are no longer migrated
Low severity internal/​config/​migration/​multi_account.go — Update documentation for the retired migration
What changed in this PR

Retires automatic execution of the legacy multi-account migration while retaining its implementation.

Changes:

  • Removes startup migration invocation.
  • Renames the deprecated migration type.
  • Updates migration and authentication tests.
File Summary
internal/​ghcmd/​cmd.go Removes automatic migration execution.
internal/​config/​migration/​multi_account.go Renames and retains the deprecated migration.
internal/​config/​migration/​multi_account_test.go Updates migration tests.
internal/​config/​auth_config_test.go Updates authentication tests.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +74 to +78
// This migration no longer runs automatically. Its compatibility window
// covered upgrades from versions that predated the multi-account schema.
// Supported configurations now already use that schema, so running it before
// every command no longer provides value. The implementation remains as
// executable documentation of the original migration and its compatibility

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I don't think we care about users that are on a pre v2.40 release, it's 3 years old, broken in many ways.

Comment thread internal/ghcmd/cmd.go
@@ -133,14 +132,6 @@ func Main() exitCode {

cmdFactory := factory.New(buildVersion, string(invokingAgent), cfgFunc, ioStreams, ghExecutablePath, telemetryService)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I don't think we care about users that are on a pre v2.40 release, it's 3 years old, broken in many ways.

This comment was marked as spam.

Comment thread internal/config/migration/multi_account.go
@williammartin
williammartin marked this pull request as ready for review September 11, 2026 16:08
@williammartin
williammartin requested a review from a team as a code owner September 11, 2026 16:08
@williammartin
williammartin requested a review from niik September 11, 2026 16:08
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants