Gracefully handle existing remote branch and/or pull request

src/service/git/git-cli.ts

@@ -1,9 +1,16 @@
 import LoggerService from "@bp/service/logger/logger-service";
 import LoggerServiceFactory from "@bp/service/logger/logger-service-factory";
-import simpleGit, { SimpleGit } from "simple-git";
+import simpleGit, { GitError, SimpleGit } from "simple-git";
 import fs from "fs";
 import { LocalGit } from "@bp/service/configs/configs.types";
 
+export class NonFastForwardError extends Error {
+  constructor(public readonly cause: unknown) {
+    super(cause instanceof Error ? cause.message : String(cause), { cause });
+    Object.setPrototypeOf(this, new.target.prototype);
+  }
+}
+
 /**
  * Command line git commands executor service
  */
@@ -123,14 +130,47 @@ export default class GitCLIService {
   }
 
   /**
-   * Check if a branch exists in a remote repository.
+   * Check if the remote branch contains commits without an equivalent in
+   * the local branch (i.e. someone added some work to the remote branch)
+   * @param cwd repository in which the local branch lives
+   * @param branch branch name to search
    * @param remote remote name or URL
+   */
+  async hasRemoteOriginalWork(cwd: string, branch: string, remote = "origin"): Promise<boolean> {
+    return this._isUpstreamMissingWork(
+      cwd,
+      `refs/heads/${branch}`,
+      `refs/remotes/${remote}/${branch}`,
+    );
+  }
+
+  /**
+   * Check if the local branch contains commits without an equivalent in
+   * the remote branch (i.e. some new work has been added to the local branch)
+   * @param cwd repository in which the local branch lives
    * @param branch branch name to search
+   * @param remote remote name or URL
+   */
+  async hasLocalOriginalWork(cwd: string, branch: string, remote = "origin"): Promise<boolean> {
+    return this._isUpstreamMissingWork(
+      cwd,
+      `refs/remotes/${remote}/${branch}`,
+      `refs/heads/${branch}`,
+    );
+  }
+
+  /**
+   * Check if `upstream` is missing work present in `head`.
+   * @param cwd repository in which the local branch lives
+   * @param upstream refspec to compare
+   * @param head reference refspec
    */
