Implemented base commit expiration validation (#4849)

ievdokdm · web-flow · commit 074f8f0c41e9 · 2025-09-12T16:11:25.000Z
Implemented base commit expiration validation To turn feature on `base_commit_allowed_days` have to be added into https://github.com/flutter/.github/blob/main/autosubmit/autosubmit.yml Issue: [170476](flutter/flutter#170476)
diff --git a/auto_submit/lib/configuration/repository_configuration.dart b/auto_submit/lib/configuration/repository_configuration.dart
@@ -21,6 +21,7 @@ class RepositoryConfiguration {
   static const String supportNoReviewRevertKey = 'support_no_review_revert';
   static const String requiredCheckRunsOnRevertKey =
       'required_checkruns_on_revert';
+  static const String baseCommitAllowedDaysKey = 'base_commit_allowed_days';
 
   static const String defaultBranchStr = 'default';
 
@@ -33,14 +34,16 @@ class RepositoryConfiguration {
     bool? runCi,
     bool? supportNoReviewReverts,
     Set<String>? requiredCheckRunsOnRevert,
+    int? baseCommitAllowedDays,
   }) : allowConfigOverride = allowConfigOverride ?? false,
        defaultBranch = defaultBranch ?? defaultBranchStr,
        autoApprovalAccounts = autoApprovalAccounts ?? <String>{},
        approvingReviews = approvingReviews ?? 2,
        approvalGroup = approvalGroup ?? 'flutter-hackers',
        runCi = runCi ?? true,
        supportNoReviewReverts = supportNoReviewReverts ?? true,
-       requiredCheckRunsOnRevert = requiredCheckRunsOnRevert ?? <String>{};
+       requiredCheckRunsOnRevert = requiredCheckRunsOnRevert ?? <String>{},
+       baseCommitAllowedDays = baseCommitAllowedDays ?? 0;
 
   /// This flag allows the repository to override the org level configuration.
   bool allowConfigOverride;
@@ -71,6 +74,10 @@ class RepositoryConfiguration {
   /// merged.
   Set<String> requiredCheckRunsOnRevert;
 
+  /// The number of days that the base commit of the pull request can not be
+  /// older than. If less than 1 then it will not be checked.
+  int baseCommitAllowedDays;
+
   @override
   String toString() {
     final stringBuffer = StringBuffer();
@@ -88,6 +95,7 @@ class RepositoryConfiguration {
     for (var checkrun in requiredCheckRunsOnRevert) {
       stringBuffer.writeln('  - $checkrun');
     }
+    stringBuffer.writeln('$baseCommitAllowedDaysKey: $baseCommitAllowedDays');
     return stringBuffer.toString();
   }
 
@@ -125,6 +133,7 @@ class RepositoryConfiguration {
       runCi: yamlDoc[runCiKey] as bool?,
       supportNoReviewReverts: yamlDoc[supportNoReviewRevertKey] as bool?,
       requiredCheckRunsOnRevert: requiredCheckRunsOnRevert,
+      baseCommitAllowedDays: yamlDoc[baseCommitAllowedDaysKey] as int?,
     );
   }
 }
diff --git a/auto_submit/lib/configuration/repository_configuration_manager.dart b/auto_submit/lib/configuration/repository_configuration_manager.dart
@@ -141,6 +141,7 @@ class RepositoryConfigurationManager {
       runCi: globalConfiguration.runCi,
       supportNoReviewReverts: globalConfiguration.supportNoReviewReverts,
       requiredCheckRunsOnRevert: globalConfiguration.requiredCheckRunsOnRevert,
+      baseCommitAllowedDays: globalConfiguration.baseCommitAllowedDays,
     );
 
     // auto approval accounts, they should be empty if nothing was defined
@@ -186,6 +187,15 @@ class RepositoryConfigurationManager {
       );
     }
 
+    // if the local configuration is not the default value then use it.
+    // Can be setted to negative value in local configuration to turn-off
+    // base commit allowed days validation.
+    final localBaseCommitAllowedDays = localConfiguration.baseCommitAllowedDays;
+    if (localBaseCommitAllowedDays != 0) {
+      mergedRepositoryConfiguration.baseCommitAllowedDays =
+          localConfiguration.baseCommitAllowedDays;
+    }
+
     return mergedRepositoryConfiguration;
   }
 }
