[fix] Address review feedback on stale-PR bot fix

Author: Eeshu-Yadav

.github/actions/bot-autoassign/stale_pr_bot.py

@@ -15,6 +15,25 @@ def __init__(self):
         self.DAYS_BEFORE_UNASSIGN = 14
         self.DAYS_BEFORE_CLOSE = 60
 
+    @staticmethod
+    def _commit_activity_date_for_author(commit, pr_author):
+        dates = []
+        if (
+            commit.author
+            and commit.author.login == pr_author
+            and commit.commit.author
+            and commit.commit.author.date
+        ):
+            dates.append(commit.commit.author.date)
+        if (
+            commit.committer
+            and commit.committer.login == pr_author
+            and commit.commit.committer
+            and commit.commit.committer.date
+        ):
+            dates.append(commit.commit.committer.date)
+        return max(dates, default=None)
+
     def _get_last_author_activity(
         self,
         pr,
@@ -32,21 +51,11 @@ def _get_last_author_activity(
             return None
         last_activity = None
         for commit in pr.get_commits():
-            author_date = commit.commit.author.date if commit.commit.author else None
-            committer_date = (
-                commit.commit.committer.date if commit.commit.committer else None
-            )
-            if author_date and committer_date:
-                commit_date = max(author_date, committer_date)
-            else:
-                commit_date = author_date or committer_date
+            commit_date = self._commit_activity_date_for_author(commit, pr_author)
             if not commit_date or commit_date <= after_date:
                 continue
-            author_login = commit.author.login if commit.author else None
-            committer_login = commit.committer.login if commit.committer else None
-            if author_login == pr_author or committer_login == pr_author:
-                if not last_activity or commit_date > last_activity:
-                    last_activity = commit_date
+            if not last_activity or commit_date > last_activity:
+                last_activity = commit_date
         if issue_comments is None:
             issue_comments = list(pr.get_issue_comments())
         for comment in issue_comments:
@@ -167,28 +176,33 @@ def is_waiting_for_maintainer(
             return False
 
     def get_last_changes_requested(self, pr, all_reviews=None):
+        """Timestamp of the latest CHANGES_REQUESTED that still represents
+        a human reviewer's current stance, or ``None``.
+        """
         try:
             if all_reviews is None:
                 all_reviews = list(pr.get_reviews())
+            # Bot reviews are advisory; COMMENTED does not change stance.
             latest_per_reviewer = {}
-            for r in all_reviews:
-                if not r.user or not r.submitted_at:
-                    continue
-                if r.user.type == "Bot":
-                    continue
-                if r.state == "COMMENTED":
+            for review in all_reviews:
+                if (
+                    not review.user
+                    or not review.submitted_at
+                    or review.user.type == "Bot"
+                    or review.state == "COMMENTED"
+                ):
                     continue
-                current = latest_per_reviewer.get(r.user.login)
-                if current is None or r.submitted_at > current.submitted_at:
-                    latest_per_reviewer[r.user.login] = r
-            blocking = [
-                r
-                for r in latest_per_reviewer.values()
-                if r.state == "CHANGES_REQUESTED"
-            ]
-            if not blocking:
-                return None
-            return max(blocking, key=lambda r: r.submitted_at).submitted_at
+                current = latest_per_reviewer.get(review.user.login)
+                if current is None or review.submitted_at > current.submitted_at:
+                    latest_per_reviewer[review.user.login] = review
+            return max(
+                (
+                    review.submitted_at
+                    for review in latest_per_reviewer.values()
+                    if review.state == "CHANGES_REQUESTED"
+                ),
+                default=None,
+            )
         except Exception as e:
             print("Error getting reviews" f" for PR #{pr.number}: {e}")
             return None

.github/actions/bot-autoassign/tests/test_stale_pr_bot.py

@@ -300,6 +300,27 @@ def test_both_unlinked_commit_skipped(self, mock_datetime, bot_env):
         last_cr = datetime(2024, 1, 1, tzinfo=timezone.utc)
         assert bot.get_days_since_activity(mock_pr, last_cr) == 9
 
+    @patch("stale_pr_bot.datetime")
+    def test_maintainer_rebase_does_not_count_as_author_activity(
+        self, mock_datetime, bot_env
+    ):
+        mock_datetime.now.return_value = datetime(2024, 3, 1, tzinfo=timezone.utc)
+        mock_datetime.side_effect = lambda *a, **kw: datetime(*a, **kw)
+        bot = StalePRBot()
+        mock_pr = Mock()
+        mock_pr.user.login = "contributor"
+        mock_pr.get_issue_comments.return_value = []
+        mock_pr.get_review_comments.return_value = []
+        mock_pr.get_reviews.return_value = []
+        mock_commit = Mock()
+        mock_commit.commit.author.date = datetime(2023, 12, 1, tzinfo=timezone.utc)
+        mock_commit.commit.committer.date = datetime(2024, 2, 25, tzinfo=timezone.utc)
+        mock_commit.author.login = "contributor"
+        mock_commit.committer.login = "maintainer"
+        mock_pr.get_commits.return_value = [mock_commit]
+        last_cr = datetime(2024, 1, 1, tzinfo=timezone.utc)
+        assert bot.get_days_since_activity(mock_pr, last_cr) == 60
+
 
 class TestIsWaitingForMaintainer:
     def _make_pr(self, author="contributor"):

docs/developer/reusable-github-utils.rst

@@ -73,6 +73,33 @@ OpenWISP repositories. The bot provides the following features:
 - **PR reopen reassignment**: When a stale PR is reopened, linked issues
   are reassigned back to the author.
 
+**How Stale PR Detection Works**
+
+The Stale PR job runs daily. For each open PR:
+
+1. **Trigger condition.** The PR is processed only when at least one human
+   reviewer's latest non-COMMENTED review is ``CHANGES_REQUESTED``. Bot
+   reviews and reviews later superseded by ``APPROVED`` or ``DISMISSED``
+   from the same reviewer do not block.
+2. **Inactivity** is the time since the more recent of: the PR author's
+   latest commit, issue comment, review comment, or review after the
+   blocking review; or the blocking review's timestamp if the author has
+   not acted since. For commits the date is taken from whichever identity
+   (author or committer) matches the PR author, so a maintainer rebasing
+   the contributor's commits does not reset the clock.
+3. **Maintainer-court skip.** If a maintainer (``OWNER``, ``MEMBER`` or
+   ``COLLABORATOR``) has commented or reviewed after the contributor's
+   last action, the PR is skipped — the ball is in the maintainer's court.
+4. **Action by days inactive:**
+
+   - **≥ 7 days:** posts a stale-warning comment.
+   - **≥ 14 days:** adds the ``stale`` label and unassigns the contributor
+     from linked issues. Pushing commits or replying revives the PR.
+   - **≥ 60 days:** closes the PR. Linked issues remain unassigned;
+     reopening the PR reassigns them and removes the ``stale`` label.
+
+   Each stage posts at most once per blocking review cycle.
+
 **Secrets**
 
 These secrets are used by the workflow to generate a ``GITHUB_TOKEN`` via