Force waiting for automated merge

README.md

@@ -36,10 +36,10 @@ are satisfied:
   text displayed by GitHub for these so called "mergeable" PRs varies
   depending on whether the changes are approved and whether the PR
   branch is out of date with its target branch.
-* All the _required_ checks have succeeded on the PR branch:
-  * If _all_ checks have succeeded, then GitHub says "All checks have
-    passed" next to a green check mark:
-    ![](./docs/images/all_passed.png)
+* All the _required_ checks have succeeded on the PR branch.
+  * _all_ checks, except for the special-purpose "automated merge status"
+    test (see below) have succeeded:
+    ![](./docs/images/merge_automatically_status.png)
   * If any _optional_ checks have failed, then GitHub will show "Some
     checks were not successful" message:
     ![](./docs/images/required_passed.png)
@@ -112,6 +112,14 @@ If a PR processing step fails for PR-specific reasons (e.g., a CI test
 failure), then the bot moves on to the next pull request, labeling the
 failed PR if/as needed (see below for PR labels).
 
+## Automated merge status
+
+The bot adds this required status automatically to PR and staging commit.
+During PR lifecycle the status is "pending", thus preventing the manual
+merge button on the GitHub PR page from becoming green. Anubis uses this
+mechanism to forbid manual merges, which result in wrong commit messages
+and missed staging checks. The bot satisfies this check just before
+merging (for the staging commit) and just after merging (for PR).
 
 ## Pull request labels
 