diff --git a/auto_submit/lib/validations/base_commit_date_allowed.dart b/auto_submit/lib/validations/base_commit_date_allowed.dart
@@ -0,0 +1,84 @@
+// Copyright 2025 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'package:cocoon_server/logging.dart';
+import 'package:github/github.dart' as github;
+
+import '../model/auto_submit_query_result.dart';
+import 'validation.dart';
+
+/// Validates that the base of the PR commit is not older than a specified in
+/// configuration number of days.
+class BaseCommitDateAllowed extends Validation {
+  BaseCommitDateAllowed({required super.config});
+
+  @override
+  String get name => 'BaseCommitDateAllowed';
+
+  @override
+  /// Implements the validation.
+  Future<ValidationResult> validate(
+    QueryResult result,
+    github.PullRequest messagePullRequest,
+  ) async {
+    final slug = messagePullRequest.base!.repo!.slug();
+    final gitHubService = await config.createGithubService(slug);
+    final sha = messagePullRequest.head!.sha!;
+    final repositoryConfiguration = await config.getRepositoryConfiguration(
+      slug,
+    );
+
+    // If the base commit allowed days is less than or equal to 0 then the
+    // validation is turned off and the base commit creation date is ignored.
+    if (repositoryConfiguration.baseCommitAllowedDays <= 0) {
+      log.info(
+        'PR base commit creation date validation turned off by setting '
+        'base_commit_allowed_days to ${repositoryConfiguration.baseCommitAllowedDays}.',
+      );
+      return ValidationResult(
+        true,
+        Action.IGNORE_FAILURE,
+        'PR base commit creation date validation turned off by setting '
+        'base_commit_allowed_days to ${repositoryConfiguration.baseCommitAllowedDays}.',
+      );
+    }
+
+    // If the base commit creation date is null then the validation fails but
+    // should not block the PR merging.
+    final commit = await gitHubService.getCommit(slug, sha);
+    if (commit.commit?.author?.date == null) {
+      log.info(
+        'PR ${slug.fullName}/${messagePullRequest.number} base commit creation '
+        'date is null.',
+      );
+      return ValidationResult(
+        false,
+        Action.IGNORE_FAILURE,
+        'Could not find the base commit creation date of the PR '
+        '${slug.fullName}/${messagePullRequest.number}.',
+      );
+    }
+
+    log.info(
+      'PR ${slug.fullName}/${messagePullRequest.number} base creation date is: '
+      '${commit.commit?.author?.date}. Allowed age is '
+      '${repositoryConfiguration.baseCommitAllowedDays} days.',
+    );
+
+    final isBaseRecent = commit.commit!.author!.date!.isAfter(
+      DateTime.now().subtract(
+        Duration(days: repositoryConfiguration.baseCommitAllowedDays),
+      ),
+    );
+
+    final message = isBaseRecent
+        ? 'The base commit of the PR is recent enough for merging.'
+        : 'The base commit of the PR is older than '
+              '${repositoryConfiguration.baseCommitAllowedDays} days and can not '
+              'be merged. Please merge the latest changes from the main into '
+              'this branch and resubmit the PR.';
+
+    return ValidationResult(isBaseRecent, Action.REMOVE_LABEL, message);
+  }
+}
diff --git a/auto_submit/lib/validations/validation_filter.dart b/auto_submit/lib/validations/validation_filter.dart
@@ -6,6 +6,7 @@ import '../configuration/repository_configuration.dart';
 import '../service/config.dart';
 import '../service/process_method.dart';
 import 'approval.dart';
+import 'base_commit_date_allowed.dart';
 import 'ci_successful.dart';
 import 'empty_checks.dart';
 import 'mergeable.dart';
