From f0ea537956e938c633345c3a45139388daef46fa Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 5 Jul 2026 20:55:13 +0200 Subject: [PATCH] Fetch pull requests using the freshly-loaded branches and remotes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The PR fetch needs the current branches (for their upstreams) and remotes to know what to query. It read them from Model().Branches / Model().Remotes on its own worker, after waiting on branchesAndRemotesWg for the branches and remotes refreshes to finish. That wait no longer guarantees fresh data: those refreshes now write the model in a bounce onto the UI thread, and Done() fires before the bounce has been processed. So the fetch read the pre-refresh lists — most visibly, checking out a branch that has a PR wouldn't show that PR until the next refresh, because the fetch queried the old branch set. Have refreshBranches / refreshReflogAndBranches / refreshRemotes return what they loaded, stash it in locals in Refresh, and hand it to the fetch. The wait on branchesAndRemotesWg orders the fetch after both loads have stored their slices, so it fetches against exactly the branches and remotes that were just loaded, with no model read on the worker. The previous commit guarantees both are always in scope when pull requests are, so no fallback is needed. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/gui/controllers/helpers/refresh_helper.go | 69 ++++++++++++------- 1 file changed, 46 insertions(+), 23 deletions(-) diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index c23c01d80..5e15e17c1 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -175,6 +175,14 @@ func (self *RefreshHelper) Refresh(options types.RefreshOptions) { } branchesAndRemotesWg := sync.WaitGroup{} + // The pull-request fetch (below) needs the just-loaded branches and + // remotes. Their model writes are bounced onto the UI thread, so the + // fetch worker can't read them back from the model without racing (and + // would see the pre-refresh values); instead the branches and remotes + // loads stash what they loaded here, and the wait on + // branchesAndRemotesWg gives the fetch the happens-before to read them. + var loadedBranches []*models.Branch + var loadedRemotes []*models.Remote includeWorktreesWithBranches := false if scopeSet.Includes(types.COMMITS) || scopeSet.Includes(types.BRANCHES) { // whenever we change commits, we should update branches because the upstream/downstream @@ -188,7 +196,7 @@ func (self *RefreshHelper) Refresh(options types.RefreshOptions) { if self.c.UserConfig().Git.LocalBranchSortOrder == "recency" { branchesAndRemotesWg.Add(1) refresh("reflog and branches", func() { - self.refreshReflogAndBranches(includeWorktreesWithBranches, options.BranchSelection, options.SelectTopReflogCommit, options.Background) + loadedBranches = self.refreshReflogAndBranches(includeWorktreesWithBranches, options.BranchSelection, options.SelectTopReflogCommit, options.Background) branchesAndRemotesWg.Done() }) } else { @@ -197,7 +205,7 @@ func (self *RefreshHelper) Refresh(options types.RefreshOptions) { // Not a recency sort, so branches doesn't depend on the reflog // being fresh; it runs concurrently with the reflog refresh // below and reads whatever's in the model, as it always has. - self.refreshBranches(includeWorktreesWithBranches, options.BranchSelection, true, self.c.Model().ReflogCommits, options.Background) + loadedBranches = self.refreshBranches(includeWorktreesWithBranches, options.BranchSelection, true, self.c.Model().ReflogCommits, options.Background) branchesAndRemotesWg.Done() }) refresh("reflog", func() { _, _ = self.refreshReflogCommits(options.Background, options.SelectTopReflogCommit) }) @@ -237,7 +245,7 @@ func (self *RefreshHelper) Refresh(options types.RefreshOptions) { if scopeSet.Includes(types.REMOTES) { branchesAndRemotesWg.Add(1) refresh("remotes", func() { - _ = self.refreshRemotes(options.Background) + loadedRemotes, _ = self.refreshRemotes(options.Background) branchesAndRemotesWg.Done() }) } @@ -245,7 +253,11 @@ func (self *RefreshHelper) Refresh(options types.RefreshOptions) { if scopeSet.Includes(types.PULL_REQUESTS) { refresh("pull requests", func() { branchesAndRemotesWg.Wait() - self.refreshGithubPullRequests(options.Background) + // Use the branches and remotes the loads above stashed, not + // Model().Branches/Remotes: those writes are bounced onto the + // UI thread and may not have landed on this worker yet. The + // wait above orders us after both loads have stashed theirs. + self.refreshGithubPullRequests(loadedBranches, loadedRemotes, options.Background) }) } @@ -404,10 +416,13 @@ func getModeName(mode types.RefreshMode) string { // order gives the immediate (non-recency) load a lower branch-load sequence // than the async (recency) load, so the sequence guard in refreshBranches keeps // the recency-sorted result even if the two loads' bounces land out of order. -func (self *RefreshHelper) refreshReflogAndBranches(refreshWorktrees bool, branchSelection types.BranchSelectionBehavior, selectTopReflogCommit bool, background bool) { +func (self *RefreshHelper) refreshReflogAndBranches(refreshWorktrees bool, branchSelection types.BranchSelectionBehavior, selectTopReflogCommit bool, background bool) []*models.Branch { switch self.c.State().GetRepoState().GetStartupStage() { case types.INITIAL: - self.refreshBranches(refreshWorktrees, branchSelection, false, self.c.Model().ReflogCommits, background) + // Return the immediate (non-recency) load's branches; the recency-sorted + // reload below runs on its own worker after we return. Both hold the same + // set of branches, which is all the caller (the PR fetch) needs. + branches := self.refreshBranches(refreshWorktrees, branchSelection, false, self.c.Model().ReflogCommits, background) self.onWorker(background, func(_ gocui.Task) error { reflogCommits, _ := self.refreshReflogCommits(background, false) @@ -416,10 +431,14 @@ func (self *RefreshHelper) refreshReflogAndBranches(refreshWorktrees bool, branc return nil }) + return branches + case types.COMPLETE: reflogCommits, _ := self.refreshReflogCommits(background, selectTopReflogCommit) - self.refreshBranches(refreshWorktrees, branchSelection, true, reflogCommits, background) + return self.refreshBranches(refreshWorktrees, branchSelection, true, reflogCommits, background) } + + return nil } func (self *RefreshHelper) refreshCommitsAndCommitFiles(commitSelection types.CommitSelectionBehavior, background bool) { @@ -733,7 +752,7 @@ func (self *RefreshHelper) refreshStateSubmoduleConfigs() ([]*models.SubmoduleCo // self.refreshStatus is called at the end of this because that's when we can // be sure there is a State.Model.Branches array to pick the current branch from -func (self *RefreshHelper) refreshBranches(refreshWorktrees bool, branchSelection types.BranchSelectionBehavior, loadBehindCounts bool, reflogCommits []*models.Commit, background bool) { +func (self *RefreshHelper) refreshBranches(refreshWorktrees bool, branchSelection types.BranchSelectionBehavior, loadBehindCounts bool, reflogCommits []*models.Commit, background bool) []*models.Branch { loadSeq := self.branchLoadSeq.Add(1) generation := self.c.State().GetRepoGeneration() @@ -820,6 +839,10 @@ func (self *RefreshHelper) refreshBranches(refreshWorktrees bool, branchSelectio self.refreshView(self.c.Contexts().Branches, background) self.refreshStatus(background) + + // Return the freshly-loaded branches so the caller can hand them to the PR + // fetch without reading them back from the (bounce-written) model. + return branches } func (self *RefreshHelper) refreshFilesAndSubmodules(background bool) error { @@ -1041,13 +1064,13 @@ func (self *RefreshHelper) refreshReflogCommits(background bool, selectTopEntry return reflogCommits, nil } -func (self *RefreshHelper) refreshRemotes(background bool) error { +func (self *RefreshHelper) refreshRemotes(background bool) ([]*models.Remote, error) { generation := self.c.State().GetRepoGeneration() prevSelectedRemote := self.c.Contexts().Remotes.GetSelected() remotes, err := self.c.Git().Loaders.RemoteLoader.GetRemotes() if err != nil { - return err + return nil, err } self.onUIThreadUnlessRepoChanged(generation, background, func() error { @@ -1075,7 +1098,7 @@ func (self *RefreshHelper) refreshRemotes(background bool) error { self.refreshView(self.c.Contexts().Remotes, background) self.refreshView(self.c.Contexts().RemoteBranches, background) - return nil + return remotes, nil } func (self *RefreshHelper) loadWorktrees() []*models.Worktree { @@ -1187,7 +1210,7 @@ func (self *RefreshHelper) refreshView(context types.Context, background bool) { }) } -func (self *RefreshHelper) refreshGithubPullRequests(background bool) { +func (self *RefreshHelper) refreshGithubPullRequests(branches []*models.Branch, remotes []*models.Remote, background bool) { generation := self.c.State().GetRepoGeneration() clearPullRequests := func() { @@ -1198,7 +1221,7 @@ func (self *RefreshHelper) refreshGithubPullRequests(background bool) { }) } - githubRemotes := getAuthenticatedGithubRemotes(self.getGithubRemotes(), self.c.Git().GitHub.GetAuthToken) + githubRemotes := getAuthenticatedGithubRemotes(self.getGithubRemotes(remotes), self.c.Git().GitHub.GetAuthToken) if len(githubRemotes) == 0 { clearPullRequests() return @@ -1209,12 +1232,12 @@ func (self *RefreshHelper) refreshGithubPullRequests(background bool) { clearPullRequests() if !self.githubBaseRemotePromptDismissed[self.c.Git().RepoPaths.RepoPath()] { - self.promptForBaseGithubRepo(githubRemotes) + self.promptForBaseGithubRepo(githubRemotes, branches) } return } - self.setGithubPullRequests(baseInfo, background) + self.setGithubPullRequests(baseInfo, branches, background) } type githubRemoteInfo struct { @@ -1223,8 +1246,8 @@ type githubRemoteInfo struct { authToken string } -func (self *RefreshHelper) getGithubRemotes() []githubRemoteInfo { - return lo.FilterMap(self.c.Model().Remotes, func(remote *models.Remote, _ int) (githubRemoteInfo, bool) { +func (self *RefreshHelper) getGithubRemotes(remotes []*models.Remote) []githubRemoteInfo { + return lo.FilterMap(remotes, func(remote *models.Remote, _ int) (githubRemoteInfo, bool) { if len(remote.Urls) == 0 { return githubRemoteInfo{}, false } @@ -1285,7 +1308,7 @@ func getGithubBaseRemote(githubRemotes []githubRemoteInfo, configuredRemoteName return nil } -func (self *RefreshHelper) promptForBaseGithubRepo(githubRemotes []githubRemoteInfo) { +func (self *RefreshHelper) promptForBaseGithubRepo(githubRemotes []githubRemoteInfo, branches []*models.Branch) { menuItems := lo.Map(githubRemotes, func(info githubRemoteInfo, _ int) *types.MenuItem { return &types.MenuItem{ LabelColumns: []string{info.remote.Name, style.FgCyan.Sprint(info.serviceInfo.RepoName)}, @@ -1295,7 +1318,7 @@ func (self *RefreshHelper) promptForBaseGithubRepo(githubRemotes []githubRemoteI self.c.Log.Error(err) } - self.setGithubPullRequests(&info, false) + self.setGithubPullRequests(&info, branches, false) return nil }) }, @@ -1323,17 +1346,17 @@ func (self *RefreshHelper) rebuildPullRequestsMap() { ) } -func (self *RefreshHelper) setGithubPullRequests(baseInfo *githubRemoteInfo, background bool) { +func (self *RefreshHelper) setGithubPullRequests(baseInfo *githubRemoteInfo, branches []*models.Branch, background bool) { generation := self.c.State().GetRepoGeneration() - if len(self.c.Model().Branches) == 0 { + if len(branches) == 0 { return } - branches := lo.Filter(self.c.Model().Branches, func(branch *models.Branch, _ int) bool { + trackingBranches := lo.Filter(branches, func(branch *models.Branch, _ int) bool { return branch.IsTrackingRemote() }) - branchNames := lo.Map(branches, func(branch *models.Branch, _ int) string { + branchNames := lo.Map(trackingBranches, func(branch *models.Branch, _ int) string { return branch.UpstreamBranch })