scriptoriumadmin/litellm
1
0
Fork 0
mirror of https://github.com/BerriAI/litellm.git synced 2026-09-26 01:12:21 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 99

fix(triage): honor --close in --reconsider mode + gate reopen on bot-close provenance

Browse source 
Address two related concerns raised in PR review on the reconsider flow:

1. **Dry-run support for --reconsider** (P1, greptile-apps):
   The previous --reconsider branch unconditionally called post_comment +
   reopen_* regardless of --close. The docstring claimed 'close is forced
   True implicitly', but the workflow's only kill switch was
   AGENT_SHIN_ENABLED — invoking the script directly without --close was
   still destructive, the opposite of the conventional dry-run
   expectation.

   triage() now honors close=False in reconsider mode: a passing verdict
   returns action='would-reopen' (with the comment body it WOULD post in
   result['comment']) and a failing verdict returns
   action='would-leave-closed-still-failing'. The reconsider workflow now
   appends --close iff AGENT_SHIN_ENABLED == 'true', mirroring the
   pattern used by close_low_quality_prs.yml.

2. **Provenance gate for reopen** (P2, both greptile-apps and veria-ai):
   The reconsider flow could be used to silently override a maintainer's
   close decision — an external author edits the closed PR to include a
   closing keyword, comments '@agent-shin reconsider', and the bot
   reopens it. There was no check that Agent Shin was the actor that
   originally closed it.

   triage() now requires was_auto_closed_by_agent_shin() to be true
   before any reopen path can fire. The check looks for a bot-authored
   comment (login ends with '[bot]') containing the auto-close marker
   'I'm **Agent Shin**'. Filtering by bot author makes the marker
   unspoofable: a contributor pasting the phrase into a manual comment
   cannot satisfy the check. When the provenance gate fails, triage
   returns action='skip-not-bot-closed' without burning LLM tokens or
   posting anything.

Unit tests cover both behaviors plus the failure modes of
was_auto_closed_by_agent_shin (no comments, non-bot author with marker,
bot comment without marker, marker found anywhere in the comment list).

Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>
This commit is contained in:
Cursor Agent 2026-05-18 05:54:47 +00:00
parent 420547f7be
commit 28ba9f7c6b
No known key found for this signature in database
3 changed files with 399 additions and 19 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

118
.github/scripts/triage_with_llm.py vendored
View file