@@ -47,6 +48,7 @@ class PullRequestValidationFilter implements ValidationFilter {
   Set<Validation> getValidations() {
     final validationsToRun = <Validation>{};
 
+    validationsToRun.add(BaseCommitDateAllowed(config: config));
     validationsToRun.add(Approval(config: config));
     // If we are running ci then we need to check the checkRuns and make sure
     // there are check runs created.
diff --git a/auto_submit/test/configuration/repository_configuration_data.dart b/auto_submit/test/configuration/repository_configuration_data.dart
@@ -45,4 +45,5 @@ const String sampleConfigWithOverride = '''
   support_no_review_revert: true
   required_checkruns_on_revert:
     - ci.yaml validation
+  base_commit_allowed_days: 7
 ''';
diff --git a/auto_submit/test/configuration/repository_configuration_manager_test.dart b/auto_submit/test/configuration/repository_configuration_manager_test.dart
@@ -51,6 +51,7 @@ void main() {
         - Google-testing
         - test (ubuntu-latest, 2.18.0)
         - cla/google
+      base_commit_allowed_days: 10
     ''';
 
     githubService.fileContentsMockList.add(sampleConfig);
@@ -92,6 +93,7 @@ void main() {
       repositoryConfiguration.requiredCheckRunsOnRevert.contains('cla/google'),
       isTrue,
     );
+    expect(repositoryConfiguration.baseCommitAllowedDays, 10);
   });
 
   test(
@@ -189,6 +191,7 @@ void main() {
       ),
       isTrue,
     );
+    expect(repositoryConfiguration.baseCommitAllowedDays, 0);
   });
 
   test('Default branch collected if omitted main', () async {
@@ -204,6 +207,7 @@ void main() {
       required_checkruns_on_revert:
         - ci.yaml validation
         - Google-testing
+      base_commit_allowed_days: 7
     ''';
 
     githubService.fileContentsMockList.add(sampleConfig);
@@ -236,6 +240,7 @@ void main() {
       ),
       isTrue,
     );
+    expect(repositoryConfiguration.baseCommitAllowedDays, 7);
   });
 
   group('Merging configurations tests', () {
@@ -253,6 +258,7 @@ void main() {
       support_no_review_revert: true
       required_checkruns_on_revert:
         - ci.yaml validation
+      base_commit_allowed_days: 7
     */
 
     test('Global config merged with default local config', () {
@@ -296,6 +302,7 @@ void main() {
         ),
         isTrue,
       );
+      expect(mergedRepositoryConfiguration.baseCommitAllowedDays, 7);
     });
 
     test('Auto approval accounts is additive, they cannot be removed', () {
@@ -398,29 +405,59 @@ void main() {
       );
     });
 
-    test('Approving reviews is not overridden if less than global config', () {
-      const expectedApprovingReviews = 2;
-      final localRepositoryConfiguration = RepositoryConfiguration(
-        approvingReviews: 1,
-      );
-      final globalRepositoryConfiguration = RepositoryConfiguration.fromYaml(
-        sampleConfigWithOverride,
-      );
-      final mergedRepositoryConfiguration = repositoryConfigurationManager
-          .mergeConfigurations(
-            globalRepositoryConfiguration,
-            localRepositoryConfiguration,
-          );
-      expect(
-        globalRepositoryConfiguration.approvingReviews ==
-            mergedRepositoryConfiguration.approvingReviews,
-        isTrue,
-      );
-      expect(
-        mergedRepositoryConfiguration.approvingReviews,
-        expectedApprovingReviews,
-      );
-    });
+    test(
+      'Base commit recent days overridden by local config if value is not 0',
+      () {
+        const expectedBaseCommitAllowedDays = -1;
+        final localRepositoryConfiguration = RepositoryConfiguration(
+          baseCommitAllowedDays: expectedBaseCommitAllowedDays,
+        );
+        final globalRepositoryConfiguration = RepositoryConfiguration.fromYaml(
+          sampleConfigWithOverride,
+        );
+        final mergedRepositoryConfiguration = repositoryConfigurationManager
+            .mergeConfigurations(
+              globalRepositoryConfiguration,
+              localRepositoryConfiguration,
+            );
+        expect(
+          globalRepositoryConfiguration.baseCommitAllowedDays !=
+              mergedRepositoryConfiguration.baseCommitAllowedDays,
+          isTrue,
+        );
+        expect(
+          mergedRepositoryConfiguration.baseCommitAllowedDays,
+          expectedBaseCommitAllowedDays,
+        );
+      },
+    );
+
+    test(
+      'Base commit recent days is not overridden by local config if value is 0',
+      () {
+        const expectedBaseCommitAllowedDays = 7;
+        final localRepositoryConfiguration = RepositoryConfiguration(
+          baseCommitAllowedDays: 0,
+        );
+        final globalRepositoryConfiguration = RepositoryConfiguration.fromYaml(
+          sampleConfigWithOverride,
+        );
+        final mergedRepositoryConfiguration = repositoryConfigurationManager
+            .mergeConfigurations(
+              globalRepositoryConfiguration,
+              localRepositoryConfiguration,
+            );
+        expect(
+          globalRepositoryConfiguration.baseCommitAllowedDays ==
+              mergedRepositoryConfiguration.baseCommitAllowedDays,
+          isTrue,
+        );
+        expect(
+          mergedRepositoryConfiguration.baseCommitAllowedDays,
+          expectedBaseCommitAllowedDays,
+        );
+      },
+    );
 
     test('Approval group is overridden if defined', () {
       const expectedApprovalGroup = 'flutter-devs';
diff --git a/auto_submit/test/validations/base_commit_date_allowed_test.dart b/auto_submit/test/validations/base_commit_date_allowed_test.dart
diff --git a/auto_submit/test/validations/base_commit_date_allowed_test_data.dart b/auto_submit/test/validations/base_commit_date_allowed_test_data.dart
