agent_shin: extract shared constants/helpers; cover review_gate.yml in guardrail tests

Bug 1: `triage_with_llm.py` and `close_low_quality_prs.py` each defined
their own copies of `extract_greptile_score`, `parse_iso8601`,
`GREPTILE_BOT_LOGINS`, `SCORE_PATTERN`, `GRACE_COMMENT_MARKER`,
`GRACE_PERIOD_SECONDS`, `IMMEDIATE_CLOSE_LOGINS`, and
`AGENT_SHIN_DEFAULT_BOT_LOGIN`. The comments explicitly said the two
copies had to stay in sync, but nothing enforced it. A future change to
one (e.g. extending `SCORE_PATTERN` for a new Greptile output format)
would silently diverge from the other and the daily sweep and the LLM
judge would disagree on which PRs have low scores.

Extract these to `.github/scripts/agent_shin_shared.py` and re-export
them from each script so the existing test attribute access
(`triage_module.GRACE_COMMENT_MARKER`, etc.) keeps working without
any test changes.

Bug 2: `review_gate.yml` is a destructive workflow (close PRs, add/remove
labels, post comments) with the same gating philosophy as the others
(`AGENT_SHIN_ENABLED = "true"` + a per-run `CLOSE_FLAG = "true"`),
but it was missing from `DESTRUCTIVE_GATE_ENV` in the guardrail tests.
Add it so a future regression (e.g. flipping to `!= "false"`) is
caught by the same parameterized invariants as every other workflow.

Co-authored-by: Yassin Kortam <yassin@berri.ai>
This commit is contained in:
Cursor Agent 2026-05-25 01:37:26 +00:00
parent 125f10402f
commit 8b5913bd35
No known key found for this signature in database
4 changed files with 166 additions and 144 deletions

79
.github/scripts/agent_shin_shared.py vendored Normal file
View file

@ -0,0 +1,79 @@
"""Constants and helpers shared by Agent Shin's triage scripts.
Both `triage_with_llm.py` (the LLM-judge entrypoint) and
`close_low_quality_prs.py` (the daily Greptile-score sweep) need to
agree on the same notions of:
* What counts as a Greptile-authored review comment
(``GREPTILE_BOT_LOGINS``) and how to extract a confidence score from
its body (``SCORE_PATTERN`` / :func:`extract_greptile_score`).
* How long the 1-day grace window is (``GRACE_PERIOD_SECONDS``) and
the HTML marker stamped into a grace-warning comment so the *other*
script can see "Agent Shin already warned" and behave accordingly
(``GRACE_COMMENT_MARKER``).
* Who Agent Shin is on GitHub (``AGENT_SHIN_DEFAULT_BOT_LOGIN``) and
which login(s) bypass the grace window entirely
(``IMMEDIATE_CLOSE_LOGINS``).
* How GitHub-style ISO-8601 timestamps round-trip into timezone-aware
:class:`datetime.datetime` (:func:`parse_iso8601`).
Keeping these in one module means a future change (new Greptile output
format, a longer grace window, a new dogfood account) is a single edit
instead of two the original split version had to call out in comments
that the two copies "must stay in sync" precisely because nothing
enforced it.
"""
from __future__ import annotations
import datetime as dt
import re
from typing import Iterable
GREPTILE_BOT_LOGINS = frozenset({"greptile-apps", "greptile-apps[bot]"})
SCORE_PATTERN = re.compile(
r"confidence\s*score\s*[:\-]?\s*(\d+)\s*/\s*5",
re.IGNORECASE,
)
GRACE_COMMENT_MARKER = "<!-- agent-shin:grace-warning -->"
GRACE_PERIOD_SECONDS = 86400
AGENT_SHIN_DEFAULT_BOT_LOGIN = "github-actions[bot]"
IMMEDIATE_CLOSE_LOGINS = frozenset({"swiftwinds"})
def extract_greptile_score(comments: Iterable[dict]) -> tuple[int, dict] | None:
"""Return (score, comment) for the most recent Greptile-authored comment
that contains a "Confidence Score: X/5". Returns None if no such comment.
"Most recent" is determined by the comment's `updated_at` (falling back to
`created_at`), so re-reviews override earlier passes.
"""
candidates: list[tuple[str, int, dict]] = []
for comment in comments:
user = (comment.get("user") or {}).get("login", "")
if user not in GREPTILE_BOT_LOGINS:
continue
body = comment.get("body") or ""
match = SCORE_PATTERN.search(body)
if not match:
continue
score = int(match.group(1))
timestamp = comment.get("updated_at") or comment.get("created_at") or ""
candidates.append((timestamp, score, comment))
if not candidates:
return None
candidates.sort(key=lambda triple: triple[0])
_, score, comment = candidates[-1]
return score, comment
def parse_iso8601(value: str) -> dt.datetime:
"""Parse a GitHub ISO-8601 timestamp into a timezone-aware datetime."""
return dt.datetime.fromisoformat(value.replace("Z", "+00:00"))

