[AUTOREVERT] add notification to shared issue (#7200)

* Adds a notification to a shared issue for autorevert
* addresses comments on #7198

Author: jeanschmidt

aws/lambda/pytorch-auto-revert/pytorch_auto_revert/__main__.py

@@ -17,6 +17,11 @@
 
 
 DEFAULT_WORKFLOWS = ["Lint", "trunk", "pull", "inductor"]
+DEFAULT_REPO_FULL_NAME = "pytorch/pytorch"
+DEFAULT_HOURS = 16
+DEFAULT_COMMENT_ISSUE_NUMBER = (
+    163650  # https://github.com/pytorch/pytorch/issues/163650
+)
 # Special constant to indicate --hud-html was passed as a flag (without a value)
 HUD_HTML_NO_VALUE_FLAG = object()
 
@@ -106,11 +111,14 @@ def get_opts() -> argparse.Namespace:
         + ' list (e.g., "pull" or "pull,trunk,inductor")',
     )
     workflow_parser.add_argument(
-        "--hours", type=int, default=48, help="Lookback window in hours (default: 48)"
+        "--hours",
+        type=int,
+        default=DEFAULT_HOURS,
+        help=f"Lookback window in hours (default: {DEFAULT_HOURS})",
     )
     workflow_parser.add_argument(
         "--repo-full-name",
-        default=os.environ.get("REPO_FULL_NAME", "pytorch/pytorch"),
+        default=os.environ.get("REPO_FULL_NAME", DEFAULT_REPO_FULL_NAME),
         help="Full repo name to filter by (owner/repo).",
     )
     workflow_parser.add_argument(
@@ -140,6 +148,14 @@ def get_opts() -> argparse.Namespace:
             "If set, write the run state to HUD HTML; omit a value to use the run timestamp as the filename."
         ),
     )
