diff --git a/.github/scripts/triage_with_llm.py b/.github/scripts/triage_with_llm.py index d2536058e01..6643e965869 100644 --- a/.github/scripts/triage_with_llm.py +++ b/.github/scripts/triage_with_llm.py @@ -24,7 +24,8 @@ Environment: GH_TOKEN / GITHUB_TOKEN - for `gh` CLI auth (auto-set in Actions) OPENAI_API_KEY - required when --close is passed OPENAI_BASE_URL - optional (route to any OpenAI-compatible API) - TRIAGE_MODEL - optional model override (default: gpt-5.4-mini) + TRIAGE_MODEL - optional model override (default: gpt-5.6-luna) + AGENT_SHIN_POLICY_URL - optional blog-post URL linked from lite-mode notices """ from __future__ import annotations @@ -61,7 +62,7 @@ from agent_shin_shared import ( # noqa: E402 -- sys.path adjusted above seconds_since_latest_marker_comment, ) -DEFAULT_MODEL = "gpt-5.4-mini" +DEFAULT_MODEL = "gpt-5.6-luna" INTERNAL_ASSOCIATIONS = frozenset({"OWNER", "MEMBER", "COLLABORATOR"}) @@ -97,8 +98,10 @@ RECONSIDER_RATE_LIMIT_SECONDS = 600 # clears BOTH quality bars: the LLM rubric (clear problem + expected/actual + # QA proof, or a linked issue) AND Greptile's most recent confidence score. READY_FOR_REVIEW_LABEL = "ready for review" +NOT_READY_LABEL = "not ready" DEFAULT_GRACE_DAYS = 1 # 24h before an un-passing, un-tagged PR is auto-closed DEFAULT_MIN_GREPTILE_SCORE = 4 # Greptile < 4/5 counts as "not passing" +LITE_NOTICE_DAYS = 7 # Hidden HTML-comment markers stamped into review-gate comments. They never # render in the GitHub UI but let the gate detect its own prior actions so it @@ -108,6 +111,7 @@ DEFAULT_MIN_GREPTILE_SCORE = 4 # Greptile < 4/5 counts as "not passing" READY_MARKER = "" REGRESSED_MARKER = "" WITHIN_GRACE_MARKER = "" +LITE_NOTICE_MARKER = "" # `GREPTILE_BOT_LOGINS` (Greptile's GitHub App login variants — # `greptile-apps[bot]` in REST API comments, `greptile-apps` in @@ -513,7 +517,18 @@ def build_pr_prompt(*, title: str, body: str) -> str: output, demonstrating the change works end-to-end against the real system. Commands whose external dependencies (LLM provider, DB, network) are mocked or stubbed do NOT - satisfy (2c); they are not end-to-end. + satisfy (2c); they are not end-to-end. When the commands + come from a custom script, the script's source must be + visible — pasted in the body (a collapsible section is + fine) or in a linked gist. Output from a script whose + source is not shown does NOT satisfy (2c). + + For a BUG FIX the proof (whichever form) must cover BOTH + sides: evidence of the failure BEFORE the change and + evidence it works AFTER. An after-only demonstration is + acceptable only for a brand-new feature, where no + meaningful "before" exists — there the proof must show the + feature working end-to-end. `has_qa_proof` must be set to `true` only when (2a), (2b), or a non-mocked (2c) is actually present in the body. If the @@ -1028,6 +1043,44 @@ def format_reconsider_still_failing_comment(kind: str, verdict: dict) -> str: ) +def format_reconsider_needs_greptile_comment( + score: int | None, min_score: int +) -> str: + """Posted when a closed PR's reconsider is blocked by the Greptile bar. + + Reopening a closed PR requires a fresh Greptile confidence score of at + least ``min_score``/5 in addition to the LLM rubric, so this comment + walks the contributor through the exact recovery loop. Carries + ``RECONSIDER_COMMENT_MARKER`` so repeated triggers stay rate-limited. + """ + current = ( + f"Greptile's most recent review scored this PR **{score}/5**" + if score is not None + else "This PR has no Greptile review with a confidence score yet" + ) + return ( + "⏸️ **Not reopening yet — a passing Greptile review is required first.**\n" + "\n" + f"{current}; reopening a closed PR requires at least " + f"**{min_score}/5**.\n" + "\n" + "To get this PR reopened:\n" + "\n" + "1. Push your fixes, then comment `@greptileai` for a re-review, " + f"until the Confidence Score is at least {min_score}/5.\n" + "2. Add end-to-end QA evidence to the PR description: for a bug " + "fix, evidence it was broken before AND works now; for a feature, " + "evidence of it working end-to-end. A video or screenshots are " + "ideal; otherwise paste the exact commands with their real output " + "(real providers, no mocks — `pytest` runs don't count). If you " + "used a script, include its source in the description (a " + "collapsible section is fine) or a linked gist.\n" + "3. Comment `@agent-shin reconsider` again.\n" + "\n" + f"{RECONSIDER_COMMENT_MARKER}" + ) + + # --------------------------------------------------------------------------- # Review gate — "ready for review" label lifecycle @@ -1156,6 +1209,43 @@ def format_regression_comment( ) +def format_lite_notice_comment( + missing: list[str], explanation: str, policy_url: str | None +) -> str: + """Posted at most once per PR while the gate runs in lite (notice-only) mode. + + Lite mode never closes: this is the launch-week heads-up that auto-closes + start in ``LITE_NOTICE_DAYS``, with a pointer to the policy blog post when + ``policy_url`` is configured. Deduped via ``LITE_NOTICE_MARKER``. + """ + policy_paragraph = ( + f"This is part of a policy change explained in " + f"[this post]({policy_url}) — it links a GitHub discussion where " + "you can share feedback.\n" + "\n" + if policy_url + else "" + ) + return ( + "🚅 Hi, thanks for the PR! This is **Agent Shin**, the automated " + "triage bot. One-time heads-up: this PR doesn't currently meet the " + "contribution bar:\n" + "\n" + f"{_format_missing(missing)}\n" + "\n" + f"> {explanation}\n" + "\n" + f"Nothing is being closed today, but starting in {LITE_NOTICE_DAYS} " + "days PRs that don't meet the bar will be auto-closed. Update the " + "description with the missing pieces and I'll re-check on the next " + "sweep and tag the PR `ready for review` once it passes — or comment " + "`@agent-shin reconsider` after updating for an immediate re-check.\n" + "\n" + f"{policy_paragraph}" + f"{LITE_NOTICE_MARKER}" + ) + + def format_within_grace_comment( missing: list[str], explanation: str, grace_days: int ) -> str: @@ -1191,16 +1281,19 @@ def review_gate( grace_days: int = DEFAULT_GRACE_DAYS, min_greptile_score: int = DEFAULT_MIN_GREPTILE_SCORE, label: str = READY_FOR_REVIEW_LABEL, + not_ready_label: str = NOT_READY_LABEL, + notice_only: bool = False, + policy_url: str | None = None, allowlist: frozenset[str] = ALLOWLIST_LOGINS, ) -> dict: - """Reconcile the `ready for review` label with a PR's current quality. + """Reconcile the `ready for review` / `not ready` labels with a PR's quality. A PR is *passing* when it clears BOTH gates: the LLM rubric (linked issue, or problem description + expected/actual + QA proof) AND Greptile's most recent confidence score (>= ``min_greptile_score``; absence of a score is not held against the PR). The gate then drives a small state machine, using - the label itself as the persisted state so comments fire only on - transitions (never on every scheduled run): + the `ready for review` label itself as the persisted state so comments fire + only on transitions (never on every scheduled run): passing, untagged -> add label + "ready for review" / "all clear" passing, tagged -> noop-passing @@ -1208,6 +1301,15 @@ def review_gate( not passing, untagged, old -> close + comment (past the grace window) not passing, untagged, new -> one-time "what's missing" notice (within grace) + The red `not ready` label mirrors the green one: it is added whenever the + PR is not passing and removed whenever it passes, so the pair always shows + the current verdict at a glance. + + ``notice_only`` (lite mode) disables the close path entirely: an untagged, + not-passing PR instead gets a one-time "closes start in 7 days" notice + linking ``policy_url``, and regression comments quote the lite window + rather than ``grace_days``. + ``close`` gates every destructive side effect: with ``close=False`` the function returns a ``would-*`` preview and touches nothing, mirroring the dry-run contract of :func:`triage`. ``judge``/``greptile_score``/ @@ -1315,6 +1417,13 @@ def review_gate( "age_days": age_days, } + not_ready_present = not_ready_label.lower() in labels_now + if close: + if passing and not_ready_present: + remove_label(repo, number, not_ready_label) + elif not passing and not not_ready_present: + add_label(repo, number, not_ready_label) + if passing: if label_present: return {**base_result, "action": "noop-passing"} @@ -1335,13 +1444,23 @@ def review_gate( missing = _combine_missing(verdict, greptile_score, min_greptile_score) if label_present: - comment = format_regression_comment(missing, explanation, grace_days) + window_days = LITE_NOTICE_DAYS if notice_only else grace_days + comment = format_regression_comment(missing, explanation, window_days) if not close: return {**base_result, "action": "would-remove-label", "comment": comment} remove_label(repo, number, label) post_comment(repo, number, comment) return {**base_result, "action": "label-removed-regressed", "comment": comment} + if notice_only: + if _has_marker(comments, LITE_NOTICE_MARKER): + return {**base_result, "action": "lite-already-notified"} + comment = format_lite_notice_comment(missing, explanation, policy_url) + if not close: + return {**base_result, "action": "would-notify-lite", "comment": comment} + post_comment(repo, number, comment) + return {**base_result, "action": "lite-notified", "comment": comment} + # Not passing and not tagged. If the PR was previously tagged and then # regressed (we removed the label and posted REGRESSED_MARKER), honor the # "PR stays open — fix it and the tag comes back" promise from @@ -1393,6 +1512,8 @@ def triage( judge: Any = None, print_prompt: bool = False, reconsider: bool = False, + min_greptile_score: int = DEFAULT_MIN_GREPTILE_SCORE, + comments: Any = _UNSET, allowlist: frozenset[str] = ALLOWLIST_LOGINS, ) -> dict: """Triage a single PR or issue. Returns a result dict for logging/tests. @@ -1420,6 +1541,17 @@ def triage( verdict on this PR/issue within `RECONSIDER_RATE_LIMIT_SECONDS`, skip — repeated triggers from the same contributor shouldn't burn CI minutes or LLM budget. + + Reopening a closed PR is held to a stricter bar than regular triage: + the PR must carry a Greptile confidence score of at least + ``min_greptile_score`` (no score at all fails — the contributor is told + to comment `@greptileai`), and the linked-issue short-circuit is + bypassed so the LLM rubric's end-to-end QA-evidence requirement always + applies. On a reconsider pass the PR is reopened AND tagged + `ready for review` (the `not ready` label is removed), since the pass + is by construction the same verdict the review gate would reach. + ``comments`` is injectable for tests; in production the live comment + list is fetched for the Greptile-score check. """ fetcher = {"pr": fetch_pr, "issue": fetch_issue}[kind] item = fetcher(repo, number) @@ -1470,10 +1602,37 @@ def triage( "rate_limit_window_seconds": RECONSIDER_RATE_LIMIT_SECONDS, } + if reconsider and kind == "pr": + if comments is _UNSET: + comments = list( + _iter_paginated_json(f"repos/{repo}/issues/{number}/comments") + ) + extraction = extract_greptile_score(comments) + greptile_score = extraction[0] if extraction else None + base_result = {**base_result, "greptile_score": greptile_score} + if greptile_score is None or greptile_score < min_greptile_score: + comment = format_reconsider_needs_greptile_comment( + greptile_score, min_greptile_score + ) + if not close: + return { + **base_result, + "action": "would-reconsider-needs-greptile", + "comment": comment, + } + post_comment(repo, number, comment) + return { + **base_result, + "action": "reconsider-needs-greptile", + "comment": comment, + } + if kind == "pr": # Short-circuit: if body very clearly links a related issue, just pass. - if has_linked_issue(body): - base = { + # Never in reconsider mode — reopening always requires the LLM rubric's + # QA-evidence check, which a linked issue alone does not satisfy. + if has_linked_issue(body) and not reconsider: + return { **base_result, "action": "pass-linked-issue", "verdict": { @@ -1482,23 +1641,6 @@ def triage( "explanation": "Linked-issue regex matched; LLM was not called.", }, } - 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, - } - post_comment(repo, number, reopen_body) - reopen_pr(repo, number) - return { - **base, - "action": "reopened", - "comment": reopen_body, - } - return base prompt = build_pr_prompt(title=title, body=body) else: prompt = build_issue_prompt(title=title, body=body) @@ -1551,6 +1693,8 @@ def triage( post_comment(repo, number, reopen_body) if kind == "pr": reopen_pr(repo, number) + add_label(repo, number, READY_FOR_REVIEW_LABEL) + remove_label(repo, number, NOT_READY_LABEL) else: reopen_issue(repo, number) return { @@ -1733,8 +1877,26 @@ def main() -> int: default=DEFAULT_MIN_GREPTILE_SCORE, choices=range(1, 6), help=( - "Review-gate only: Greptile score below which a PR counts as not " - f"passing (default: {DEFAULT_MIN_GREPTILE_SCORE} -> <4/5 regresses)." + "Greptile score below which a PR counts as not passing, for both " + "the review gate and PR reconsiders " + f"(default: {DEFAULT_MIN_GREPTILE_SCORE} -> <4/5 fails)." + ), + ) + parser.add_argument( + "--notice-only", + action="store_true", + help=( + "Review-gate only (lite mode): never close; post a one-time " + f'"closes start in {LITE_NOTICE_DAYS} days" notice on failing, ' + "untagged PRs instead. Labels still reconcile." + ), + ) + parser.add_argument( + "--policy-url", + default=os.environ.get("AGENT_SHIN_POLICY_URL") or None, + help=( + "Blog-post URL linked from the lite-mode notice " + "(default: $AGENT_SHIN_POLICY_URL)." ), ) args = parser.parse_args() @@ -1752,6 +1914,8 @@ def main() -> int: model=args.model, grace_days=args.grace_days, min_greptile_score=args.min_greptile_score, + notice_only=args.notice_only, + policy_url=args.policy_url, ) else: result = triage( @@ -1762,6 +1926,7 @@ def main() -> int: model=args.model, print_prompt=args.print_prompt, reconsider=args.reconsider, + min_greptile_score=args.min_greptile_score, ) if result.get("action") == "print-prompt": diff --git a/.github/workflows/review_gate.yml b/.github/workflows/review_gate.yml index 46284289652..80c10154cce 100644 --- a/.github/workflows/review_gate.yml +++ b/.github/workflows/review_gate.yml @@ -1,13 +1,20 @@ name: Agent Shin — review gate -# Keeps the `ready for review` label in sync with whether an external PR -# currently clears BOTH the LLM rubric AND Greptile's confidence score. +# Keeps the `ready for review` / `not ready` label pair in sync with whether +# an external PR currently clears BOTH the LLM rubric AND Greptile's +# confidence score. # -# pass -> add `ready for review` + a "passed / all clear" comment -# regress -> remove the label + a "what's missing" comment (PR stays open) +# pass -> swap to `ready for review` + a "passed / all clear" comment +# regress -> swap to `not ready` + a "what's missing" comment (PR stays open) # fail, <24h old -> a one-time "what's missing" notice (grace window) # fail, >24h old -> close + a comment (reopen via `@agent-shin reconsider`) # +# LITE MODE: while the repo variable `AGENT_SHIN_MODE` is the exact string +# "lite", the close path is replaced by a one-time "closes start in 7 days" +# notice linking the policy blog post (`AGENT_SHIN_POLICY_URL`). Labels still +# reconcile. Unset the variable (or set anything else) for the full +# experience. +# # DRY-RUN BY DEFAULT. Every side effect (label add/remove, comment, close) is # gated behind `--close`, which is only added when the repo variable # `AGENT_SHIN_ENABLED == "true"`. Until then runs only write the verdict to the @@ -85,6 +92,8 @@ jobs: OPENAI_BASE_URL: ${{ vars.OPENAI_BASE_URL }} TRIAGE_MODEL: ${{ vars.TRIAGE_MODEL }} AGENT_SHIN_ENABLED: ${{ vars.AGENT_SHIN_ENABLED }} + AGENT_SHIN_MODE: ${{ vars.AGENT_SHIN_MODE }} + AGENT_SHIN_POLICY_URL: ${{ vars.AGENT_SHIN_POLICY_URL }} CLOSE_FLAG: ${{ github.event.inputs.close || 'false' }} GRACE_DAYS: ${{ github.event.inputs.grace_days || '1' }} MIN_GREPTILE_SCORE: ${{ github.event.inputs.min_greptile_score || '4' }} @@ -92,6 +101,10 @@ jobs: run: | set -euo pipefail COMMON=(--review-gate --grace-days "${GRACE_DAYS}" --min-greptile-score "${MIN_GREPTILE_SCORE}") + if [ "${AGENT_SHIN_MODE:-}" = "lite" ]; then + COMMON+=(--notice-only) + echo "::notice::AGENT_SHIN_MODE=lite -> notice-only (no closes; one-time 7-day notices)." + fi # Fail-safe gating, identical philosophy to the Greptile closer: # - AGENT_SHIN_ENABLED must be the EXACT string "true" to act at all. diff --git a/.github/workflows/triage_reconsider.yml b/.github/workflows/triage_reconsider.yml index f35f681d09a..48b53286b8b 100644 --- a/.github/workflows/triage_reconsider.yml +++ b/.github/workflows/triage_reconsider.yml @@ -1,13 +1,19 @@ name: Agent Shin — reconsider # Comment-trigger workflow: when the PR/issue author (or an internal -# collaborator) comments `@agent-shin reconsider` on a CLOSED PR/issue, -# Agent Shin re-runs LLM-judge triage on the current title+body and: +# collaborator) comments `@agent-shin reconsider`, Agent Shin re-evaluates: # -# - on PASS: posts a "re-evaluated and reopened" comment + reopens. +# CLOSED PR/issue — re-runs LLM-judge triage on the current title+body: +# - on PASS: posts a "re-evaluated and reopened" comment + reopens (for +# PRs this additionally requires a Greptile confidence score +# of at least 4/5, and swaps the label to `ready for review`). # - on FAIL: posts a "still missing X" comment and leaves it closed, # so the contributor can iterate again. # +# OPEN PR — runs the review gate for that PR instead, so a contributor who +# just fixed their description gets the `ready for review` / `not ready` +# label pair flipped immediately rather than waiting for the daily sweep. +# # This exists because GitHub does NOT let an external (non-write-access) # OSS contributor reopen a PR/issue closed by a bot or maintainer. Without # this comment trigger, a contributor whose PR Agent Shin auto-closed @@ -120,14 +126,24 @@ jobs: OPENAI_BASE_URL: ${{ vars.OPENAI_BASE_URL }} TRIAGE_MODEL: ${{ vars.TRIAGE_MODEL }} AGENT_SHIN_ENABLED: ${{ vars.AGENT_SHIN_ENABLED }} + AGENT_SHIN_MODE: ${{ vars.AGENT_SHIN_MODE }} + AGENT_SHIN_POLICY_URL: ${{ vars.AGENT_SHIN_POLICY_URL }} # `issue_comment` events fire for both issues and PR comments. # `issue.pull_request` is set iff this is a PR comment, so we use # its presence to decide whether to invoke `--pr N` or `--issue N`. IS_PR: ${{ github.event.issue.pull_request != null }} + STATE: ${{ github.event.issue.state }} NUMBER: ${{ github.event.issue.number }} run: | set -euo pipefail - if [ "${IS_PR}" = "true" ]; then + if [ "${IS_PR}" = "true" ] && [ "${STATE}" = "open" ]; then + # Reconsider on an OPEN PR re-runs the review gate so the label + # pair flips right away instead of on the next daily sweep. + ARGS=(--repo "${{ github.repository }}" --pr "${NUMBER}" --review-gate) + if [ "${AGENT_SHIN_MODE:-}" = "lite" ]; then + ARGS+=(--notice-only) + fi + elif [ "${IS_PR}" = "true" ]; then ARGS=(--repo "${{ github.repository }}" --pr "${NUMBER}" --reconsider) else ARGS=(--repo "${{ github.repository }}" --issue "${NUMBER}" --reconsider) diff --git a/tests/test_litellm/test_github_review_gate.py b/tests/test_litellm/test_github_review_gate.py index 001fa8f43f5..7e9b95b63f6 100644 --- a/tests/test_litellm/test_github_review_gate.py +++ b/tests/test_litellm/test_github_review_gate.py @@ -255,7 +255,8 @@ class TestReviewGateGraceAndClose: result = _gate(triage_module, judge=_fail, greptile_score=None) assert result["action"] == "within-grace-notified" - assert rec.closed == [] and rec.added == [] and rec.removed == [] + assert rec.closed == [] and rec.removed == [] + assert rec.added == [triage_module.NOT_READY_LABEL] assert triage_module.WITHIN_GRACE_MARKER in rec.comments[0] assert "QA proof" in rec.comments[0] @@ -488,6 +489,203 @@ class TestReviewGateGuards: assert "all clear" in state["comments"][-1]["body"].lower() +class TestReviewGateNotReadyLabel: + """The red `not ready` label mirrors `ready for review`: added whenever + the PR is not passing, removed whenever it passes, so the pair always + shows the current verdict.""" + + def test_fail_untagged_adds_not_ready_label(self, triage_module, monkeypatch): + monkeypatch.setattr( + triage_module, "fetch_pr", lambda repo, n: _make_pr(created_at=JUST_NOW) + ) + rec = _Recorder(triage_module, monkeypatch) + + result = _gate(triage_module, judge=_fail, greptile_score=None) + + assert result["action"] == "within-grace-notified" + assert rec.added == [triage_module.NOT_READY_LABEL] + assert rec.closed == [] + + def test_regression_swaps_ready_for_not_ready(self, triage_module, monkeypatch): + pr = _make_pr(labels=[{"name": "ready for review"}]) + monkeypatch.setattr(triage_module, "fetch_pr", lambda repo, n: pr) + rec = _Recorder(triage_module, monkeypatch) + + result = _gate(triage_module, judge=_fail, greptile_score=5) + + assert result["action"] == "label-removed-regressed" + assert rec.added == [triage_module.NOT_READY_LABEL] + assert rec.removed == [triage_module.READY_FOR_REVIEW_LABEL] + + def test_pass_removes_not_ready_label(self, triage_module, monkeypatch): + pr = _make_pr(labels=[{"name": "not ready"}]) + monkeypatch.setattr(triage_module, "fetch_pr", lambda repo, n: pr) + rec = _Recorder(triage_module, monkeypatch) + + result = _gate(triage_module, judge=_pass, greptile_score=5) + + assert result["action"] == "labeled-ready" + assert rec.added == [triage_module.READY_FOR_REVIEW_LABEL] + assert rec.removed == [triage_module.NOT_READY_LABEL] + + def test_pass_already_tagged_still_clears_stale_not_ready( + self, triage_module, monkeypatch + ): + pr = _make_pr(labels=[{"name": "ready for review"}, {"name": "not ready"}]) + monkeypatch.setattr(triage_module, "fetch_pr", lambda repo, n: pr) + rec = _Recorder(triage_module, monkeypatch) + + result = _gate(triage_module, judge=_pass, greptile_score=5) + + assert result["action"] == "noop-passing" + assert rec.removed == [triage_module.NOT_READY_LABEL] + assert rec.added == [] and rec.comments == [] + + def test_labels_already_in_sync_are_untouched(self, triage_module, monkeypatch): + pr = _make_pr(labels=[{"name": "not ready"}], created_at=JUST_NOW) + monkeypatch.setattr(triage_module, "fetch_pr", lambda repo, n: pr) + rec = _Recorder(triage_module, monkeypatch) + prior = [ + { + "user": {"login": "github-actions[bot]"}, + "body": triage_module.WITHIN_GRACE_MARKER, + } + ] + + result = _gate(triage_module, judge=_fail, greptile_score=None, comments=prior) + + assert result["action"] == "within-grace-already-notified" + assert rec.added == [] and rec.removed == [] + + def test_dry_run_never_touches_labels(self, triage_module, monkeypatch): + monkeypatch.setattr( + triage_module, "fetch_pr", lambda repo, n: _make_pr(created_at=JUST_NOW) + ) + rec = _Recorder(triage_module, monkeypatch) + + result = _gate(triage_module, close=False, judge=_fail, greptile_score=None) + + assert result["action"] == "would-notify-within-grace" + assert rec.added == [] and rec.removed == [] + + +class TestReviewGateLiteMode: + """Lite mode (notice_only=True): the launch-week posture. Never closes; + failing untagged PRs get a one-time "closes start in 7 days" notice + linking the policy post. Labels still reconcile.""" + + def test_lite_posts_one_time_notice_instead_of_closing( + self, triage_module, monkeypatch + ): + # TWO_DAYS_AGO is past the full-mode grace window, so this PR would + # be CLOSED in full mode — lite must notice instead. + monkeypatch.setattr( + triage_module, + "fetch_pr", + lambda repo, n: _make_pr(created_at=TWO_DAYS_AGO), + ) + rec = _Recorder(triage_module, monkeypatch) + + result = _gate(triage_module, judge=_fail, greptile_score=None, notice_only=True) + + assert result["action"] == "lite-notified" + assert rec.closed == [] + assert len(rec.comments) == 1 + assert triage_module.LITE_NOTICE_MARKER in rec.comments[0] + assert "7 days" in rec.comments[0] + assert "@agent-shin reconsider" in rec.comments[0] + assert rec.added == [triage_module.NOT_READY_LABEL] + + def test_lite_notice_links_policy_url(self, triage_module, monkeypatch): + monkeypatch.setattr( + triage_module, + "fetch_pr", + lambda repo, n: _make_pr(created_at=TWO_DAYS_AGO), + ) + rec = _Recorder(triage_module, monkeypatch) + url = "https://docs.litellm.ai/blog/agent-shin" + + _gate( + triage_module, + judge=_fail, + greptile_score=None, + notice_only=True, + policy_url=url, + ) + + assert url in rec.comments[0] + + def test_lite_does_not_double_notify(self, triage_module, monkeypatch): + monkeypatch.setattr( + triage_module, + "fetch_pr", + lambda repo, n: _make_pr(created_at=TWO_DAYS_AGO), + ) + rec = _Recorder(triage_module, monkeypatch) + prior = [ + { + "user": {"login": "github-actions[bot]"}, + "body": triage_module.LITE_NOTICE_MARKER, + } + ] + + result = _gate( + triage_module, + judge=_fail, + greptile_score=None, + notice_only=True, + comments=prior, + ) + + assert result["action"] == "lite-already-notified" + assert rec.comments == [] and rec.closed == [] + + def test_lite_dry_run_previews_without_side_effects( + self, triage_module, monkeypatch + ): + monkeypatch.setattr( + triage_module, + "fetch_pr", + lambda repo, n: _make_pr(created_at=TWO_DAYS_AGO), + ) + rec = _Recorder(triage_module, monkeypatch) + + result = _gate( + triage_module, + close=False, + judge=_fail, + greptile_score=None, + notice_only=True, + ) + + assert result["action"] == "would-notify-lite" + assert rec.comments == [] and rec.closed == [] and rec.added == [] + assert "comment" in result + + def test_lite_regression_quotes_lite_window_and_never_closes( + self, triage_module, monkeypatch + ): + pr = _make_pr(labels=[{"name": "ready for review"}], created_at=TWO_DAYS_AGO) + monkeypatch.setattr(triage_module, "fetch_pr", lambda repo, n: pr) + rec = _Recorder(triage_module, monkeypatch) + + result = _gate(triage_module, judge=_fail, greptile_score=2, notice_only=True) + + assert result["action"] == "label-removed-regressed" + assert rec.closed == [] + assert "7 days" in rec.comments[0] + assert "24 hours" not in rec.comments[0] + + def test_lite_pass_still_labels_ready(self, triage_module, monkeypatch): + monkeypatch.setattr(triage_module, "fetch_pr", lambda repo, n: _make_pr()) + rec = _Recorder(triage_module, monkeypatch) + + result = _gate(triage_module, judge=_pass, greptile_score=5, notice_only=True) + + assert result["action"] == "labeled-ready" + assert rec.added == [triage_module.READY_FOR_REVIEW_LABEL] + + class TestReviewGateAllowlist: """While the dogfood allowlist is active it is the sole author gate: only the named accounts pass, and for them the internal-author exemption diff --git a/tests/test_litellm/test_github_triage_with_llm.py b/tests/test_litellm/test_github_triage_with_llm.py index f50cf126c36..c5930e4cb44 100644 --- a/tests/test_litellm/test_github_triage_with_llm.py +++ b/tests/test_litellm/test_github_triage_with_llm.py @@ -726,6 +726,14 @@ class TestBuildPrompts: class TestMainModelDefault: """`--model` falls back to DEFAULT_MODEL even when TRIAGE_MODEL is empty.""" + def test_default_model_is_gpt_5_6_luna(self, triage_module): + # Pinned deliberately: the judge should track the latest model in the + # family, and the gpt-5 prefix check below relies on the family name. + assert triage_module.DEFAULT_MODEL == "gpt-5.6-luna" + assert triage_module.DEFAULT_MODEL.startswith( + triage_module.GPT5_FAMILY_PREFIX + ), "reasoning-effort handling in call_llm_judge keys off the gpt-5 prefix" + def _stub_triage(self, triage_module, monkeypatch): captured: dict = {} @@ -1104,6 +1112,38 @@ class TestTriageOrchestration: lambda *a, **kw: None, ) + @staticmethod + def _greptile_comments(score: int) -> list: + """A comment list whose most recent Greptile review carries `score`/5. + + PR reconsiders are gated on this score, so tests exercising the + judge/reopen paths must inject a passing one (or explicitly a failing + one to exercise the gate itself). + """ + return [ + { + "user": {"login": "greptile-apps[bot]"}, + "body": f"Confidence Score: {score}/5", + "created_at": "2026-05-24T10:00:00Z", + } + ] + + @staticmethod + def _stub_labels(triage_module, monkeypatch) -> dict: + """Capture add_label/remove_label calls (reconsider reopen flips labels).""" + labels: dict = {"added": [], "removed": []} + monkeypatch.setattr( + triage_module, + "add_label", + lambda repo, n, label: labels["added"].append(label), + ) + monkeypatch.setattr( + triage_module, + "remove_label", + lambda repo, n, label: labels["removed"].append(label), + ) + return labels + @staticmethod def _stub_grace_aged_out(triage_module, monkeypatch): """Pretend the grace warning has aged out. @@ -1129,14 +1169,17 @@ class TestTriageOrchestration: ) def test_should_reopen_on_reconsider_pass(self, triage_module, monkeypatch): - # Reconsider on a closed PR with a passing verdict -> reopen + post a - # friendly "re-evaluated" comment. close=True is the production path - # (the workflow only adds --close when AGENT_SHIN_ENABLED=true). + # Reconsider on a closed PR with a passing verdict AND a passing + # Greptile score -> reopen + post a friendly "re-evaluated" comment + # + swap the label pair to `ready for review`. close=True is the + # production path (the workflow only adds --close when + # AGENT_SHIN_ENABLED=true). pr = self._make_pr( state="closed", body="Updated body with QA proof + screenshots." ) monkeypatch.setattr(triage_module, "fetch_pr", lambda repo, n: pr) self._stub_reconsider_guards(triage_module, monkeypatch) + labels = self._stub_labels(triage_module, monkeypatch) posted = {} reopened = {} monkeypatch.setattr( @@ -1166,11 +1209,14 @@ class TestTriageOrchestration: {"verdict": "pass", "missing": [], "explanation": "ok now"} ), reconsider=True, + comments=self._greptile_comments(5), ) assert result["action"] == "reopened" assert reopened["n"] == 42 assert posted["n"] == 42 assert "reopened" in posted["body"].lower() + assert labels["added"] == [triage_module.READY_FOR_REVIEW_LABEL] + assert labels["removed"] == [triage_module.NOT_READY_LABEL] def test_should_dry_run_reconsider_pass_when_close_false( self, triage_module, monkeypatch @@ -1206,6 +1252,7 @@ class TestTriageOrchestration: {"verdict": "pass", "missing": [], "explanation": "ok now"} ), reconsider=True, + comments=self._greptile_comments(5), ) assert result["action"] == "would-reopen" # The previewed comment body is still returned so a step-summary @@ -1249,6 +1296,7 @@ class TestTriageOrchestration: model="m", judge=lambda p: json.dumps(verdict), reconsider=True, + comments=self._greptile_comments(5), ) assert result["action"] == "reconsider-still-failing" assert posted["n"] == 42 @@ -1288,6 +1336,7 @@ class TestTriageOrchestration: {"verdict": v, "missing": [], "explanation": "weird"} ), reconsider=True, + comments=self._greptile_comments(5), ) assert result["action"] == "reconsider-still-failing", ambiguous assert "body" in posted, ambiguous @@ -1321,19 +1370,72 @@ class TestTriageOrchestration: model="m", judge=lambda p: json.dumps(verdict), reconsider=True, + comments=self._greptile_comments(5), ) assert result["action"] == "would-reconsider-still-failing" assert "QA proof" in result["comment"] - def test_should_reopen_on_reconsider_with_linked_issue_short_circuit( + def test_linked_issue_alone_must_not_reopen_on_reconsider( self, triage_module, monkeypatch ): - # The linked-issue short-circuit also has to honor reconsider mode: - # if the contributor edited the body to add `Fixes #1234`, the regex - # path should reopen the PR without calling the LLM. + # Reopening a closed PR requires end-to-end QA evidence, so the + # linked-issue short-circuit is bypassed in reconsider mode: the LLM + # judge MUST run, and a body that only links an issue (no QA proof) + # stays closed. Without this bypass, `Fixes #1234` alone would reopen + # any bot-closed PR. pr = self._make_pr(state="closed", body="Fixes #1234\n\nAddresses the bug.") monkeypatch.setattr(triage_module, "fetch_pr", lambda repo, n: pr) self._stub_reconsider_guards(triage_module, monkeypatch) + judged = {} + posted = {} + monkeypatch.setattr( + triage_module, + "post_comment", + lambda repo, n, body: posted.update({"body": body}), + ) + monkeypatch.setattr( + triage_module, + "reopen_pr", + lambda *a, **kw: pytest.fail("must not reopen without QA evidence"), + ) + + def judge(prompt): + judged["prompt"] = prompt + return json.dumps( + { + "verdict": "fail", + "missing": ["end-to-end QA proof"], + "explanation": "Linked issue but no QA evidence.", + } + ) + + result = triage_module.triage( + repo="o/r", + kind="pr", + number=55, + close=True, + model="m", + judge=judge, + reconsider=True, + comments=self._greptile_comments(5), + ) + assert result["action"] == "reconsider-still-failing" + assert "prompt" in judged, "the LLM judge must run despite the linked issue" + assert "QA proof" in posted["body"] + + def test_linked_issue_with_qa_evidence_reopens_on_reconsider( + self, triage_module, monkeypatch + ): + # The judge (not the regex) decides reconsider reopens: when the body + # carries both the linked issue and real QA evidence, the pass verdict + # reopens as before. + pr = self._make_pr( + state="closed", + body="Fixes #1234\n\nBefore/after curl output:\n```\n$ curl ...\n```", + ) + monkeypatch.setattr(triage_module, "fetch_pr", lambda repo, n: pr) + self._stub_reconsider_guards(triage_module, monkeypatch) + self._stub_labels(triage_module, monkeypatch) posted = {} reopened = {} monkeypatch.setattr( @@ -1353,42 +1455,16 @@ class TestTriageOrchestration: number=55, close=True, model="m", - judge=lambda p: pytest.fail("LLM must not run when linked-issue matches"), + judge=lambda p: json.dumps( + {"verdict": "pass", "missing": [], "explanation": "proof present"} + ), reconsider=True, + comments=self._greptile_comments(5), ) assert result["action"] == "reopened" assert reopened["n"] == 55 assert "reopened" in posted["body"].lower() - def test_should_dry_run_reconsider_with_linked_issue_when_close_false( - self, triage_module, monkeypatch - ): - # Linked-issue short-circuit must ALSO honor dry-run. - pr = self._make_pr(state="closed", body="Fixes #1234\n\nAddresses the bug.") - monkeypatch.setattr(triage_module, "fetch_pr", lambda repo, n: pr) - self._stub_reconsider_guards(triage_module, monkeypatch) - monkeypatch.setattr( - triage_module, - "post_comment", - lambda *a, **kw: pytest.fail("must not post in dry-run"), - ) - monkeypatch.setattr( - triage_module, - "reopen_pr", - lambda *a, **kw: pytest.fail("must not reopen in dry-run"), - ) - - 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" - def test_should_skip_internal_in_reconsider_mode(self, triage_module, monkeypatch): # Internal authors are exempt from triage in both regular and # reconsider mode — Agent Shin should never reopen one of their PRs @@ -1534,6 +1610,7 @@ class TestTriageOrchestration: lambda repo, n: reopened.update({"n": n}), ) + self._stub_labels(triage_module, monkeypatch) result = triage_module.triage( repo="o/r", kind="pr", @@ -1544,10 +1621,150 @@ class TestTriageOrchestration: {"verdict": "pass", "missing": [], "explanation": "ok"} ), reconsider=True, + comments=self._greptile_comments(5), ) assert result["action"] == "reopened" assert reopened["n"] == 1 + def test_reconsider_without_greptile_score_stays_closed_without_llm( + self, triage_module, monkeypatch + ): + # A closed PR with no Greptile confidence score cannot be reopened — + # the contributor must comment `@greptileai` first. The gate runs + # BEFORE the judge so refused reconsiders never burn LLM budget. + pr = self._make_pr(state="closed", body="Now with screenshots.") + monkeypatch.setattr(triage_module, "fetch_pr", lambda repo, n: pr) + self._stub_reconsider_guards(triage_module, monkeypatch) + posted = {} + monkeypatch.setattr( + triage_module, + "post_comment", + lambda repo, n, body: posted.update({"n": n, "body": body}), + ) + monkeypatch.setattr( + triage_module, + "reopen_pr", + lambda *a, **kw: pytest.fail("must not reopen without a Greptile score"), + ) + + result = triage_module.triage( + repo="o/r", + kind="pr", + number=42, + close=True, + model="m", + judge=lambda p: pytest.fail("LLM must not run before the Greptile gate"), + reconsider=True, + comments=[], + ) + assert result["action"] == "reconsider-needs-greptile" + assert result["greptile_score"] is None + assert "@greptileai" in posted["body"] + assert "4/5" in posted["body"] + # Must carry the rate-limit marker so spamming reconsider while the + # score is missing still hits the cooldown. + assert triage_module.RECONSIDER_COMMENT_MARKER in posted["body"] + + def test_reconsider_with_low_greptile_score_stays_closed_without_llm( + self, triage_module, monkeypatch + ): + pr = self._make_pr(state="closed", body="Now with screenshots.") + monkeypatch.setattr(triage_module, "fetch_pr", lambda repo, n: pr) + self._stub_reconsider_guards(triage_module, monkeypatch) + posted = {} + monkeypatch.setattr( + triage_module, + "post_comment", + lambda repo, n, body: posted.update({"body": body}), + ) + monkeypatch.setattr( + triage_module, + "reopen_pr", + lambda *a, **kw: pytest.fail("must not reopen below the Greptile bar"), + ) + + result = triage_module.triage( + repo="o/r", + kind="pr", + number=42, + close=True, + model="m", + judge=lambda p: pytest.fail("LLM must not run below the Greptile bar"), + reconsider=True, + comments=self._greptile_comments(3), + ) + assert result["action"] == "reconsider-needs-greptile" + assert result["greptile_score"] == 3 + assert "3/5" in posted["body"] + + def test_reconsider_greptile_gate_honors_dry_run( + self, triage_module, monkeypatch + ): + pr = self._make_pr(state="closed", body="Now with screenshots.") + monkeypatch.setattr(triage_module, "fetch_pr", lambda repo, n: pr) + self._stub_reconsider_guards(triage_module, monkeypatch) + monkeypatch.setattr( + triage_module, + "post_comment", + lambda *a, **kw: pytest.fail("must not post in dry-run"), + ) + + result = triage_module.triage( + repo="o/r", + kind="pr", + number=42, + close=False, + model="m", + judge=lambda p: pytest.fail("LLM must not run before the Greptile gate"), + reconsider=True, + comments=[], + ) + assert result["action"] == "would-reconsider-needs-greptile" + assert "@greptileai" in result["comment"] + + def test_reconsider_greptile_gate_does_not_apply_to_issues( + self, triage_module, monkeypatch + ): + # Issues have no Greptile reviews; the score gate is PR-only. An + # issue reconsider with a passing verdict must reopen even though no + # comment list or score exists. + issue = { + "number": 9, + "title": "Bug: with repro now", + "body": "```\n$ curl ...\ntraceback\n```\nExpected X, got Y.", + "state": "closed", + "author_association": "NONE", + "user": {"login": "mateo-berri"}, + } + monkeypatch.setattr(triage_module, "fetch_issue", lambda repo, n: issue) + self._stub_reconsider_guards(triage_module, monkeypatch) + reopened = {} + monkeypatch.setattr(triage_module, "post_comment", lambda repo, n, body: None) + monkeypatch.setattr( + triage_module, + "reopen_issue", + lambda repo, n: reopened.update({"n": n}), + ) + monkeypatch.setattr( + triage_module, + "_iter_paginated_json", + lambda *a, **kw: pytest.fail("issue reconsider must not fetch comments"), + ) + + result = triage_module.triage( + repo="o/r", + kind="issue", + number=9, + close=True, + model="m", + judge=lambda p: json.dumps( + {"verdict": "pass", "missing": [], "explanation": "repro present"} + ), + reconsider=True, + ) + assert result["action"] == "reopened" + assert reopened["n"] == 9 + def test_should_reopen_issue_on_reconsider_pass(self, triage_module, monkeypatch): issue = { "number": 7, diff --git a/tests/test_litellm/test_github_triage_workflows.py b/tests/test_litellm/test_github_triage_workflows.py index ec6e9fc2381..9d68101ef73 100644 --- a/tests/test_litellm/test_github_triage_workflows.py +++ b/tests/test_litellm/test_github_triage_workflows.py @@ -270,6 +270,40 @@ def _reaction_steps(steps: list[dict], content: str) -> list[tuple[int, dict]]: ] +def test_reconsider_routes_open_prs_to_review_gate() -> None: + """`@agent-shin reconsider` on an OPEN PR must run the review gate (label + flip) rather than `--reconsider` (which skips non-closed items) — without + this branch the advertised "reconsider can change the tag" flow silently + does nothing. + """ + steps = _reconsider_steps() + run = steps[_index_of_run_step(steps, "triage_with_llm.py")]["run"] + assert '"${STATE}" = "open"' in run, ( + "the reconsider run step must branch on the PR's open/closed state" + ) + assert "--review-gate" in run, ( + "open-PR reconsiders must invoke the review gate so the label pair flips" + ) + assert "--reconsider" in run, "closed PRs/issues must still use --reconsider" + + +def test_lite_mode_gates_are_exact_string_matches() -> None: + """Lite mode must only engage on the EXACT string "lite" — any other value + (typos, "Lite", "true") falls through to full behavior, mirroring the + fail-safe `= "true"` convention used for AGENT_SHIN_ENABLED. Both the + review gate and the reconsider workflow carry the branch. + """ + for workflow_file in ("review_gate.yml", "triage_reconsider.yml"): + text = "\n".join(_all_run_blocks(_load_workflow(workflow_file))) + assert '"${AGENT_SHIN_MODE:-}" = "lite"' in text, ( + f"{workflow_file} must select lite mode with an exact-string " + 'comparison against "lite"' + ) + assert "--notice-only" in text, ( + f"{workflow_file} must pass --notice-only when AGENT_SHIN_MODE=lite" + ) + + class TestReconsiderReactions: """The reconsider workflow acknowledges the triggering comment with a 👀 reaction the moment it accepts the trigger, and a 👍 once the run finishes,