Skip to content

Commit bbc01e6

Browse files
committed
[change] Drop auto-close from stale-PR bot in favor of final follow-up
1 parent 98ba0a6 commit bbc01e6

3 files changed

Lines changed: 84 additions & 102 deletions

File tree

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

Lines changed: 42 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ def __init__(self):
1313
super().__init__()
1414
self.DAYS_BEFORE_STALE_WARNING = 7
1515
self.DAYS_BEFORE_UNASSIGN = 14
16-
self.DAYS_BEFORE_CLOSE = 60
16+
self.DAYS_BEFORE_FINAL_FOLLOWUP = 60
1717

1818
@staticmethod
1919
def _commit_activity_date_for_author(commit, pr_author):
@@ -244,69 +244,31 @@ def unassign_linked_issues(self, pr):
244244
print(f"Error processing linked issues for PR #{pr.number}: {e}")
245245
return 0
246246

247-
def close_stale_pr(self, pr, days_inactive):
248-
if pr.state == "closed":
249-
print(f"PR #{pr.number} is already closed, skipping")
250-
return True
247+
def send_final_followup(self, pr, days_inactive):
251248
try:
252249
pr_author = pr.user.login if pr.user else None
253250
if not pr_author:
254251
return False
255-
close_lines = [
256-
"<!-- bot:closed -->",
257-
f"Hi @{pr_author} 👋,",
258-
"",
259-
(
260-
"This pull request has been automatically"
261-
" closed due to"
262-
f" **{days_inactive} days of inactivity**."
263-
" After changes were requested,"
264-
" the PR remained inactive."
265-
),
266-
"",
267-
(
268-
"We understand that life gets busy,"
269-
" and we appreciate your initial"
270-
" contribution! 💙"
271-
),
252+
followup_lines = [
253+
"<!-- bot:final_followup -->",
254+
f"Hi @{pr_author},",
272255
"",
273-
("**The door is always open**" " for you to come back:"),
274-
(
275-
"- You can **reopen this PR** at any time"
276-
" if you'd like to continue working on it"
277-
),
278-
("- Feel free to push new commits" " addressing the requested changes"),
279256
(
280-
"- If you reopen the PR, the linked issue"
281-
" will be reassigned to you"
257+
f"This PR has been inactive for **{days_inactive} days**"
258+
" since changes were requested. Are you still working on it?"
282259
),
283260
"",
284261
(
285-
"If you have any questions or need help,"
286-
" don't hesitate to reach out."
287-
" We're here to support you!"
262+
"If yes, push new commits or reply to let us know."
263+
" If you've moved on, please close the PR or comment"
264+
" so another contributor can pick it up."
288265
),
289-
"",
290-
("Thank you for your interest in" " contributing to OpenWISP! 🙏"),
291266
]
292-
try:
293-
pr.create_issue_comment("\n".join(close_lines))
294-
except Exception as comment_error:
295-
print(
296-
f"Warning: Could not post closing comment"
297-
f" on PR #{pr.number}: {comment_error}"
298-
)
299-
finally:
300-
pr.edit(state="closed")
301-
unassigned_count = self.unassign_linked_issues(pr)
302-
print(
303-
f"Closed PR #{pr.number} after"
304-
f" {days_inactive} days of inactivity,"
305-
f" unassigned {unassigned_count} issues"
306-
)
267+
pr.create_issue_comment("\n".join(followup_lines))
268+
print(f"Sent final follow-up for PR #{pr.number}")
307269
return True
308270
except Exception as e:
309-
print(f"Error closing PR #{pr.number}: {e}")
271+
print(f"Error sending final follow-up for PR #{pr.number}: {e}")
310272
return False
311273

312274
def mark_pr_stale(self, pr, days_inactive):
@@ -347,14 +309,6 @@ def mark_pr_stale(self, pr, days_inactive):
347309
" let us know."
348310
" We're happy to help! 🤝"
349311
),
350-
"",
351-
(
352-
"If there's no further activity within"
353-
f" **{self.DAYS_BEFORE_CLOSE - days_inactive}"
354-
" more days**, this PR will be"
355-
" automatically closed"
356-
" (but can be reopened anytime)."
357-
),
358312
]
359313
pr.create_issue_comment("\n".join(unassign_lines))
360314
unassigned_count = self.unassign_linked_issues(pr)
@@ -463,27 +417,40 @@ def process_stale_prs(self):
463417
" maintainer review, skipping"
464418
)
465419
continue
466-
if days_inactive >= self.DAYS_BEFORE_CLOSE:
467-
if self.close_stale_pr(pr, days_inactive):
468-
processed_count += 1
469-
elif days_inactive >= self.DAYS_BEFORE_UNASSIGN:
470-
if not self.has_bot_comment(
471-
pr,
420+
stages = (
421+
(
422+
self.DAYS_BEFORE_STALE_WARNING,
423+
self.DAYS_BEFORE_UNASSIGN,
424+
"stale_warning",
425+
self.send_stale_warning,
426+
),
427+
(
428+
self.DAYS_BEFORE_UNASSIGN,
429+
None,
472430
"stale",
473-
after_date=last_changes_requested,
474-
issue_comments=issue_comments,
475-
):
476-
if self.mark_pr_stale(pr, days_inactive):
477-
processed_count += 1
478-
elif days_inactive >= self.DAYS_BEFORE_STALE_WARNING:
479-
if not self.has_bot_comment(
431+
self.mark_pr_stale,
432+
),
433+
(
434+
self.DAYS_BEFORE_FINAL_FOLLOWUP,
435+
None,
436+
"final_followup",
437+
self.send_final_followup,
438+
),
439+
)
440+
for low, high, marker, action in stages:
441+
if days_inactive < low:
442+
continue
443+
if high is not None and days_inactive >= high:
444+
continue
445+
if self.has_bot_comment(
480446
pr,
481-
"stale_warning",
447+
marker,
482448
after_date=last_changes_requested,
483449
issue_comments=issue_comments,
484450
):
485-
if self.send_stale_warning(pr, days_inactive):
486-
processed_count += 1
451+
continue
452+
if action(pr, days_inactive):
453+
processed_count += 1
487454
except Exception as e:
488455
print(f"Error processing" f" PR #{pr.number}: {e}")
489456
continue

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

Lines changed: 34 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ def test_thresholds(self, bot_env):
4343
bot = StalePRBot()
4444
assert bot.DAYS_BEFORE_STALE_WARNING == 7
4545
assert bot.DAYS_BEFORE_UNASSIGN == 14
46-
assert bot.DAYS_BEFORE_CLOSE == 60
46+
assert bot.DAYS_BEFORE_FINAL_FOLLOWUP == 60
4747

4848

4949
class TestGetLastChangesRequested:
@@ -563,33 +563,15 @@ def test_success(self, bot_env):
563563
mock_issue.remove_from_assignees.assert_called_once_with("testuser")
564564

565565

566-
class TestCloseStalePR:
566+
class TestSendFinalFollowup:
567567
def test_success(self, bot_env):
568568
bot = StalePRBot()
569569
mock_pr = Mock()
570-
mock_pr.body = "Fixes #123"
571570
mock_pr.user.login = "testuser"
572-
mock_pr.state = "open"
573-
mock_assignee = Mock()
574-
mock_assignee.login = "testuser"
575-
mock_issue = Mock()
576-
mock_issue.pull_request = None
577-
mock_issue.assignees = [mock_assignee]
578-
mock_issue.repository.full_name = "openwisp/openwisp-utils"
579-
bot_env["repo"].get_issue.return_value = mock_issue
580-
assert bot.close_stale_pr(mock_pr, 60)
571+
assert bot.send_final_followup(mock_pr, 60)
581572
mock_pr.create_issue_comment.assert_called_once()
582573
comment = mock_pr.create_issue_comment.call_args[0][0]
583-
assert "<!-- bot:closed -->" in comment
584-
mock_pr.edit.assert_called_once_with(state="closed")
585-
mock_issue.remove_from_assignees.assert_called_once_with("testuser")
586-
587-
def test_already_closed(self, bot_env):
588-
bot = StalePRBot()
589-
mock_pr = Mock()
590-
mock_pr.state = "closed"
591-
assert bot.close_stale_pr(mock_pr, 60)
592-
mock_pr.create_issue_comment.assert_not_called()
574+
assert "<!-- bot:final_followup -->" in comment
593575
mock_pr.edit.assert_not_called()
594576

595577

@@ -674,6 +656,36 @@ def test_skips_pr_with_superseded_changes_requested(self, mock_datetime, bot_env
674656
mock_pr.create_issue_comment.assert_not_called()
675657
mock_pr.edit.assert_not_called()
676658

659+
@patch("stale_pr_bot.datetime")
660+
def test_pr_first_processed_past_60_days_marks_stale_and_followup(
661+
self, mock_datetime, bot_env
662+
):
663+
mock_datetime.now.return_value = datetime(2024, 5, 10, tzinfo=timezone.utc)
664+
mock_datetime.side_effect = lambda *a, **kw: datetime(*a, **kw)
665+
bot = StalePRBot()
666+
mock_pr = Mock()
667+
mock_pr.body = ""
668+
mock_pr.number = 7
669+
mock_pr.user.login = "contributor"
670+
cr_review = Mock()
671+
cr_review.state = "CHANGES_REQUESTED"
672+
cr_review.submitted_at = datetime(2024, 2, 1, tzinfo=timezone.utc)
673+
cr_review.user.login = "maintainer"
674+
cr_review.user.type = "User"
675+
mock_pr.get_reviews.return_value = [cr_review]
676+
mock_pr.get_commits.return_value = []
677+
mock_pr.get_issue_comments.return_value = []
678+
mock_pr.get_review_comments.return_value = []
679+
mock_pr.get_labels.return_value = []
680+
bot_env["repo"].get_pulls.return_value = [mock_pr]
681+
bot.process_stale_prs()
682+
bodies = [c[0][0] for c in mock_pr.create_issue_comment.call_args_list]
683+
assert any("<!-- bot:stale -->" in b for b in bodies)
684+
assert any("<!-- bot:final_followup -->" in b for b in bodies)
685+
assert not any("<!-- bot:stale_warning -->" in b for b in bodies)
686+
mock_pr.add_to_labels.assert_called_once_with("stale")
687+
mock_pr.edit.assert_not_called()
688+
677689

678690
class TestRun:
679691
def test_no_github_client(self, bot_env):

docs/developer/reusable-github-utils.rst

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,8 @@ OpenWISP repositories. The bot provides the following features:
6969
assigned, the bot responds with contributing guidelines explaining that
7070
no assignment is needed — just open a PR.
7171
- **Stale PR management**: Warns PR authors after 7 days of inactivity,
72-
marks stale and unassigns after 14 days, and closes after 60 days.
72+
marks stale and unassigns after 14 days, and posts a final follow-up
73+
encouragement after 60 days. The bot does not auto-close PRs.
7374
- **PR reopen reassignment**: When a stale PR is reopened, linked issues
7475
are reassigned back to the author.
7576

@@ -92,11 +93,13 @@ The Stale PR job runs daily. For each open PR:
9293
last action, the PR is skipped — the ball is in the maintainer's court.
9394
4. **Action by days inactive:**
9495

95-
- **≥ 7 days:** posts a stale-warning comment.
96+
- **7–13 days:** posts a stale-warning comment.
9697
- **≥ 14 days:** adds the ``stale`` label and unassigns the contributor
97-
from linked issues. Pushing commits or replying revives the PR.
98-
- **≥ 60 days:** closes the PR. Linked issues remain unassigned;
99-
reopening the PR reassigns them and removes the ``stale`` label.
98+
from linked issues. An author comment on the PR clears the label and
99+
reassigns the issues; a push alone only resets the inactivity timer.
100+
- **≥ 60 days:** posts a final follow-up comment asking whether the
101+
contributor is still working on it. The PR is not auto-closed;
102+
maintainers may close manually if needed.
100103

101104
Each stage posts at most once per blocking review cycle.
102105

0 commit comments

Comments
 (0)