fix(triage): anchor reconsider provenance on most recent close event

Reconsider previously treated any historical Agent Shin auto-close
comment as sufficient proof that Agent Shin owns the current closure.
If the PR was reopened and later re-closed by a maintainer (e.g. as
a duplicate or out-of-scope), the contributor could override that
maintainer-initiated closure by commenting `@agent-shin reconsider`.

`was_auto_closed_by_agent_shin` now fetches the issue events, finds
the most recent `closed` event, and requires its actor login to (a)
end with [bot] and (b) match the author of a comment containing the
Agent Shin marker. Closures by maintainers or unrelated bots (stale,
cla-assistant, etc.) no longer satisfy provenance.
This commit is contained in:
mateo-berri 2026-05-18 14:52:38 +00:00
parent 76aa7b7772
commit b7578817f8
No known key found for this signature in database
2 changed files with 131 additions and 41 deletions

View file

@ -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:

View file

@ -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."""