-  async remoteBranchExists(remote: string, branch: string): Promise<boolean> {
-    this.logger.info(`Checking if branch ${branch} exists on ${remote}`);
-    const output = await simpleGit().raw(["ls-remote", "--heads", this.remoteWithAuth(remote), branch]);
-    return output.trim().length > 0;
+  async _isUpstreamMissingWork(cwd: string, upstream: string, head: string): Promise<boolean> {
+    const stdout = await this.git(cwd).raw(["cherry", upstream, head]);
+    // prefix:
+    // - means equivalent present in both refs
+    // + means missing from upstream
+    return stdout.split("\n").some(line => line.startsWith("+"));
   }
 
   /**
@@ -187,7 +227,15 @@ export default class GitCLIService {
     if (force) {
       options.push("--force-with-lease");
     }
-    await this.git(cwd).push(remote, branch, options);
+    try {
+      await this.git(cwd).push(remote, branch, options);
+    } catch (err) {
+      // hacky, but is there a better way to detect a non-fast-forward rejection?
+      if (err instanceof GitError && err.message.includes("[rejected] (non-fast-forward)")) {
+        throw new NonFastForwardError(err);
+      }
+      throw err;
+    }
   }
 
   /**

src/service/git/git-client.ts

@@ -1,5 +1,16 @@
 import { BackportPullRequest, GitClientType, GitPullRequest } from "@bp/service/git/git.types";
 
+export class PullRequestExists extends Error {
+    constructor(public readonly url?: string) {
+        let msg = "Pull request already exists";
+        if (url) {
+            msg += ": " + url;
+        }
+        super(msg);
+        Object.setPrototypeOf(this, new.target.prototype);
+    }
+}
+
 /**
  * Git management service interface, which provides a common API for interacting 
  * with several git management services like GitHub, Gitlab or Bitbucket.

src/service/git/github/github-client.ts

@@ -1,11 +1,12 @@
-import GitClient from "@bp/service/git/git-client";
+import GitClient, { PullRequestExists } from "@bp/service/git/git-client";
 import { inferSquash } from "@bp/service/git/git-util";
 import { BackportPullRequest, GitClientType, GitPullRequest } from "@bp/service/git/git.types";
 import GitHubMapper from "@bp/service/git/github/github-mapper";
 import OctokitFactory from "@bp/service/git/github/octokit-factory";
 import LoggerService from "@bp/service/logger/logger-service";
 import LoggerServiceFactory from "@bp/service/logger/logger-service-factory";
 import { Octokit } from "@octokit/rest";
+import { RequestError } from "@octokit/request-error";
 import { PullRequest } from "@octokit/webhooks-types";
 
 export default class GitHubClient implements GitClient {
@@ -192,16 +193,36 @@
     this.logger.info(`Creating pull request ${backport.head} -> ${backport.base}`);
     this.logger.info(`${JSON.stringify(backport, null, 2)}`);
 
-    const { data } = await this.octokit.pulls.create({
-      owner: backport.owner,
-      repo: backport.repo,
-      head: backport.headRepo ? `${backport.headRepo.owner}:${backport.head}` : backport.head,
-      ...(backport.headRepo ? { head_repo: backport.headRepo.project } : {}),
-      base: backport.base,
-      title: backport.title,
-      body: backport.body,
-      maintainer_can_modify: true,
-    });
+    let data: PullRequest;
+    try {
+      const resp = await this.octokit.pulls.create({
+        owner: backport.owner,
+        repo: backport.repo,
+        head: backport.headRepo ? `${backport.headRepo.owner}:${backport.head}` : backport.head,
+        ...(backport.headRepo ? { head_repo: backport.headRepo.project } : {}),
+        base: backport.base,
+        title: backport.title,
+        body: backport.body,
+        maintainer_can_modify: true,
+      });
+      data = resp.data as PullRequest;
+    } catch (err) {
+      if (err instanceof RequestError) {
+        if (this.compatibleForge && err.status === 409) {
+          // Forgejo returns unstructured message:
+          // pull request already exists for these targets [id: .., issue_id: .., head_repo_id: .., base_repo_id: .., head_branch: bp-..., base_branch: ...]
+          this.logger.warn(err.message);
+          throw new PullRequestExists();
+        }
+        if (!this.compatibleForge && err.status === 422) {
+          // GitHub returns a structured message without the URL
+          // {"resource":"PullRequest","code":"custom","message":"A pull request already exists for lampajr:bp-prod-6353aff."}
+          this.logger.warn(err.message);
+          throw new PullRequestExists();
+        }
+      }
+      throw err;
+    }
 
     if (!data) {
       throw new Error("Pull request creation failed");
@@ -214,7 +235,7 @@
         this.octokit.issues.addLabels({
           owner: backport.owner,
           repo: backport.repo,
-          issue_number: (data as PullRequest).number,
+          issue_number: data.number,
           labels: backport.labels,
         }).catch(error => this.logger.error(`Error setting labels: ${error}`))
       );
@@ -225,7 +246,7 @@
         this.octokit.pulls.requestReviewers({
           owner: backport.owner,
           repo: backport.repo,
-          pull_number: (data as PullRequest).number,
+          pull_number: data.number,
           reviewers: backport.reviewers,
         }).catch(error => this.logger.error(`Error requesting reviewers: ${error}`))
       );
@@ -236,7 +257,7 @@
         this.octokit.issues.addAssignees({
           owner: backport.owner,
           repo: backport.repo,
-          issue_number: (data as PullRequest).number,
+          issue_number: data.number,
           assignees: backport.assignees,
         }).catch(error => this.logger.error(`Error setting assignees: ${error}`))
       );
@@ -248,7 +269,7 @@
           this.octokit.issues.createComment({
             owner: backport.owner,
             repo: backport.repo,
-            issue_number: (data as PullRequest).number,
+            issue_number: data.number,
             body: c,
           }).catch(error => this.logger.error(`Error posting comment: ${error}`))
         );
@@ -298,4 +319,4 @@
       id: parseInt(prUrl.substring(prUrl.lastIndexOf("/") + 1, prUrl.length)),
     };
   }
-}
+}

src/service/runner/runner.ts

@@ -2,8 +2,8 @@
 import { Args } from "@bp/service/args/args.types";
 import { Configs } from "@bp/service/configs/configs.types";
 import PullRequestConfigsParser from "@bp/service/configs/pullrequest/pr-configs-parser";
-import GitCLIService from "@bp/service/git/git-cli";
-import GitClient from "@bp/service/git/git-client";
+import GitCLIService, { NonFastForwardError } from "@bp/service/git/git-cli";
+import GitClient, { PullRequestExists } from "@bp/service/git/git-client";
 import GitClientFactory from "@bp/service/git/git-client-factory";
 import { BackportPullRequest, GitClientType } from "@bp/service/git/git.types";
 import LoggerService from "@bp/service/logger/logger-service";
@@ -14,7 +14,7 @@
 interface Git {
   gitClientType: GitClientType;
   gitClientApi: Pick<GitClient, ("getLatestPullRequestComments" | "createPullRequest" | "createPullRequestComment")>;
-  gitCli: Pick<GitCLIService, ("clone" | "createLocalBranch" | "fetch" | "remoteBranchExists" | "cherryPick" | "addRemote" | "push" | "pointToSameCommit" )>;
+  gitCli: Pick<GitCLIService, ("clone" | "createLocalBranch" | "fetch" | "hasRemoteOriginalWork" | "hasLocalOriginalWork" | "cherryPick" | "addRemote" | "push" | "pointToSameCommit" )>;
 }
 
 /**
@@ -142,13 +142,6 @@
   }
 
   async executeBackport(configs: Configs, backportPR: BackportPullRequest, git: Git): Promise<void> {
-    const remote = backportPR.headRepo?.cloneUrl ?? backportPR.cloneUrl;
-    const branchExists = await git.gitCli.remoteBranchExists(remote, backportPR.head);
-    if (branchExists) {
-      this.logger.warn(`Backport branch ${backportPR.head} already exists on ${remote}, skipping`);
-      return;
-    }
-
     const notifyError = !configs.dryRun && configs.errorNotification.enabled && configs.errorNotification.message.length > 0;
 
     if (notifyError && await this.failureAlreadyReported(configs, backportPR, git)) {
@@ -274,13 +267,46 @@
   if (!configs.dryRun) {
     // 10. push the new branch to origin
     yield async () => {
+      try {
         await git.gitCli.push(configs.folder, backportPR.head, target_remote);
+      } catch (err) {
+        if (!(err instanceof NonFastForwardError)) {
+          throw err;
+        }
+        logger.info("Non-fast-forward remote branch");
+        // fetch the diverting remote
+        await git.gitCli.fetch(configs.folder, backportPR.head, target_remote);
+        if (
+          await git.gitCli.hasRemoteOriginalWork(configs.folder, backportPR.head, target_remote)
+        ) {
+          logger.info("Remote branch contains original work, aborting");
+          // throw original error: we cannot push in non-fast-forwarding way
+          throw err.cause;
+        }
+        if (
+          await git.gitCli.hasLocalOriginalWork(configs.folder, backportPR.head, target_remote)
+        ) {
+          logger.info("Force-pushing rebased branch");
+          await git.gitCli.push(configs.folder, backportPR.head, target_remote, true);
+        }
+      }
     };
 
     // 11. create pull request new branch -> target branch (using octokit)
     yield async () => {
-      const prUrl = await git.gitClientApi.createPullRequest(backportPR);
-      logger.info(`Pull request created: ${prUrl}`);
+      try{
+        const prUrl = await git.gitClientApi.createPullRequest(backportPR);
+        logger.info(`Pull request created: ${prUrl}`);
+      } catch (err) {
+        if (!(err instanceof PullRequestExists)) {
+          throw err;
+        }
+        if (err.url){
+          logger.info(`Pull request updated: ${err.url}`);
+        } else {
+          logger.info("The existing pull request has been updated");
+        }
+      }
     };
   } else {
     yield async () => {
@@ -309,9 +335,12 @@
     async fetch(_cwd: string, branch: string, remote = "origin"): Promise<void> {
       s += `git fetch ${remote} ${branch}`;
     },
-    async remoteBranchExists(_remote: string, _branch: string): Promise<boolean> {
+    async hasRemoteOriginalWork(_cwd: string, _branch: string, _remote = "origin"): Promise<boolean> {
       return false;
     },
+    async hasLocalOriginalWork(_cwd: string, _branch: string, _remote = "origin"): Promise<boolean> {
+      return true;
+    },
     async cherryPick(_cwd: string, sha: string, strategy = "recursive", strategyOption = "theirs", cherryPickOptions: string | undefined, emptyCommit = "stop"): Promise<void> {
       s += `git cherry-pick -m 1 --strategy=${strategy} --strategy-option=${strategyOption} --empty=${emptyCommit} `;
       if (cherryPickOptions !== undefined) {

test/service/git/git-cli.test.ts

@@ -1,4 +1,4 @@
-import GitCLIService from "@bp/service/git/git-cli";
+import GitCLIService, { NonFastForwardError } from "@bp/service/git/git-cli";
 import { FileState, GitActionTypes, MockGithub } from "@kie/mock-github";
 import { spawnSync } from "child_process";
 import { assert } from "console";
@@ -125,6 +125,17 @@ describe("git cli service", () => {
     expect(unchanged).toBe(true);
   });
 
+  test("push diverting local branch throws a dedicated error", async () => {
+    spawnSync("git", ["switch", "sbranch"], { cwd });
+    spawnSync("git", ["commit","--amend", "-m", "bla"], { cwd });
+    expect(git.push(cwd, "sbranch", "origin", false)).rejects.toBeInstanceOf(NonFastForwardError);
+    const remoteOriginal = await git.hasRemoteOriginalWork(cwd, "sbranch", "origin");
+    expect(remoteOriginal).toBe(false);
+    const localOriginal = await git.hasLocalOriginalWork(cwd, "sbranch", "origin");
+    expect(localOriginal).toBe(false);
+    // force push now succeeds
+    await git.push(cwd, "sbranch", "origin", true);
+  });
 
   test("git clone on already created repo", async () => {
     await git.clone("remote", cwd, "tbranch");