View file

@ -39,24 +39,32 @@ import argparse
import datetime as dt
import json
import os
import re
import subprocess
import sys
from typing import Iterable
# Greptile's GitHub App appears as `greptile-apps[bot]` in REST API comments
# and `greptile-apps` in `gh pr view --json` output. Accept either form.
GREPTILE_BOT_LOGINS = frozenset({"greptile-apps", "greptile-apps[bot]"})
# Add this script's directory to `sys.path` so the sibling
# `agent_shin_shared` module is importable when the script is invoked
# directly (e.g. `python3 .github/scripts/close_low_quality_prs.py ...`).
sys.path.insert(0, os.path.dirname(os.path.abspath(__file__)))
# Matches lines like:
# <h3>Confidence Score: 3/5</h3>
# **Confidence Score: 4/5**
# Confidence Score: 5 / 5
SCORE_PATTERN = re.compile(
r"confidence\s*score\s*[:\-]?\s*(\d+)\s*/\s*5",
re.IGNORECASE,
from agent_shin_shared import ( # noqa: E402 -- sys.path adjusted above
AGENT_SHIN_DEFAULT_BOT_LOGIN,
GRACE_COMMENT_MARKER,
GRACE_PERIOD_SECONDS,
GREPTILE_BOT_LOGINS,
IMMEDIATE_CLOSE_LOGINS,
SCORE_PATTERN,
extract_greptile_score,
parse_iso8601,
)
# `GREPTILE_BOT_LOGINS` and `SCORE_PATTERN` (Greptile's GitHub App login
# variants and the "Confidence Score: X/5" regex) are imported from
# `agent_shin_shared` so the LLM judge in `triage_with_llm.py` and this
# daily Greptile sweep read the score through the same set of logins
# and the same regex.
# `author_association` values for internal BerriAI contributors who should be
# exempt from auto-triage.
INTERNAL_AUTHOR_ASSOCIATIONS = frozenset({"OWNER", "MEMBER", "COLLABORATOR"})
@ -67,32 +75,20 @@ 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 = "<!-- agent-shin:grace-warning -->"
# 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"})
# `GRACE_COMMENT_MARKER` (HTML marker appended to grace-period warning
# comments — used by either script to recognize that a warning was
# already posted) and `GRACE_PERIOD_SECONDS` (length of the grace
# period between the warning and the actual auto-close, 24 hours) are
# imported from `agent_shin_shared` so the Agent Shin LLM judge and
# this daily Greptile sweep agree on the same marker and duration.
#
# `AGENT_SHIN_DEFAULT_BOT_LOGIN` (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` works the same here) and
# `IMMEDIATE_CLOSE_LOGINS` (case-insensitive logins that bypass BOTH
# the grace period AND the dry-run gating; useful for dogfooding the
# bot from external test accounts) are imported too.
def gh(*args: str) -> str:
@ -186,39 +182,6 @@ def fetch_pr_comments(pr_number: int, repo: str | None) -> list[dict]:
return comments
def extract_greptile_score(comments: Iterable[dict]) -> tuple[int, dict] | None:
"""Return (score, comment) for the most recent Greptile-authored comment
that contains a "Confidence Score: X/5". Returns None if no such comment.
"Most recent" is determined by the comment's `updated_at` (falling back to
`created_at`), so re-reviews override earlier passes.
"""
candidates: list[tuple[str, int, dict]] = []
for comment in comments:
user = (comment.get("user") or {}).get("login", "")
if user not in GREPTILE_BOT_LOGINS:
continue
body = comment.get("body") or ""
match = SCORE_PATTERN.search(body)
if not match:
continue
score = int(match.group(1))
timestamp = comment.get("updated_at") or comment.get("created_at") or ""
candidates.append((timestamp, score, comment))
if not candidates:
return None
candidates.sort(key=lambda triple: triple[0])
_, score, comment = candidates[-1]
return score, comment
def parse_iso8601(value: str) -> dt.datetime:
"""Parse a GitHub ISO-8601 timestamp into a timezone-aware datetime."""
return dt.datetime.fromisoformat(value.replace("Z", "+00:00"))
def has_optout_label(pr: dict, optout_labels: set[str]) -> bool:
labels = {label.get("name", "").lower() for label in pr.get("labels", [])}
return bool(labels & {lbl.lower() for lbl in optout_labels})

View file

@ -40,15 +40,33 @@ import textwrap
import urllib.parse
from typing import Any, Iterable
# Add this script's directory to `sys.path` so the sibling
# `agent_shin_shared` module is importable when the script is invoked
# directly (e.g. `python3 .github/scripts/triage_with_llm.py ...`) and
# also when the tests load this script via
# `importlib.util.spec_from_file_location`.
sys.path.insert(0, os.path.dirname(os.path.abspath(__file__)))
from agent_shin_shared import ( # noqa: E402 -- sys.path adjusted above
AGENT_SHIN_DEFAULT_BOT_LOGIN,
GRACE_COMMENT_MARKER,
GRACE_PERIOD_SECONDS,
GREPTILE_BOT_LOGINS,
IMMEDIATE_CLOSE_LOGINS,
SCORE_PATTERN,
extract_greptile_score,
parse_iso8601,
)
DEFAULT_MODEL = "gpt-5.4-mini"
INTERNAL_ASSOCIATIONS = frozenset({"OWNER", "MEMBER", "COLLABORATOR"})
# Login of the account that performs Agent Shin's GitHub writes. When the
# workflow uses `secrets.GITHUB_TOKEN` (our default), the closure / reopen
# event's `actor.login` is `github-actions[bot]`. The env override exists
# for local debugging and for repos that wire Agent Shin to a PAT.
AGENT_SHIN_DEFAULT_BOT_LOGIN = "github-actions[bot]"
# `AGENT_SHIN_DEFAULT_BOT_LOGIN` is imported from `agent_shin_shared`.
# When the workflow uses the default `secrets.GITHUB_TOKEN`, the
# closure / reopen event's `actor.login` is `github-actions[bot]`. The
# env override `AGENT_SHIN_BOT_LOGIN` exists for local debugging and for
# repos that wire Agent Shin to a PAT.
# HTML marker appended to every reconsider verdict comment. We grep for this
# on subsequent reconsider triggers to enforce a short cooldown so that
@ -63,26 +81,20 @@ RECONSIDER_COMMENT_MARKER = "<!-- agent-shin:reconsider-verdict -->"
# 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 = "<!-- agent-shin:grace-warning -->"
# 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"})
# `GRACE_COMMENT_MARKER` (HTML marker on the grace-period warning comment
# posted on the first low-quality detection — used on subsequent triage
# runs to detect that a warning was already posted and measure how long
# ago it was posted) and `GRACE_PERIOD_SECONDS` (length of the grace
# period between the warning and the actual auto-close, 24 hours) are
# imported from `agent_shin_shared` so the daily Greptile sweep and the
# LLM judge agree on the same marker and duration.
#
# `IMMEDIATE_CLOSE_LOGINS` (case-insensitive logins that bypass BOTH the
# 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) is shared too.
# Useful for dogfooding the bot from an external account that has no
# push permissions to the repo. Callers compare via `login.lower() in ...`.
# --- Review-gate ("ready for review" label lifecycle) configuration ----------
# The review gate keeps a single label in sync with whether a PR currently
@ -101,21 +113,12 @@ READY_MARKER = "<!-- agent-shin:ready -->"
REGRESSED_MARKER = "<!-- agent-shin:regressed -->"
WITHIN_GRACE_MARKER = "<!-- agent-shin:within-grace -->"
# Greptile's GitHub App appears as `greptile-apps[bot]` in REST API comments
# and `greptile-apps` in `gh pr view --json` output. Accept either form.
# Mirrors the constant of the same name in `close_low_quality_prs.py`; keep
# them in sync so the daily sweep and the review gate read the score through
# the same set of logins.
GREPTILE_BOT_LOGINS = frozenset({"greptile-apps", "greptile-apps[bot]"})
# Matches lines like:
# <h3>Confidence Score: 3/5</h3>
# **Confidence Score: 4/5**
# Confidence Score: 5 / 5
SCORE_PATTERN = re.compile(
r"confidence\s*score\s*[:\-]?\s*(\d+)\s*/\s*5",
re.IGNORECASE,
)
# `GREPTILE_BOT_LOGINS` (Greptile's GitHub App login variants —
# `greptile-apps[bot]` in REST API comments, `greptile-apps` in
# `gh pr view --json` output) and `SCORE_PATTERN` (regex matching lines
# like `Confidence Score: 3/5`) are imported from `agent_shin_shared`
# so the daily sweep and the review gate read the score through the
# same set of logins / patterns.
# Marker phrase Agent Shin always includes in its auto-close comments
# (see `format_pr_close_comment` / `format_issue_close_comment`). The
@ -427,40 +430,11 @@ def is_internal_contributor(item: dict) -> bool:
# ---------------------------------------------------------------------------
# Greptile score + age helpers (mirrored from close_low_quality_prs.py)
def extract_greptile_score(comments: Iterable[dict]) -> tuple[int, dict] | None:
"""Return (score, comment) for the most recent Greptile-authored comment
that contains a "Confidence Score: X/5". Returns None if no such comment.
"Most recent" is determined by the comment's `updated_at` (falling back to
`created_at`), so re-reviews override earlier passes.
"""
candidates: list[tuple[str, int, dict]] = []
for comment in comments:
user = (comment.get("user") or {}).get("login", "")
if user not in GREPTILE_BOT_LOGINS:
continue
body = comment.get("body") or ""
match = SCORE_PATTERN.search(body)
if not match:
continue
score = int(match.group(1))
timestamp = comment.get("updated_at") or comment.get("created_at") or ""
candidates.append((timestamp, score, comment))
if not candidates:
return None
candidates.sort(key=lambda triple: triple[0])
_, score, comment = candidates[-1]
return score, comment
def parse_iso8601(value: str) -> dt.datetime:
"""Parse a GitHub ISO-8601 timestamp into a timezone-aware datetime."""
return dt.datetime.fromisoformat(value.replace("Z", "+00:00"))
# Greptile score + age helpers (`extract_greptile_score`, `parse_iso8601`)
# live in `agent_shin_shared` — they're imported at the top of this module
# so both `triage_with_llm.py` and `close_low_quality_prs.py` share a
# single source of truth for the Confidence-Score regex and ISO-8601
# parsing.
# ---------------------------------------------------------------------------

View file

@ -47,6 +47,12 @@ DESTRUCTIVE_GATE_ENV: dict[str, str] = {
# only kill switch is `AGENT_SHIN_ENABLED`, which already serves as
# both the destructive gate and the global enablement gate.
"triage_reconsider.yml": "AGENT_SHIN_ENABLED",
# The review gate can add/remove labels, post comments, and close PRs.
# Its per-run knob is `CLOSE_FLAG` (from the workflow_dispatch input),
# gated by an outer `AGENT_SHIN_ENABLED = "true"` check. Listing it
# here ensures the same fail-safe `= "true"` and kill-switch invariants
# we enforce on every other destructive workflow are enforced here too.
"review_gate.yml": "CLOSE_FLAG",
}