+    workflow_parser.add_argument(
+        "--notify-issue-number",
+        type=int,
+        default=int(
+            os.environ.get("NOTIFY_ISSUE_NUMBER", DEFAULT_COMMENT_ISSUE_NUMBER)
+        ),
+        help=f"Issue number to notify (default: {DEFAULT_COMMENT_ISSUE_NUMBER})",
+    )
 
     # workflow-restart-checker subcommand
     workflow_restart_parser = subparsers.add_parser(
@@ -219,17 +235,21 @@ def main(*args, **kwargs) -> None:
 
     if opts.subcommand is None:
         autorevert_v2(
-            os.environ.get("WORKFLOWS", "Lint,trunk,pull,inductor").split(","),
-            hours=int(os.environ.get("HOURS", 16)),
-            repo_full_name=os.environ.get("REPO_FULL_NAME", "pytorch/pytorch"),
+            os.environ.get("WORKFLOWS", ",".join(DEFAULT_WORKFLOWS)).split(","),
+            hours=int(os.environ.get("HOURS", DEFAULT_HOURS)),
+            notify_issue_number=int(
+                os.environ.get("NOTIFY_ISSUE_NUMBER", DEFAULT_COMMENT_ISSUE_NUMBER)
+            ),
+            repo_full_name=os.environ.get("REPO_FULL_NAME", DEFAULT_REPO_FULL_NAME),
             restart_action=(RestartAction.LOG if opts.dry_run else RestartAction.RUN),
-            revert_action=(RevertAction.LOG if opts.dry_run else RevertAction.RUN_LOG),
+            revert_action=(RevertAction.LOG if opts.dry_run else RevertAction.RUN_NOTIFY),
         )
     elif opts.subcommand == "autorevert-checker":
         # New default behavior under the same subcommand
-        _signals, _pairs, state_json = autorevert_v2(
+        _, _, state_json = autorevert_v2(
             opts.workflows,
             hours=opts.hours,
+            notify_issue_number=opts.notify_issue_number,
             repo_full_name=opts.repo_full_name,
             restart_action=(RestartAction.LOG if opts.dry_run else opts.restart_action),
             revert_action=(RevertAction.LOG if opts.dry_run else opts.revert_action),

aws/lambda/pytorch-auto-revert/pytorch_auto_revert/signal_actions.py

@@ -226,8 +226,10 @@ def execute_revert(
             )
             return False
 
-        if not dry_run:
-            self._comment_pr_notify_revert(commit_sha, sources, ctx)
+        if ctx.revert_action == RevertAction.RUN_REVERT:
+            self._comment_pr_do_revert(commit_sha, sources, ctx)
+        elif ctx.revert_action == RevertAction.RUN_NOTIFY:
+            self._comment_issue_notify(commit_sha, sources, ctx)
 
         self._logger.insert_event(
             repo=ctx.repo_full_name,
@@ -323,6 +325,32 @@ def execute_restart(
             )
         return True
 
+    def _commit_message_check_pr_is_revert(
+        self, commit_message: str, ctx: RunContext
+    ) -> Optional[int]:
+        # Look for "Reverted #XXXXX" - indicates a revert action
+        revert_matches = re.findall(
+            f"Reverted https://github.com/{ctx.repo_full_name}/pull/(\\d+)",
+            commit_message,
+        )
+        if revert_matches:
+            pr_number = int(revert_matches[-1])
+            return pr_number
+        return None
+
+    def _commit_message_check_pr_is_merge(
+        self, commit_message: str, ctx: RunContext
+    ) -> Optional[int]:
+        # Look for "Pull Request resolved: #XXXXX" - indicates a merge action
+        merge_matches = re.findall(
+            f"Pull Request resolved: https://github.com/{ctx.repo_full_name}/pull/(\\d+)",
+            commit_message,
+        )
+        if merge_matches:
+            pr_number = int(merge_matches[-1])
+            return pr_number
+        return None
+
     def _find_pr_by_sha(
         self, commit_sha: str, ctx: RunContext
     ) -> Optional[Tuple[CommitPRSourceAction, github.PullRequest.PullRequest]]:
@@ -348,13 +376,8 @@ def _find_pr_by_sha(
             # This is the most reliable way to determine the pytorchbot action
             # Use findall to get all matches and pick the last one (pytorchbot appends at the end)
 
-            # Look for "Reverted #XXXXX" - indicates a revert action
-            revert_matches = re.findall(
-                r"Reverted https://github.com/pytorch/pytorch/pull/(\d+)",
-                commit_message,
-            )
-            if revert_matches:
-                pr_number = int(revert_matches[-1])  # Use the last match
+            pr_number = self._commit_message_check_pr_is_revert(commit_message, ctx)
+            if pr_number is not None:
                 try:
                     pr = repo.get_pull(pr_number)
                     logging.info(
@@ -370,13 +393,8 @@ def _find_pr_by_sha(
                         str(e),
                     )
 
-            # Look for "Pull Request resolved: #XXXXX" - indicates a merge action
-            pr_resolved_matches = re.findall(
-                r"Pull Request resolved: https://github.com/pytorch/pytorch/pull/(\d+)",
-                commit_message,
-            )
-            if pr_resolved_matches:
-                pr_number = int(pr_resolved_matches[-1])  # Use the last match
+            pr_number = self._commit_message_check_pr_is_merge(commit_message, ctx)
+            if pr_number is not None:
                 try:
                     pr = repo.get_pull(pr_number)
                     logging.info(
@@ -434,13 +452,65 @@ def _find_pr_by_sha(
             )
             return None
 
-    def _comment_pr_notify_revert(
+    def _comment_issue_notify(
+        self, commit_sha: str, sources: List[SignalMetadata], ctx: RunContext
+    ) -> bool:
+        """Comment on the issue to notify interested stakeholders about the detected autorevert"""
+
+        if ctx.revert_action != RevertAction.RUN_NOTIFY:
+            return False
+
+        logging.debug(
+            "[v2][action] notify for sha %s: finding the issue and notifying stakeholders on issue %s",
+            commit_sha[:8],
+            ctx.notify_issue_number,
+        )
+
+        # find the PR from commit_sha on main
+        pr_result = self._find_pr_by_sha(commit_sha, ctx)
+        if not pr_result:
+            logging.error(
+                "[v2][action] revert for sha %s: no PR found!", commit_sha[:8]
+            )
+            return False
+
+        try:
+            issue = (
+                GHClientFactory()
+                .client.get_repo(ctx.repo_full_name)
+                .get_issue(number=ctx.notify_issue_number)
+            )
+            action_type, pr = pr_result
+            issue.create_comment(
+                f"Autorevert detected a possible offender: {commit_sha[:8]} from PR #{pr.number}.\n"
+                + (
+                    "The commit is a revert"
+                    if action_type == CommitPRSourceAction.REVERT
+                    else "The commit is a PR merge"
+                )
+                + "\n"
+                + "This commit is breaking the following workflows:\n"
+                + "- {}".format("\n- ".join(source.workflow_name for source in sources))
+                + "\n"
+            )
+        except Exception:
+            logging.exception(
+                "[v2][action] revert for sha %s: failed to comment on issue #%d",
+                commit_sha[:8],
+                ctx.notify_issue_number,
+            )
+            return False
+
+    def _comment_pr_do_revert(
         self, commit_sha: str, sources: List[SignalMetadata], ctx: RunContext
     ) -> bool:
-        """Comment on the pull request to notify the author about that their PR is breaking signals."""
+        """Comment on the pull request to pytorchbot to revert it."""
+
+        if ctx.revert_action != RevertAction.RUN_REVERT:
+            return False
 
         logging.debug(
-            "[v2][action] revert for sha %s: finding the PR andnotifying author",
+            "[v2][action] revert for sha %s: finding the PR and notifying author",
             commit_sha[:8],
         )
 
@@ -461,35 +531,33 @@ def _comment_pr_notify_revert(
             )
             return False
 
-        # Comment on the PR to notify the author about the revert
-        comment_body = (
-            "This PR is breaking the following workflows:\n"
-            + "- {}".format("\n- ".join(source.workflow_name for source in sources))
-            + "\n\nPlease investigate and fix the issues."
-        )
-
-        pr.create_issue_comment(comment_body)
-        logging.warning(
-            "[v2][action] revert for sha %s: notified author in PR #%d",
-            commit_sha[:8],
-            pr.number,
-        )
+        # TODO Add autorevert cause for pytorchbot OR decide if we need to use
+        # other causes like weird
 
-        if ctx.revert_action == RevertAction.RUN_REVERT:
-            # TODO Add autorevert cause for pytorchbot OR decide if we need to use
-            # other causes like weird
-
-            # TODO check if the tag `autorevert:disable` is present and don't do the revert
-            # comment, instead limiting to poke the author
-            comment_body = (
-                "XXXX revert -m \"Reverted automatically by pytorch's autorevert, "
-                + 'to avoid this behaviour add the tag autorevert:disable" -c autorevert'
+        # TODO check if the tag `autorevert:disable` is present and don't do the revert
+        # comment, instead limiting to poke the author
+        try:
+            pr.create_issue_comment(
+                "@pytorchbot revert -m \"Reverted automatically by pytorch's autorevert, "
+                + 'to avoid this behaviour add the tag autorevert:disable" -c autorevert\n'
+                + "\n"
+                + "This PR is breaking the following workflows:\n"
+                + "- {}".format("\n- ".join(source.workflow_name for source in sources))
+                + "\n\nPlease investigate and fix the issues."
             )
-            pr.create_issue_comment(comment_body)
-            logging.warning(
-                "[v2][action] revert for sha %s: requested pytorchbot revert in PR #%d",
+        except Exception as e:
+            logging.error(  # noqa: G200
+                "[v2][action] revert for sha %s: failed to comment on PR #%d: %s",
                 commit_sha[:8],
                 pr.number,
+                str(e),
             )
+            return False
+
+        logging.warning(
+            "[v2][action] revert for sha %s: requested pytorchbot revert in PR #%d",
+            commit_sha[:8],
+            pr.number,
+        )
 
         return True

aws/lambda/pytorch-auto-revert/pytorch_auto_revert/signal_extraction_types.py

@@ -32,6 +32,7 @@
 @dataclass(frozen=True)
 class RunContext:
     lookback_hours: int
+    notify_issue_number: int
     repo_full_name: str
     restart_action: RestartAction
     revert_action: RevertAction

aws/lambda/pytorch-auto-revert/pytorch_auto_revert/testers/autorevert_v2.py

@@ -13,6 +13,7 @@
 def autorevert_v2(
     workflows: Iterable[str],
     *,
+    notify_issue_number: int,
     hours: int = 24,
     repo_full_name: str = "pytorch/pytorch",
     restart_action: RestartAction = RestartAction.RUN,
@@ -32,12 +33,13 @@ def autorevert_v2(
     ts = datetime.now(timezone.utc)
 
     logging.info(
-        "[v2] Start: workflows=%s hours=%s repo=%s restart_action=%s revert_action=%s",
+        "[v2] Start: workflows=%s hours=%s repo=%s restart_action=%s revert_action=%s notify_issue_number=%s",
         ",".join(workflows),
         hours,
         repo_full_name,
         restart_action,
         revert_action,
+        notify_issue_number,
     )
     logging.info("[v2] Run timestamp (CH log ts) = %s", ts.isoformat())
 
@@ -59,6 +61,7 @@ def autorevert_v2(
     # Build run context
     run_ctx = RunContext(
         lookback_hours=hours,
+        notify_issue_number=notify_issue_number,
         repo_full_name=repo_full_name,
         restart_action=restart_action,
         revert_action=revert_action,