fix: debounce failed backports, based on marker in failure comment

README.md

@@ -70,7 +70,7 @@ It works in this way: given the provided `pull/merge request` it infers the serv
 
 After that it clones the corresponding git repository, check out in the provided `target branch` and create a new branch from that (name automatically generated if not provided as option).
 
-By default the tool will try to cherry-pick the single squashed/merged commit into the newly created branch. The `--no-squash` and `--auto-no-squash` options control this behavior according the following table. 
+By default the tool will try to cherry-pick the single squashed/merged commit into the newly created branch. The `--no-squash` and `--auto-no-squash` options control this behavior according the following table.
 
 | No squash | Auto no squash |Behavior|
 |---|---|---|
@@ -135,7 +135,7 @@ This tool comes with some inputs that allow users to override the default behavi
 | Strategy Option       | --strategy-option        | N            | Cherry pick merging strategy option, see [git-merge](https://git-scm.com/docs/git-merge#_merge_strategies) doc for all possible values                                                           | "theirs"       |
 | Cherry-pick Options       | --cherry-pick-options        | N            | Additional cherry-pick options, see [git-cherry-pick](https://git-scm.com/docs/git-cherry-pick) doc for all possible values                                                           | "theirs"       |
 | Additional comments       | --comments        | N            | Semicolon separated list of additional comments to be posted to the backported pull request                                                           | []       |
-| Enable error notification       | --enable-err-notification        | N            | If true, enable the error notification as comment on the original pull request                                                           | false       |
+| Enable error notification       | --enable-err-notification        | N            | If true, enable the error notification as comment on the original pull request, see [Error notification](#error-notification)           | false       |
 | Dry Run       | -d, --dry-run        | N            | If enabled the tool does not push nor create anything remotely, use this to skip PR creation                                                           | false       |
 
 > **NOTE**: `pull request` and (`target branch` or `target branch pattern`) are *mandatory*, they must be provided as CLI options or as part of the configuration file (if used).
@@ -179,6 +179,16 @@ $ git-backporting -tb v1 -pr https://github.com/upstream/project/pull/123 -a ***
 
 When using `--tb-repo` alone, the backport branch is pushed directly to the target repository, so your PAT needs push access there. When combining it with `--bp-repo`, the branch is pushed to the fork instead, so your PAT needs push access on the fork and only the ability to open a pull request on the target repository.
 
+#### Error notification
+
+If error notifications are enabled, every failed backport is reported via comment on the original pull request.
+The comment contains the error, a reconstruction of the attempted steps and a hidden marker identifying the target branch.
+
+This marker is used to debounce the reporting of the same backport failing multiple times in quick succession.
+Before backporting to a target branch, the tool inspects the most recent comments of the original pull request.
+If a failure has already been reported, the backport attempt will be skipped.
+To retry the backport, delete the failure comment (or edit out the marker).
+
 #### Configuration file example
 
 This is an example of a configuration file that can be used.
@@ -211,7 +221,7 @@ This action can be used in any *GitHub* workflow, below you can find a simple ex
 ```yml
 name: Pull Request Backporting using Git Backporting
 
-on: 
+on:
   workflow_dispatch:
     inputs:
       targetBranch:
@@ -220,12 +230,12 @@ on:
         type: string
       pullRequest:
         description: 'Pull request'
-        required: true 
+        required: true
         type: string
       dryRun:
         description: 'Dry run'
         required: false
-        default: "true" 
+        default: "true"
         type: string
 
 jobs:

dist/cli/index.js

@@ -1002,6 +1002,48 @@ class GitHubClient {
         const { owner, project, id } = this.extractPullRequestData(prUrl);
         return this.getPullRequest(owner, project, id, squash);
     }
+    async getLatestPullRequestComments(prUrl) {
+        const { owner, project, id } = this.extractPullRequestData(prUrl);
+        this.logger.debug(`Fetching latest comments of pull request ${owner}/${project}/${id}`);
+        const perPage = 100;
+        const params = {
+            owner: owner,
+            repo: project,
+            issue_number: id,
+            per_page: perPage,
+        };
+        const { data, headers } = await this.octokit.issues.listComments(params);
+        const firstPage = data.map(c => c.body ?? "");
+        // Forgejo so far didn't implement pagination, and just always returns all comments.
+        const total = this.extractTotalCount(headers);
+        if (total !== undefined && data.length >= total) {
+            return firstPage;
+        }
+        const lastPage = this.extractLastPage(headers, perPage);
+        if (lastPage <= 1) {
+            return firstPage;
+        }
+        // last page may hold only one single comment, so always fetch last two pages
+        const fetchPage = async (page) => {
+            const { data } = await this.octokit.issues.listComments({ ...params, page });
+            return data.map(c => c.body ?? "");
+        };
+        const secondLast = lastPage === 2 ? firstPage : await fetchPage(lastPage - 1);
+        const last = await fetchPage(lastPage);
+        return [...secondLast, ...last];
+    }
+    extractTotalCount(headers) {
+        const total = parseInt(`${headers["x-total-count"]}`);
+        return isNaN(total) ? undefined : total;
+    }
+    extractLastPage(headers, perPage) {
+        const last = /[?&]page=(\d+)[^>]*>;\s*rel="last"/.exec(headers.link ?? "");
+        if (last) {
+            return parseInt(last[1]);
+        }
+        const total = this.extractTotalCount(headers);
+        return total === undefined ? 1 : Math.ceil(total / perPage);
+    }
     /**
      * Read the message out of a git.getCommit response. GitHub returns it at the top level
      * (`message`), whereas Gitea/Forgejo nest it under `commit` (`commit.message`). Support
@@ -1382,6 +1424,14 @@ class GitLabClient {
         }
         return commentUrl;
     }
+    // https://docs.gitlab.com/ee/api/notes.html#list-all-merge-request-notes
+    async getLatestPullRequestComments(mrUrl) {
+        const { namespace, project, id } = this.extractMergeRequestData(mrUrl);
+        const projectId = this.getProjectId(namespace, project);
+        const { data } = await this.client.get(`/projects/${projectId}/merge_requests/${id}/notes?sort=desc&per_page=100`);
+        const notes = (data ?? []);
+        return notes.map(n => n.body ?? "");
+    }
     // UTILS
     /**
      * Retrieve a gitlab user given its username
@@ -1762,21 +1812,43 @@ class Runner {
         }
         return token;
     }
+    /**
+     * Check whether a previous run already reported a failed backport to the same
+     * target branch, by looking for a hidden marker in the latest comments of the
+     * original pull request.
+     * @returns true if the failure has already been reported, false otherwise or if
+     *          the comments could not be fetched
+     */
+    async failureAlreadyReported(configs, backportPR, git) {
+        try {
+            const comments = await git.gitClientApi.getLatestPullRequestComments(configs.originalPullRequest.url);
+            return comments.some(c => c.includes(failureMarker(backportPR.base)));
+        }
+        catch (error) {
+            this.logger.warn(`Unable to fetch the comments of ${configs.originalPullRequest.url}: ${error}`);
+            return false;
+        }
+    }
     async executeBackport(configs, backportPR, git) {
         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)) {
+            this.logger.warn(`Backport to ${backportPR.base} already failed. Delete failure comment to retry. Skipping.`);
+            return;
+        }
         let i = 0;
         for (const step of backportSteps(this.logger, configs, backportPR, git)) {
             try {
                 await step();
             }
             catch (error) {
                 this.logger.error(`Something went wrong backporting to ${backportPR.base}: ${error}`);
-                if (!configs.dryRun && configs.errorNotification.enabled && configs.errorNotification.message.length > 0) {
+                if (notifyError) {
                     // notify the failure as comment in the original pull request
                     let comment = (0, runner_util_1.injectError)(configs.errorNotification.message, error);
                     comment = (0, runner_util_1.injectTargetBranch)(comment, backportPR.base);
@@ -1789,6 +1861,8 @@ class Runner {
                     catch (scriptError) {
                         this.logger.error(`Something went wrong reconstructing the script: ${scriptError}`);
                     }
+                    comment += `\n\nThe backport to \`${backportPR.base}\` will not be retried until this comment is deleted.`;
+                    comment += `\n\n${failureMarker(backportPR.base)}`;
                     await git.gitClientApi.createPullRequestComment(configs.originalPullRequest.url, comment);
                 }
                 throw error;
@@ -1798,6 +1872,10 @@ class Runner {
     }
 }
 exports["default"] = Runner;
+// Hidden marker appended to the failure notification comment.
+function failureMarker(base) {
+    return `<!-- git-backporting: backport to ${base} failed -->`;
+}
 function* backportSteps(logger, configs, backportPR, git) {
     // every failible operation should be in one dedicated closure
     // whether the backport pr targets a different repository than the original pull request's one (--tb-repo),
@@ -1919,6 +1997,9 @@ async function backportScript(configs, backportPR, git, failed) {
     for (const step of backportSteps(fakeLogger, configs, backportPR, {
         gitClientType: git.gitClientType,
         gitClientApi: {
+            async getLatestPullRequestComments(_prUrl) {
+                return [];
+            },
             async createPullRequest(_backport) {
                 s += `# ${git.gitClientType}.createPullRequest`;
                 return "";

dist/gha/index.js

@@ -965,6 +965,48 @@ class GitHubClient {
         const { owner, project, id } = this.extractPullRequestData(prUrl);
         return this.getPullRequest(owner, project, id, squash);
     }
+    async getLatestPullRequestComments(prUrl) {
+        const { owner, project, id } = this.extractPullRequestData(prUrl);
+        this.logger.debug(`Fetching latest comments of pull request ${owner}/${project}/${id}`);
+        const perPage = 100;
+        const params = {
+            owner: owner,
+            repo: project,
+            issue_number: id,
+            per_page: perPage,
+        };
+        const { data, headers } = await this.octokit.issues.listComments(params);
+        const firstPage = data.map(c => c.body ?? "");
+        // Forgejo so far didn't implement pagination, and just always returns all comments.
+        const total = this.extractTotalCount(headers);
+        if (total !== undefined && data.length >= total) {
+            return firstPage;
+        }
+        const lastPage = this.extractLastPage(headers, perPage);
+        if (lastPage <= 1) {
+            return firstPage;
+        }
+        // last page may hold only one single comment, so always fetch last two pages
+        const fetchPage = async (page) => {
+            const { data } = await this.octokit.issues.listComments({ ...params, page });
+            return data.map(c => c.body ?? "");
+        };
+        const secondLast = lastPage === 2 ? firstPage : await fetchPage(lastPage - 1);
+        const last = await fetchPage(lastPage);
+        return [...secondLast, ...last];
+    }
+    extractTotalCount(headers) {
+        const total = parseInt(`${headers["x-total-count"]}`);
+        return isNaN(total) ? undefined : total;
+    }
+    extractLastPage(headers, perPage) {
+        const last = /[?&]page=(\d+)[^>]*>;\s*rel="last"/.exec(headers.link ?? "");
+        if (last) {
+            return parseInt(last[1]);
+        }
+        const total = this.extractTotalCount(headers);
+        return total === undefined ? 1 : Math.ceil(total / perPage);
+    }
     /**
      * Read the message out of a git.getCommit response. GitHub returns it at the top level
      * (`message`), whereas Gitea/Forgejo nest it under `commit` (`commit.message`). Support
@@ -1345,6 +1387,14 @@ class GitLabClient {
         }
         return commentUrl;
     }
+    // https://docs.gitlab.com/ee/api/notes.html#list-all-merge-request-notes
+    async getLatestPullRequestComments(mrUrl) {
+        const { namespace, project, id } = this.extractMergeRequestData(mrUrl);
+        const projectId = this.getProjectId(namespace, project);
+        const { data } = await this.client.get(`/projects/${projectId}/merge_requests/${id}/notes?sort=desc&per_page=100`);
+        const notes = (data ?? []);
+        return notes.map(n => n.body ?? "");
+    }
     // UTILS
     /**
      * Retrieve a gitlab user given its username
@@ -1725,21 +1775,43 @@ class Runner {
         }
         return token;
     }
+    /**
+     * Check whether a previous run already reported a failed backport to the same
+     * target branch, by looking for a hidden marker in the latest comments of the
+     * original pull request.
+     * @returns true if the failure has already been reported, false otherwise or if
+     *          the comments could not be fetched
+     */
+    async failureAlreadyReported(configs, backportPR, git) {
+        try {
+            const comments = await git.gitClientApi.getLatestPullRequestComments(configs.originalPullRequest.url);
+            return comments.some(c => c.includes(failureMarker(backportPR.base)));
+        }
+        catch (error) {
+            this.logger.warn(`Unable to fetch the comments of ${configs.originalPullRequest.url}: ${error}`);
+            return false;
+        }
+    }
     async executeBackport(configs, backportPR, git) {
         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)) {
+            this.logger.warn(`Backport to ${backportPR.base} already failed. Delete failure comment to retry. Skipping.`);
+            return;
+        }
         let i = 0;
         for (const step of backportSteps(this.logger, configs, backportPR, git)) {
             try {
                 await step();
             }
             catch (error) {
                 this.logger.error(`Something went wrong backporting to ${backportPR.base}: ${error}`);
-                if (!configs.dryRun && configs.errorNotification.enabled && configs.errorNotification.message.length > 0) {
+                if (notifyError) {
                     // notify the failure as comment in the original pull request
                     let comment = (0, runner_util_1.injectError)(configs.errorNotification.message, error);
                     comment = (0, runner_util_1.injectTargetBranch)(comment, backportPR.base);
@@ -1752,6 +1824,8 @@ class Runner {
                     catch (scriptError) {
                         this.logger.error(`Something went wrong reconstructing the script: ${scriptError}`);
                     }
+                    comment += `\n\nThe backport to \`${backportPR.base}\` will not be retried until this comment is deleted.`;
+                    comment += `\n\n${failureMarker(backportPR.base)}`;
                     await git.gitClientApi.createPullRequestComment(configs.originalPullRequest.url, comment);
                 }
                 throw error;
@@ -1761,6 +1835,10 @@ class Runner {
     }
 }
 exports["default"] = Runner;
+// Hidden marker appended to the failure notification comment.
+function failureMarker(base) {
+    return `<!-- git-backporting: backport to ${base} failed -->`;
+}
 function* backportSteps(logger, configs, backportPR, git) {
     // every failible operation should be in one dedicated closure
     // whether the backport pr targets a different repository than the original pull request's one (--tb-repo),
@@ -1882,6 +1960,9 @@ async function backportScript(configs, backportPR, git, failed) {
     for (const step of backportSteps(fakeLogger, configs, backportPR, {
         gitClientType: git.gitClientType,
         gitClientApi: {
+            async getLatestPullRequestComments(_prUrl) {
+                return [];
+            },
             async createPullRequest(_backport) {
                 s += `# ${git.gitClientType}.createPullRequest`;
                 return "";

src/service/git/git-client.ts

@@ -35,6 +35,13 @@ import { BackportPullRequest, GitClientType, GitPullRequest } from "@bp/service/
    */
    getPullRequestFromUrl(prUrl: string, squash: boolean | undefined): Promise<GitPullRequest>;
 
+  /**
+   * Get the most recent comments of the pull request.
+   * @param prUrl pull request's URL
+   * @returns {Promise<string[]>} the list of comments
+   */
+  getLatestPullRequestComments(prUrl: string): Promise<string[]>;
+
   // WRITE
 
   /**