diff --git a/.github/scripts/triage_with_llm.py b/.github/scripts/triage_with_llm.py index 52d17f769ab..60c051e3352 100644 --- a/.github/scripts/triage_with_llm.py +++ b/.github/scripts/triage_with_llm.py @@ -44,10 +44,11 @@ INTERNAL_ASSOCIATIONS = frozenset({"OWNER", "MEMBER", "COLLABORATOR"}) # Marker phrase Agent Shin always includes in its auto-close comments # (see `format_pr_close_comment` / `format_issue_close_comment`). The -# provenance check for reconsider uses this string + a bot-author filter -# to confirm a PR/issue was actually auto-closed by Agent Shin before -# letting a reconsider trigger reopen it. Keep the marker in sync with -# the literal text in those formatter functions. +# provenance check for reconsider matches this marker against a comment +# authored by the same bot login that performed the most recent `closed` +# event, so a contributor cannot reopen a PR/issue that a maintainer +# closed after a prior Agent Shin auto-close. Keep the marker in sync +# with the literal text in those formatter functions. AGENT_SHIN_AUTO_CLOSE_MARKER = "I'm **Agent Shin**" # Model families that require `reasoning_effort` to be set, and that reject @@ -199,26 +200,67 @@ def fetch_issue_comments(repo: str, number: int) -> list[dict]: return comments -def was_auto_closed_by_agent_shin(repo: str, number: int) -> bool: - """Return True iff this PR/issue carries an Agent Shin auto-close comment. +def fetch_issue_events(repo: str, number: int) -> list[dict]: + """Fetch all issue events for a PR/issue (paginated, ascending order). - Provenance check for the `@agent-shin reconsider` flow. We require: - - 1. A comment authored by a bot account (login ends with "[bot]") — - the auto-close workflow uses GH_TOKEN which posts as - `github-actions[bot]`. Filtering by bot author makes it impossible - for a contributor to spoof an Agent Shin close by pasting the - marker text into a manual comment. - 2. The comment body contains the Agent Shin auto-close marker - (`AGENT_SHIN_AUTO_CLOSE_MARKER`). - - Without this check, a maintainer who closes a PR as a duplicate or - out-of-scope could be silently overridden by the original author - commenting `@agent-shin reconsider` and polishing the description. + Used by the reconsider provenance check to identify the actor of the + most recent `closed` event. Returns [] on error so the reconsider + path fails safe (no proof of Agent Shin close -> refuse to reopen). """ + try: + raw = gh( + "api", + "--paginate", + f"repos/{repo}/issues/{number}/events?per_page=100", + ) + except subprocess.CalledProcessError: + return [] + events: list[dict] = [] + for line in raw.strip().splitlines(): + line = line.strip() + if not line: + continue + try: + parsed = json.loads(line) + except json.JSONDecodeError: + continue + if isinstance(parsed, list): + events.extend(parsed) + else: + events.append(parsed) + return events + + +def was_auto_closed_by_agent_shin(repo: str, number: int) -> bool: + """Return True iff Agent Shin is responsible for the *current* closure. + + Provenance check for the `@agent-shin reconsider` flow. We require ALL of: + + 1. The most recent `closed` event on the PR/issue was performed by + a bot account (actor login ends with "[bot]"). Agent Shin's + auto-close workflow uses GH_TOKEN, which posts as + `github-actions[bot]`. Anchoring on the most recent close — not + just any historical close — prevents a contributor from + overriding a *later* maintainer-initiated closure (e.g. + duplicate, out-of-scope) by polishing the description and + commenting `@agent-shin reconsider`. + 2. A comment authored by the same bot login that performed the + close contains the Agent Shin auto-close marker + (`AGENT_SHIN_AUTO_CLOSE_MARKER`). Matching the comment author + to the closer rules out closures by unrelated bots (stale, + cla-assistant, etc.) and spoofing via marker text pasted by + non-bot accounts. + """ + events = fetch_issue_events(repo, number) + last_closer: str | None = None + for event in events: + if (event.get("event") or "").lower() == "closed": + last_closer = ((event.get("actor") or {}).get("login") or "").lower() + if not last_closer or not last_closer.endswith("[bot]"): + return False for comment in fetch_issue_comments(repo, number): login = ((comment.get("user") or {}).get("login") or "").lower() - if not login.endswith("[bot]"): + if login != last_closer: continue body = comment.get("body") or "" if AGENT_SHIN_AUTO_CLOSE_MARKER in body: diff --git a/tests/test_litellm/test_github_triage_with_llm.py b/tests/test_litellm/test_github_triage_with_llm.py index 078612e89f9..542e5bcea57 100644 --- a/tests/test_litellm/test_github_triage_with_llm.py +++ b/tests/test_litellm/test_github_triage_with_llm.py @@ -301,9 +301,17 @@ class TestMainModelDefault: class TestWasAutoClosedByAgentShin: """Provenance check that gates reconsider's reopen path.""" - def test_should_return_true_when_bot_comment_has_marker( + @staticmethod + def _install(monkeypatch, triage_module, events, comments): + monkeypatch.setattr(triage_module, "fetch_issue_events", lambda repo, n: events) + monkeypatch.setattr( + triage_module, "fetch_issue_comments", lambda repo, n: comments + ) + + def test_should_return_true_when_latest_close_was_bot_with_marker( self, triage_module, monkeypatch ): + events = [{"event": "closed", "actor": {"login": "github-actions[bot]"}}] comments = [ { "user": {"login": "github-actions[bot]"}, @@ -313,33 +321,32 @@ class TestWasAutoClosedByAgentShin: ), } ] - monkeypatch.setattr( - triage_module, "fetch_issue_comments", lambda repo, n: comments - ) + self._install(monkeypatch, triage_module, events, comments) assert triage_module.was_auto_closed_by_agent_shin("o/r", 1) is True - def test_should_return_false_when_no_comments(self, triage_module, monkeypatch): - monkeypatch.setattr(triage_module, "fetch_issue_comments", lambda repo, n: []) + def test_should_return_false_when_no_close_event(self, triage_module, monkeypatch): + self._install(monkeypatch, triage_module, [], []) assert triage_module.was_auto_closed_by_agent_shin("o/r", 1) is False def test_should_ignore_non_bot_author_with_marker(self, triage_module, monkeypatch): # A contributor pasting the marker into a manual comment must NOT - # be treated as proof Agent Shin closed the PR. Only bot accounts - # (login ends with "[bot]") count — they can't be spoofed. + # be treated as proof Agent Shin closed the PR. Even if a bot did + # the most recent close, the marker comment must be authored by + # that same bot login — not by the human. + events = [{"event": "closed", "actor": {"login": "github-actions[bot]"}}] comments = [ { "user": {"login": "outside-dev"}, "body": "I'm **Agent Shin**, just kidding — please reconsider this.", } ] - monkeypatch.setattr( - triage_module, "fetch_issue_comments", lambda repo, n: comments - ) + self._install(monkeypatch, triage_module, events, comments) assert triage_module.was_auto_closed_by_agent_shin("o/r", 1) is False def test_should_ignore_bot_comment_without_marker(self, triage_module, monkeypatch): # Other bots (codecov, cla-assistant, etc.) post on every PR; their # presence must not satisfy the provenance check. + events = [{"event": "closed", "actor": {"login": "github-actions[bot]"}}] comments = [ { "user": {"login": "codecov[bot]"}, @@ -350,15 +357,16 @@ class TestWasAutoClosedByAgentShin: "body": "Confidence Score: 2/5", }, ] - monkeypatch.setattr( - triage_module, "fetch_issue_comments", lambda repo, n: comments - ) + self._install(monkeypatch, triage_module, events, comments) assert triage_module.was_auto_closed_by_agent_shin("o/r", 1) is False - def test_should_find_marker_in_any_bot_comment(self, triage_module, monkeypatch): - # The auto-close comment may not be the most recent (e.g. the - # contributor commented after Agent Shin closed it). Any matching - # bot comment counts. + def test_should_anchor_on_most_recent_close_event(self, triage_module, monkeypatch): + # Agent Shin auto-closed first, contributor commented after; the + # bot-authored marker comment is anywhere in the timeline. + events = [ + {"event": "labeled", "actor": {"login": "krrishdholakia"}}, + {"event": "closed", "actor": {"login": "github-actions[bot]"}}, + ] comments = [ { "user": {"login": "codecov[bot]"}, @@ -373,11 +381,51 @@ class TestWasAutoClosedByAgentShin: "body": "Replying after auto-close ...", }, ] - monkeypatch.setattr( - triage_module, "fetch_issue_comments", lambda repo, n: comments - ) + self._install(monkeypatch, triage_module, events, comments) assert triage_module.was_auto_closed_by_agent_shin("o/r", 1) is True + def test_should_refuse_when_maintainer_re_closed_after_agent_shin( + self, triage_module, monkeypatch + ): + # Agent Shin auto-closed, the PR was reopened, then a maintainer + # closed it again (e.g. as a duplicate). `@agent-shin reconsider` + # must NOT override the maintainer's later closure even though the + # historical Agent Shin marker comment still exists. + events = [ + {"event": "closed", "actor": {"login": "github-actions[bot]"}}, + {"event": "reopened", "actor": {"login": "github-actions[bot]"}}, + {"event": "closed", "actor": {"login": "krrishdholakia"}}, + ] + comments = [ + { + "user": {"login": "github-actions[bot]"}, + "body": "I'm **Agent Shin**, the automated triage bot ...", + } + ] + self._install(monkeypatch, triage_module, events, comments) + assert triage_module.was_auto_closed_by_agent_shin("o/r", 1) is False + + def test_should_refuse_when_unrelated_bot_re_closed_after_agent_shin( + self, triage_module, monkeypatch + ): + # Agent Shin closed, the PR was reopened, then a different bot + # (stale, etc.) closed it. The marker comment is from + # `github-actions[bot]` but the most recent closer is + # `stale[bot]`, so the logins don't match -> refuse to reopen. + events = [ + {"event": "closed", "actor": {"login": "github-actions[bot]"}}, + {"event": "reopened", "actor": {"login": "github-actions[bot]"}}, + {"event": "closed", "actor": {"login": "stale[bot]"}}, + ] + comments = [ + { + "user": {"login": "github-actions[bot]"}, + "body": "I'm **Agent Shin**, the automated triage bot ...", + } + ] + self._install(monkeypatch, triage_module, events, comments) + assert triage_module.was_auto_closed_by_agent_shin("o/r", 1) is False + class TestCallLlmJudge: """call_llm_judge sets gpt-5 specific kwargs correctly."""