Fix "Pull all" button when a linked worktree has changes

Also move the logic to app-store where it belongs

Author: pol-rivero

app/src/lib/git/for-each-ref.ts

@@ -91,6 +91,7 @@ export async function getBranchesDifferingFromUpstream(
     upstream: '%(upstream)',
     symref: '%(symref)',
     head: '%(HEAD)',
+    worktreePath: '%(worktreepath)',
   })
 
   const prefixes = ['refs/heads', 'refs/remotes']
@@ -117,6 +118,10 @@ export async function getBranchesDifferingFromUpstream(
       // Exclude symbolic refs and the current branch
       continue
     }
+    if (ref.worktreePath.length > 0 && ref.worktreePath !== repository.path) {
+      // Exclude branches checked out in other worktrees, since they can't be fast-forwarded from here
+      continue
+    }
 
     if (ref.fullName.startsWith('refs/heads')) {
       if (ref.upstream.length === 0) {

app/src/lib/stores/app-store.ts

@@ -5663,12 +5663,39 @@ export class AppStore extends TypedBaseStore<IAppState> {
     }
   }
 
+  private async withRepoInfoInError(
+    errorMessagePrefix: string,
+    repository: Repository,
+    fn: () => Promise<void>
+  ): Promise<void> {
+    try {
+      await fn()
+    } catch (error) {
+      const message = error instanceof Error ? error.message : String(error)
+      throw new Error(
+        `${errorMessagePrefix} '${repository.name}' (${repository.path}): ${message}`,
+        { cause: error }
+      )
+    }
+  }
+
   public async _pull(repository: Repository): Promise<void> {
     return this.withRefreshedGitHubRepository(repository, repository => {
       return this.performPull(repository)
     })
   }
 
+  public async _pullAllRepositories(): Promise<void> {
+    const repositories = await this.repositoriesStore.getAll()
+    await Promise.all(
+      repositories.map(repository =>
+        this.withRepoInfoInError('Error pulling', repository, () =>
+          this._pull(repository)
+        )
+      )
+    )
+  }
+
   /** This shouldn't be called directly. See `Dispatcher`. */
   private async performPull(repository: Repository): Promise<void> {
     return this.withPushPullFetch(repository, async () => {

app/src/models/repository.ts

@@ -113,6 +113,9 @@ export class Repository {
   }
 
   public get path(): string {
+    // NOTE: This is not actually the main worktree. We preserve the name "mainWorkTree" to
+    // minimize merge conflicts when pulling changes from the official repo (desktop/desktop).
+    // If isLinkedWorktree = true, this is actually the path to the linked worktree
     return this.mainWorkTree.path
   }
 

app/src/ui/dispatcher/dispatcher.ts

@@ -780,6 +780,14 @@ export class Dispatcher {
     return this.appStore._pull(repository)
   }
 
+  public async pullAllRepositories(): Promise<void> {
+    try {
+      await this.appStore._pullAllRepositories()
+    } catch (error) {
+      this.postError(error)
+    }
+  }
+
   /** Fetch a specific refspec for the repository. */
   public fetchRefspec(
     repository: Repository,

app/src/ui/repositories-list/repositories-list.tsx

@@ -126,36 +126,6 @@ function findMatchingListItem(
   return null
 }
 
-function isPullableRepository(
-  repository: Repositoryish,
-  repositories: ReadonlyArray<Repositoryish>
-): repository is Repository {
-  if (!(repository instanceof Repository)) {
-    return false
-  }
-
-  if (!repository.isLinkedWorktree) {
-    return true
-  }
-
-  const mainWorktreePath = normalizePath(repository.mainWorktreePath)
-  const candidatesWithSameMain = repositories.filter(
-    (candidate): candidate is Repository =>
-      candidate instanceof Repository &&
-      normalizePath(candidate.mainWorktreePath) === mainWorktreePath
-  )
-
-  if (candidatesWithSameMain.length === 0) {
-    return false
-  }
-
-  const preferred =
-    candidatesWithSameMain.find(candidate => !candidate.isLinkedWorktree) ??
-    candidatesWithSameMain[0]
-
-  return preferred.id === repository.id
-}
-
 /** The list of user-added repositories. */
 export class RepositoriesList extends React.Component<
   IRepositoriesListProps,
@@ -682,27 +652,8 @@ export class RepositoriesList extends React.Component<
 
   private onPullRepositoriesButtonClick = async () => {
     this.setState({ pullingRepositories: true })
-    try {
-      const repositoriesToPull = this.props.repositories.filter(repository =>
-        isPullableRepository(repository, this.props.repositories)
-      )
-
-      await Promise.all(
-        repositoriesToPull.map(repository =>
-          this.props.dispatcher.pull(repository).catch(e => {
-            const message = e instanceof Error ? e.message : String(e)
-            throw new Error(
-              `Error pulling '${repository.name}' (${repository.path}): ${message}`,
-              { cause: e }
-            )
-          })
-        )
-      )
-    } catch (e) {
-      this.props.dispatcher.postError(e)
-    } finally {
-      this.setState({ pullingRepositories: false })
-    }
+    await this.props.dispatcher.pullAllRepositories()
+    this.setState({ pullingRepositories: false })
   }
 
   private onCloneRepository = () => {