diff --git a/.github/workflows/pr_link_issue_reminder.yml b/.github/workflows/pr_link_issue_reminder.yml index b2de62f1b890..a0221284057e 100644 --- a/.github/workflows/pr_link_issue_reminder.yml +++ b/.github/workflows/pr_link_issue_reminder.yml @@ -7,8 +7,11 @@ on: jobs: remind: - # Reminds external contributors to link an issue. PRs from maintainers, users - # with write/admin access, and collaborators are skipped by the script. + # Reminds external contributors to link an issue, reports still-unlinked PRs to + # Slack (one message per PR, sent once) 5 days after the reminder, and auto-closes + # them 10 days after the reminder unless rescued (issue linked or `no-issue-needed` + # label added). PRs from maintainers, users with write/admin access, and collaborators + # are skipped by the script. name: Remind external contributors to link an issue if: github.repository == 'huggingface/diffusers' runs-on: ubuntu-22.04 @@ -18,6 +21,10 @@ jobs: issues: write env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_URL_PR_LINK_ISSUE }} + # Comma-separated Slack member IDs pinged in the Slack rescue messages + # (Sayak Paul, YiYi Xu, Dhruv Nair, Daniel Gu). + SLACK_MENTION_IDS: ${{ secrets.SLACK_PR_LINK_ISSUE_MENTION_IDS }} steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 diff --git a/utils/remind_link_issue.py b/utils/remind_link_issue.py index 5250792be5ae..ddfc736a3e4e 100644 --- a/utils/remind_link_issue.py +++ b/utils/remind_link_issue.py @@ -12,14 +12,24 @@ # See the License for the specific language governing permissions and # limitations under the License. """ -Script to remind PR authors to link an issue. +Script to remind PR authors to link an issue, and to escalate unresolved reminders. Behavior: - Scans open, non-draft PRs. - A PR is considered "linked" if GitHub's GraphQL `closingIssuesReferences` returns > 0 (covers both `Fixes #N` keywords in the body and issues linked via the GitHub UI). -- If a PR is not linked and no prior reminder is present, the script posts a single - friendly reminder comment. +- If a PR is not linked and has no reminder yet, the script posts a single friendly + reminder comment warning that the PR may be auto-closed. +- If a PR only has an old-style reminder (posted before the auto-close notice existed), + the script posts a single follow-up carrying the notice instead, so those PRs enter + the same escalation path with the full warning window. +- Once the warning is `SLACK_RESCUE_DAYS` old and the PR is still not linked, it is + reported to Slack (one message per PR, so each can be tracked individually) for + maintainers to rescue it (by linking an issue or adding the `no-issue-needed` label). + Each PR is reported exactly once, so a PR the team decided not to rescue does not + come back on every run. +- Once the warning is `AUTOCLOSE_DAYS` old and the PR is still not linked, the PR is + closed with an explanatory comment. - PRs labeled `no-issue-needed` and bot-authored PRs are skipped. - PRs authored by maintainers, users with write (or admin) access, and collaborators are skipped; the reminder only targets external contributors. @@ -38,8 +48,23 @@ REPO = "huggingface/diffusers" REMINDER_MARKER = "" +# Present only in comments that warn about auto-closure; the escalation clock starts +# from the bot comment carrying this marker. +AUTOCLOSE_MARKER = "" +# Appended to the warning comment once the PR has been reported to Slack, so it is +# reported exactly once instead of on every run of the rescue window. +SLACK_NOTIFIED_MARKER = "" +# Login the reminder comments are authored under (the workflow's GITHUB_TOKEN). +BOT_LOGIN = "github-actions[bot]" BYPASS_LABELS = {"no-issue-needed"} -LOOKBACK_DAYS = 2 +# Days after the warning at which a still-unlinked PR is reported to Slack for rescue. +# Kept well ahead of `AUTOCLOSE_DAYS` so the single Slack message leaves maintainers a +# few working days to react (a message sent on a Friday still gets a mid-week reply). +SLACK_RESCUE_DAYS = 5 +# Days after the warning at which a still-unlinked PR is automatically closed. +AUTOCLOSE_DAYS = 10 +# Upper bound on how far back to paginate open PRs; older PRs are left alone. +SCAN_LOOKBACK_DAYS = 30 # Collaborator permission levels that mark a PR author as a maintainer / writer / # collaborator. Authors with any of these are skipped (the reminder is only for # external contributors). @@ -89,10 +114,6 @@ def author_checkbox_checked(pr): return bool(AUTHOR_CHECKBOX_PATTERN.search(pr.body or "")) -def has_existing_reminder(pr): - return any(REMINDER_MARKER in (c.body or "") for c in pr.get_issue_comments()) - - def is_privileged_author(repo, pr, author): """Return True if the author is a maintainer, has write/admin access, or is a collaborator.""" # `author_association` is on the PR payload and needs no extra token scope. @@ -113,21 +134,85 @@ def is_privileged_author(repo, pr, author): def reminder_body(author): return ( f"{REMINDER_MARKER}\n" + f"{AUTOCLOSE_MARKER}\n" f"Hi @{author}, thanks for the PR! It does not appear to link an issue it fixes. " "If this PR addresses an existing issue, please add a closing keyword " "(e.g. `Fixes #1234`) to the PR description so the issue is linked. " f"See the [contribution guide]({CONTRIBUTION_GUIDE_URL}) for more details. " "If this PR intentionally does not fix a tracked issue, a maintainer can " - "add the `no-issue-needed` label to silence this reminder." + "add the `no-issue-needed` label to silence this reminder.\n\n" + f"**Please note that PRs without a linked issue are likely to be automatically " + f"closed {AUTOCLOSE_DAYS} days after this notice.**\n\n" + "Once the PR links an issue (or gets the `no-issue-needed` label), you can ignore " + "this message \u2014 it stays here as a comment, but it no longer applies." + ) + + +def followup_body(author): + return ( + f"{AUTOCLOSE_MARKER}\n" + f"Hi @{author}, a follow-up on the reminder above: this PR still does not link " + "an issue it fixes.\n\n" + f"**Please note that PRs without a linked issue are likely to be automatically " + f"closed {AUTOCLOSE_DAYS} days after this notice.** Adding a closing keyword " + "(e.g. `Fixes #1234`) to the PR description, or a maintainer adding the " + "`no-issue-needed` label, will prevent that." + ) + + +def autoclose_body(): + return ( + "This PR has been automatically closed because it does not link an issue and " + f"the reminder above was not addressed within {AUTOCLOSE_DAYS} days. " + "If this PR is still relevant, please link the issue it fixes " + "(e.g. `Fixes #1234`) or ask a maintainer to add the `no-issue-needed` label, " + "and it can be reopened.\n\n" + "We are experimenting with this process to keep the review queue manageable, " + "and it will sometimes get it wrong. If you think this PR should stay open, " + "please just say so here and we will reopen it \u2014 no need to justify it at length.\n\n" + "Thanks again for contributing, and sorry for the noise if we closed this by mistake!" ) +def post_to_slack(webhook_url, text): + response = requests.post(webhook_url, json={"text": text}, timeout=30) + response.raise_for_status() + + +def mark_slack_notified(warning): + """Append the notified marker to the warning comment so the PR is not reported again.""" + warning.edit(f"{warning.body or ''}\n{SLACK_NOTIFIED_MARKER}") + + +def send_slack_rescue_messages(webhook_url, mention_ids, at_risk): + """Post one header message plus one message per PR, so each PR can be tracked + individually (e.g. with a ✅ reaction once handled). Each PR is reported once.""" + header = ( + f"⚠️ {len(at_risk)} open PR(s) without a linked issue will be auto-closed soon. " + "Link an issue or add the `no-issue-needed` label to rescue them:" + ) + if mention_ids: + header = "cc " + " ".join(f"<@{mid}>" for mid in mention_ids) + "\n" + header + post_to_slack(webhook_url, header) + for pr, warning, days_left in at_risk: + post_to_slack( + webhook_url, f"<{pr.html_url}|#{pr.number} {pr.title}> by `{pr.user.login}` — closes in {days_left} day(s)" + ) + mark_slack_notified(warning) + + def main(): token = os.environ["GITHUB_TOKEN"] + slack_webhook_url = os.getenv("SLACK_WEBHOOK_URL") + # Comma-separated Slack member IDs (e.g. "U0123ABC,U0456DEF") pinged in the Slack rescue messages. + mention_ids = [m.strip() for m in os.getenv("SLACK_MENTION_IDS", "").split(",") if m.strip()] g = Github(token) repo = g.get_repo(REPO) owner, name = REPO.split("/", 1) - cutoff = datetime.now(timezone.utc) - timedelta(days=LOOKBACK_DAYS) + now = datetime.now(timezone.utc) + scan_cutoff = now - timedelta(days=SCAN_LOOKBACK_DAYS) + # (pr, warning_comment, days_left) triples reported to Slack for rescue. + at_risk = [] try: pulls = repo.get_pulls(state="open", sort="created", direction="desc") @@ -136,9 +221,9 @@ def main(): created_at = pr.created_at if created_at.tzinfo is None: created_at = created_at.replace(tzinfo=timezone.utc) - # PRs are sorted newest-first, so once we cross the cutoff every + # PRs are sorted newest-first, so once we cross the scan cutoff every # remaining PR is older too and we can stop paginating. - if created_at < cutoff: + if created_at < scan_cutoff: break if pr.draft: continue @@ -156,9 +241,33 @@ def main(): continue if has_linked_issue(token, owner, name, pr.number): continue - if has_existing_reminder(pr): + comments = list(pr.get_issue_comments()) + # Only a marker comment authored by the bot itself starts the + # escalation clock; a pasted marker in a user comment does not. + warning = next( + ( + c + for c in comments + if AUTOCLOSE_MARKER in (c.body or "") and c.user is not None and c.user.login == BOT_LOGIN + ), + None, + ) + if warning is None: + # A PR with only an old-style reminder (without the auto-close + # notice) gets a follow-up carrying the notice; the escalation + # clock starts from whichever comment carries the marker. + already_reminded = any(REMINDER_MARKER in (c.body or "") for c in comments) + pr.create_issue_comment(followup_body(author) if already_reminded else reminder_body(author)) continue - pr.create_issue_comment(reminder_body(author)) + warned_at = warning.created_at + if warned_at.tzinfo is None: + warned_at = warned_at.replace(tzinfo=timezone.utc) + days_since_warning = (now - warned_at).days + if days_since_warning >= AUTOCLOSE_DAYS: + pr.create_issue_comment(autoclose_body()) + pr.edit(state="closed") + elif days_since_warning >= SLACK_RESCUE_DAYS and SLACK_NOTIFIED_MARKER not in (warning.body or ""): + at_risk.append((pr, warning, AUTOCLOSE_DAYS - days_since_warning)) except Exception as e: logger.warning("Skipping PR #%s: %s", getattr(pr, "number", "?"), e) continue @@ -166,6 +275,14 @@ def main(): logger.error("Failed to fetch open PRs: %s", e) raise + if at_risk: + if slack_webhook_url: + send_slack_rescue_messages(slack_webhook_url, mention_ids, at_risk) + else: + logger.warning( + "SLACK_WEBHOOK_URL is not set; skipping rescue messages for %d at-risk PR(s).", len(at_risk) + ) + if __name__ == "__main__": logging.basicConfig(level=logging.INFO, format="%(asctime)s %(levelname)s %(message)s")