Only reopen on an explicit 'pass' verdict. Previously any non-'fail'
value (including missing/empty/unknown verdicts) would trigger a
reopen, undoing a prior auto-close based on an ambiguous response.
Co-authored-by: Yassin Kortam <yassin@berri.ai>
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.
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>
Follow-up to PR #28117. Three behavior changes + one new workflow,
addressing the team's concerns on the original review:
1) Apply auto-close to ALL open PRs, not just those over a week old.
- close_low_quality_prs.py: --min-age-days default flipped from 7 to
0. The flag is preserved as an opt-in safety net for one-off
backfill runs that want to spare very-young PRs, but the daily
scheduled sweep now closes external-author PRs as soon as Greptile
scores them <4/5.
- close_low_quality_prs.yml: workflow_dispatch input default also
flipped to 0; doc comments updated.
2) Apply auto-close to draft PRs too.
- close_low_quality_prs.py: removed the skip-draft branch in
evaluate_pr. Drafts are NOT a free pass — the team's intent is
'open PR count == PRs internal collaborators need to action on',
so a draft Greptile scored 2/5 still belongs in the closed bucket.
Authors who genuinely need a long-lived draft can attach the 'wip'
opt-out label, which is unchanged.
- The 'skip-draft' action is gone; the 'wip' label still skips.
3) Address the 'OSS contributors cannot reopen a bot-closed PR' wrinkle.
GitHub does NOT let an external (non-write-access) contributor
reopen a PR that was closed by a bot or maintainer (long-standing
limitation). The original PR's close-comments told contributors to
'Reopen the PR — I'll re-evaluate automatically', which is broken
for the very audience this triage targets. Two changes:
a) Reword every close-comment (Greptile sweep + Agent Shin PR
close + Agent Shin issue close + PR template) to recommend:
- Open a new PR with the updated branch (primary path).
- Or comment '@agent-shin reconsider' on the closed PR for a
re-evaluation that, on pass, reopens the PR via the bot's
GH_TOKEN write access.
b) Add the @agent-shin reconsider workflow:
- .github/workflows/triage_reconsider.yml: new
'issue_comment'-triggered workflow. Authorizes only the
PR/issue author or an internal collaborator
(OWNER/MEMBER/COLLABORATOR), gated via a step output so
unauthorized commenters never reach the destructive steps.
Globally gated on AGENT_SHIN_ENABLED='true' (positive form,
matching the test_github_triage_workflows guardrail
patterns).
- triage_with_llm.py: --reconsider mode. On a closed PR/issue,
re-runs the LLM judge (or linked-issue regex short-circuit)
and:
- on pass: reopens via reopen_pr/reopen_issue + posts a
'Re-evaluated and reopened' comment.
- on fail: leaves closed and posts a 'still missing X'
comment so the contributor can iterate again.
Reconsider-on-open is a no-op ('skip-not-closed').
Internal-author + bot-account skips still take priority over
reconsider.
4) Greptile-on-closed-PRs question: the team asked whether Greptile can
re-review a closed PR. Greptile's docs don't address this and we
shouldn't promise behavior we can't verify, so the new close-comment
wording does NOT instruct contributors to 're-request greptile on
the closed PR'. Instead it points them at the new-PR path (which
Greptile definitely reviews) or the @agent-shin reconsider trigger
(which re-runs the LiteLLM-side rubric judge, not Greptile).
Tests: 93 passing (was 59).
- test_github_close_low_quality_prs.py: replaced 'skip drafts' test
with 'closes drafts when score is low' + 'closes brand-new PR when
min_age=0' + 'no skip when min_age=0'. The 'skip too young'
assertion is preserved as opt-in.
- test_github_triage_with_llm.py: 6 new TestTriageOrchestration cases
for reconsider mode (skip-not-closed on open, reopen on pass,
still-failing comment on fail, linked-issue short-circuit reopen,
skip internal author in reconsider, reopen-issue on pass) + a new
TestCloseCommentText class that pins the user-facing 'open a new
PR' + '@agent-shin reconsider' wording.
- test_github_triage_workflows.py: added triage_reconsider.yml to
the destructive-gate guardrail table; AGENT_SHIN_ENABLED is its
own destructive gate (no separate per-run flag needed).
Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>
Three independent bugbot findings against triage_with_llm.py:
1. LINKED_ISSUE_PATTERN included weak keywords (`see`, `ref`,
`addresses`) so casual mentions like "See #1234 for context" were
short-circuited to pass-linked-issue without ever calling the LLM —
contradicting the prompt's own "a bare issue number without a closing
keyword counts only if it's clearly the related issue (not a passing
mention)" rubric. Limit the regex to GitHub's documented PR-closing
keywords (fixes/fix/fixed/closes/close/closed/resolves/resolve/resolved).
2. is_internal_contributor() treated an empty/missing author_association
as external (eligible for the destructive close path), while the sibling
is_external_pr_author() in close_low_quality_prs.py fail-safes the same
case as internal. Align the two so a partial/unknown GitHub response can
never make a PR eligible for auto-close.
3. argparse `default=os.environ.get("TRIAGE_MODEL", DEFAULT_MODEL)` returns
the empty string when GitHub Actions exposes an unset repo variable as
an empty-string env var (the optional vars.TRIAGE_MODEL case in the
workflow). Use `os.environ.get(...) or DEFAULT_MODEL` so empty -> default,
matching the existing OPENAI_BASE_URL pattern.
Tests:
- Casual mentions now must fall through to the LLM (parametrized);
added an orchestration test ensuring "See #1234" reaches the judge.
- Empty/missing author_association now fails safe (parametrized).
- Empty TRIAGE_MODEL env var falls back to DEFAULT_MODEL; explicit
TRIAGE_MODEL is still honored.
Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>
- Bump DEFAULT_MODEL from gpt-4o-mini to gpt-5.4-mini (more modern;
4M total context window per OpenAI catalog, JSON-schema response
format, function calling all supported).
- For gpt-5.x family models, pass reasoning_effort="none" via
extra_body. gpt-5.x rejects temperature != 1 unless reasoning_effort
is explicitly "none"; setting it lets us keep temperature=0 for
deterministic JSON rubric judgments. extra_body works across openai
SDK versions regardless of whether they natively type the kwarg.
- For non-gpt5 overrides (TRIAGE_MODEL=gpt-4o-mini etc.), reasoning_effort
is not sent.
- 4 new unit tests cover: gpt-5.4-mini -> reasoning_effort=none,
capitalized/dated gpt-5 variants -> reasoning_effort=none,
gpt-4o-mini -> no extra_body, base_url passthrough.
Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>
- close_low_quality_prs.py: Treat author_association API lookup failures
as internal (fail-safe) so transient errors don't cause internal
contributors' PRs to be auto-closed.
- triage_with_llm.py: Update summary heading from 'Would post comment:'
to 'Posted comment:' since this branch only runs after the comment
has already been posted.
Co-authored-by: Yassin Kortam <yassin@berri.ai>
- close_low_quality_prs.yml: only workflow_dispatch with close=true (and
AGENT_SHIN_ENABLED=true) actually closes PRs. Scheduled runs are always
dry-run, matching the safety invariant documented for triage_pr/issue.
- triage_with_llm.py: textwrap.dedent on an f-string with multi-line
interpolated bodies fails because the body's 2nd+ lines start at column 0,
making the common-indent zero. Dedent the static template first, then
.format() the title/body in.
Co-authored-by: Yassin Kortam <yassin@berri.ai>
Adds a new triage flow that evaluates external pull requests and issues
against the project's contribution rubric and, when configured to do so,
auto-closes non-conforming ones with an explanatory comment. Contributors
can update + reopen to be re-evaluated.
Scope:
- Internal BerriAI contributors (author_association OWNER/MEMBER/COLLABORATOR)
and bot accounts are skipped entirely.
- 'Fixes #1234' / 'Resolves https://github.com/.../issues/N' in the PR body
short-circuits to PASS without burning LLM tokens.
- LLM judge returns structured JSON (verdict, missing[], explanation);
parser tolerates markdown fences and embedded JSON.
- LLM errors NEVER close PRs/issues — failure surfaces as 'skip-llm-error'.
Safety:
- pull_request_target / issues triggers are FORCED dry-run in the workflow;
only manual workflow_dispatch with close=true (and AGENT_SHIN_ENABLED=true)
takes destructive action.
- Default mode writes verdicts to GITHUB_STEP_SUMMARY only — no public
comments until the team flips the AGENT_SHIN_ENABLED repo variable.
- LLM uses an OpenAI-compatible endpoint (model and base URL configurable
via repo variables; key via OPENAI_API_KEY secret).
Files:
- .github/scripts/triage_with_llm.py - judge orchestrator + CLI
- .github/workflows/triage_pr_with_llm.yml
- .github/workflows/triage_issue_with_llm.yml
- tests/test_litellm/test_github_triage_with_llm.py - 33 unit tests
End-to-end validated against four real PRs (#28117 internal collaborator,
#28108 bot, #28129 'Fixes #28128', #28116 no linked issue) and issue
#28132 with a stubbed LLM judge: each path produces the expected action.
Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>
