From 8b5913bd35686e476b12e94f2eca02eacc6b0624 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 25 May 2026 01:37:26 +0000 Subject: [PATCH] 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 --- .github/scripts/agent_shin_shared.py | 79 ++++++++++++ .github/scripts/close_low_quality_prs.py | 103 +++++---------- .github/scripts/triage_with_llm.py | 122 +++++++----------- .../test_github_triage_workflows.py | 6 + 4 files changed, 166 insertions(+), 144 deletions(-) create mode 100644 .github/scripts/agent_shin_shared.py diff --git a/.github/scripts/agent_shin_shared.py b/.github/scripts/agent_shin_shared.py new file mode 100644 index 00000000000..6b763a2e20b --- /dev/null +++ b/.github/scripts/agent_shin_shared.py @@ -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 = "" + +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")) diff --git a/.github/scripts/close_low_quality_prs.py b/.github/scripts/close_low_quality_prs.py index 746807f4a58..5e9832ccd85 100644 --- a/.github/scripts/close_low_quality_prs.py +++ b/.github/scripts/close_low_quality_prs.py @@ -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: -#

Confidence Score: 3/5

-# **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 = "" - -# 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}) diff --git a/.github/scripts/triage_with_llm.py b/.github/scripts/triage_with_llm.py index 243d2f9144d..a47b5c782a1 100644 --- a/.github/scripts/triage_with_llm.py +++ b/.github/scripts/triage_with_llm.py @@ -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 = "" # 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 = "" - -# 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 = "" REGRESSED_MARKER = "" WITHIN_GRACE_MARKER = "" -# 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: -#

Confidence Score: 3/5

-# **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. # --------------------------------------------------------------------------- diff --git a/tests/test_litellm/test_github_triage_workflows.py b/tests/test_litellm/test_github_triage_workflows.py index da2743da585..ac316b1988a 100644 --- a/tests/test_litellm/test_github_triage_workflows.py +++ b/tests/test_litellm/test_github_triage_workflows.py @@ -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", }