Do not ignore staged PRs

src/Main.js

@@ -2,6 +2,7 @@ const createHandler = require('github-webhook-handler');
 const Config = require('./Config.js');
 const Log = require('./Logger.js');
 const Merger = require('./RepoMerger.js');
+const Util = require('./Util.js');
 
 const Logger = Log.Logger;
 
@@ -22,48 +23,89 @@ WebhookHandler.on('error', (err) => {
 WebhookHandler.on('pull_request_review', (ev) => {
     const pr = ev.payload.pull_request;
     Logger.info("pull_request_review event:", ev.payload.id, pr.number, pr.head.sha, pr.state);
-    Merger.run();
+    Merger.run(Util.PrId.PrNum(pr.number));
 });
 
 // https://developer.github.com/v3/activity/events/types/#pullrequestevent
 WebhookHandler.on('pull_request', (ev) => {
     const pr = ev.payload.pull_request;
     Logger.info("pull_request event:", ev.payload.id, pr.number, pr.head.sha, pr.state);
-    Merger.run();
+    Merger.run(Util.PrId.PrNum(pr.number));
 });
 
 // https://developer.github.com/v3/activity/events/types/#statusevent
 WebhookHandler.on('status', (ev) => {
     const e = ev.payload;
     Logger.info("status event:", e.id, e.sha, e.context, e.state);
-    Merger.run();
+    const branches = Array.from(e.branches, b => b.name);
+    if (branches.includes(Config.stagingBranch()))
+        Merger.run(Util.PrId.PrMessage(e.commit.commit.message));
+    else
+        Merger.run(Util.PrId.BranchList(branches));
 });
 
 // https://developer.github.com/v3/activity/events/types/#pushevent
 WebhookHandler.on('push', (ev) => {
     const e = ev.payload;
     Logger.info("push event:", e.ref);
-    Merger.run();
+
+    // e.ref as refs/heads/branch_name
+    const parts = e.ref.split('/')
+    const branch = parts[parts.length-1];
+
+    if (branch !== Config.stagingBranch()) {
+        Merger.run(Util.PrId.Branch(branch));
+        return;
+    }
+
+    if (e.head_commit) {
+        Merger.run(Util.PrId.PrMessage(e.head_commit.message));
+        return;
+    }
+    Logger.warn("push event: e.head_commit is null");
+    Merger.run(Util.PrId.Empty());
 });
 
 // https://docs.github.com/ru/webhooks/webhook-events-and-payloads#workflow_run
 WebhookHandler.on('workflow_run', (ev) => {
     const e = ev.payload.workflow_run;
     Logger.info("workflow_run event:", e.head_sha);
-    Merger.run();
+    // e.pull_requests is empty for the staged commit
+    if (e.head_branch === Config.stagingBranch()) {
+        Merger.run(Util.PrId.Sha(e.head_sha));
+        return;
+    }
+    if (!e.pull_requests.length) {
+        Logger.warn("workflow_run event: pull_requests array is empty");
+        Merger.run(Util.PrId.Empty());
+        return;
+    }
+    const list = Array.from(e.pull_requests, v => v.number);
+    Merger.run(Util.PrId.PrNumList(list));
 });
 
 // https://docs.github.com/en/webhooks/webhook-events-and-payloads#check_run
 WebhookHandler.on('check_run', (ev) => {
     const e = ev.payload.check_run;
     Logger.info("check_run event:", e.head_sha);
-    Merger.run();
+    // e.check_suite.pull_requests is empty for the staged commit
+    if (e.check_suite.head_branch === Config.stagingBranch()) {
+        Merger.run(Util.PrId.Sha(e.head_sha));
+        return;
+    }
+    if (!e.check_suite.pull_requests.length) {
+        Logger.warn("check_run event: pull_requests array is empty");
+        Merger.run(Util.PrId.Empty());
+        return;
+    }
+    const list = Array.from(e.check_suite.pull_requests, v => v.number);
+    Merger.run(Util.PrId.PrNumList(list));
 });
 
 WebhookHandler.on('ping', (ev) => {
     const e = ev.payload;
     Logger.info("ping event, hook_id:", e.hook_id);
 });
 
-Merger.run(WebhookHandler);
+Merger.run(null, WebhookHandler);
 

src/PrMerger.js

@@ -49,18 +49,32 @@ class PrMerger {
         this._ignored = 0; // the number of PRs skipped due to human markings; TODO: Rename to _ignoredAsMarked
         this._ignoredAsUnchanged = 0; // the number of PRs skipped due to lack of PR updates
         this._todo = null; // raw PRs to be processed
+        this._stagedBranchSha; // the SHA of the branch head
     }
 
     // Implements a single Anubis processing step.
     // Returns suggested wait time until the next step (in milliseconds).
-    async execute(lastScan) {
+    async execute(lastScan, prIds) {
         Logger.info("runStep running");
+        Logger.info('prIds: [' + prIds.join() + ']');
 
         this._todo = await GH.getOpenPrs();
         this._total = this._todo.length;
         Logger.info(`Received ${this._total} PRs from GitHub:`, this._prNumbers());
 
-        await this._determineProcessingOrder(await this._current());
+        const currentPr = await this._current();
+        await this._determineProcessingOrder(currentPr);
+
+        let updatedPrs = await this._prNumbersFromIds(prIds, currentPr, this._todo);
+        // Treat the 'null' updatedPrs below as if all PRs have been 'updated'.
+        // An empty updatedPrs means that none of the PRs has been updated.
+        if (updatedPrs === null) {
+            Logger.warn('discarding PR scan optimization');
+        } else {
+            // remove duplicates
+            updatedPrs = updatedPrs.filter((v, idx) => updatedPrs.indexOf(v) === idx);
+            Logger.info('recently updated PRs: [' + updatedPrs.join() + ']');
+        }
 
         let somePrWasStaged = false;
         let currentScan = new PrScanResult(this._todo);
@@ -74,7 +88,14 @@ class PrMerger {
                     continue;
                 }
 
-                if (lastScan && lastScan.isStillUnchanged(rawPr, currentScan.scanDate)) {
+                const clearedForMerge = rawPr.labels.some(el => el.name === Config.clearedForMergeLabel());
+                const updated = !updatedPrs || updatedPrs.some(el => el === rawPr.number);
+
+                // If a PR A was cleared for merge, it may be ready for merge (no updates are expected)
+                // but waiting for another PR B which is currently being merged. If we switched back to PR A
+                // it may remain still ready for merge so we need to process it anyway.
+                // The 'lastScan' check turns off optimization for the initial scan.
+                if (!clearedForMerge && !updated && lastScan && lastScan.isStillUnchanged(rawPr, currentScan.scanDate)) {
                     const updatedAt = new Date(rawPr.updated_at);
                     Logger.info(`Ignoring PR${rawPr.number} because it has not changed since ${updatedAt.toISOString()}`);
                     this._ignoredAsUnchanged++;
@@ -145,12 +166,50 @@ class PrMerger {
         Logger.info("PR processing order:", this._prNumbers());
     }
 
+    // Translates each element of prIds into a PR number.
+    // Returns an array of PR numbers if it could translate all Ids or null otherwise.
+    async _prNumbersFromIds(prIds, currentPr, prList) {
+        let prNumList = [];
+
+        for (let id of prIds) {
+            if (id.type === "prNum") {
+                prNumList.push(id.value);
+            } else if (id.type === "sha") {
+                if (currentPr && (id.value === this._stagedBranchSha)) {
+                    prNumList.push(currentPr.number.toString());
+                } else {
+                    const commit = await GH.getCommit(id.value);
+                    const prNum = Util.ParsePrNumber(commit.message);
+                    if (prNum === null) {
+                        Logger.warn(`Could not get a PR number by parsing ${id.value} message`);
+                        return null;
+                    } else {
+                        Logger.info(`Got PR${prNum} from ${id.value} message`);
+                        prNumList.push(prNum);
+                    }
+                }
+            } else if (id.type === "branch") {
+                const pr = prList.find(p => p.head.ref === id.value);
+                if (pr) {
+                    prNumList.push(pr.number.toString());
+                } else {
+                    Logger.warn(`Could not find a PR by ${id} branch`);
+                    return null;
+                }
+            } else {
+                assert(id.isEmpty());
+                return null;
+            }
+        }
+        return prNumList;
+    }
+
     // Returns a raw PR with a staged commit (or null).
     // If that PR exists, it is in either a "staged" or "merged" state.
     async _current() {
         Logger.info("Looking for the current PR...");
-        const stagedBranchSha = await GH.getReference(Config.stagingBranchPath());
-        const stagedBranchCommit = await GH.getCommit(stagedBranchSha);
+        this._stagedBranchSha = await GH.getReference(Config.stagingBranchPath());
+        const stagedBranchCommit = await GH.getCommit(this._stagedBranchSha);
         Logger.info("Staged branch head sha: " + stagedBranchCommit.sha);
         const prNum = Util.ParsePrNumber(stagedBranchCommit.message);
         if (prNum === null) {
@@ -169,11 +228,11 @@ class PrMerger {
 } // PrMerger
 
 // promises to process all PRs once, hiding PrMerger from callers
-async function Step() {
+async function Step(prIds) {
     const lastScan = _LastScan;
     _LastScan = null;
     const mergerer = new PrMerger();
-    _LastScan = await mergerer.execute(lastScan);
+    _LastScan = await mergerer.execute(lastScan, prIds);
     return _LastScan.minDelay;
 }
 

src/RepoMerger.js

@@ -16,6 +16,7 @@ class RepoMerger {
         this._running = false;
         this._handler = null;
         this._server = null;
+        this._prIds = [];
     }
 
     _createServer() {
@@ -46,11 +47,14 @@ class RepoMerger {
         });
     }
 
-    // prNum (if provided) corresponds to a PR, scheduled this 'run'
-    async run(handler) {
+    // prIds (if provided) an array of Util.PrId elements filled by an event
+    async run(prIds, handler) {
         if (handler)
             this._handler = handler;
 
+        if (prIds)
+            this._prIds.push(...prIds);
+
         if (this._running) {
             Logger.info("Already running, planning rerun.");
             this._rerun = true;
@@ -64,7 +68,9 @@ class RepoMerger {
                 this._rerun = false;
                 if (!this._server)
                     await this._createServer();
-                rerunIn = await Step();
+                const ids = this._prIds;
+                this._prIds = [];
+                rerunIn = await Step(ids);
             } catch (e) {
                 Log.LogError(e, "RepoMerger.run");
                 this._rerun = true;

src/Util.js

@@ -64,10 +64,41 @@ class ErrorContext extends Error {
     }
 }
 
+// Represents a PR 'identificator' as a string, either of
+// PR number, or
+// PR branch name (without 'refs' or 'heads' prefixes), or
+// staging branch commit SHA (including stale commits).
+class PrId
+{
+    constructor(type, val) {
+        assert(type !== undefined && val !== undefined);
+        this.type = type; // a PR identificator type ("branch", "sha", "prNum") or null
+        this.value = val; // a PR identificator or null
+    }
+
+    static Empty() { return [new PrId(null, null)]; }
+    static Branch(branch) { return [new PrId("branch", branch)]; }
+    static BranchList(branches) { return Array.from(branches, b => new PrId("branch", b)); }
+    static Sha(sha) { return [new PrId("sha", sha)]; }
+    static PrNum(prNum) { return [new PrId("prNum", prNum.toString())]; }
+    static PrMessage(message) {
+        const prNum = ParsePrNumber(message);
+        if (prNum !== null)
+            return [new PrId("prNum", prNum)];
+        return PrId.Empty();
+    }
+    static PrNumList(list) { return Array.from(list, prNum => new PrId("prNum", prNum.toString())); }
+
+    isEmpty() { return this.type === null; }
+
+    toString() { return `${this.type}:${this.value}`; }
+}
+
 module.exports = {
     sleep: sleep,
     commonParams: commonParams,
     ParsePrNumber: ParsePrNumber,
-    ErrorContext: ErrorContext
+    ErrorContext: ErrorContext,
+    PrId: PrId,
 };