@ -42,6 +42,14 @@ DEFAULT_MODEL = "gpt-5.4-mini"
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.
AGENT_SHIN_AUTO_CLOSE_MARKER = "I'm **Agent Shin**"
# 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
@ -160,6 +168,64 @@ def reopen_issue(repo: str, number: int) -> None:
)
def fetch_issue_comments(repo: str, number: int) -> list[dict]:
"""Fetch all issue-style comments on a PR/issue (paginated).
`gh api --paginate` returns one JSON array per page; iterate them and
flatten. Returns [] on error (the reconsider path treats "no comments
found" as "no proof Agent Shin closed this", which fails-safe).
"""
try:
raw = gh(
"api",
"--paginate",
f"repos/{repo}/issues/{number}/comments?per_page=100",
)
except subprocess.CalledProcessError:
return []
comments: 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):
comments.extend(parsed)
else:
comments.append(parsed)
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.
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.
"""
for comment in fetch_issue_comments(repo, number):
login = ((comment.get("user") or {}).get("login") or "").lower()
if not login.endswith("[bot]"):
continue
body = comment.get("body") or ""
if AGENT_SHIN_AUTO_CLOSE_MARKER in body:
return True
return False
# ---------------------------------------------------------------------------
# Author classification
@ -514,10 +580,20 @@ def triage(
fail-but-no-comment is replaced with a "still failing" comment + leave
closed; a pass triggers `reopen_pr`/`reopen_issue` plus a reopen comment.
Reconsider mode is intended for the `@agent-shin reconsider` comment
trigger. `close` is forced True implicitly when `reconsider` is set
because the bot has already decided this is a real (non-dry-run)
invocation; it's the caller's responsibility to gate on
AGENT_SHIN_ENABLED before calling reconsider mode.
trigger.
`close` still controls whether destructive side effects fire. When
`close=False` and `reconsider=True`, the function previews the
decision (`would-reopen`, `would-leave-closed-still-failing`, or
`skip-not-bot-closed`) without posting comments or reopening — this
mirrors the regular `would-close` dry-run behavior so the reconsider
workflow can be exercised safely with `AGENT_SHIN_ENABLED != "true"`.
Provenance: when `reconsider=True`, we additionally check that the
PR/issue was actually auto-closed by Agent Shin (via the bot-authored
auto-close marker comment). If not, we refuse to reopen so a
maintainer-closed PR cannot be silently overridden by the author
polishing the description and commenting `@agent-shin reconsider`.
"""
fetcher = {"pr": fetch_pr, "issue": fetch_issue}[kind]
item = fetcher(repo, number)
@ -551,6 +627,15 @@ def triage(
if is_internal_contributor(item):
return {**base_result, "action": "skip-internal-author"}
# Provenance gate for reconsider: only reopen items Agent Shin auto-closed.
# Done up-front so a maintainer-closed PR short-circuits before we burn
# LLM tokens or touch comments. The check requires a bot-authored
# comment containing the auto-close marker (see
# `was_auto_closed_by_agent_shin`), so a contributor cannot spoof it by
# quoting the close template themselves.
if reconsider and not was_auto_closed_by_agent_shin(repo, number):
return {**base_result, "action": "skip-not-bot-closed"}
if kind == "pr":
prompt = build_pr_prompt(title=title, body=body)
# Short-circuit: if body very clearly links a related issue, just pass.
@ -565,8 +650,14 @@ def triage(
},
}
if reconsider:
# Pass-on-reconsider -> reopen the PR with a friendly comment.
reopen_body = format_reopen_comment(kind)
if not close:
return {
**base,
"action": "would-reopen",
"comment": reopen_body,
}
# Pass-on-reconsider -> reopen the PR with a friendly comment.
post_comment(repo, number, reopen_body)
reopen_pr(repo, number)
return {
@ -606,9 +697,17 @@ def triage(
if reconsider:
# Reconsider: pass -> reopen + post reopen comment;
# fail -> leave closed + post a "still failing" comment so the
# contributor can iterate again.
# contributor can iterate again. When `close=False` we preview
# the action instead of actually posting/reopening.
if decision != "fail":
reopen_body = format_reopen_comment(kind)
if not close:
return {
**base_result,
"action": "would-reopen",
"verdict": verdict,
"comment": reopen_body,
}
post_comment(repo, number, reopen_body)
if kind == "pr":
reopen_pr(repo, number)
@ -621,6 +720,13 @@ def triage(
"comment": reopen_body,
}
still_failing = format_reconsider_still_failing_comment(kind, verdict)
if not close:
return {
**base_result,
"action": "would-leave-closed-still-failing",
"verdict": verdict,
"comment": still_failing,
}
post_comment(repo, number, still_failing)
return {
**base_result,

18
.github/workflows/triage_reconsider.yml vendored
View file

@ -107,20 +107,22 @@ jobs:
else
ARGS=(--repo "${{ github.repository }}" --issue "${NUMBER}" --reconsider)
fi
# Reconsider IS the destructive path here (it can post comments
# and reopen) — there's no separate `--close` flag because the
# script's reconsider mode handles both pass (reopen) and fail
# (still-failing comment) outcomes itself.
# Reconsider is the destructive path here (it can post comments
# and reopen). The triage script honors `--close` even in
# reconsider mode — without it the script previews actions
# (`would-reopen` / `would-leave-closed-still-failing`) without
# writing to GitHub. Append `--close` only when AGENT_SHIN_ENABLED
# is the literal string "true"; everything else (unset, "false",
# "True", "yes", "1", typos) stays in dry-run.
#
# Use the positive `= "true"` gate (instead of `!= "true" -> exit`)
# so the workflow guardrails in
# tests/test_litellm/test_github_triage_workflows.py see the
# canonical fail-safe enable pattern. Unknown values like "True",
# "yes", "1", or typos will fall through to the dry-run else
# branch, which is the safe default.
# canonical fail-safe enable pattern.
if [ "${AGENT_SHIN_ENABLED:-false}" = "true" ]; then
ARGS+=(--close)
echo "::notice::Agent Shin reconsider ENABLED — running real triage."
python3 .github/scripts/triage_with_llm.py "${ARGS[@]}"
else
echo "::notice::AGENT_SHIN_ENABLED is not 'true' -> reconsider stays in dry-run (no comment, no reopen)."
fi
python3 .github/scripts/triage_with_llm.py "${ARGS[@]}"

282
tests/test_litellm/test_github_triage_with_llm.py
View file

@ -298,6 +298,87 @@ class TestMainModelDefault:
assert captured["model"] == "gpt-4o-mini"
class TestWasAutoClosedByAgentShin:
"""Provenance check that gates reconsider's reopen path."""
def test_should_return_true_when_bot_comment_has_marker(
self, triage_module, monkeypatch
):
comments = [
{
"user": {"login": "github-actions[bot]"},
"body": (
"👋 Hi, thanks for the PR! I'm **Agent Shin**, the automated "
"triage bot for this repository.\n\nThis PR is being **auto-closed**..."
),
}
]
monkeypatch.setattr(
triage_module, "fetch_issue_comments", lambda repo, n: 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: [])
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.
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
)
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.
comments = [
{
"user": {"login": "codecov[bot]"},
"body": "## Codecov Report ...",
},
{
"user": {"login": "greptile-apps[bot]"},
"body": "Confidence Score: 2/5",
},
]
monkeypatch.setattr(
triage_module, "fetch_issue_comments", lambda repo, n: 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.
comments = [
{
"user": {"login": "codecov[bot]"},
"body": "## Codecov Report",
},
{
"user": {"login": "github-actions[bot]"},
"body": "I'm **Agent Shin**, the automated triage bot ...",
},
{
"user": {"login": "outside-dev"},
"body": "Replying after auto-close ...",
},
]
monkeypatch.setattr(
triage_module, "fetch_issue_comments", lambda repo, n: comments
)
assert triage_module.was_auto_closed_by_agent_shin("o/r", 1) is True
class TestCallLlmJudge:
"""call_llm_judge sets gpt-5 specific kwargs correctly."""
@ -604,6 +685,11 @@ class TestTriageOrchestration:
state="closed", body="Updated body with QA proof + screenshots."
)
monkeypatch.setattr(triage_module, "fetch_pr", lambda repo, n: pr)
# Provenance: the PR was auto-closed by Agent Shin (a bot-authored
# auto-close comment exists), so reconsider is allowed to reopen.
monkeypatch.setattr(
triage_module, "was_auto_closed_by_agent_shin", lambda repo, n: True
)
posted = {}
reopened = {}
monkeypatch.setattr(
@ -627,7 +713,7 @@ class TestTriageOrchestration:
repo="o/r",
kind="pr",
number=42,
close=False,
close=True,
model="m",
judge=lambda p: json.dumps(
{"verdict": "pass", "missing": [], "explanation": "ok now"}
@ -644,6 +730,9 @@ class TestTriageOrchestration:
):
pr = self._make_pr(state="closed", body="still empty")
monkeypatch.setattr(triage_module, "fetch_pr", lambda repo, n: pr)
monkeypatch.setattr(
triage_module, "was_auto_closed_by_agent_shin", lambda repo, n: True
)
posted = {}
monkeypatch.setattr(
triage_module,
@ -671,7 +760,7 @@ class TestTriageOrchestration:
repo="o/r",
kind="pr",
number=42,
close=False,
close=True,
model="m",
judge=lambda p: json.dumps(verdict),
reconsider=True,
@ -688,6 +777,9 @@ class TestTriageOrchestration:
# path should reopen the PR without calling the LLM.
pr = self._make_pr(state="closed", body="Fixes #1234\n\nAddresses the bug.")
monkeypatch.setattr(triage_module, "fetch_pr", lambda repo, n: pr)
monkeypatch.setattr(
triage_module, "was_auto_closed_by_agent_shin", lambda repo, n: True
)
posted = {}
reopened = {}
monkeypatch.setattr(
@ -705,7 +797,7 @@ class TestTriageOrchestration:
repo="o/r",
kind="pr",
number=55,
close=False,
close=True,
model="m",
judge=lambda p: pytest.fail("LLM must not run when linked-issue matches"),
reconsider=True,
@ -729,17 +821,194 @@ class TestTriageOrchestration:
"reopen_pr",
lambda *a, **kw: pytest.fail("must not reopen for internal author"),
)
# Internal check must fire *before* provenance, so the provenance
# helper should never be invoked for an internal author.
monkeypatch.setattr(
triage_module,
"was_auto_closed_by_agent_shin",
lambda *a, **kw: pytest.fail("must not check provenance for internal"),
)
result = triage_module.triage(
repo="o/r",
kind="pr",
number=1,
close=False,
close=True,
model="m",
judge=lambda p: pytest.fail("LLM must not run for internal author"),
reconsider=True,
)
assert result["action"] == "skip-internal-author"
def test_should_skip_reconsider_when_not_bot_closed(
self, triage_module, monkeypatch
):
# A maintainer-closed PR (no Agent Shin auto-close comment) must
# never be reopened by `@agent-shin reconsider`, regardless of how
# good the LLM verdict would be. Otherwise the original author
# could polish the description and silently override a maintainer's
# "closed as duplicate / out of scope" decision.
pr = self._make_pr(state="closed", body="Fixes #1234")
monkeypatch.setattr(triage_module, "fetch_pr", lambda repo, n: pr)
monkeypatch.setattr(
triage_module, "was_auto_closed_by_agent_shin", lambda repo, n: False
)
monkeypatch.setattr(
triage_module,
"post_comment",
lambda *a, **kw: pytest.fail("must not comment when not bot-closed"),
)
monkeypatch.setattr(
triage_module,
"reopen_pr",
lambda *a, **kw: pytest.fail("must not reopen when not bot-closed"),
)
result = triage_module.triage(
repo="o/r",
kind="pr",
number=1,
close=True,
model="m",
judge=lambda p: pytest.fail("LLM must not run when not bot-closed"),
reconsider=True,
)
assert result["action"] == "skip-not-bot-closed"
def test_should_skip_reconsider_issue_when_not_bot_closed(
self, triage_module, monkeypatch
):
# Same provenance gate for issues: only Agent Shin auto-closed
# issues are eligible for reopen-on-reconsider.
issue = {
"number": 7,
"title": "Bug",
"body": "Repro: curl ...",
"state": "closed",
"author_association": "NONE",
"user": {"login": "outside"},
}
monkeypatch.setattr(triage_module, "fetch_issue", lambda repo, n: issue)
monkeypatch.setattr(
triage_module, "was_auto_closed_by_agent_shin", lambda repo, n: False
)
monkeypatch.setattr(
triage_module,
"reopen_issue",
lambda *a, **kw: pytest.fail("must not reopen maintainer-closed issue"),
)
result = triage_module.triage(
repo="o/r",
kind="issue",
number=7,
close=True,
model="m",
judge=lambda p: pytest.fail("LLM must not run for non-bot-closed issue"),
reconsider=True,
)
assert result["action"] == "skip-not-bot-closed"
def test_should_preview_reopen_in_reconsider_dry_run(
self, triage_module, monkeypatch
):
# When `close=False` and `reconsider=True`, a passing verdict must
# produce a `would-reopen` preview WITHOUT posting a comment or
# reopening — same dry-run pattern as `would-close` in regular mode.
pr = self._make_pr(state="closed", body="Now with screenshots + repro.")
monkeypatch.setattr(triage_module, "fetch_pr", lambda repo, n: pr)
monkeypatch.setattr(
triage_module, "was_auto_closed_by_agent_shin", lambda repo, n: True
)
monkeypatch.setattr(
triage_module,
"post_comment",
lambda *a, **kw: pytest.fail("dry-run must not post"),
)
monkeypatch.setattr(
triage_module,
"reopen_pr",
lambda *a, **kw: pytest.fail("dry-run must not reopen"),
)
result = triage_module.triage(
repo="o/r",
kind="pr",
number=42,
close=False,
model="m",
judge=lambda p: json.dumps(
{"verdict": "pass", "missing": [], "explanation": "ok now"}
),
reconsider=True,
)
assert result["action"] == "would-reopen"
# The preview should include the comment body the bot WOULD post
# (useful for $GITHUB_STEP_SUMMARY).
assert "reopened" in result["comment"].lower()
def test_should_preview_reopen_in_reconsider_dry_run_linked_issue(
self, triage_module, monkeypatch
):
# The linked-issue short-circuit also has to honor dry-run in
# reconsider mode — no LLM call AND no destructive side effects.
pr = self._make_pr(state="closed", body="Fixes #1234\n\nDetails.")
monkeypatch.setattr(triage_module, "fetch_pr", lambda repo, n: pr)
monkeypatch.setattr(
triage_module, "was_auto_closed_by_agent_shin", lambda repo, n: True
)
monkeypatch.setattr(
triage_module,
"post_comment",
lambda *a, **kw: pytest.fail("dry-run must not post"),
)
monkeypatch.setattr(
triage_module,
"reopen_pr",
lambda *a, **kw: pytest.fail("dry-run must not reopen"),
)
result = triage_module.triage(
repo="o/r",
kind="pr",
number=55,
close=False,
model="m",
judge=lambda p: pytest.fail("LLM must not run when linked-issue matches"),
reconsider=True,
)
assert result["action"] == "would-reopen"
assert "reopened" in result["comment"].lower()
def test_should_preview_still_failing_in_reconsider_dry_run(
self, triage_module, monkeypatch
):
# When `close=False` and the verdict is fail, the dry-run preview
# must say `would-leave-closed-still-failing` and not post the
# "still failing" comment.
pr = self._make_pr(state="closed", body="still thin")
monkeypatch.setattr(triage_module, "fetch_pr", lambda repo, n: pr)
monkeypatch.setattr(
triage_module, "was_auto_closed_by_agent_shin", lambda repo, n: True
)
monkeypatch.setattr(
triage_module,
"post_comment",
lambda *a, **kw: pytest.fail("dry-run must not post"),
)
verdict = {
"verdict": "fail",
"missing": ["QA proof"],
"explanation": "Still no QA proof.",
}
result = triage_module.triage(
repo="o/r",
kind="pr",
number=42,
close=False,
model="m",
judge=lambda p: json.dumps(verdict),
reconsider=True,
)
assert result["action"] == "would-leave-closed-still-failing"
assert "QA proof" in result["comment"]
def test_should_reopen_issue_on_reconsider_pass(self, triage_module, monkeypatch):
issue = {
"number": 7,
@ -750,6 +1019,9 @@ class TestTriageOrchestration:
"user": {"login": "outside"},
}
monkeypatch.setattr(triage_module, "fetch_issue", lambda repo, n: issue)
monkeypatch.setattr(
triage_module, "was_auto_closed_by_agent_shin", lambda repo, n: True
)
posted = {}
reopened = {}
monkeypatch.setattr(
@ -767,7 +1039,7 @@ class TestTriageOrchestration:
repo="o/r",
kind="issue",
number=7,
close=False,
close=True,
model="m",
judge=lambda p: json.dumps(
{"verdict": "pass", "missing": [], "explanation": "now reproducible"}