@@ -298,6 +306,7 @@ All configuration fields are required.
 *voting_delay_max* | The maximum merging age of a PR that has fewer than `config::sufficient_approvals` votes. The PR age string should comply with [timestring](https://github.com/mike182uk/timestring) parser. | "10d"
 *staging_checks*| The expected number of CI tests executed against the staging branch. | 2
 *approval_url*| The URL associated with an approval status test description. | ""
+*automated_merge_url*| The URL associated with an automated merge status test description. | ""
 *logger_params* | A JSON-formatted parameter list for the [Bunyan](https://github.com/trentm/node-bunyan) logging library [constructor](https://github.com/trentm/node-bunyan#constructor-api). | <pre>{<br>    "name": "anubis",<br>    "streams": [ ... ]<br>}</pre>
 
 TODO: Merge all three "mutually exclusive" boolean `*_run` options into one `run_mode` option accepting on of four mode names, including "production". Document individual string values in a separate table (here).

src/Config.js

@@ -32,6 +32,7 @@ class ConfigOptions {
         this._stagingChecks = conf.staging_checks;
         this._loggerParams = conf.logger_params;
         this._approvalUrl = conf.approval_url;
+        this._automatedMergeUrl = conf.automated_merge_url;
 
         // unused
         this._githubUserNoreplyEmail = null;
@@ -112,6 +113,15 @@ class ConfigOptions {
     approvalContext() { return "PR approval"; }
 
     copiedDescriptionSuffix() { return " (copied from PR by Anubis)"; }
+
+    // an URL of the description of the automated merge test status
+    automatedMergeStatusUrl() { return this._automatedMergeUrl; }
+
+    // the 'context name' of the automated merge test status
+    automatedMergeStatusContext() { return "Merge automatically"; }
+
+    // whether the bot will create the automated merge test statuses for PR and staged commit
+    manageAutomatedMergeStatus() { return this.automatedMergeStatusUrl().length > 0; }
 }
 
 const configFile = process.argv.length > 2 ? process.argv[2] : './config.json';

src/MergeContext.js

@@ -160,16 +160,16 @@ class StatusChecks
         assert(expectedStatusCount !== undefined);
         assert(expectedStatusCount !== null);
         assert(context);
-        this.expectedStatusCount = expectedStatusCount;
+        this._expectedStatusCount = expectedStatusCount;
         this.context = context;
-        this.requiredStatuses = [];
+        this._requiredStatuses = [];
         this.optionalStatuses = [];
     }
 
     addRequiredStatus(requiredStatus) {
         assert(requiredStatus);
         assert(!this.hasStatus(requiredStatus.context));
-        this.requiredStatuses.push(requiredStatus);
+        this._requiredStatuses.push(requiredStatus);
     }
 
     addOptionalStatus(optionalStatus) {
@@ -179,19 +179,19 @@ class StatusChecks
     }
 
     hasStatus(context) {
-        return this.requiredStatuses.some(el => el.context.trim() === context.trim()) ||
+        return this._requiredStatuses.some(el => el.context.trim() === context.trim()) ||
             this.optionalStatuses.some(el => el.context.trim() === context.trim());
     }
 
     hasApprovalStatus(approval) {
-        return this.requiredStatuses.some(el =>
+        return this._requiredStatuses.some(el =>
                 el.context.trim() === Config.approvalContext() &&
                 el.state === approval.state &&
                 el.description === approval.description);
     }
 
     setApprovalStatus(approval) {
-        this.requiredStatuses = this.requiredStatuses.filter(st => st.context !== Config.approvalContext());
+        this._requiredStatuses = this._requiredStatuses.filter(st => st.context !== Config.approvalContext());
         let raw = {
             state: approval.state,
             target_url: Config.approvalUrl(),
@@ -201,10 +201,40 @@ class StatusChecks
         this.addRequiredStatus(new StatusCheck(raw));
     }
 
+    hasAutomatedMergeStatus(state, description) {
+        return this._requiredStatuses.some(el =>
+                el.context.trim() === Config.automatedMergeStatusContext() &&
+                el.state === state &&
+                el.description === description);
+    }
+
+    setAutomatedMergeStatus(state, description) {
+        this._requiredStatuses = this._requiredStatuses.filter(st => st.context !== Config.automatedMergeStatusContext());
+        let raw = {
+            state: state,
+            target_url: Config.automatedMergeStatusUrl(),
+            description: description,
+            context: Config.automatedMergeStatusContext()
+        };
+        this.addRequiredStatus(new StatusCheck(raw));
+    }
+
+    // all required statuses except the 'automated merge test' status
+    requiredStatuses() {
+        return this._requiredStatuses.filter(st => st.context !== Config.automatedMergeStatusContext());
+    }
+
+    // the 'expected status count' except the 'automated merge test' status
+    expectedStatusCount() {
+        const hasAutomated = this._requiredStatuses.some(el => el.context.trim() === Config.automatedMergeStatusContext());
+        return hasAutomated ? this._expectedStatusCount - 1 : this._expectedStatusCount;
+    }
+
     // no more required status changes or additions are expected
     final() {
-        return (this.requiredStatuses.length >= this.expectedStatusCount) &&
-            this.requiredStatuses.every(check => !check.pending());
+        const required = this.requiredStatuses();
+        return (required.length >= this.expectedStatusCount()) &&
+            required.every(check => !check.pending());
     }
 
     // something went wrong with at least one of the required status checks
@@ -215,18 +245,18 @@ class StatusChecks
     // Whether at least one of the required status checks failed.
     // Ignores checks with the given context, when searching.
     _failedExcept(context) {
-        const filteredChecks = this.requiredStatuses.filter(st => context ? st.context !== context : true);
+        const filteredChecks = this.requiredStatuses().filter(st => context ? st.context !== context : true);
         return filteredChecks.some(check => check.failed());
     }
 
     // the results are final and all checks were a success
     succeeded() {
-        return this.final() && this.requiredStatuses.every(check => check.success());
+        return this.final() && this.requiredStatuses().every(check => check.success());
     }
 
     toString() {
-        let combinedStatus = "context: " + this.context + " expected/required/optional: " + this.expectedStatusCount + "/" +
-            this.requiredStatuses.length + "/" + this.optionalStatuses.length + ", combined: ";
+        let combinedStatus = "context: " + this.context + " expected/required/optional: " + this.expectedStatusCount() + "/" +
+            this.requiredStatuses().length + "/" + this.optionalStatuses.length + ", combined: ";
         if (this.failed())
             combinedStatus += "failure";
         else if (this.succeeded())
@@ -237,7 +267,7 @@ class StatusChecks
             combinedStatus += "to-be-determined";
 
         let requiredDetail = "";
-        for (let st of this.requiredStatuses) {
+        for (let st of this.requiredStatuses()) {
             if (requiredDetail !== "")
                 requiredDetail += ", ";
             requiredDetail += st.context + ": " + st.state;
@@ -599,6 +629,41 @@ class PullRequest {
         await GH.createStatus(sha, this._approval.state, Config.approvalUrl(), this._approval.description, Config.approvalContext());
     }
 
+    async _setAutomatedMergeStatuses() {
+        if (!Config.manageAutomatedMergeStatus())
+            return;
+
+        const state = this._prState.merged() ? "success" : "pending";
+        const description = this._automatedMergeStatusDescription();
+
+        if (!this._prStatuses.hasAutomatedMergeStatus(state, description)) {
+            await this._createAutomatedMergeStatus(this._prHeadSha(), state);
+            this._prStatuses.setAutomatedMergeStatus(state, description);
+        }
+
+        await this._setAutomatedMergeStatusForStaged(state);
+    }
+
+    async _setAutomatedMergeStatusForStaged(state) {
+        const description = this._automatedMergeStatusDescription();
+        if (!Config.manageAutomatedMergeStatus())
+            return;
+
+        if (this._stagedStatuses && !this._stagedStatuses.hasAutomatedMergeStatus(state, description)) {
+            await this._createAutomatedMergeStatus(this._stagedSha(), state);
+            this._stagedStatuses.setAutomatedMergeStatus(state, description);
+        }
+    }
+
+    async _createAutomatedMergeStatus(sha, state) {
+        if (this._dryRun("creating automated merge status"))
+            return;
+        await GH.createStatus(sha, state, Config.automatedMergeStatusUrl(),
+                this._automatedMergeStatusDescription(), Config.automatedMergeStatusContext());
+    }
+
+    _automatedMergeStatusDescription() { return this._prState.toString(); }
+
     async _getRequiredContexts() {
         if (this._requiredContextsCache)
             return this._requiredContextsCache;
@@ -639,7 +704,9 @@ class PullRequest {
     // returns filled StatusChecks object
     async _getStagingStatuses() {
         const combinedStagingStatuses = await GH.getStatuses(this._stagedSha());
-        const genuineStatuses = combinedStagingStatuses.statuses.filter(st => !st.description.endsWith(Config.copiedDescriptionSuffix()));
+        const genuineStatuses = combinedStagingStatuses.statuses.filter(st =>
+                !st.description.endsWith(Config.copiedDescriptionSuffix()) &&
+                st.context !== Config.automatedMergeStatusContext());
         assert(genuineStatuses.length <= Config.stagingChecks());
         let statusChecks = new StatusChecks(Config.stagingChecks(), "Staging");
         // all genuine checks are 'required'
@@ -763,6 +830,8 @@ class PullRequest {
 
         assert(!this._prState.merged());
 
+        await this._setAutomatedMergeStatuses();
+
         this._messageValid = this._prMessageValid();
         this._log("messageValid: " + this._messageValid);
 
@@ -792,6 +861,8 @@ class PullRequest {
 
         assert(this._prState.merged());
 
+        await this._setAutomatedMergeStatuses();
+
         // Clear any positive labels (there should be no negatives here)
         // because Config.mergedLabel() set below already implies that all
         // intermediate processing steps have succeeded.
@@ -1076,7 +1147,7 @@ class PullRequest {
                 this._log("_supplyStagingWithPrRequired: skip existing " + requiredContext);
                 continue;
             }
-            const requiredPrStatus = this._prStatuses.requiredStatuses.find(el => el.context.trim() === requiredContext.trim());
+            const requiredPrStatus = this._prStatuses.requiredStatuses().find(el => el.context.trim() === requiredContext.trim());
             assert(requiredPrStatus);
             assert(!requiredPrStatus.description.endsWith(Config.copiedDescriptionSuffix()));
 
@@ -1166,6 +1237,8 @@ class PullRequest {
 
         assert(!this._stagingBanned);
 
+        await this._setAutomatedMergeStatusForStaged("success");
+
         try {
             await GH.updateReference(this._prBaseBranchPath(), this._stagedSha(), false);
         } catch (e) {