[change] Drop auto-close from stale-PR bot in favor of final follow-up

Author: Eeshu-Yadav

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

@@ -13,7 +13,7 @@ def __init__(self):
         super().__init__()
         self.DAYS_BEFORE_STALE_WARNING = 7
         self.DAYS_BEFORE_UNASSIGN = 14
-        self.DAYS_BEFORE_CLOSE = 60
+        self.DAYS_BEFORE_FINAL_FOLLOWUP = 60
 
     @staticmethod
     def _commit_activity_date_for_author(commit, pr_author):
@@ -244,75 +244,31 @@ def unassign_linked_issues(self, pr):
             print(f"Error processing linked issues for PR #{pr.number}: {e}")
             return 0
 
-    def close_stale_pr(self, pr, days_inactive):
-        # TEMPORARY: auto-close disabled. The stale-detection heuristic
-        # has been closing PRs that are merely blocked by bot reviews
-        # (or by reviews the same reviewer later approved). The proper
-        # fix lives in PR #668; until it lands, no PR is auto-closed.
-        print(f"Auto-close currently disabled, skipping PR #{pr.number}")
-        return False
-        if pr.state == "closed":
-            print(f"PR #{pr.number} is already closed, skipping")
-            return True
+    def send_final_followup(self, pr, days_inactive):
         try:
             pr_author = pr.user.login if pr.user else None
             if not pr_author:
                 return False
-            close_lines = [
-                "<!-- bot:closed -->",
-                f"Hi @{pr_author} 👋,",
-                "",
-                (
-                    "This pull request has been automatically"
-                    " closed due to"
-                    f" **{days_inactive} days of inactivity**."
-                    " After changes were requested,"
-                    " the PR remained inactive."
-                ),
-                "",
-                (
-                    "We understand that life gets busy,"
-                    " and we appreciate your initial"
-                    " contribution! 💙"
-                ),
+            followup_lines = [
+                "<!-- bot:final_followup -->",
+                f"Hi @{pr_author},",
                 "",
-                ("**The door is always open**" " for you to come back:"),
-                (
-                    "- You can **reopen this PR** at any time"
-                    " if you'd like to continue working on it"
-                ),
-                ("- Feel free to push new commits" " addressing the requested changes"),
                 (
-                    "- If you reopen the PR, the linked issue"
-                    " will be reassigned to you"
+                    f"This PR has been inactive for **{days_inactive} days**"
+                    " since changes were requested. Are you still working on it?"
                 ),
                 "",
                 (
-                    "If you have any questions or need help,"
-                    " don't hesitate to reach out."
-                    " We're here to support you!"
+                    "If yes, push new commits or reply to let us know."
+                    " If you've moved on, please close the PR or comment"
+                    " so another contributor can pick it up."
                 ),
-                "",
-                ("Thank you for your interest in" " contributing to OpenWISP! 🙏"),
             ]
-            try:
-                pr.create_issue_comment("\n".join(close_lines))
-            except Exception as comment_error:
-                print(
-                    f"Warning: Could not post closing comment"
-                    f" on PR #{pr.number}: {comment_error}"
-                )
-            finally:
-                pr.edit(state="closed")
-            unassigned_count = self.unassign_linked_issues(pr)
-            print(
-                f"Closed PR #{pr.number} after"
-                f" {days_inactive} days of inactivity,"
-                f" unassigned {unassigned_count} issues"
-            )
+            pr.create_issue_comment("\n".join(followup_lines))
+            print(f"Sent final follow-up for PR #{pr.number}")
             return True
         except Exception as e:
-            print(f"Error closing PR #{pr.number}: {e}")
+            print(f"Error sending final follow-up for PR #{pr.number}: {e}")
             return False
 
     def mark_pr_stale(self, pr, days_inactive):
@@ -353,14 +309,6 @@ def mark_pr_stale(self, pr, days_inactive):
                     " let us know."
                     " We're happy to help! 🤝"
                 ),
-                "",
-                (
-                    "If there's no further activity within"
-                    f" **{self.DAYS_BEFORE_CLOSE - days_inactive}"
-                    " more days**, this PR will be"
-                    " automatically closed"
-                    " (but can be reopened anytime)."
-                ),
             ]
             pr.create_issue_comment("\n".join(unassign_lines))
             unassigned_count = self.unassign_linked_issues(pr)
@@ -469,27 +417,40 @@ def process_stale_prs(self):
                             " maintainer review, skipping"
                         )
                         continue
