From f876173b78f493bc661023cb928f66efb6d79e7b Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 19 May 2026 07:35:24 +0000 Subject: [PATCH] feat(triage): 1-day grace period before close + SwiftWinds immediate-close bypass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add a 24-hour grace window between the first low-quality detection and the actual auto-close. The first detection posts a warning comment that explicitly says "You have 1 day to address this before this PR is auto-closed" and points the contributor at: * `@agent-shin reconsider` to request another look (and re-open) * `@greptileai` to request a fresh Greptile review — works even after the PR is closed - Both `triage_with_llm.py` (LLM judge) and `close_low_quality_prs.py` (Greptile-score closer) share the same `` HTML marker so a warning posted by either path is recognized by both. - Add IMMEDIATE_CLOSE_LOGINS = {swiftwinds} to bypass BOTH the grace period AND the dry-run / AGENT_SHIN_ENABLED gating. SwiftWinds is the user's personal account (no push permissions to litellm) used to dogfood the bot; user explicitly asked: "For SwiftWinds, just close immediately. Faster iteration that way." - Update the standard close comments to mention that `@greptileai` works even after the PR is closed. - Add 23 new tests covering: warn-grace on first detection, skip during grace window, close after grace expires, SwiftWinds bypass (case insensitive, with close=False, no random-login false positives), the grace-warning text invariants, and the SwiftWinds entry in the IMMEDIATE_CLOSE_LOGINS constant. Co-authored-by: Mateo Wang --- .github/scripts/close_low_quality_prs.py | 214 +++++++- .github/scripts/triage_with_llm.py | 218 +++++++- .../test_github_close_low_quality_prs.py | 242 ++++++++- .../test_github_triage_with_llm.py | 478 +++++++++++++++++- 4 files changed, 1120 insertions(+), 32 deletions(-) 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("

Confidence Score: 1/5

")], + lambda *a, **kw: [ + _greptile_comment( + "

Confidence Score: 1/5

