SERVER-123002 auto revert bot skips todo failures (#50770)

GitOrigin-RevId: 8f77f465ff0400882c85695bd575f8b54cd7512a

buildscripts/BUILD.bazel

@@ -211,6 +211,10 @@ py_binary(
     visibility = ["//visibility:public"],
     deps = [
         "//buildscripts/linter",
+        dependency(
+            "evergreen-py",
+            group = "testing",
+        ),
     ],
 )
 

buildscripts/tests/test_todo_linter.py

@@ -0,0 +1,69 @@
+"""Unit tests for todo_linter.py."""
+
+import os
+import unittest
+from unittest.mock import patch
+
+import buildscripts.todo_linter as under_test
+
+
+class TestShouldIgnoreTodoLintFailure(unittest.TestCase):
+    @patch.object(
+        under_test,
+        "get_patch_description",
+        return_value="SERVER-12345 auto-revert-app[bot] follow-up",
+    )
+    def test_github_pr_auto_revert_patch_description_is_ignored(self, mock_get_patch_description):
+        with patch.dict(
+            os.environ, {"requester": "github_pr", "version_id": "version-123"}, clear=True
+        ):
+            self.assertTrue(under_test.should_ignore_todo_lint_failure())
+
+        mock_get_patch_description.assert_called_once_with("version-123")
+
+    @patch.object(
+        under_test,
+        "get_patch_description",
+        return_value="SERVER-12345 auto-revert-app[bot] follow-up",
+    )
+    def test_github_pull_request_user_auto_revert_patch_description_is_ignored(
+        self, mock_get_patch_description
+    ):
+        with patch.dict(
+            os.environ, {"AUTHOR": "github_pull_request", "VERSION_ID": "version-123"}, clear=True
+        ):
+            self.assertTrue(under_test.should_ignore_todo_lint_failure())
+
+        mock_get_patch_description.assert_called_once_with("version-123")
+
+    @patch.object(
+        under_test,
+        "get_patch_description",
+        return_value="SERVER-12345 regular pull request",
+    )
+    def test_non_auto_revert_patch_description_is_not_ignored(self, mock_get_patch_description):
+        with patch.dict(
+            os.environ, {"requester": "github_pr", "version_id": "version-123"}, clear=True
+        ):
+            self.assertFalse(under_test.should_ignore_todo_lint_failure())
+
+        mock_get_patch_description.assert_called_once_with("version-123")
+
+
+class TestLintFiles(unittest.TestCase):
+    @patch.object(under_test.parallel, "parallel_process", return_value=False)
+    @patch.object(under_test, "should_ignore_todo_lint_failure", return_value=True)
+    def test_lint_files_does_not_exit_for_auto_revert_patch(
+        self, _mock_should_ignore, _mock_parallel_process
+    ):
+        under_test._lint_files(["buildscripts/todo_linter.py"])
+
+    @patch.object(under_test.parallel, "parallel_process", return_value=False)
+    @patch.object(under_test, "should_ignore_todo_lint_failure", return_value=False)
+    def test_lint_files_exits_when_exemption_does_not_apply(
+        self, _mock_should_ignore, _mock_parallel_process
+    ):
+        with self.assertRaises(SystemExit) as context:
+            under_test._lint_files(["buildscripts/todo_linter.py"])
+
+        self.assertEqual(context.exception.code, 1)

buildscripts/todo_linter.py

@@ -19,11 +19,15 @@ if __name__ == "__main__" and __package__ is None:
     sys.path.append(os.path.dirname(os.path.dirname(os.path.abspath(os.path.realpath(__file__)))))
 
 from buildscripts.linter import git, parallel
+from evergreen import RetryingEvergreenApi
 
 EXCLUSION_VALUE = "(Ignore linting)"
 TODO_REGEX = re.compile(r'[^"]TODO[^"]')
 JIRA_TICKET_REGEX = re.compile(r"\w+-\d+")
 FILES_RE = re.compile(r"\.(cpp|c|h|hpp|py|js|mjs|inl|idl|yml|bazel)$")
+EVG_CONFIG_FILE = "./.evergreen.yml"
+AUTO_REVERT_APP_BOT_MARKER = "auto-revert-app[bot]"
+GITHUB_PULL_REQUEST_IDENTIFIERS = {"github_pr", "github_pull_request"}
 
 
 def is_interesting_file(file_name: str) -> bool:
@@ -48,7 +52,7 @@ def check_file(file_name: str) -> bool:
     except (OSError, UnicodeDecodeError):
         return True
 
-    errors = []
+    errors: list[tuple[int, str]] = []
     for lineno, line in enumerate(lines, 1):
         if EXCLUSION_VALUE in line:
             continue
@@ -66,8 +70,47 @@ def check_file(file_name: str) -> bool:
     return len(errors) == 0
 
 
+def get_patch_description(version_id: str) -> str:
+    """Return the Evergreen patch description for the given version."""
+    evg_api = RetryingEvergreenApi.get_api(config_file=EVG_CONFIG_FILE)
+    version = evg_api.version_by_id(version_id)
+    return version.message or ""
+
+
+def should_ignore_todo_lint_failure() -> bool:
+    """Return whether TODO lint failures should be ignored for this run."""
+    requester = os.environ.get("requester") or os.environ.get("REQUESTER")
+    evergreen_user = os.environ.get("author") or os.environ.get("AUTHOR")
+    if not any(
+        identifier in GITHUB_PULL_REQUEST_IDENTIFIERS
+        for identifier in (requester, evergreen_user)
+        if identifier
+    ):
+        return False
+
+    version_id = os.environ.get("version_id") or os.environ.get("VERSION_ID")
+    if not version_id:
+        return False
+
+    try:
+        patch_description = get_patch_description(version_id)
+    except Exception as exc:  # pylint: disable=broad-except
+        logging.warning("Unable to determine patch description for TODO lint skip: %s", exc)
+        return False
+
+    # The auto-revert marker is attached to Evergreen's patch description, so use the version
+    # message rather than trying to infer it from GitHub PR metadata.
+    return AUTO_REVERT_APP_BOT_MARKER in patch_description.lower()
+
+
 def _lint_files(file_names: list[str]) -> None:
     if not parallel.parallel_process([os.path.abspath(f) for f in file_names], check_file):
+        if should_ignore_todo_lint_failure():
+            print(
+                "Skipping TODO lint failure because this Evergreen GitHub pull request was "
+                "created by auto-revert-app[bot]."
+            )
+            return
         print(
             "ERROR: Found TODO comments referencing unlinked SERVER tickets."
             " Please resolve or remove them before committing."