-                    if days_inactive >= self.DAYS_BEFORE_CLOSE:
-                        if self.close_stale_pr(pr, days_inactive):
-                            processed_count += 1
-                    elif days_inactive >= self.DAYS_BEFORE_UNASSIGN:
-                        if not self.has_bot_comment(
-                            pr,
+                    stages = (
+                        (
+                            self.DAYS_BEFORE_STALE_WARNING,
+                            self.DAYS_BEFORE_UNASSIGN,
+                            "stale_warning",
+                            self.send_stale_warning,
+                        ),
+                        (
+                            self.DAYS_BEFORE_UNASSIGN,
+                            None,
                             "stale",
-                            after_date=last_changes_requested,
-                            issue_comments=issue_comments,
-                        ):
-                            if self.mark_pr_stale(pr, days_inactive):
-                                processed_count += 1
-                    elif days_inactive >= self.DAYS_BEFORE_STALE_WARNING:
-                        if not self.has_bot_comment(
+                            self.mark_pr_stale,
+                        ),
+                        (
+                            self.DAYS_BEFORE_FINAL_FOLLOWUP,
+                            None,
+                            "final_followup",
+                            self.send_final_followup,
+                        ),
+                    )
+                    for low, high, marker, action in stages:
+                        if days_inactive < low:
+                            continue
+                        if high is not None and days_inactive >= high:
+                            continue
+                        if self.has_bot_comment(
                             pr,
-                            "stale_warning",
+                            marker,
                             after_date=last_changes_requested,
                             issue_comments=issue_comments,
                         ):
-                            if self.send_stale_warning(pr, days_inactive):
-                                processed_count += 1
+                            continue
+                        if action(pr, days_inactive):
+                            processed_count += 1
                 except Exception as e:
                     print(f"Error processing" f" PR #{pr.number}: {e}")
                     continue

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

@@ -43,7 +43,7 @@ def test_thresholds(self, bot_env):
         bot = StalePRBot()
         assert bot.DAYS_BEFORE_STALE_WARNING == 7
         assert bot.DAYS_BEFORE_UNASSIGN == 14
-        assert bot.DAYS_BEFORE_CLOSE == 60
+        assert bot.DAYS_BEFORE_FINAL_FOLLOWUP == 60
 
 
 class TestGetLastChangesRequested:
@@ -563,34 +563,15 @@ def test_success(self, bot_env):
         mock_issue.remove_from_assignees.assert_called_once_with("testuser")
 
 
-@pytest.mark.skip(reason="Auto-close temporarily disabled; see close_stale_pr stub.")
-class TestCloseStalePR:
+class TestSendFinalFollowup:
     def test_success(self, bot_env):
         bot = StalePRBot()
         mock_pr = Mock()
-        mock_pr.body = "Fixes #123"
         mock_pr.user.login = "testuser"
-        mock_pr.state = "open"
-        mock_assignee = Mock()
-        mock_assignee.login = "testuser"
-        mock_issue = Mock()
-        mock_issue.pull_request = None
-        mock_issue.assignees = [mock_assignee]
-        mock_issue.repository.full_name = "openwisp/openwisp-utils"
-        bot_env["repo"].get_issue.return_value = mock_issue
-        assert bot.close_stale_pr(mock_pr, 60)
+        assert bot.send_final_followup(mock_pr, 60)
         mock_pr.create_issue_comment.assert_called_once()
         comment = mock_pr.create_issue_comment.call_args[0][0]
-        assert "<!-- bot:closed -->" in comment
-        mock_pr.edit.assert_called_once_with(state="closed")
-        mock_issue.remove_from_assignees.assert_called_once_with("testuser")
-
-    def test_already_closed(self, bot_env):
-        bot = StalePRBot()
-        mock_pr = Mock()
-        mock_pr.state = "closed"
-        assert bot.close_stale_pr(mock_pr, 60)
-        mock_pr.create_issue_comment.assert_not_called()
+        assert "<!-- bot:final_followup -->" in comment
         mock_pr.edit.assert_not_called()
 
 
@@ -675,6 +656,36 @@ def test_skips_pr_with_superseded_changes_requested(self, mock_datetime, bot_env
         mock_pr.create_issue_comment.assert_not_called()
         mock_pr.edit.assert_not_called()
 
+    @patch("stale_pr_bot.datetime")
+    def test_pr_first_processed_past_60_days_marks_stale_and_followup(
+        self, mock_datetime, bot_env
+    ):
+        mock_datetime.now.return_value = datetime(2024, 5, 10, tzinfo=timezone.utc)
+        mock_datetime.side_effect = lambda *a, **kw: datetime(*a, **kw)
+        bot = StalePRBot()
+        mock_pr = Mock()
+        mock_pr.body = ""
+        mock_pr.number = 7
+        mock_pr.user.login = "contributor"
+        cr_review = Mock()
+        cr_review.state = "CHANGES_REQUESTED"
+        cr_review.submitted_at = datetime(2024, 2, 1, tzinfo=timezone.utc)
+        cr_review.user.login = "maintainer"
+        cr_review.user.type = "User"
+        mock_pr.get_reviews.return_value = [cr_review]
+        mock_pr.get_commits.return_value = []
+        mock_pr.get_issue_comments.return_value = []
+        mock_pr.get_review_comments.return_value = []
+        mock_pr.get_labels.return_value = []
+        bot_env["repo"].get_pulls.return_value = [mock_pr]
+        bot.process_stale_prs()
+        bodies = [c[0][0] for c in mock_pr.create_issue_comment.call_args_list]
+        assert any("<!-- bot:stale -->" in b for b in bodies)
+        assert any("<!-- bot:final_followup -->" in b for b in bodies)
+        assert not any("<!-- bot:stale_warning -->" in b for b in bodies)
+        mock_pr.add_to_labels.assert_called_once_with("stale")
+        mock_pr.edit.assert_not_called()
+
 
 class TestRun:
     def test_no_github_client(self, bot_env):

docs/developer/reusable-github-utils.rst

@@ -69,7 +69,8 @@ OpenWISP repositories. The bot provides the following features:
   assigned, the bot responds with contributing guidelines explaining that
   no assignment is needed — just open a PR.
 - **Stale PR management**: Warns PR authors after 7 days of inactivity,
-  marks stale and unassigns after 14 days, and closes after 60 days.
+  marks stale and unassigns after 14 days, and posts a final follow-up
+  encouragement after 60 days. The bot does not auto-close PRs.
 - **PR reopen reassignment**: When a stale PR is reopened, linked issues
   are reassigned back to the author.
 
@@ -92,11 +93,13 @@ The Stale PR job runs daily. For each open PR:
    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.
+   - **7–13 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.
+     from linked issues. An author comment on the PR clears the label and
+     reassigns the issues; a push alone only resets the inactivity timer.
+   - **≥ 60 days:** posts a final follow-up comment asking whether the
+     contributor is still working on it. The PR is not auto-closed;
+     maintainers may close manually if needed.
 
    Each stage posts at most once per blocking review cycle.