", + updated_at="2026-05-15T00:00:00Z", + ), + old_warning, + ], ) action, score, _ = closer_module.evaluate_pr( self._make_pr(created_days_ago=14), @@ -322,6 +352,82 @@ class TestEvaluatePr: assert action == "close" assert score == 1 + def test_should_skip_when_grace_warning_within_window( + self, closer_module, _now, monkeypatch + ): + # Within the 24-hour grace window the closer must NOT close the + # PR even if the score is still low. The warning is only an hour + # old; give the contributor time to push fixes before destruction. + recent_warning = { + "user": {"login": "github-actions[bot]"}, + "body": "warning text\n\n" + closer_module.GRACE_COMMENT_MARKER, + "created_at": (_now - dt.timedelta(hours=1)) + .isoformat() + .replace("+00:00", "Z"), + } + monkeypatch.setattr( + closer_module, + "fetch_pr_comments", + lambda *a, **kw: [ + _greptile_comment("Confidence Score: 2/5"), + recent_warning, + ], + ) + action, score, _ = closer_module.evaluate_pr( + self._make_pr(created_days_ago=10), + now=_now, + min_age_days=0, + min_score=4, + repo=None, + optout_labels=set(), + ) + assert action == "skip-in-grace-period" + assert score == 2 + + def test_should_close_immediately_for_swiftwinds_login( + self, closer_module, _now, monkeypatch + ): + # SwiftWinds is in `IMMEDIATE_CLOSE_LOGINS` for dogfooding the bot + # from an external account. Skip grace; close on first detection + # so the iteration loop is fast. + monkeypatch.setattr( + closer_module, + "fetch_pr_comments", + lambda *a, **kw: [_greptile_comment("Confidence Score: 1/5")], + ) + action, score, _ = closer_module.evaluate_pr( + self._make_pr(created_days_ago=0, author_login="SwiftWinds"), + now=_now, + min_age_days=0, + min_score=4, + repo=None, + optout_labels=set(), + ) + assert action == "close" + assert score == 1 + + def test_should_close_immediately_for_swiftwinds_login_case_insensitive( + self, closer_module, _now, monkeypatch + ): + # GitHub login matching is case-insensitive on GitHub's side; the + # API returns the original casing. Make sure the bypass fires + # regardless of how the login was registered. + monkeypatch.setattr( + closer_module, + "fetch_pr_comments", + lambda *a, **kw: [_greptile_comment("Confidence Score: 1/5")], + ) + for login in ("SwiftWinds", "swiftwinds", "SWIFTWINDS"): + action, _, _ = closer_module.evaluate_pr( + self._make_pr(created_days_ago=0, author_login=login), + now=_now, + min_age_days=0, + min_score=4, + repo=None, + optout_labels=set(), + ) + assert action == "close", login + def test_should_skip_internal_authors(self, closer_module, _now, monkeypatch): # Override the fixture for this one test. monkeypatch.setattr( @@ -422,6 +528,120 @@ class TestMainOptoutLabelDefault: assert default not in captured["optout_labels"], default +class TestSecondsSinceLastGraceWarning: + """Grace-period detection: only counts comments by the bot identity + that contain the shared `GRACE_COMMENT_MARKER`.""" + + def _make_marker_comment( + self, + closer_module, + *, + login: str = "github-actions[bot]", + created_at: str = "2026-05-16T00:00:00Z", + include_marker: bool = True, + ) -> dict: + body = "warning text" + if include_marker: + body += "\n\n" + closer_module.GRACE_COMMENT_MARKER + return { + "user": {"login": login}, + "body": body, + "created_at": created_at, + } + + def test_should_return_none_when_no_marker_comment(self, closer_module): + comments = [ + { + "user": {"login": "github-actions[bot]"}, + "body": "Some other bot comment", + "created_at": "2026-05-16T00:00:00Z", + } + ] + assert closer_module.seconds_since_last_grace_warning(comments) is None + + def test_should_return_none_for_empty(self, closer_module): + assert closer_module.seconds_since_last_grace_warning([]) is None + + def test_should_ignore_non_bot_comments_with_marker(self, closer_module): + # If a curious user quotes the marker in a comment, we must NOT + # treat it as a bot warning. The grace timer would then never fire. + comments = [ + self._make_marker_comment(closer_module, login="random-user"), + ] + assert closer_module.seconds_since_last_grace_warning(comments) is None + + def test_should_pick_latest_marker_comment(self, closer_module): + # When multiple grace warnings exist (e.g. a re-open cycle), use + # the most recent one to compute the age. + comments = [ + self._make_marker_comment(closer_module, created_at="2026-05-15T00:00:00Z"), + self._make_marker_comment(closer_module, created_at="2026-05-16T23:00:00Z"), + ] + now = dt.datetime(2026, 5, 17, 0, 0, 0, tzinfo=dt.timezone.utc) + age = closer_module.seconds_since_last_grace_warning(comments, now=now) + # 1h = 3600s + assert age == 3600.0 + + +class TestImmediateCloseLoginsConstant: + """SwiftWinds is the dogfood account the user explicitly named — pin + its presence so a future cleanup that removes the constant or + forgets to keep the entry doesn't silently break the test path.""" + + def test_should_include_swiftwinds(self, closer_module): + assert "swiftwinds" in closer_module.IMMEDIATE_CLOSE_LOGINS + + def test_should_be_lowercase_for_case_insensitive_match(self, closer_module): + for login in closer_module.IMMEDIATE_CLOSE_LOGINS: + assert login == login.lower(), login + + +class TestGraceWarningCommentText: + """Pin the user-facing language in the grace warning comment so the + `1 day grace` and `@greptileai still works after close` promises + don't get accidentally dropped in a future refactor. + """ + + def test_should_state_one_day_grace_period(self, closer_module): + body = closer_module.format_grace_warning_comment(score=2, threshold=4) + # The user's PR explicitly said "specify in the comment" — pin + # that the literal "1 day" appears in the comment. + assert "1 day" in body + + def test_should_mention_agent_shin_reconsider(self, closer_module): + body = closer_module.format_grace_warning_comment(score=2, threshold=4) + assert "@agent-shin reconsider" in body + + def test_should_promise_greptileai_works_after_close(self, closer_module): + body = closer_module.format_grace_warning_comment(score=2, threshold=4) + assert "@greptileai" in body + assert "even after the PR is closed" in body + + def test_should_carry_grace_marker(self, closer_module): + # The marker is what `seconds_since_last_grace_warning` greps for + # to detect a prior warning — dropping it would silently break + # the cooldown. + body = closer_module.format_grace_warning_comment(score=2, threshold=4) + assert closer_module.GRACE_COMMENT_MARKER in body + + def test_close_comment_should_mention_greptileai_post_close(self, closer_module): + # The actual close comment should ALSO point at the @greptileai + # post-close re-review path so contributors see the same options + # whether they read the warning or only catch the close comment. + # `close_pr` writes the close comment via `gh pr comment` — we + # don't easily call it directly here, but the comment body is + # constructed inline. Re-creating it via a no-op `gh` stub is + # awkward, so we assert against the same string template by + # asserting that the close path's text constant is updated. + # `close_pr` source must contain the marker text — guarded by + # this whole-module read-and-assert. + from pathlib import Path + + source = Path(closer_module.__file__).read_text() + assert "even after the PR is closed" in source + assert "@greptileai" in source + + class TestHasOptoutLabel: def test_should_match_label_case_insensitively(self, closer_module): pr = {"labels": [{"name": "Do Not Close"}, {"name": "bug"}]} diff --git a/tests/test_litellm/test_github_triage_with_llm.py b/tests/test_litellm/test_github_triage_with_llm.py index a8ecd0fbea0..020de81c391 100644 --- a/tests/test_litellm/test_github_triage_with_llm.py +++ b/tests/test_litellm/test_github_triage_with_llm.py @@ -619,9 +619,12 @@ class TestTriageOrchestration: self, triage_module, monkeypatch ): # "See #1234" is a passing mention, not a closing keyword. The LLM - # must get a chance to apply the stricter rubric. + # must get a chance to apply the stricter rubric. With no prior + # grace warning, the first failing verdict triggers the warning + # path (`would-warn-grace` in dry-run). pr = self._make_pr(body="See #1234 for context. No QA proof here.") monkeypatch.setattr(triage_module, "fetch_pr", lambda repo, n: pr) + self._stub_grace_no_warning(triage_module, monkeypatch) called = {"judge": False} def judge(prompt): @@ -639,7 +642,7 @@ class TestTriageOrchestration: judge=judge, ) assert called["judge"] is True - assert result["action"] == "would-close" + assert result["action"] == "would-warn-grace" def test_should_return_pass_llm_when_judge_passes(self, triage_module, monkeypatch): pr = self._make_pr(body="Long body, no linked issue.") @@ -656,9 +659,16 @@ class TestTriageOrchestration: assert result["action"] == "pass-llm" assert "Long body" in captured["prompt"] - def test_should_return_would_close_in_dry_run(self, triage_module, monkeypatch): + def test_should_return_would_close_in_dry_run_after_grace_aged_out( + self, triage_module, monkeypatch + ): + # When the grace warning has already aged out (>= GRACE_PERIOD_SECONDS) + # AND the rubric still fails, the dry-run preview returns + # `would-close` so a step-summary writer can render the close + # comment without touching GitHub state. pr = self._make_pr(body="just a sentence.") monkeypatch.setattr(triage_module, "fetch_pr", lambda repo, n: pr) + self._stub_grace_aged_out(triage_module, monkeypatch) def fake_post(*a, **kw): pytest.fail("should not post comments in dry-run") @@ -685,11 +695,15 @@ class TestTriageOrchestration: assert result["action"] == "would-close" assert result["verdict"]["missing"] == ["problem description", "QA proof"] - def test_should_post_comment_and_close_when_close_enabled( + def test_should_post_comment_and_close_after_grace_window( self, triage_module, monkeypatch ): + # The "real close" path: --close passed AND the grace warning has + # aged out AND the rubric still fails. The bot posts the close + # comment and closes the PR. pr = self._make_pr(body="just a sentence.") monkeypatch.setattr(triage_module, "fetch_pr", lambda repo, n: pr) + self._stub_grace_aged_out(triage_module, monkeypatch) posted = {} closed = {} monkeypatch.setattr( @@ -784,6 +798,30 @@ class TestTriageOrchestration: lambda *a, **kw: None, ) + @staticmethod + def _stub_grace_aged_out(triage_module, monkeypatch): + """Pretend the grace warning has aged out. + + For tests that exercise the post-grace close path. Set the age + to twice the grace window so a future tweak to + `GRACE_PERIOD_SECONDS` doesn't accidentally make the stub fall + back inside the window. + """ + monkeypatch.setattr( + triage_module, + "seconds_since_last_grace_warning", + lambda *a, **kw: triage_module.GRACE_PERIOD_SECONDS * 2, + ) + + @staticmethod + def _stub_grace_no_warning(triage_module, monkeypatch): + """Pretend no grace warning has been posted yet (first detection).""" + monkeypatch.setattr( + triage_module, + "seconds_since_last_grace_warning", + lambda *a, **kw: None, + ) + def test_should_reopen_on_reconsider_pass(self, triage_module, monkeypatch): # Reconsider on a closed PR with a passing verdict -> reopen + post a # friendly "re-evaluated" comment. close=True is the production path @@ -1214,6 +1252,9 @@ class TestTriageOrchestration: "user": {"login": "outside"}, } monkeypatch.setattr(triage_module, "fetch_issue", lambda repo, n: issue) + # Grace already aged out -> close path. (Issues use the same + # GRACE_COMMENT_MARKER detection as PRs.) + self._stub_grace_aged_out(triage_module, monkeypatch) closed = {} posted = {} monkeypatch.setattr( @@ -1243,3 +1284,432 @@ class TestTriageOrchestration: assert result["action"] == "closed" assert closed["n"] == 7 assert "reproduction" in posted["body"] + + # ---- Grace-period flow ------------------------------------------------ + + def test_should_post_grace_warning_on_first_failing_run_in_close_mode( + self, triage_module, monkeypatch + ): + # First low-quality detection -> bot posts a warning comment with + # the GRACE_COMMENT_MARKER. The PR must NOT be closed yet. + pr = self._make_pr(body="just a sentence.") + monkeypatch.setattr(triage_module, "fetch_pr", lambda repo, n: pr) + self._stub_grace_no_warning(triage_module, monkeypatch) + posted = {} + monkeypatch.setattr( + triage_module, + "post_comment", + lambda repo, n, body: posted.update({"n": n, "body": body}), + ) + monkeypatch.setattr( + triage_module, + "close_pr", + lambda *a, **kw: pytest.fail("must not close on first detection"), + ) + + verdict = { + "verdict": "fail", + "missing": ["QA proof"], + "explanation": "Body too thin.", + } + result = triage_module.triage( + repo="o/r", + kind="pr", + number=42, + close=True, + model="m", + judge=lambda p: json.dumps(verdict), + ) + assert result["action"] == "warned-grace" + assert posted["n"] == 42 + # Pin the user-facing language pieces the user explicitly asked for. + assert "1 day" in posted["body"] + assert "@agent-shin reconsider" in posted["body"] + assert "@greptileai" in posted["body"] + assert "even after the PR is closed" in posted["body"] + assert triage_module.GRACE_COMMENT_MARKER in posted["body"] + + def test_should_skip_close_inside_grace_window(self, triage_module, monkeypatch): + # A warning was posted recently; do nothing on this run regardless + # of close=True. The next run after `GRACE_PERIOD_SECONDS` elapses + # is the one that flips to actual close. + pr = self._make_pr(body="just a sentence.") + monkeypatch.setattr(triage_module, "fetch_pr", lambda repo, n: pr) + monkeypatch.setattr( + triage_module, + "seconds_since_last_grace_warning", + lambda *a, **kw: 60.0, + ) + monkeypatch.setattr( + triage_module, + "post_comment", + lambda *a, **kw: pytest.fail("must not comment during grace window"), + ) + monkeypatch.setattr( + triage_module, + "close_pr", + lambda *a, **kw: pytest.fail("must not close during grace window"), + ) + + verdict = { + "verdict": "fail", + "missing": ["QA proof"], + "explanation": "Body too thin.", + } + result = triage_module.triage( + repo="o/r", + kind="pr", + number=42, + close=True, + model="m", + judge=lambda p: json.dumps(verdict), + ) + assert result["action"] == "skip-in-grace-period" + assert result["grace_age_seconds"] == 60.0 + assert result["grace_period_seconds"] == triage_module.GRACE_PERIOD_SECONDS + + def test_should_dry_run_grace_warning_when_close_false( + self, triage_module, monkeypatch + ): + # In dry-run mode the FIRST failing detection returns + # `would-warn-grace` (with the previewed comment body) and never + # touches GitHub state. Lets a local operator preview the + # warning before flipping --close on. + pr = self._make_pr(body="thin") + monkeypatch.setattr(triage_module, "fetch_pr", lambda repo, n: pr) + self._stub_grace_no_warning(triage_module, monkeypatch) + monkeypatch.setattr( + triage_module, + "post_comment", + lambda *a, **kw: pytest.fail("must not post in dry-run grace warn"), + ) + + verdict = { + "verdict": "fail", + "missing": ["QA proof"], + "explanation": "thin", + } + result = triage_module.triage( + repo="o/r", + kind="pr", + number=1, + close=False, + model="m", + judge=lambda p: json.dumps(verdict), + ) + assert result["action"] == "would-warn-grace" + assert "1 day" in result["comment"] + + # ---- IMMEDIATE_CLOSE_LOGINS bypass (e.g. SwiftWinds) ----------------- + + def test_should_skip_grace_for_swiftwinds_login(self, triage_module, monkeypatch): + # SwiftWinds is in `IMMEDIATE_CLOSE_LOGINS` for dogfooding. + # Even though no grace warning has been posted, the bypass must + # take the close path immediately. + pr = self._make_pr(body="just a sentence.", user={"login": "SwiftWinds"}) + monkeypatch.setattr(triage_module, "fetch_pr", lambda repo, n: pr) + # The grace check must NOT be consulted at all for an immediate + # login — pin that here. + monkeypatch.setattr( + triage_module, + "seconds_since_last_grace_warning", + lambda *a, **kw: pytest.fail( + "grace check must not run for immediate-close login" + ), + ) + posted = {} + closed = {} + monkeypatch.setattr( + triage_module, + "post_comment", + lambda repo, n, body: posted.update({"n": n, "body": body}), + ) + monkeypatch.setattr( + triage_module, + "close_pr", + lambda repo, n: closed.update({"n": n}), + ) + + verdict = { + "verdict": "fail", + "missing": ["QA proof"], + "explanation": "Body too thin.", + } + result = triage_module.triage( + repo="o/r", + kind="pr", + number=99, + close=True, + model="m", + judge=lambda p: json.dumps(verdict), + ) + assert result["action"] == "closed" + assert result["immediate_close"] is True + assert closed["n"] == 99 + # The close comment is the standard one (no warning was needed). + assert "Agent Shin" in posted["body"] + + def test_should_close_swiftwinds_even_when_close_flag_false( + self, triage_module, monkeypatch + ): + # The `IMMEDIATE_CLOSE_LOGINS` bypass intentionally overrides the + # workflow-level dry-run gating. The user explicitly asked to + # "turn on full (non dry run) mode with SwiftWinds" because their + # personal account has no push permissions to litellm and is the + # perfect testing target. With `close=False` (workflow stripped + # --close on pull_request_target), the script must STILL post + + # close for SwiftWinds. + pr = self._make_pr(body="just a sentence.", user={"login": "SwiftWinds"}) + monkeypatch.setattr(triage_module, "fetch_pr", lambda repo, n: pr) + posted = {} + closed = {} + monkeypatch.setattr( + triage_module, + "post_comment", + lambda repo, n, body: posted.update({"n": n, "body": body}), + ) + monkeypatch.setattr( + triage_module, + "close_pr", + lambda repo, n: closed.update({"n": n}), + ) + + verdict = { + "verdict": "fail", + "missing": ["QA proof"], + "explanation": "thin", + } + result = triage_module.triage( + repo="o/r", + kind="pr", + number=99, + close=False, + model="m", + judge=lambda p: json.dumps(verdict), + ) + assert result["action"] == "closed" + assert closed["n"] == 99 + + def test_should_match_immediate_close_login_case_insensitively( + self, triage_module, monkeypatch + ): + # GitHub returns the original casing of a login. The bypass must + # work regardless of the exact case the API returns. + for login in ("SwiftWinds", "swiftwinds", "SWIFTWINDS"): + pr = self._make_pr(body="thin", user={"login": login}) + monkeypatch.setattr(triage_module, "fetch_pr", lambda repo, n: pr) + monkeypatch.setattr( + triage_module, + "post_comment", + lambda *a, **kw: None, + ) + monkeypatch.setattr( + triage_module, + "close_pr", + lambda *a, **kw: None, + ) + verdict = { + "verdict": "fail", + "missing": ["QA proof"], + "explanation": "thin", + } + result = triage_module.triage( + repo="o/r", + kind="pr", + number=1, + close=False, + model="m", + judge=lambda p: json.dumps(verdict), + ) + assert result["action"] == "closed", login + assert result.get("immediate_close") is True, login + + def test_should_not_treat_random_external_login_as_immediate_close( + self, triage_module, monkeypatch + ): + # Sanity check — the bypass must NOT fire for any login other + # than the explicitly-listed test accounts. + pr = self._make_pr(body="thin", user={"login": "random-oss-dev"}) + monkeypatch.setattr(triage_module, "fetch_pr", lambda repo, n: pr) + self._stub_grace_no_warning(triage_module, monkeypatch) + posted = {} + monkeypatch.setattr( + triage_module, + "post_comment", + lambda repo, n, body: posted.update({"body": body}), + ) + monkeypatch.setattr( + triage_module, + "close_pr", + lambda *a, **kw: pytest.fail("must not close non-immediate login"), + ) + + verdict = { + "verdict": "fail", + "missing": ["QA proof"], + "explanation": "thin", + } + result = triage_module.triage( + repo="o/r", + kind="pr", + number=1, + close=True, + model="m", + judge=lambda p: json.dumps(verdict), + ) + assert result["action"] == "warned-grace" + + +class TestImmediateCloseLoginsConstant: + """SwiftWinds is the dogfood account the user explicitly named — pin + its presence so a future cleanup that removes the constant or + forgets to keep the entry doesn't silently break the test path.""" + + def test_should_include_swiftwinds(self, triage_module): + assert "swiftwinds" in triage_module.IMMEDIATE_CLOSE_LOGINS + + def test_should_be_lowercase_for_case_insensitive_match(self, triage_module): + for login in triage_module.IMMEDIATE_CLOSE_LOGINS: + assert login == login.lower(), login + + +class TestGraceWarningCommentText: + """Pin the user-facing promises in the grace warning so a future + refactor can't silently drop them.""" + + def test_pr_grace_warning_should_state_one_day_grace(self, triage_module): + body = triage_module.format_grace_warning_pr_comment( + {"verdict": "fail", "missing": ["QA proof"], "explanation": "thin"} + ) + # The user explicitly asked: "specify in the comment" that there + # is a 1-day grace. + assert "1 day" in body + + def test_pr_grace_warning_should_mention_reconsider_during_grace( + self, triage_module + ): + body = triage_module.format_grace_warning_pr_comment( + {"verdict": "fail", "missing": [], "explanation": ""} + ) + assert "@agent-shin reconsider" in body + + def test_pr_grace_warning_should_promise_greptileai_works_post_close( + self, triage_module + ): + body = triage_module.format_grace_warning_pr_comment( + {"verdict": "fail", "missing": [], "explanation": ""} + ) + # Per user: comment should state @greptileai works even after close. + assert "@greptileai" in body + assert "even after the PR is closed" in body + + def test_pr_grace_warning_should_carry_grace_marker(self, triage_module): + # The marker is what `seconds_since_last_grace_warning` greps for + # on subsequent runs to detect that a warning has been posted. + # Dropping it would silently break the close-after-grace path. + body = triage_module.format_grace_warning_pr_comment( + {"verdict": "fail", "missing": [], "explanation": ""} + ) + assert triage_module.GRACE_COMMENT_MARKER in body + + def test_issue_grace_warning_should_carry_grace_marker(self, triage_module): + body = triage_module.format_grace_warning_issue_comment( + {"verdict": "fail", "missing": [], "explanation": ""} + ) + assert triage_module.GRACE_COMMENT_MARKER in body + assert "1 day" in body + assert "@agent-shin reconsider" in body + + def test_pr_close_comment_should_promise_greptileai_works_post_close( + self, triage_module + ): + # The standard close comment must ALSO point at @greptileai so + # contributors see the same options whether they read the warning + # or only catch the close comment. + body = triage_module.format_pr_close_comment( + {"verdict": "fail", "missing": [], "explanation": ""} + ) + assert "@greptileai" in body + assert "even after the PR is closed" in body + + +class TestSecondsSinceLastGraceWarning: + """Mirror of TestSecondsSinceLastReconsiderVerdict for the new helper. + Both helpers share `_seconds_since_latest_marker_comment` underneath + so the parsing logic is exercised either way; these tests pin the + grace-marker-specific behavior.""" + + def _make_comment( + self, + *, + login: str, + body: str, + created_at: str | None = "2026-05-18T05:00:00Z", + ) -> dict: + comment: dict = {"user": {"login": login}, "body": body} + if created_at is not None: + comment["created_at"] = created_at + return comment + + def test_should_return_none_when_no_grace_marker(self, triage_module, monkeypatch): + comments = [ + self._make_comment( + login="github-actions[bot]", + body="Some other bot message", + ), + self._make_comment(login="random-user", body="ping?"), + ] + monkeypatch.setattr( + triage_module, "_iter_paginated_json", lambda *a, **kw: iter(comments) + ) + assert triage_module.seconds_since_last_grace_warning("o/r", 1) is None + + def test_should_ignore_non_bot_comments_with_marker( + self, triage_module, monkeypatch + ): + # A user who quotes the marker in a question must NOT be treated + # as the bot warning; otherwise the close-after-grace path would + # never fire because the timer keeps resetting. + comments = [ + self._make_comment( + login="random-user", + body=f"What is {triage_module.GRACE_COMMENT_MARKER}?", + ) + ] + monkeypatch.setattr( + triage_module, "_iter_paginated_json", lambda *a, **kw: iter(comments) + ) + assert triage_module.seconds_since_last_grace_warning("o/r", 1) is None + + def test_should_pick_latest_grace_marker(self, triage_module, monkeypatch): + comments = [ + self._make_comment( + login="github-actions[bot]", + body="old warning " + triage_module.GRACE_COMMENT_MARKER, + created_at="2026-05-18T03:00:00Z", + ), + self._make_comment( + login="github-actions[bot]", + body="newer warning " + triage_module.GRACE_COMMENT_MARKER, + created_at="2026-05-18T04:55:00Z", + ), + ] + monkeypatch.setattr( + triage_module, "_iter_paginated_json", lambda *a, **kw: iter(comments) + ) + + import datetime as real_dt + + class FrozenDateTime(real_dt.datetime): + @classmethod + def now(cls, tz=None): + return real_dt.datetime(2026, 5, 18, 5, 0, 0, tzinfo=tz) + + frozen_module = type(triage_module.dt)("datetime") + frozen_module.datetime = FrozenDateTime + frozen_module.timezone = real_dt.timezone + monkeypatch.setattr(triage_module, "dt", frozen_module) + + age = triage_module.seconds_since_last_grace_warning("o/r", 1) + # Newer warning is 5 minutes (300s) before "now". + assert age == 300.0