[IMP] runbot_merge: concurrency issue with GH closing PRs being merged

Once more unto the breach, with the issue of pushing stagings (with
"closes" annotations) to the target branch making GH close the PR &
send the hook, which makes runbot_merge consider the PR closed and the
staging cancelled.

This probably still doesn't fix the issue, but it reduces the
problematic window: before this, the process first updates the
branches, then marks the PRs, then comments & closes the PRs, and
finally commits the PR update.

This means as runbot_merge is sending a comment & a status update to
each PR in a staging, GH has some time to send the "closed" webhook
behind its back, making the controller immediately cancel the current
staging, especially if the v3 endpoint is a bit slow.

By moving the commenting & closing out of the critical path (to the
feedback queue), this window should be significantly shortened.

runbot_merge/models/pull_requests.py

@@ -121,10 +121,14 @@ class Project(models.Model):
                 gh = ghs[repo] = repo.github()
 
             try:
-                gh.comment(f.pull_request, f.message)
+                if f.close:
+                    gh.close(f.pull_request, f.message)
+                else:
+                    gh.comment(f.pull_request, f.message)
             except Exception:
                 _logger.exception(
-                    "Error while trying to send a comment to %s:%s (%s)",
+                    "Error while trying to %s %s:%s (%s)",
+                    'close' if f.close else 'send a comment to',
                     repo.name, f.pull_request,
                     f.message and f.message[:200]
                 )
@@ -836,6 +840,7 @@ class Feedback(models.Model):
     # objects on non-handled branches
     pull_request = fields.Integer()
     message = fields.Char()
+    close = fields.Boolean()
 
 class Commit(models.Model):
     """Represents a commit onto which statuses might be posted,
@@ -1132,7 +1137,12 @@ class Stagings(models.Model):
                 )
                 prs.write({'state': 'merged'})
                 for pr in prs:
-                    gh[pr.repository.name].close(pr.number, 'Merged, thanks!')
+                    self.env['runbot_merge.pull_requests.feedback'].create({
+                        'repository': pr.repository.id,
+                        'pull_request': pr.number,
+                        'message': "Merged, thanks!",
+                        'close': True,
+                    })
             finally:
                 self.batch_ids.write({'active': False})
                 self.write({'active': False})

runbot_merge/tests/test_basic.py

@@ -846,6 +846,7 @@ class TestMergeMethod:
         repo.post_status(staging.id, 'success', 'legal/cla')
         repo.post_status(staging.id, 'success', 'ci/runbot')
         env['runbot_merge.project']._check_progress()
+        env['runbot_merge.project']._send_feedback()
         assert env['runbot_merge.pull_requests'].search([
             ('repository.name', '=', repo.name),
             ('number', '=', prx.number)