[mq] remove autosubmit label on dequeue only (#4041)

Only remove the autosubmit label if the PR is dequeued.

This is a speculative fix for flutter/flutter#159139

app_dart/lib/src/request_handlers/github/webhook_subscription.dart

@@ -17,6 +17,7 @@ import '../../model/github/checks.dart' as cocoon_checks;
 import '../../request_handling/exceptions.dart';
 import '../../request_handling/subscription_handler.dart';
 import '../../service/datastore.dart';
+import '../../service/github_service.dart';
 import '../../service/logging.dart';
 
 // Filenames which are not actually tests.
@@ -202,6 +203,9 @@ class GithubWebhookSubscription extends SubscriptionHandler {
         // and schedule new ones.
         await _scheduleIfMergeable(pullRequestEvent);
         break;
+      case 'dequeued':
+        await _respondToPullRequestDequeued(pullRequestEvent);
+        break;
       // Ignore the rest of the events.
       case 'ready_for_review':
       case 'unlabeled':
@@ -269,6 +273,36 @@ class GithubWebhookSubscription extends SubscriptionHandler {
     return gobCommit != null;
   }
 
+  /// Responds to the "dequeued" pull request event.
+  ///
+  /// See also: https://docs.github.com/en/webhooks/webhook-events-and-payloads?actionType=dequeued#pull_request
+  Future<void> _respondToPullRequestDequeued(
+    PullRequestEvent pullRequestEvent,
+  ) async {
+    final PullRequest pr = pullRequestEvent.pullRequest!;
+    final RepositorySlug slug = pullRequestEvent.repository!.slug();
+    final GithubService githubService = await config.createGithubService(slug);
+
+    // Remove the autosubmit label when a pull request is kicked out of the
+    // merge queue to avoid infinite loops.
+    //
+    // An example of an infinite loop:
+    //
+    // 1. Autosubmit bot is notified that a PR is ready (reviewed, all green, has `autosubmit` label).
+    // 2. Autosubmit bot puts the PR onto the merge queue.
+    // 3. The PR fails some tests in the merge queue.
+    // 4. Github kicks the PR back, removing it fom the merge queue.
+    // 5. GOTO step 1.
+    //
+    // Removing the `autosubmit` label will prevent the autosubmit bot from
+    // repeating the process, until a human looks at the PR, decides that it's
+    // ready again, and manually adds the `autosubmit` label on it.
+    final bool hasAutosubmitLabel = pr.labels?.any((label) => label.name == Config.kAutosubmitLabel) ?? false;
+    if (hasAutosubmitLabel) {
+      await githubService.removeLabel(slug, pr.number!, Config.kAutosubmitLabel);
+    }
+  }
+
   /// This method assumes that jobs should be cancelled if they are already
   /// runnning.
   Future<void> _scheduleIfMergeable(

app_dart/lib/src/service/config.dart

@@ -32,6 +32,11 @@ const String kDefaultBranchName = 'master';
 class Config {
   Config(this._db, this._cache);
 
+  /// Labels autosubmit looks for on pull requests
+  ///
+  /// Keep this in sync with the similar `Config` class in `auto_submit`.
+  static const String kAutosubmitLabel = 'autosubmit';
+
   final DatastoreDB _db;
 
   final CacheService _cache;

app_dart/lib/src/service/github_service.dart

@@ -189,6 +189,15 @@ class GithubService {
     return github.issues.listByRepo(slug, labels: labels, state: state).toList();
   }
 
+  /// Removes a label from a pull request.
+  Future<bool> removeLabel(
+    RepositorySlug slug,
+    int issueNumber,
+    String label,
+  ) {
+    return github.issues.removeLabelForIssue(slug, issueNumber, label);
+  }
+
   /// Get an issue with the issue number
   Future<Issue>? getIssue(
     RepositorySlug slug, {

app_dart/test/request_handlers/github/webhook_subscription_test.dart

@@ -2329,6 +2329,37 @@ void foo() {
       expect(batchRequestCalled, isTrue);
     });
 
+    test('Removes the "autosubmit" label on dequeued', () async {
+      const int issueNumber = 123;
+
+      tester.message = generateGithubWebhookMessage(
+        action: 'dequeued',
+        number: issueNumber,
+        withAutosubmit: true,
+      );
+
+      await tester.post(webhook);
+
+      expect(
+        githubService.removedLabels,
+        [(RepositorySlug('flutter', 'flutter'), 123, 'autosubmit')],
+      );
+    });
+
+    test('Does not try to remove the "autosubmit" label on dequeued if it is not there', () async {
+      const int issueNumber = 123;
+
+      tester.message = generateGithubWebhookMessage(
+        action: 'dequeued',
+        number: issueNumber,
+        withAutosubmit: false,
+      );
+
+      await tester.post(webhook);
+
+      expect(githubService.removedLabels, isEmpty);
+    });
+
     group('BuildBucket', () {
       const int issueNumber = 123;
 

app_dart/test/src/service/fake_github_service.dart

@@ -39,6 +39,14 @@ class FakeGithubService implements GithubService {
     return <IssueLabel>[];
   }
 
+  final List<(RepositorySlug slug, int issueNumber, String label)> removedLabels = [];
+
+  @override
+  Future<bool> removeLabel(RepositorySlug slug, int issueNumber, String label) async {
+    removedLabels.add(((slug, issueNumber, label)));
+    return true;
+  }
+
   @override
   Future<void> assignReviewer(RepositorySlug slug, {int? pullRequestNumber, String? reviewer}) async {}
 

app_dart/test/src/utilities/webhook_generators.dart

@@ -24,6 +24,7 @@ PushMessage generateGithubWebhookMessage({
   String mergeCommitSha = 'fd6b46416c18de36ce87d0241994b2da180cab4c',
   RepositorySlug? slug,
   bool includeChanges = false,
+  bool withAutosubmit = false,
 }) {
   final String data = (pb.GithubWebhookMessage.create()
         ..event = event
@@ -40,6 +41,7 @@ PushMessage generateGithubWebhookMessage({
           slug: slug,
           mergeCommitSha: mergeCommitSha,
           includeChanges: includeChanges,
+          withAutosubmit: withAutosubmit,
         ))
       .writeToJson();
   return PushMessage(data: data, messageId: 'abc123');
@@ -59,6 +61,7 @@ String _generatePullRequestEvent(
   bool isMergeable = true,
   String mergeCommitSha = 'fd6b46416c18de36ce87d0241994b2da180cab4c',
   bool includeChanges = false,
+  bool withAutosubmit = false,
 }) {
   slug ??= Config.flutterSlug;
   baseRef ??= Config.defaultBranch(slug);
@@ -134,6 +137,16 @@ String _generatePullRequestEvent(
         "color": "5319e7",
         "default": false
       },''' : ''}
+      ${withAutosubmit ? '''
+      {
+        "id": 4232992339,
+        "node_id": "LA_kwDOAeUeuM78TlZT",
+        "url": "https://api.github.com/repos/${slug.fullName}/labels/autosubmit",
+        "name": "autosubmit",
+        "color": "008820",
+        "default": false,
+        "description": "Merge PR when tree becomes green via auto submit App"
+      },''' : ''}
       {
         "id": 283480100,
         "node_id": "MDU6TGFiZWwyODM0ODAxMDA=",

auto_submit/lib/service/config.dart

@@ -60,6 +60,8 @@ class Config {
   static const String kTreeStatusDiscordUrl = 'TREE_STATUS_DISCORD_WEBHOOK_URL';
 
   /// Labels autosubmit looks for on pull requests
+  ///
+  /// Keep this in sync with the similar `Config` class in `app_dart`.
   static const String kAutosubmitLabel = 'autosubmit';
 
   /// GitHub check stale threshold.

auto_submit/lib/service/pull_request_validation_service.dart

@@ -140,20 +140,6 @@ class PullRequestValidationService extends ValidationService {
       await githubService.createComment(slug, prNumber, message);
       log.info(message);
     } else {
-      // Remove the autosubmit label post enqueue/merge to avoid infinite loops.
-      // Here's an example of an infinite loop:
-      //
-      // 1. Autosubmit bot is notified that a PR is ready (reviewed, all green, has `autosubmit` label).
-      // 2. Autosubmit bot puts the PR onto the merge queue.
-      // 3. The PR fails some tests in the merge queue.
-      // 4. Github kicks the PR back, removing it fom the merge queue.
-      // 5. GOTO step 1.
-      //
-      // Removing the `autosubmit` label will prevent the autosubmit bot from
-      // repeating the process, until a human looks at the PR, decides that it's
-      // ready again, and manually adds the `autosubmit` label on it.
-      await githubService.removeLabel(slug, prNumber, Config.kAutosubmitLabel);
-
       log.info('Pull Request ${slug.fullName}/$prNumber was merged successfully!');
       log.info('Attempting to insert a pull request record into the database for $prNumber');
       await insertPullRequestRecord(

auto_submit/test/service/pull_request_validation_service_test.dart

@@ -136,7 +136,7 @@ void main() {
     // was removed, there was no issue comment generated, and the message was
     // acknowledged.
     expect(githubService.issueComment, isNull);
-    expect(githubService.labelRemoved, isTrue);
+    expect(githubService.labelRemoved, isFalse);
     assert(pubsub.messagesQueue.isEmpty);
   });
 
@@ -353,7 +353,7 @@ void main() {
       // was removed, there was no issue comment generated, and the message was
       // acknowledged.
       expect(githubService.issueComment, isNull);
-      expect(githubService.labelRemoved, isTrue);
+      expect(githubService.labelRemoved, isFalse);
       assert(pubsub.messagesQueue.isEmpty);
     });