diff --git a/.github/scripts/close_low_quality_prs.py b/.github/scripts/close_low_quality_prs.py index d7363f88328..85b281e959d 100644 --- a/.github/scripts/close_low_quality_prs.py +++ b/.github/scripts/close_low_quality_prs.py @@ -38,6 +38,7 @@ from __future__ import annotations import argparse import datetime as dt import json +import os import re import subprocess import sys @@ -66,6 +67,33 @@ INTERNAL_AUTHOR_ASSOCIATIONS = frozenset({"OWNER", "MEMBER", "COLLABORATOR"}) # `default=[...]` combination silently mutates the shared default list. DEFAULT_OPTOUT_LABELS = ("do not close", "keep open", "wip") +# HTML marker appended to grace-period warning comments. Shared with the +# Agent Shin LLM-judge script (`triage_with_llm.py`) so a warning posted +# by either path is recognized by both: the LLM judge can see "Greptile +# already warned this contributor 12 hours ago" and skip re-warning, and +# the Greptile closer can see "Agent Shin already warned" and close on +# the next run if Greptile still has a low score. +GRACE_COMMENT_MARKER = "" + +# Length of the grace period between the warning comment and the actual +# auto-close. Set to 24 hours so the contributor has at least one full +# working day across any time zone to push fixes or comment +# `@agent-shin reconsider`. Mirrors the constant of the same name in +# `triage_with_llm.py` — keep them in sync if either changes. +GRACE_PERIOD_SECONDS = 86400 + +# Default login of the GitHub identity that performs Agent Shin's writes; +# used for matching the author of a grace-warning comment so we don't +# count somebody quoting the marker. The env override +# `AGENT_SHIN_BOT_LOGIN` mirrors `triage_with_llm.py`. +AGENT_SHIN_DEFAULT_BOT_LOGIN = "github-actions[bot]" + +# Logins (case-insensitive) that bypass BOTH the 1-day grace period AND +# the dry-run gating. Mirrors `IMMEDIATE_CLOSE_LOGINS` in +# `triage_with_llm.py`. Used for dogfooding the bot from external test +# accounts that have no push permissions to the repo. +IMMEDIATE_CLOSE_LOGINS = frozenset({"swiftwinds"}) + def gh(*args: str) -> str: """Run a `gh` CLI command and return stdout. Raises on non-zero exit.""" @@ -196,6 +224,124 @@ def has_optout_label(pr: dict, optout_labels: set[str]) -> bool: return bool(labels & {lbl.lower() for lbl in optout_labels}) +def seconds_since_last_grace_warning( + comments: Iterable[dict], + *, + bot_login: str | None = None, + now: dt.datetime | None = None, +) -> float | None: + """Return seconds since the bot's most recent grace-period warning, or + None if no such warning has ever been posted on this PR. + + Detects warnings by matching `GRACE_COMMENT_MARKER` in comments + authored by the bot identity. Operates on an already-fetched + comments list (avoids a second `gh api` call when the caller has + already pulled the page for Greptile-score extraction). + + `now` is injectable so callers (and tests) can pin the reference + time. The closer runs everything against a single `now` snapshot + captured at the top of `main()` so age calculations stay consistent + across many PRs in a single run. + """ + expected_login = ( + bot_login + or os.environ.get("AGENT_SHIN_BOT_LOGIN") + or AGENT_SHIN_DEFAULT_BOT_LOGIN + ).lower() + latest: dt.datetime | None = None + for comment in comments: + author = ((comment.get("user") or {}).get("login") or "").lower() + if author != expected_login: + continue + body = comment.get("body") or "" + if GRACE_COMMENT_MARKER not in body: + continue + created = comment.get("created_at") + if not created: + continue + try: + ts = parse_iso8601(created) + except ValueError: + continue + if latest is None or ts > latest: + latest = ts + if latest is None: + return None + reference = now if now is not None else dt.datetime.now(dt.timezone.utc) + return (reference - latest).total_seconds() + + +def format_grace_warning_comment(score: int, threshold: int) -> str: + """Comment posted on the FIRST low-Greptile-score detection — gives + the contributor a 1-day grace window before the auto-close fires on + the next daily cron run. + + Mirrors `format_grace_warning_pr_comment` in + `triage_with_llm.py` in spirit (1-day grace + escape hatches), but + framed around Greptile's confidence score instead of the LLM judge's + rubric since the close trigger here is the Greptile signal. + """ + return ( + "👋 Hi, thanks for the PR! I'm **Agent Shin**, the automated triage bot for this repository.\n" + "\n" + "Heads up — Greptile's most recent review scored this PR " + f"**{score}/5**, below our merge bar of **{threshold}/5**.\n" + "\n" + "⏳ **You have 1 day to address Greptile's feedback before this PR is auto-closed.** " + "We close low-confidence PRs aggressively to keep the review queue manageable for " + "maintainers and contributors alike. **This isn't a rejection of the idea.**\n" + "\n" + "During the grace period:\n" + "\n" + "1. Push fixes that address Greptile's feedback (continue using your existing branch is fine).\n" + "2. Either:\n" + " - Comment `@greptileai` to request a fresh Greptile review. If the new score is " + f"**{threshold}/5 or higher**, the PR stays open.\n" + " - Or comment `@agent-shin reconsider` to have Agent Shin re-evaluate the PR description.\n" + "\n" + "If this PR is auto-closed in 24 hours, you'll still have options:\n" + "\n" + "- Comment `@agent-shin reconsider` after pushing fixes — Agent Shin will re-run triage " + "and reopen the PR if it now meets the bar.\n" + "- Comment `@greptileai` to request a re-review — that works **even after the PR is closed**.\n" + "\n" + "Thanks for contributing to LiteLLM. We know auto-closures can sting; the goal is to keep " + "the project healthy, not to dismiss your work.\n" + "\n" + f"{GRACE_COMMENT_MARKER}" + ) + + +def post_grace_warning( + pr: dict, + score: int, + threshold: int, + repo: str | None, + dry_run: bool, +) -> None: + """Post the 1-day grace-period warning comment on `pr`. + + The warning carries `GRACE_COMMENT_MARKER` so subsequent runs can + detect that the contributor has already been told about the + pending close. Does NOT close the PR — the close happens on the + next eligible run after `GRACE_PERIOD_SECONDS` elapses (handled + by `close_pr`). + """ + pr_number = pr["number"] + repo_args = ["--repo", repo] if repo else [] + + if dry_run: + print( + f" [DRY RUN] Would post grace warning to PR #{pr_number} " + f"(greptile={score}/5): {pr['title']}" + ) + return + + comment_body = format_grace_warning_comment(score, threshold) + gh("pr", "comment", str(pr_number), "--body", comment_body, *repo_args) + print(f" Posted grace warning on PR #{pr_number} (greptile={score}/5)") + + def close_pr( pr: dict, score: int, @@ -219,7 +365,8 @@ def close_pr( comment_body = ( f"Closing as part of automated PR triage.\n\n" f"Greptile's most recent review scored this PR **{score}/5**, below " - f"our merge bar of **{threshold}/5**.\n\n" + f"our merge bar of **{threshold}/5**, and the 1-day grace period since " + "the warning has elapsed.\n\n" "We close low-confidence PRs aggressively to keep the review queue " "manageable for maintainers and contributors alike. **This is not a " "rejection of the idea** — to bring this back:\n\n" @@ -233,7 +380,9 @@ def close_pr( "maintainer, so a fresh PR is the most reliable path forward. If you " "would prefer this exact PR re-evaluated, comment " "`@agent-shin reconsider` once you've pushed the fixes — Agent Shin " - "will re-run triage and reopen this PR if it now meets the bar.\n\n" + "will re-run triage and reopen this PR if it now meets the bar. " + "You can also comment `@greptileai` to request a fresh Greptile " + "review — that works **even after the PR is closed**.\n\n" "Thanks for contributing to LiteLLM. We know auto-closures can sting; " "the goal is to keep the project healthy, not to dismiss your work." ) @@ -258,16 +407,28 @@ def evaluate_pr( repo: str | None, optout_labels: set[str], ) -> tuple[str, int | None, int | None]: - """Decide whether to close `pr`. + """Decide what to do with `pr` on this triage run. Returns (action, score_or_none, age_days_or_none) where action is one of: "skip-too-young", "skip-optout-label", "skip-internal", - "skip-no-greptile-score", "skip-score-ok", or "close". + "skip-no-greptile-score", "skip-score-ok", + "warn-grace", "skip-in-grace-period", or "close". Drafts are NOT skipped — the goal is "open PR count == PRs internal collaborators need to action on", and a draft that Greptile scored <4/5 is still in that queue. Authors can opt out via the `wip` label (see `DEFAULT_OPTOUT_LABELS`) if they need to keep a long-lived draft open. + + Grace-period semantics: the first time a PR fails the rubric, the + action is `warn-grace` — the caller should post a warning comment but + NOT close the PR. On a subsequent run, if the warning is still less + than `GRACE_PERIOD_SECONDS` old AND the PR still fails, the action is + `skip-in-grace-period`. Once the warning ages out and the rubric is + still failing, the action is `close`. + + Grace is bypassed for `IMMEDIATE_CLOSE_LOGINS` (test/dogfood + accounts), which always go straight to `close` on the first failing + run so the bot is dogfoodable end-to-end without a 24h delay. """ if has_optout_label(pr, optout_labels): return ("skip-optout-label", None, None) @@ -294,6 +455,16 @@ def evaluate_pr( if score >= min_score: return ("skip-score-ok", score, age_days) + login = ((pr.get("author") or {}).get("login") or "").lower() + if login in IMMEDIATE_CLOSE_LOGINS: + return ("close", score, age_days) + + grace_age = seconds_since_last_grace_warning(comments, now=now) + if grace_age is None: + return ("warn-grace", score, age_days) + if grace_age < GRACE_PERIOD_SECONDS: + return ("skip-in-grace-period", score, age_days) + return ("close", score, age_days) @@ -370,6 +541,8 @@ def main() -> int: closed = 0 summary = { "close": 0, + "warn-grace": 0, + "skip-in-grace-period": 0, "skip-too-young": 0, "skip-optout-label": 0, "skip-internal": 0, @@ -388,6 +561,30 @@ def main() -> int: ) summary[action] = summary.get(action, 0) + 1 + # Per-PR dry-run override: `IMMEDIATE_CLOSE_LOGINS` accounts (e.g. + # SwiftWinds) always run in real-close mode regardless of the + # global `--close` flag. Lets a maintainer dogfood the bot from + # an external account while the rest of the open-PR queue stays + # on the safe dry-run default. + author_login = ((pr.get("author") or {}).get("login") or "").lower() + is_immediate = author_login in IMMEDIATE_CLOSE_LOGINS + pr_dry_run = dry_run and not is_immediate + + if action == "warn-grace": + assert score is not None + print( + f"#{pr['number']}: \"{pr['title']}\" " + f"(age={age_days}d, greptile={score}/5) -> warn-grace" + ) + post_grace_warning( + pr, + score=score, + threshold=args.min_score, + repo=args.repo, + dry_run=pr_dry_run, + ) + continue + if action != "close": continue @@ -395,6 +592,7 @@ def main() -> int: print( f"#{pr['number']}: \"{pr['title']}\" " f"(age={age_days}d, greptile={score}/5) -> close" + + (" [immediate-close login]" if is_immediate else "") ) close_pr( pr, @@ -402,11 +600,11 @@ def main() -> int: threshold=args.min_score, age_days=age_days, repo=args.repo, - dry_run=dry_run, + dry_run=pr_dry_run, label=args.close_label, ) - if not dry_run: + if not pr_dry_run: closed += 1 if args.limit is not None and closed >= args.limit: print(f"\nReached --limit={args.limit}; stopping.") @@ -416,6 +614,10 @@ def main() -> int: for key, value in summary.items(): print(f" {key:28s} {value}") print(f"\nTotal {'would close' if dry_run else 'closed'}: {summary['close']}") + print( + f"Total {'would warn (grace)' if dry_run else 'warned (grace)'}: " + f"{summary['warn-grace']}" + ) return 0 diff --git a/.github/scripts/triage_with_llm.py b/.github/scripts/triage_with_llm.py index 94052c32e7b..2442c061b98 100644 --- a/.github/scripts/triage_with_llm.py +++ b/.github/scripts/triage_with_llm.py @@ -62,6 +62,27 @@ RECONSIDER_COMMENT_MARKER = "" # the body" iteration loop isn't punished. RECONSIDER_RATE_LIMIT_SECONDS = 600 +# HTML marker appended to the grace-period warning comment posted on the +# first low-quality detection. We grep for this on subsequent triage runs +# to (a) detect that a warning was already posted (so we don't spam the +# contributor with duplicate warnings) and (b) measure how long ago it +# was posted so we know when the grace period has elapsed. +GRACE_COMMENT_MARKER = "" + +# Length of the grace period between the warning comment and the actual +# auto-close. Set to 24 hours so the contributor has at least one full +# working day across any time zone to push fixes or comment +# `@agent-shin reconsider`. +GRACE_PERIOD_SECONDS = 86400 + +# Logins (case-insensitive) that bypass BOTH the 1-day grace period AND +# the dry-run / `AGENT_SHIN_ENABLED` workflow gating — every Agent Shin +# verdict against a PR/issue from one of these accounts is treated as a +# real run with immediate close on fail. Useful for dogfooding the bot +# from an external account that has no push permissions to the repo. +# Listed lower-case so callers compare via `login.lower() in ...`. +IMMEDIATE_CLOSE_LOGINS = frozenset({"swiftwinds"}) + # Model families that require `reasoning_effort` to be set, and that reject # `temperature != 1` unless `reasoning_effort` is "none". For these models we # pass `reasoning_effort="none"` so a `temperature=0` deterministic judgment @@ -238,17 +259,20 @@ def was_closed_by_agent_shin( return actor.lower() == expected -def seconds_since_last_reconsider_verdict( - repo: str, number: int, *, bot_login: str | None = None +def _seconds_since_latest_marker_comment( + repo: str, + number: int, + *, + marker: str, + bot_login: str | None = None, ) -> float | None: - """Return seconds since the bot's most recent reconsider verdict comment. + """Shared helper: return seconds since the bot's most recent comment + that contains the given HTML marker, or None if no such comment exists. - Detects comments by matching the HTML marker `RECONSIDER_COMMENT_MARKER` - appended by `format_reopen_comment` and - `format_reconsider_still_failing_comment`. Returns None when the bot - has never posted a reconsider verdict on this PR/issue (or when the - only matching comments are missing a `created_at` timestamp, which - shouldn't happen on a real GitHub response). + Used by both the reconsider-verdict cooldown and the grace-period + warning detection — keeping the iteration logic centralized stops the + two paths from drifting (e.g. one fixing a tz parsing bug and the + other forgetting to mirror it). """ expected_login = ( bot_login @@ -261,7 +285,7 @@ def seconds_since_last_reconsider_verdict( if author != expected_login: continue body = comment.get("body") or "" - if RECONSIDER_COMMENT_MARKER not in body: + if marker not in body: continue created = comment.get("created_at") if not created: @@ -277,6 +301,39 @@ def seconds_since_last_reconsider_verdict( return (dt.datetime.now(dt.timezone.utc) - latest).total_seconds() +def seconds_since_last_reconsider_verdict( + repo: str, number: int, *, bot_login: str | None = None +) -> float | None: + """Return seconds since the bot's most recent reconsider verdict comment. + + Detects comments by matching the HTML marker `RECONSIDER_COMMENT_MARKER` + appended by `format_reopen_comment` and + `format_reconsider_still_failing_comment`. Returns None when the bot + has never posted a reconsider verdict on this PR/issue (or when the + only matching comments are missing a `created_at` timestamp, which + shouldn't happen on a real GitHub response). + """ + return _seconds_since_latest_marker_comment( + repo, number, marker=RECONSIDER_COMMENT_MARKER, bot_login=bot_login + ) + + +def seconds_since_last_grace_warning( + repo: str, number: int, *, bot_login: str | None = None +) -> float | None: + """Return seconds since the bot's most recent grace-period warning. + + Detects warning comments by matching the HTML marker + `GRACE_COMMENT_MARKER` appended by `format_grace_warning_pr_comment` + and `format_grace_warning_issue_comment`. Returns None when no + grace warning has ever been posted on this PR/issue — that's the + "first low-quality detection" signal that drives the warning path. + """ + return _seconds_since_latest_marker_comment( + repo, number, marker=GRACE_COMMENT_MARKER, bot_login=bot_login + ) + + # --------------------------------------------------------------------------- # Author classification @@ -510,6 +567,9 @@ def format_pr_close_comment(verdict: dict) -> str: "to get back into the review queue.\n" " - **Or** comment `@agent-shin reconsider` on this closed PR after updating the description. " "I'll re-run the triage; if it now passes, I'll reopen this PR automatically.\n" + " - You can also comment `@greptileai` on this PR to request a fresh Greptile review — that " + "still works **even after the PR is closed**, and a higher score is one of the signals that " + "lifts the PR back into the queue.\n" "\n" "Internal BerriAI contributors: this rubric doesn't apply to you — ping a maintainer.\n" "\n" @@ -550,6 +610,90 @@ def format_issue_close_comment(verdict: dict) -> str: ) +def format_grace_warning_pr_comment(verdict: dict) -> str: + """Comment posted on the FIRST low-quality detection — gives the + contributor a 1-day grace window to fix the PR before the next + triage run actually closes it. + + This is the "before-close" warning. On the second triage run, if the + grace marker is older than `GRACE_PERIOD_SECONDS` AND the PR still + fails the rubric, the close path runs (which posts + `format_pr_close_comment` and closes the PR). + """ + missing_lines = _format_missing(verdict.get("missing") or []) + explanation = verdict.get("explanation") or "" + return ( + "👋 Hi, thanks for the PR! I'm **Agent Shin**, the automated triage bot for this repository.\n" + "\n" + "Heads up — this PR does not yet meet the bar described in our " + "[pull-request template](https://github.com/BerriAI/litellm/blob/main/.github/pull_request_template.md). " + "Specifically, I couldn't find:\n" + "\n" + f"{missing_lines}\n" + "\n" + f"> {explanation}\n" + "\n" + "⏳ **You have 1 day to address this before this PR is auto-closed.** " + "During the grace period:\n" + "\n" + "1. Update the PR description to either:\n" + " - Link a related GitHub issue (e.g. `Fixes #1234`), OR\n" + " - Add a clear **problem description**, **expected vs. actual behavior**, and **visual QA proof** " + "(before/after screenshots, a short screen recording, or terminal/log output).\n" + "2. Comment `@agent-shin reconsider` on this PR after updating it. If your update meets the " + "bar, I'll skip the auto-close and a maintainer will take another look.\n" + "\n" + "If this PR is auto-closed in 24 hours, you'll still have options:\n" + "\n" + "- Comment `@agent-shin reconsider` to have me re-evaluate (and reopen the PR if it now meets the bar).\n" + "- Comment `@greptileai` to request a fresh Greptile review — that works **even after the PR is closed**.\n" + "\n" + "Internal BerriAI contributors: this rubric doesn't apply to you — ping a maintainer.\n" + "\n" + "_(I'm an LLM, so I'm not infallible. If you think I got this wrong, comment " + "`@agent-shin reconsider` or ping a maintainer — they'll override me.)_\n" + "\n" + f"{GRACE_COMMENT_MARKER}" + ) + + +def format_grace_warning_issue_comment(verdict: dict) -> str: + """Issue analogue of `format_grace_warning_pr_comment`.""" + missing_lines = _format_missing(verdict.get("missing") or []) + explanation = verdict.get("explanation") or "" + return ( + "👋 Hi, thanks for filing this! I'm **Agent Shin**, the automated triage bot for this repository.\n" + "\n" + "Heads up — this issue doesn't yet have enough detail for a maintainer to act on. " + "Specifically, I couldn't find:\n" + "\n" + f"{missing_lines}\n" + "\n" + f"> {explanation}\n" + "\n" + "⏳ **You have 1 day to address this before this issue is auto-closed.** " + "During the grace period:\n" + "\n" + "1. Edit the issue to add the missing pieces:\n" + " - For **bug reports**: a runnable reproduction (code / curl / config), expected vs. actual behavior, " + "and a screenshot / traceback / log showing the bug.\n" + " - For **feature requests**: a concrete description of what should change, plus a use case and example " + "(config / API call / UI flow).\n" + "2. Comment `@agent-shin reconsider` on this issue after updating it. If your update meets the bar, " + "I'll skip the auto-close and a maintainer will take another look.\n" + "\n" + "If this issue is auto-closed in 24 hours, you can still comment `@agent-shin reconsider` to have " + "me re-evaluate (and reopen the issue if it now meets the bar).\n" + "\n" + "Internal BerriAI contributors: this rubric doesn't apply to you — ping a maintainer.\n" + "\n" + "_(I'm an LLM, so I'm not infallible. If you think I got this wrong, comment " + "`@agent-shin reconsider` or ping a maintainer — they'll override me.)_\n" + "\n" + f"{GRACE_COMMENT_MARKER}" + ) + + # --------------------------------------------------------------------------- # Step-summary helpers @@ -809,7 +953,58 @@ def triage( if decision != "fail": return {**base_result, "action": "pass-llm", "verdict": verdict} - if not close: + # Grace-period flow: on the first low-quality detection, post a warning + # comment instead of closing immediately. On a subsequent triage run + # (manual re-trigger, or the daily `close_low_quality_prs.py` cron + # finding the same PR in its own pass), if `GRACE_PERIOD_SECONDS` has + # elapsed since the warning AND the PR still fails the rubric, close. + # + # `IMMEDIATE_CLOSE_LOGINS` (e.g. test/dogfood accounts like SwiftWinds) + # bypass the grace period entirely — every fail is treated as a real + # close run. This is intentional: those accounts exist specifically to + # exercise the bot end-to-end, and waiting a day per iteration kills + # the feedback loop. + is_immediate = login.lower() in IMMEDIATE_CLOSE_LOGINS + + if not is_immediate: + grace_age = seconds_since_last_grace_warning(repo, number) + if grace_age is None: + warning_body = ( + format_grace_warning_pr_comment(verdict) + if kind == "pr" + else format_grace_warning_issue_comment(verdict) + ) + if not close: + return { + **base_result, + "action": "would-warn-grace", + "verdict": verdict, + "comment": warning_body, + } + post_comment(repo, number, warning_body) + return { + **base_result, + "action": "warned-grace", + "verdict": verdict, + "comment": warning_body, + } + if grace_age < GRACE_PERIOD_SECONDS: + return { + **base_result, + "action": "skip-in-grace-period", + "verdict": verdict, + "grace_age_seconds": grace_age, + "grace_period_seconds": GRACE_PERIOD_SECONDS, + } + + # Either the grace window has elapsed or this author bypasses grace — + # proceed to the actual close path. `--close` still gates the + # destructive write for the regular-author path so a local operator + # can preview a "would close after grace" verdict; immediate-close + # accounts (`is_immediate`) ignore `--close` so a workflow that + # forces dry-run for the global population can still take real + # action on those test accounts. + if not close and not is_immediate: return {**base_result, "action": "would-close", "verdict": verdict} comment_body = ( @@ -828,6 +1023,7 @@ def triage( "action": "closed", "verdict": verdict, "comment": comment_body, + "immediate_close": is_immediate, } diff --git a/tests/test_litellm/test_github_close_low_quality_prs.py b/tests/test_litellm/test_github_close_low_quality_prs.py index 141fb7f8c02..cda28036c75 100644 --- a/tests/test_litellm/test_github_close_low_quality_prs.py +++ b/tests/test_litellm/test_github_close_low_quality_prs.py @@ -125,6 +125,7 @@ class TestEvaluatePr: created_days_ago: int = 10, is_draft: bool = False, labels: list[str] | None = None, + author_login: str = "someone", ) -> dict: created = dt.datetime(2026, 5, 17, tzinfo=dt.timezone.utc) - dt.timedelta( days=created_days_ago @@ -135,7 +136,7 @@ class TestEvaluatePr: "createdAt": created.isoformat().replace("+00:00", "Z"), "isDraft": is_draft, "labels": [{"name": lbl} for lbl in (labels or [])], - "author": {"login": "someone"}, + "author": {"login": author_login}, "url": f"https://example.com/pr/{number}", } @@ -146,10 +147,13 @@ class TestEvaluatePr: closer_module, "is_external_pr_author", lambda pr, repo: True ) - def test_should_close_drafts_when_score_low(self, closer_module, _now, monkeypatch): + def test_should_warn_drafts_when_score_low_first_time( + self, closer_module, _now, monkeypatch + ): # Drafts are NOT a free pass — the open-PR queue should reflect any # PR that needs human attention regardless of draft status. Authors # who need a long-lived draft can use the `wip` opt-out label. + # First run: warn the contributor (1-day grace), don't close yet. monkeypatch.setattr( closer_module, "fetch_pr_comments", @@ -163,15 +167,17 @@ class TestEvaluatePr: repo=None, optout_labels=set(), ) - assert action == "close" + assert action == "warn-grace" assert score == 2 and age == 0 - def test_should_close_brand_new_pr_when_min_age_zero( + def test_should_warn_brand_new_pr_when_min_age_zero( self, closer_module, _now, monkeypatch ): # `min_age_days=0` means no age filter — a freshly-opened PR is - # eligible the moment Greptile scores it below threshold. This is - # the new default behavior. + # eligible the moment Greptile scores it below threshold. The + # first detection still goes through the warn-grace step rather + # than closing immediately, giving the contributor 24 hours to + # respond before the next run actually closes the PR. monkeypatch.setattr( closer_module, "fetch_pr_comments", @@ -185,7 +191,7 @@ class TestEvaluatePr: repo=None, optout_labels=set(), ) - assert action == "close" + assert action == "warn-grace" assert score == 1 and age == 0 def test_should_skip_optout_label_case_insensitive( @@ -284,9 +290,12 @@ class TestEvaluatePr: assert action == "skip-score-ok" assert score == 4 and age == 10 - def test_should_close_when_old_and_low_score( + def test_should_warn_when_old_and_low_score_no_prior_warning( self, closer_module, _now, monkeypatch ): + # Even an old PR that still has no grace warning gets one on the + # first eligible run — the daily cron is the natural cadence, so + # an existing-but-never-warned PR enters the grace flow normally. monkeypatch.setattr( closer_module, "fetch_pr_comments", @@ -300,16 +309,37 @@ class TestEvaluatePr: repo=None, optout_labels=set(), ) - assert action == "close" + assert action == "warn-grace" assert score == 3 and age == 10 - def test_should_close_when_old_and_very_low_score( + def test_should_close_when_grace_warning_aged_out_and_score_still_low( self, closer_module, _now, monkeypatch ): + # Day-1 the closer posted a warning. Day-2 the PR still scores <4 + # AND the warning is older than `GRACE_PERIOD_SECONDS`, so the + # action flips to `close`. This is the "grace expired" path. + old_warning = { + "user": {"login": "github-actions[bot]"}, + "body": ( + "you have 1 day to fix this\n\n" + closer_module.GRACE_COMMENT_MARKER + ), + "created_at": ( + _now - dt.timedelta(seconds=closer_module.GRACE_PERIOD_SECONDS + 60) + ) + .isoformat() + .replace("+00:00", "Z"), + "updated_at": "2026-05-15T00:00:00Z", + } monkeypatch.setattr( closer_module, "fetch_pr_comments", - lambda *a, **kw: [_greptile_comment("