From 06145a7d71d5e66da08317e09b66dd20f87a77d6 Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Mon, 25 May 2026 20:37:52 -0700 Subject: [PATCH] feat(triage): day-7 enactment sweep + flip AGENT_SHIN_ENABLED to live-by-default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is the second-step PR of the Agent Shin rollout: it merges 7 days after the heads-up (#28759 + this branch) and turns the bot on for real. Two things happen on merge: 1. A one-shot enactment sweep runs over every open external PR/issue, driving each through the steady-state triage logic with --close=true. PRs that fixed their description in the grace week get tagged `ready for review`; PRs/issues still failing the rubric get the standard 24h grace warning (or close, if they already had one and 24h elapsed). The sweep uses the same `_agent_shin_actions` dry-run wrappers as the heads-up so a single boolean toggles real vs. log. 2. The four existing triage workflows (triage_pr_with_llm, triage_issue_with_llm, close_low_quality_prs, review_gate, triage_reconsider) flip from "dry-run unless AGENT_SHIN_ENABLED=true" to "live unless AGENT_SHIN_ENABLED=false". The variable becomes a kill switch instead of an opt-in. Default semantics: unset means live. Files ----- .github/scripts/triage_rollout_enact.py — the enactment sweep. * Calls review_gate(close=False, now=current_time) for PRs and triage(close=False) (wrapped in `_fake_now(current_time)`) for issues, then routes the verdict through the matching maybe_* wrapper. Two single-page dispatch tables (_apply_pr_result, _apply_issue_result) make it easy to audit which actions map to which mutations. * Time-travel dry-run: --simulate-future-hours N (default 24+1s when --close is not set) shifts the clock forward N hours so you can preview what the next daily cron will do. Implemented in exactly two places: review_gate's `now=` parameter for PRs, and a `_fake_now` context manager that patches `agent_shin_shared.dt.datetime.now` for issues. The context manager restores the original module on exit (and on exception). * --simulate-now ISO_TS pins the clock to a specific timestamp. * --close forbids the simulate flags so a real run is always at wall-clock time. .github/workflows/triage_rollout_enact.yml — thin wrapper. Fires --close on push to litellm_internal_staging (the enactment merge) and offers a workflow_dispatch with dry_run + simulate_future_hours inputs for safe re-runs. .github/workflows/{triage_pr_with_llm,triage_issue_with_llm, close_low_quality_prs,review_gate,triage_reconsider}.yml — inverted: * `${AGENT_SHIN_ENABLED:-true}` (default live) * Conditional: `= "false"` enters kill-switch branch * OPENAI_API_KEY env: exposed unless `vars.AGENT_SHIN_ENABLED == 'false'` Comments updated to call out the new kill-switch semantics. tests/test_litellm/test_github_triage_workflows.py — split the destructive-gate constant into PER_RUN_GATE_ENV (per-input gates that must still match `= "true"`) and KILL_SWITCH_WORKFLOWS (all five workflows, must match the inverted `= "false"` / `!= "false"` pattern). Updates the kill-switch test wording to describe the new semantics. tests/test_litellm/test_triage_rollout_enact.py — 22 tests covering: * _fake_now patches and restores (including on exception) * Each branch of _apply_pr_result / _apply_issue_result with a recorder that captures the maybe_* call sequence and dry_run flag * _process_one skip-not-open / skip-internal-author * run() end-to-end: dry-run threads dry_run=True through every wrapper, real run threads False, current_time threads through to the per-item evaluators, --kind / --only-numbers restrict scope. Local preview commands ---------------------- # What this script would do right now (no GitHub writes): python3 .github/scripts/triage_rollout_enact.py --repo BerriAI/litellm # What the next daily cron will do (24h+1s in the future): python3 .github/scripts/triage_rollout_enact.py --repo BerriAI/litellm \\ --simulate-future-hours 24 # Pin to a specific moment: python3 .github/scripts/triage_rollout_enact.py --repo BerriAI/litellm \\ --simulate-now '2026-06-02T09:00:00Z' Co-Authored-By: Claude Opus 4.7 --- .github/scripts/triage_rollout_enact.py | 512 ++++++++++++++++++ .github/workflows/close_low_quality_prs.yml | 17 +- .github/workflows/review_gate.yml | 20 +- .github/workflows/triage_issue_with_llm.yml | 25 +- .github/workflows/triage_pr_with_llm.yml | 37 +- .github/workflows/triage_reconsider.yml | 37 +- .github/workflows/triage_rollout_enact.yml | 84 +++ .../test_github_triage_workflows.py | 72 +-- .../test_litellm/test_triage_rollout_enact.py | 374 +++++++++++++ 9 files changed, 1085 insertions(+), 93 deletions(-) create mode 100644 .github/scripts/triage_rollout_enact.py create mode 100644 .github/workflows/triage_rollout_enact.yml create mode 100644 tests/test_litellm/test_triage_rollout_enact.py diff --git a/.github/scripts/triage_rollout_enact.py b/.github/scripts/triage_rollout_enact.py new file mode 100644 index 00000000000..86994b7979a --- /dev/null +++ b/.github/scripts/triage_rollout_enact.py @@ -0,0 +1,512 @@ +#!/usr/bin/env python3 +"""One-shot day-7 enactment sweep for the Agent Shin rollout. + +This runs once, on the merge commit of the enactment PR, exactly 7 days after +the heads-up sweep. It walks every open external PR/issue and lets Agent +Shin's steady-state logic decide what to do for each: + + * PR passing the rubric -> tag `ready for review` (via `review_gate`) + * PR failing the rubric, has the heads-up marker, past 24h since the warning + -> close + post the standard close comment + * PR failing the rubric, no heads-up marker yet (created this week) + -> post the 24h grace warning (steady-state path) + * Issue passing -> no-op + * Issue failing past grace -> close + comment + * Issue failing in grace -> warn (or already-warned skip) + * Internal-author / closed -> skip + +The script doesn't replicate the rubric logic — it calls into the existing +``review_gate`` and ``triage`` paths in dry-run mode, then routes their +verdicts through the ``maybe_*`` wrappers so a single ``dry_run`` boolean +toggles between logging and real GitHub mutations. + +Time-travel dry-run +------------------- +``--simulate-future-hours N`` (default 24h+1s when --dry-run is set and no +explicit value is given) shifts the script's notion of "now" forward by N +hours. This lets you preview what the *next* scheduled run will do: any PR +currently in the grace window will tip into "past grace" after 24h, and the +preview shows those would-close decisions before they actually fire. + +Time-travel is implemented in exactly one place: a ``current_time`` variable +is computed at the top of ``run()`` and threaded through ``review_gate(now=)`` +for PRs. For issues (whose grace check goes through +``seconds_since_latest_marker_comment``), we patch ``agent_shin_shared``'s +``dt.datetime.now`` under a context manager for the duration of each +``triage()`` call — one small surface, easy to audit. + +CLI examples +------------ + +:: + + # Pure preview at current time (no GitHub writes): + python3 .github/scripts/triage_rollout_enact.py --repo BerriAI/litellm + + # Preview what the next daily cron will do (24h+1s in the future): + python3 .github/scripts/triage_rollout_enact.py --repo BerriAI/litellm \\ + --simulate-future-hours 24 + + # Real run (what the workflow does on merge): + python3 .github/scripts/triage_rollout_enact.py --repo BerriAI/litellm --close +""" + +from __future__ import annotations + +import argparse +import contextlib +import datetime as dt +import json +import os +import sys +from pathlib import Path +from typing import Any, Iterator + +_SCRIPTS_DIR = Path(__file__).resolve().parent +if str(_SCRIPTS_DIR) not in sys.path: + sys.path.insert(0, str(_SCRIPTS_DIR)) + +import agent_shin_shared # noqa: E402 +import triage_with_llm # noqa: E402 +from _agent_shin_actions import ( # noqa: E402 + maybe_add_label, + maybe_close_issue, + maybe_close_pr, + maybe_post_comment, + maybe_remove_label, +) +from triage_with_llm import ( # noqa: E402 + DEFAULT_MODEL, + READY_FOR_REVIEW_LABEL, + fetch_issue, + fetch_pr, + format_grace_warning_issue_comment, + format_grace_warning_pr_comment, + format_issue_close_comment, + format_pr_close_comment, + gh, + is_internal_contributor, + review_gate, + triage, +) + +# Default time-travel offset: the next daily cron runs 24h after this script's +# real run, so 24h+1s gives a "what fires tomorrow" preview without any edge +# cases at the boundary itself. +DEFAULT_SIMULATE_HOURS = 24 +_DEFAULT_FUTURE_SECONDS = DEFAULT_SIMULATE_HOURS * 3600 + 1 + + +@contextlib.contextmanager +def _fake_now(when: dt.datetime) -> Iterator[None]: + """Patch ``dt.datetime.now`` in agent_shin_shared so issue triage's grace + check resolves against ``when`` rather than wall-clock time. + + Patches ``agent_shin_shared.dt`` (the module's local alias) rather than + the global ``datetime`` so we don't leak the override into unrelated code. + The patch is scoped to the ``with`` block — once it exits, the original + ``dt`` module is restored, so a per-item call to ``triage()`` is the only + code that ever sees the fake clock. + """ + real_dt = agent_shin_shared.dt + + class _FakeDt: + """Drop-in replacement for ``datetime`` with a frozen ``now``.""" + + timezone = real_dt.timezone + datetime = real_dt.datetime + + @staticmethod + def datetime_now(tz: dt.tzinfo | None = None) -> dt.datetime: + return when if tz is None else when.astimezone(tz) + + # We only need to override `dt.datetime.now`. Easiest path: install a + # tiny shim that proxies to the real `datetime` module for everything + # except `.now()`. + class _DtShim: + timezone = real_dt.timezone + + class datetime(real_dt.datetime): # noqa: N801 - mirror stdlib name + @classmethod + def now(cls, tz: dt.tzinfo | None = None) -> dt.datetime: + return when if tz is None else when.astimezone(tz) + + # Pass everything else through to the real module. + def __getattr__(self, name: str) -> Any: # pragma: no cover - shim + return getattr(real_dt, name) + + agent_shin_shared.dt = _DtShim() + try: + yield + finally: + agent_shin_shared.dt = real_dt + + +def _list_open_numbers(repo: str, kind: str) -> list[int]: + cmd = "pr" if kind == "pr" else "issue" + raw = gh( + cmd, + "list", + "--repo", + repo, + "--state", + "open", + "--limit", + "1000", + "--json", + "number", + ) + return [item["number"] for item in json.loads(raw)] + + +# --------------------------------------------------------------------------- +# Per-PR / per-issue dispatch. +# +# Each helper takes the verdict-shaped result from review_gate / triage, then +# routes it to the matching maybe_* wrapper. The wrappers each carry the +# `dry_run` boolean, so a dry-run preview hits exactly the same code path as +# the real run except for the final GitHub API call. + + +def _apply_pr_result(*, repo: str, number: int, result: dict, dry_run: bool) -> dict: + """Translate a ``review_gate`` result into the matching GitHub mutation + (or a dry-run log line). Returns the augmented result with the action + taken (``"applied"``, ``"would-apply"``, or ``"noop"``).""" + action = result.get("action") or "unknown" + comment = result.get("comment") + base = {"kind": "pr", "number": number, "review_gate_action": action} + + # `review_gate(close=False)` returns `would-*` strings for every + # transition; `review_gate(close=True)` returns the already-applied + # counterparts. The dispatcher below treats both forms identically so the + # enactment script can be driven in either mode (we always run it in + # close=False mode to capture the would-* preview, then re-apply the + # mutations through the dry-run wrappers). + if action in ("noop-passing", "skip-not-open", "skip-internal-author"): + return {**base, "result": "noop"} + if action in ("skip-no-llm-key", "skip-llm-error"): + return { + **base, + "result": "noop-llm-unavailable", + "error": result.get("error"), + } + if action in ("would-label-ready", "labeled-ready"): + assert comment, "review_gate must supply a comment for label-ready" + maybe_post_comment(repo, number, comment, dry_run=dry_run) + maybe_add_label(repo, number, READY_FOR_REVIEW_LABEL, dry_run=dry_run) + return {**base, "result": "labeled-ready"} + if action in ("would-remove-label", "label-removed-regressed"): + assert comment + maybe_remove_label(repo, number, READY_FOR_REVIEW_LABEL, dry_run=dry_run) + maybe_post_comment(repo, number, comment, dry_run=dry_run) + return {**base, "result": "label-removed-regressed"} + if action in ("would-close", "closed"): + assert comment + maybe_post_comment(repo, number, comment, dry_run=dry_run) + maybe_close_pr(repo, number, dry_run=dry_run) + return {**base, "result": "closed"} + if action in ("would-notify-within-grace", "within-grace-notified"): + assert comment + maybe_post_comment(repo, number, comment, dry_run=dry_run) + return {**base, "result": "warned-within-grace"} + if action in ("within-grace-already-notified", "regressed-already-notified"): + return {**base, "result": "noop-already-notified"} + # Anything else falls through as a no-op so an unexpected verdict from + # review_gate (e.g. a future action string) doesn't cause a partial write. + return {**base, "result": "noop-unknown-action"} + + +def _apply_issue_result(*, repo: str, number: int, result: dict, dry_run: bool) -> dict: + """Translate a ``triage`` (kind='issue') result into the matching + mutation. Mirrors `_apply_pr_result` for the issue half of the flow.""" + action = result.get("action") or "unknown" + verdict = result.get("verdict") or {} + base = {"kind": "issue", "number": number, "triage_action": action} + + if action in ( + "pass-llm", + "pass-linked-issue", + "skip-not-open", + "skip-internal-author", + ): + return {**base, "result": "noop"} + if action in ("skip-no-llm-key", "skip-llm-error"): + return { + **base, + "result": "noop-llm-unavailable", + "error": result.get("error"), + } + if action in ("would-warn-grace", "warned-grace"): + body = format_grace_warning_issue_comment(verdict) + maybe_post_comment(repo, number, body, dry_run=dry_run) + return {**base, "result": "warned-within-grace"} + if action in ("skip-in-grace-period",): + return {**base, "result": "noop-already-warned"} + if action in ("would-close", "closed"): + body = format_issue_close_comment(verdict) + maybe_post_comment(repo, number, body, dry_run=dry_run) + maybe_close_issue(repo, number, dry_run=dry_run) + return {**base, "result": "closed"} + return {**base, "result": "noop-unknown-action"} + + +def _evaluate_pr( + *, + repo: str, + number: int, + model: str, + current_time: dt.datetime, + judge: Any = None, +) -> dict: + """Run ``review_gate`` in preview mode against ``current_time``. + + Always uses ``close=False`` so the underlying review_gate never mutates + GitHub directly — the enactment script is the single source of mutations + and routes everything through the dry-run wrappers. + """ + return review_gate( + repo=repo, + number=number, + close=False, + model=model, + judge=judge, + now=current_time, + ) + + +def _evaluate_issue( + *, + repo: str, + number: int, + model: str, + current_time: dt.datetime, + judge: Any = None, +) -> dict: + """Run ``triage(kind='issue')`` in preview mode against ``current_time``. + + ``triage`` doesn't accept a ``now`` parameter, so the time-travel patch is + applied here (the only place issues touch the wall clock is the + grace-warning age check inside ``seconds_since_latest_marker_comment``). + """ + with _fake_now(current_time): + return triage( + repo=repo, + kind="issue", + number=number, + close=False, + model=model, + judge=judge, + ) + + +def _process_one( + *, + repo: str, + kind: str, + number: int, + model: str, + dry_run: bool, + current_time: dt.datetime, + judge: Any = None, +) -> dict: + """Evaluate one PR/issue and apply the resulting mutation via the + maybe_* wrappers. Skip-cases (not-open, internal author, no key) short- + circuit before any LLM call.""" + fetcher = fetch_pr if kind == "pr" else fetch_issue + item = fetcher(repo, number) + + if (item.get("state") or "") != "open": + return {"kind": kind, "number": number, "result": "skip-not-open"} + if is_internal_contributor(item): + return {"kind": kind, "number": number, "result": "skip-internal-author"} + + if kind == "pr": + result = _evaluate_pr( + repo=repo, + number=number, + model=model, + current_time=current_time, + judge=judge, + ) + return _apply_pr_result( + repo=repo, number=number, result=result, dry_run=dry_run + ) + result = _evaluate_issue( + repo=repo, + number=number, + model=model, + current_time=current_time, + judge=judge, + ) + return _apply_issue_result(repo=repo, number=number, result=result, dry_run=dry_run) + + +def _print_summary(results: list[dict], *, current_time: dt.datetime) -> None: + counts: dict[str, int] = {} + for r in results: + counts[r.get("result") or "unknown"] = ( + counts.get(r.get("result") or "unknown", 0) + 1 + ) + print(f"\n=== enactment summary (clock={current_time.isoformat()}) ===") + for action in sorted(counts): + print(f" {action:35s} {counts[action]}") + print(f" total {len(results)}") + + +def run( + *, + repo: str, + close: bool, + model: str, + current_time: dt.datetime, + kinds: tuple[str, ...] = ("pr", "issue"), + judge: Any = None, + only_numbers: dict[str, list[int]] | None = None, +) -> list[dict]: + """Sweep ``repo`` and apply the enactment verdicts. Returns per-item results.""" + dry_run = not close + mode_label = "DRY RUN" if dry_run else "REAL RUN" + print( + f"[{mode_label}] enactment sweep over {repo} at clock={current_time.isoformat()}" + ) + + results: list[dict] = [] + for kind in kinds: + numbers = list((only_numbers or {}).get(kind, [])) or _list_open_numbers( + repo, kind + ) + print(f"\n--- {kind}s: {len(numbers)} open ---") + for n in numbers: + try: + result = _process_one( + repo=repo, + kind=kind, + number=n, + model=model, + dry_run=dry_run, + current_time=current_time, + judge=judge, + ) + except Exception as exc: # noqa: BLE001 - per-item errors don't abort + result = { + "kind": kind, + "number": n, + "result": "error", + "error": str(exc), + } + print(f"!! {kind}#{n}: {exc}", file=sys.stderr) + print(f" {kind}#{n}: {result.get('result')}") + results.append(result) + _print_summary(results, current_time=current_time) + return results + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--repo", required=True, help="owner/repo") + parser.add_argument( + "--close", + action="store_true", + help=( + "Actually post comments and close PRs/issues. Without this flag " + "the script runs in dry-run mode and only logs what it would do." + ), + ) + parser.add_argument( + "--simulate-future-hours", + type=float, + default=None, + help=( + "Dry-run only: pretend the wall clock is N hours in the future " + f"(default when --close is NOT set: {DEFAULT_SIMULATE_HOURS}h+1s, " + "so you preview exactly what the next daily run will do). Set to " + "0 to preview at the current clock instead." + ), + ) + parser.add_argument( + "--simulate-now", + type=str, + default=None, + help=( + "Dry-run only: pin the wall clock to this ISO-8601 timestamp " + "(e.g. '2026-06-02T09:00:00Z'). Overrides --simulate-future-hours." + ), + ) + parser.add_argument( + "--model", + default=os.environ.get("TRIAGE_MODEL") or DEFAULT_MODEL, + help=f"Model for the rubric LLM judge (default: {DEFAULT_MODEL}).", + ) + parser.add_argument( + "--kind", + choices=("pr", "issue", "both"), + default="both", + help="Restrict the sweep to PRs or issues only (default: both).", + ) + parser.add_argument( + "--only-pr", + type=int, + action="append", + default=[], + help="Limit the PR sweep to these PR numbers (repeat for several).", + ) + parser.add_argument( + "--only-issue", + type=int, + action="append", + default=[], + help="Limit the issue sweep to these issue numbers (repeat for several).", + ) + args = parser.parse_args() + + if args.close and ( + args.simulate_future_hours is not None or args.simulate_now is not None + ): + parser.error( + "--simulate-future-hours / --simulate-now are only valid in dry-run " + "(omit --close to preview a future clock)." + ) + + if args.close and not os.environ.get("OPENAI_API_KEY"): + parser.error("OPENAI_API_KEY must be set for --close (real-run) mode.") + + # Resolve the script's notion of "now". + if args.simulate_now is not None: + current_time = dt.datetime.fromisoformat( + args.simulate_now.replace("Z", "+00:00") + ) + else: + offset = ( + args.simulate_future_hours + if args.simulate_future_hours is not None + else (0 if args.close else DEFAULT_SIMULATE_HOURS + 1 / 3600) + ) + current_time = dt.datetime.now(dt.timezone.utc) + dt.timedelta(hours=offset) + + kinds: tuple[str, ...] + if args.kind == "pr": + kinds = ("pr",) + elif args.kind == "issue": + kinds = ("issue",) + else: + kinds = ("pr", "issue") + + only: dict[str, list[int]] = {} + if args.only_pr: + only["pr"] = args.only_pr + if args.only_issue: + only["issue"] = args.only_issue + + run( + repo=args.repo, + close=args.close, + model=args.model, + current_time=current_time, + kinds=kinds, + only_numbers=only or None, + ) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/.github/workflows/close_low_quality_prs.yml b/.github/workflows/close_low_quality_prs.yml index 2401be84000..6fce371ca1e 100644 --- a/.github/workflows/close_low_quality_prs.yml +++ b/.github/workflows/close_low_quality_prs.yml @@ -64,10 +64,10 @@ jobs: - name: Run low-quality PR closer env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - # Scheduled runs are ALWAYS dry-run, even when AGENT_SHIN_ENABLED is - # "true", so the team can QA the closer's verdicts in step summaries - # before any contributor sees a PR closed. Real closures only happen - # on manual workflow_dispatch with close=true (and the variable set). + # Scheduled runs are ALWAYS dry-run, even after the rollout, so the + # team can QA the closer's verdicts in step summaries before any + # contributor sees a PR closed. Real closures only happen on manual + # workflow_dispatch with close=true (and the kill switch off). CLOSE_FLAG: ${{ github.event.inputs.close || 'false' }} AGENT_SHIN_ENABLED: ${{ vars.AGENT_SHIN_ENABLED }} MIN_AGE_DAYS: ${{ github.event.inputs.min_age_days || '0' }} @@ -81,12 +81,15 @@ jobs: --min-score "${MIN_SCORE}" --limit "${LIMIT}" ) - if [ "${AGENT_SHIN_ENABLED:-false}" != "true" ]; then - echo "::notice::AGENT_SHIN_ENABLED is not 'true' -> forcing dry-run regardless of close input." + # Kill switch: AGENT_SHIN_ENABLED="false" forces dry-run. Default + # (unset / any other value) is "live", matching the post-enactment + # rollout state. + if [ "${AGENT_SHIN_ENABLED:-true}" = "false" ]; then + echo "::notice::Agent Shin kill switch is ON (AGENT_SHIN_ENABLED='false'). Forcing dry-run." elif [ "${GITHUB_EVENT_NAME:-}" = "workflow_dispatch" ] && [ "${CLOSE_FLAG}" = "true" ]; then ARGS+=(--close) echo "::notice::Running in close-on-fail mode." else - echo "::notice::AGENT_SHIN_ENABLED is true but this trigger is dry-run (scheduled event or close=false)." + echo "::notice::Kill switch off but this trigger is dry-run (scheduled event or close=false)." fi python3 .github/scripts/close_low_quality_prs.py "${ARGS[@]}" diff --git a/.github/workflows/review_gate.yml b/.github/workflows/review_gate.yml index 73215f359e4..27ae69b8b5c 100644 --- a/.github/workflows/review_gate.yml +++ b/.github/workflows/review_gate.yml @@ -79,7 +79,9 @@ jobs: # enabled or a collaborator triggers it manually, so an external user # can't force paid LLM calls by churning a fork PR while the bot is # still in dry-run. - OPENAI_API_KEY: ${{ (vars.AGENT_SHIN_ENABLED == 'true' || github.event_name == 'workflow_dispatch') && secrets.OPENAI_API_KEY || '' }} + # Kill-switch semantics: only suppress the LLM key when the variable + # is literally "false". Unset / any other value -> bot is live. + OPENAI_API_KEY: ${{ (vars.AGENT_SHIN_ENABLED != 'false' || github.event_name == 'workflow_dispatch') && secrets.OPENAI_API_KEY || '' }} OPENAI_BASE_URL: ${{ vars.OPENAI_BASE_URL }} TRIAGE_MODEL: ${{ vars.TRIAGE_MODEL }} AGENT_SHIN_ENABLED: ${{ vars.AGENT_SHIN_ENABLED }} @@ -92,20 +94,20 @@ jobs: set -euo pipefail COMMON=(--review-gate --grace-days "${GRACE_DAYS}" --min-greptile-score "${MIN_GREPTILE_SCORE}") - # Fail-safe gating, identical philosophy to the Greptile closer: - # - AGENT_SHIN_ENABLED must be the EXACT string "true" to act at all. - # - A manual dispatch can still preview with close=false. - # - Automatic triggers (PR events, schedule) act once enabled — that - # is the whole point of the gate (re-tag / un-tag automatically). + # Kill-switch gating: + # - AGENT_SHIN_ENABLED="false" (the exact string) forces dry-run. + # - Any other value, including unset, leaves the bot live. + # - A manual dispatch can still preview with close=false even + # when the bot is live. DO_CLOSE="false" - if [ "${AGENT_SHIN_ENABLED:-false}" != "true" ]; then - echo "::notice::AGENT_SHIN_ENABLED is not 'true' -> dry-run (no labels/comments/closes)." + if [ "${AGENT_SHIN_ENABLED:-true}" = "false" ]; then + echo "::notice::Agent Shin kill switch is ON (AGENT_SHIN_ENABLED='false'). Forcing dry-run." elif [ "${GITHUB_EVENT_NAME:-}" = "workflow_dispatch" ] && [ "${CLOSE_FLAG:-false}" = "true" ]; then DO_CLOSE="true" echo "::notice::Manual run -> acting for real." elif [ "${GITHUB_EVENT_NAME:-}" != "workflow_dispatch" ]; then DO_CLOSE="true" - echo "::notice::Enabled automatic trigger (${GITHUB_EVENT_NAME:-}) -> acting for real." + echo "::notice::Automatic trigger (${GITHUB_EVENT_NAME:-}) -> acting for real (kill switch off)." else echo "::notice::Manual dispatch with close=false -> dry-run." fi diff --git a/.github/workflows/triage_issue_with_llm.yml b/.github/workflows/triage_issue_with_llm.yml index 5161165beef..c4d5a5f8d31 100644 --- a/.github/workflows/triage_issue_with_llm.yml +++ b/.github/workflows/triage_issue_with_llm.yml @@ -2,9 +2,9 @@ name: Agent Shin — Issue triage # LLM-as-judge triage for external GitHub issues. # -# DRY-RUN BY DEFAULT. See .github/workflows/triage_pr_with_llm.yml for the -# enablement procedure — same repo variable (`AGENT_SHIN_ENABLED=true`) -# unlocks the PR and issue triage flows together. +# LIVE BY DEFAULT. See .github/workflows/triage_pr_with_llm.yml for the +# kill-switch procedure — same repo variable (`AGENT_SHIN_ENABLED="false"`) +# forces the PR and issue triage flows back into dry-run together. on: issues: @@ -15,7 +15,7 @@ on: description: "Issue number to triage manually." required: true close: - description: "If true and AGENT_SHIN_ENABLED=true, actually close on fail." + description: "If true (and AGENT_SHIN_ENABLED != 'false'), actually close on fail." required: false default: "false" type: choice @@ -55,7 +55,9 @@ jobs: # The Python script calls the LLM whenever this var is set # (regardless of `--close`); stripping `--close` doesn't suppress # the API call, only the destructive side effects. - OPENAI_API_KEY: ${{ (vars.AGENT_SHIN_ENABLED == 'true' || github.event_name == 'workflow_dispatch') && secrets.OPENAI_API_KEY || '' }} + # Kill-switch semantics: only suppress the LLM key when the variable + # is literally "false". Unset / any other value -> bot is live. + OPENAI_API_KEY: ${{ (vars.AGENT_SHIN_ENABLED != 'false' || github.event_name == 'workflow_dispatch') && secrets.OPENAI_API_KEY || '' }} OPENAI_BASE_URL: ${{ vars.OPENAI_BASE_URL }} TRIAGE_MODEL: ${{ vars.TRIAGE_MODEL }} AGENT_SHIN_ENABLED: ${{ vars.AGENT_SHIN_ENABLED }} @@ -71,13 +73,16 @@ jobs: # string, and a `!= "false"` check would treat "True", "yes", # "1", "TRUE", typos, and accidental whitespace as enabling # closure. Mirror the Greptile closer's `= "true"` pattern. - if [ "${AGENT_SHIN_ENABLED:-false}" = "true" ] && [ "${DISPATCH_CLOSE:-false}" = "true" ]; then + # Kill switch: AGENT_SHIN_ENABLED="false" forces dry-run even when + # the dispatch input asks for close. Default (unset / any other + # value) is "live", matching the post-enactment rollout state. + if [ "${AGENT_SHIN_ENABLED:-true}" != "false" ] && [ "${DISPATCH_CLOSE:-false}" = "true" ]; then ARGS+=(--close) - echo "::notice::Agent Shin is ENABLED and running in close-on-fail mode." - elif [ "${AGENT_SHIN_ENABLED:-false}" = "true" ]; then - echo "::notice::Agent Shin is ENABLED but this trigger is dry-run (workflow_dispatch close != 'true')." + echo "::notice::Agent Shin is LIVE — running in close-on-fail mode." + elif [ "${AGENT_SHIN_ENABLED:-true}" != "false" ]; then + echo "::notice::Agent Shin is LIVE but this trigger is dry-run (workflow_dispatch close != 'true')." else - echo "::notice::Agent Shin is in DRY-RUN mode (AGENT_SHIN_ENABLED is not 'true'). No comments will be posted; no issues will be closed." + echo "::notice::Agent Shin kill switch is ON (AGENT_SHIN_ENABLED='false'). Forcing dry-run." fi # Automatic `issues` events stay dry-run regardless until the team # explicitly invokes workflow_dispatch with close=true. diff --git a/.github/workflows/triage_pr_with_llm.yml b/.github/workflows/triage_pr_with_llm.yml index 4aee1b14305..8f56060efcd 100644 --- a/.github/workflows/triage_pr_with_llm.yml +++ b/.github/workflows/triage_pr_with_llm.yml @@ -2,15 +2,15 @@ name: Agent Shin — PR triage # LLM-as-judge triage for external pull requests. # -# DRY-RUN BY DEFAULT. Closures and public comments are gated on the repo -# variable `AGENT_SHIN_ENABLED` being set to the string `"true"`. Until then, -# every run only writes its verdict to the workflow step summary so the team -# can QA the judge's decisions before flipping it on. +# LIVE BY DEFAULT. Closures and public comments fire whenever the workflow is +# triggered and the dispatch input asks for `close=true`. The repo variable +# `AGENT_SHIN_ENABLED` acts as a kill switch — set it to the exact string +# `"false"` (Settings > Secrets and variables > Actions > Variables) to force +# every run back to dry-run. Any other value, including unset, leaves the bot +# enabled. # -# To enable for real: -# 1. Add a repo secret `OPENAI_API_KEY` (or compatible). -# 2. Set repo variable `AGENT_SHIN_ENABLED` to `true` -# (Settings > Secrets and variables > Actions > Variables). +# Required setup: +# - Repo secret `OPENAI_API_KEY` (or compatible) for the LLM judge. # # We use `pull_request_target` so the workflow has access to repo secrets # and runs against PRs from forks. We never check out fork code — only read @@ -25,7 +25,7 @@ on: description: "PR number to triage manually." required: true close: - description: "If true and AGENT_SHIN_ENABLED=true, actually close on fail." + description: "If true (and AGENT_SHIN_ENABLED != 'false'), actually close on fail." required: false default: "false" type: choice @@ -66,7 +66,11 @@ jobs: # The Python script calls the LLM whenever this var is set # (regardless of `--close`); stripping `--close` doesn't suppress # the API call, only the destructive side effects. - OPENAI_API_KEY: ${{ (vars.AGENT_SHIN_ENABLED == 'true' || github.event_name == 'workflow_dispatch') && secrets.OPENAI_API_KEY || '' }} + # Kill-switch semantics: only suppress the LLM key when the variable + # is literally "false". Unset / any other value -> bot is live, key + # is exposed. Manual dispatch always gets the key so a collaborator + # can force-run even with the kill switch on. + OPENAI_API_KEY: ${{ (vars.AGENT_SHIN_ENABLED != 'false' || github.event_name == 'workflow_dispatch') && secrets.OPENAI_API_KEY || '' }} OPENAI_BASE_URL: ${{ vars.OPENAI_BASE_URL }} TRIAGE_MODEL: ${{ vars.TRIAGE_MODEL }} AGENT_SHIN_ENABLED: ${{ vars.AGENT_SHIN_ENABLED }} @@ -82,13 +86,16 @@ jobs: # string, and a `!= "false"` check would treat "True", "yes", # "1", "TRUE", typos, and accidental whitespace as enabling # closure. Mirror the Greptile closer's `= "true"` pattern. - if [ "${AGENT_SHIN_ENABLED:-false}" = "true" ] && [ "${DISPATCH_CLOSE:-false}" = "true" ]; then + # Kill switch: AGENT_SHIN_ENABLED="false" forces dry-run even when + # the dispatch input asks for close. The default (unset / any other + # value) is "live", matching the post-enactment rollout state. + if [ "${AGENT_SHIN_ENABLED:-true}" != "false" ] && [ "${DISPATCH_CLOSE:-false}" = "true" ]; then ARGS+=(--close) - echo "::notice::Agent Shin is ENABLED and running in close-on-fail mode." - elif [ "${AGENT_SHIN_ENABLED:-false}" = "true" ]; then - echo "::notice::Agent Shin is ENABLED but this trigger is dry-run (workflow_dispatch close != 'true' or scheduled event)." + echo "::notice::Agent Shin is LIVE — running in close-on-fail mode." + elif [ "${AGENT_SHIN_ENABLED:-true}" != "false" ]; then + echo "::notice::Agent Shin is LIVE but this trigger is dry-run (workflow_dispatch close != 'true' or scheduled event)." else - echo "::notice::Agent Shin is in DRY-RUN mode (AGENT_SHIN_ENABLED is not 'true'). No comments will be posted; no PRs will be closed." + echo "::notice::Agent Shin kill switch is ON (AGENT_SHIN_ENABLED='false'). Forcing dry-run." fi # On the scheduled/automatic pull_request_target trigger we default to # dry-run regardless, so the team can review verdicts in the step diff --git a/.github/workflows/triage_reconsider.yml b/.github/workflows/triage_reconsider.yml index 5b23a8d5697..ff055fd8c24 100644 --- a/.github/workflows/triage_reconsider.yml +++ b/.github/workflows/triage_reconsider.yml @@ -15,11 +15,11 @@ name: Agent Shin — reconsider # (which loses the original PR's history). The bot, on the other hand, # has write access via GH_TOKEN and can reopen on their behalf. # -# DRY-RUN BY DEFAULT — gated on `vars.AGENT_SHIN_ENABLED == 'true'` just -# like the other Agent Shin workflows. The workflow also gates on the -# commenter being either the PR/issue author or an internal collaborator -# (OWNER/MEMBER/COLLABORATOR) so random commenters cannot DOS the LLM -# judge or force a reopen. +# LIVE BY DEFAULT — disabled only when `vars.AGENT_SHIN_ENABLED == 'false'` +# (the kill switch shared with the other Agent Shin workflows). The workflow +# also gates on the commenter being either the PR/issue author or an internal +# collaborator (OWNER/MEMBER/COLLABORATOR) so random commenters cannot DOS the +# LLM judge or force a reopen. on: issue_comment: @@ -108,23 +108,20 @@ jobs: ARGS=(--repo "${{ github.repository }}" --issue "${NUMBER}" --reconsider) fi # Reconsider's destructive actions (post comment + reopen) are - # gated on `--close`, mirroring the regular triage workflows. - # When AGENT_SHIN_ENABLED is not the EXACT string "true", we - # still run the script so its verdict + would-X action lands in - # the step summary for QA — but without `--close`, the script - # returns `would-reopen` / `would-reconsider-still-failing` - # instead of touching GitHub state. + # gated on `--close`. The kill switch is the shared + # AGENT_SHIN_ENABLED variable: setting it to the literal string + # "false" forces reconsider back to dry-run (the script still + # runs so the would-X verdict lands in the step summary for QA, + # but without `--close` no GitHub state changes). # - # Use the positive `= "true"` gate (not `!= "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 fall through to the dry-run - # branch, which is the safe default. - if [ "${AGENT_SHIN_ENABLED:-false}" = "true" ]; then + # Negative `!= "false"` against the `:-true` default keeps the + # kill-switch semantics symmetric with the other triage + # workflows — unknown values (typos, "True", "1") leave the bot + # live, matching the post-enactment live-by-default policy. + if [ "${AGENT_SHIN_ENABLED:-true}" != "false" ]; then ARGS+=(--close) - echo "::notice::Agent Shin reconsider ENABLED — running real triage (close=true)." + echo "::notice::Agent Shin reconsider is LIVE — running real triage (close=true)." else - echo "::notice::AGENT_SHIN_ENABLED is not 'true' -> reconsider stays in dry-run (no comment, no reopen)." + echo "::notice::Agent Shin kill switch is ON (AGENT_SHIN_ENABLED='false'). Reconsider stays in dry-run." fi python3 .github/scripts/triage_with_llm.py "${ARGS[@]}" diff --git a/.github/workflows/triage_rollout_enact.yml b/.github/workflows/triage_rollout_enact.yml new file mode 100644 index 00000000000..9bacdd3f07f --- /dev/null +++ b/.github/workflows/triage_rollout_enact.yml @@ -0,0 +1,84 @@ +name: Agent Shin — rollout enactment (one-shot) + +# Fires the day-7 enactment sweep: closes any open external PR/issue that's +# still failing the rubric 7 days after the heads-up (because the contributor +# didn't update the description), and applies steady-state actions +# (ready-for-review tag, in-grace warnings) to everything else. From the +# merge of this workflow onward, the existing daily/cron triage workflows +# go live for real (the AGENT_SHIN_ENABLED gates are removed by the same PR). +# +# Thin shell over `.github/scripts/triage_rollout_enact.py`. The dry-run +# preview path is the SAME code with --close stripped, so a local preview +# (with --simulate-future-hours 24 to peek at tomorrow's cron run) is a +# high-fidelity preview of what the workflow will actually do. + +on: + push: + branches: + - litellm_internal_staging + paths: + - ".github/scripts/triage_rollout_enact.py" + workflow_dispatch: + inputs: + dry_run: + description: "Dry run (true = preview only, false = actually close/comment)." + required: false + default: "true" + type: choice + options: + - "true" + - "false" + simulate_future_hours: + description: "Dry-run only: pretend N hours have passed (default 24 = next cron preview)." + required: false + default: "24" + +permissions: + contents: read + issues: write + pull-requests: write + +jobs: + enact: + if: github.repository == 'BerriAI/litellm' + runs-on: ubuntu-latest + steps: + - name: Checkout triage scripts + uses: actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955 # v4.3.0 + with: + sparse-checkout: .github/scripts + persist-credentials: false + + - name: Set up Python + uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0 + with: + python-version: "3.12" + + - name: Install LLM client + run: pip install --no-cache-dir "openai>=1.40.0" + + - name: Run enactment sweep + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} + OPENAI_BASE_URL: ${{ vars.OPENAI_BASE_URL }} + TRIAGE_MODEL: ${{ vars.TRIAGE_MODEL }} + DRY_RUN_INPUT: ${{ github.event.inputs.dry_run }} + SIM_HOURS: ${{ github.event.inputs.simulate_future_hours }} + run: | + set -euo pipefail + ARGS=(--repo "${{ github.repository }}") + if [ "${GITHUB_EVENT_NAME:-}" = "workflow_dispatch" ]; then + if [ "${DRY_RUN_INPUT:-true}" = "false" ]; then + ARGS+=(--close) + echo "::notice::Manual dispatch with dry_run=false -> closures and comments WILL be applied." + else + ARGS+=(--simulate-future-hours "${SIM_HOURS:-24}") + echo "::notice::Manual dispatch in dry-run, simulating +${SIM_HOURS:-24}h." + fi + else + # push to litellm_internal_staging = the enactment merge -> real run + ARGS+=(--close) + echo "::notice::Enactment-merge trigger -> closures and comments WILL be applied." + fi + python3 .github/scripts/triage_rollout_enact.py "${ARGS[@]}" diff --git a/tests/test_litellm/test_github_triage_workflows.py b/tests/test_litellm/test_github_triage_workflows.py index ac316b1988a..644b146949a 100644 --- a/tests/test_litellm/test_github_triage_workflows.py +++ b/tests/test_litellm/test_github_triage_workflows.py @@ -35,26 +35,30 @@ import yaml REPO_ROOT = Path(__file__).resolve().parents[2] WORKFLOWS_DIR = REPO_ROOT / ".github" / "workflows" -# Map of workflow file -> the env var name that drives the destructive -# gate inside that workflow's `run:` block. Keeping this table explicit -# (rather than scraping every workflow file) means a new workflow file -# that bypasses the dry-run gating doesn't silently slip past this test. -DESTRUCTIVE_GATE_ENV: dict[str, str] = { +# Workflows that consult a per-run user-input env var (e.g. CLOSE_FLAG, +# DISPATCH_CLOSE) before adding `--close`. Those gates must stay positive +# `= "true"` so typos like "True"/"yes"/"1" fail-closed to dry-run. +PER_RUN_GATE_ENV: dict[str, str] = { "triage_pr_with_llm.yml": "DISPATCH_CLOSE", "triage_issue_with_llm.yml": "DISPATCH_CLOSE", "close_low_quality_prs.yml": "CLOSE_FLAG", - # The reconsider workflow has no per-run "really do it?" knob — its - # 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", } +# Every workflow that can post comments or close PRs/issues must consult +# the `AGENT_SHIN_ENABLED` kill switch. Post-enactment the gate is +# inverted: bot is live by default, only forced into dry-run when the +# variable is literally "false". The reconsider workflow has no per-run +# knob — AGENT_SHIN_ENABLED IS its only gate — so it appears here but +# NOT in PER_RUN_GATE_ENV. +KILL_SWITCH_WORKFLOWS: tuple[str, ...] = ( + "triage_pr_with_llm.yml", + "triage_issue_with_llm.yml", + "close_low_quality_prs.yml", + "review_gate.yml", + "triage_reconsider.yml", +) + def _load_workflow(name: str) -> dict: return yaml.safe_load((WORKFLOWS_DIR / name).read_text()) @@ -74,7 +78,7 @@ def _all_run_blocks(workflow: dict) -> list[str]: return commands -@pytest.mark.parametrize("workflow_file,env_var", sorted(DESTRUCTIVE_GATE_ENV.items())) +@pytest.mark.parametrize("workflow_file,env_var", sorted(PER_RUN_GATE_ENV.items())) def test_should_use_failsafe_equals_true_comparison( workflow_file: str, env_var: str ) -> None: @@ -116,30 +120,34 @@ def test_should_use_failsafe_equals_true_comparison( ) -@pytest.mark.parametrize("workflow_file", sorted(DESTRUCTIVE_GATE_ENV)) +@pytest.mark.parametrize("workflow_file", sorted(KILL_SWITCH_WORKFLOWS)) def test_should_require_agent_shin_enabled_for_close(workflow_file: str) -> None: - """Every destructive gate must also gate on the global enablement - variable, so flipping `AGENT_SHIN_ENABLED` off is a kill switch - regardless of any per-run input. + """Every destructive gate must consult ``AGENT_SHIN_ENABLED`` so the + variable is a usable kill switch regardless of any per-run input. - Two patterns are equally fine: - - Positive: `[ "${AGENT_SHIN_ENABLED:-false}" = "true" ]` to enter - the close branch (Agent Shin workflows). - - Negative: `[ "${AGENT_SHIN_ENABLED:-false}" != "true" ]` then - bail out / force dry-run (Greptile closer). + Post-enactment the variable is INVERTED — Agent Shin is live by default + and the variable forces dry-run when explicitly set to ``"false"``. Two + patterns are equally fine: - What matters is that the comparison value is the literal "true"; - `!= "false"` or `= "1"` etc. would not be a true kill switch. + - Positive enter-kill-switch branch: + ``[ "${AGENT_SHIN_ENABLED:-true}" = "false" ]`` + - Negative bypass-kill-switch branch: + ``[ "${AGENT_SHIN_ENABLED:-true}" != "false" ]`` + + What matters is that the comparison value is the literal ``"false"`` + AND the default-when-unset is ``"true"`` (i.e. live). ``= "true"`` + against ``${AGENT_SHIN_ENABLED:-false}`` would re-introduce the + pre-rollout dry-run-by-default semantics. """ workflow = _load_workflow(workflow_file) text = "\n".join(_all_run_blocks(workflow)) accepted_patterns = ( - '"${AGENT_SHIN_ENABLED:-false}" = "true"', - '"${AGENT_SHIN_ENABLED:-false}" != "true"', + '"${AGENT_SHIN_ENABLED:-true}" = "false"', + '"${AGENT_SHIN_ENABLED:-true}" != "false"', ) assert any(p in text for p in accepted_patterns), ( - f"{workflow_file} must gate destructive actions on " - '`AGENT_SHIN_ENABLED = "true"` (or the inverted `!= "true"` ' - "guard that forces dry-run). Without this, an unset repo " - "variable would not be treated as a kill switch." + f"{workflow_file} must gate destructive actions on the inverted " + '`AGENT_SHIN_ENABLED` kill switch (`= "false"` or `!= "false"` ' + 'against the `${AGENT_SHIN_ENABLED:-true}` default). Without this, ' + "an unset repo variable could be confused for an opt-in/opt-out." ) diff --git a/tests/test_litellm/test_triage_rollout_enact.py b/tests/test_litellm/test_triage_rollout_enact.py new file mode 100644 index 00000000000..b41631c80b7 --- /dev/null +++ b/tests/test_litellm/test_triage_rollout_enact.py @@ -0,0 +1,374 @@ +"""Unit tests for the day-7 enactment sweep. + +Covers: + + * ``_fake_now`` — the time-travel context manager. Patches + ``agent_shin_shared.dt`` so ``datetime.now()`` returns a pinned value, + and restores the original module on exit (even on exception). + * ``_apply_pr_result`` / ``_apply_issue_result`` — dispatch tables that + turn a review_gate/triage verdict into one or two ``maybe_*`` calls. + These are where the dry-run boolean actually reaches the side-effect + wrappers, so a regression here breaks the entire enactment. + * ``_process_one`` — per-item skip cases (not-open, internal author). + * ``run`` — sweep loop end-to-end with stubbed evaluators, exercising + both dry-run and real modes plus the time-travel offset. + +Nothing in this file ever shells out — gh / openai / GitHub mutations are +stubbed end-to-end. +""" + +from __future__ import annotations + +import datetime as dt +import importlib.util +import sys +from pathlib import Path + +import pytest + +_SCRIPTS_DIR = Path(__file__).resolve().parents[2] / ".github" / "scripts" + + +def _load(name: str, filename: str | None = None): + spec = importlib.util.spec_from_file_location( + name, _SCRIPTS_DIR / (filename or f"{name}.py") + ) + assert spec and spec.loader + module = importlib.util.module_from_spec(spec) + sys.modules[name] = module + spec.loader.exec_module(module) + return module + + +@pytest.fixture(scope="module") +def shared_module(): + return _load("agent_shin_shared") + + +@pytest.fixture(scope="module") +def triage_module(shared_module): + return _load("triage_with_llm") + + +@pytest.fixture(scope="module") +def actions_module(triage_module): + return _load("_agent_shin_actions") + + +@pytest.fixture(scope="module") +def enact_module(triage_module, actions_module, shared_module): + return _load("triage_rollout_enact") + + +# --------------------------------------------------------------------------- +# _fake_now context manager + + +class TestFakeNow: + def test_patches_dt_now_inside_context(self, enact_module, shared_module): + when = dt.datetime(2099, 1, 1, tzinfo=dt.timezone.utc) + with enact_module._fake_now(when): + assert shared_module.dt.datetime.now(dt.timezone.utc) == when + + def test_restores_dt_on_exit(self, enact_module, shared_module): + original = shared_module.dt + with enact_module._fake_now(dt.datetime(2099, 1, 1, tzinfo=dt.timezone.utc)): + pass + assert shared_module.dt is original + + def test_restores_dt_on_exception(self, enact_module, shared_module): + original = shared_module.dt + with pytest.raises(RuntimeError): + with enact_module._fake_now( + dt.datetime(2099, 1, 1, tzinfo=dt.timezone.utc) + ): + raise RuntimeError("boom") + assert shared_module.dt is original + + +# --------------------------------------------------------------------------- +# _apply_pr_result and _apply_issue_result + + +@pytest.fixture +def recorder(enact_module, monkeypatch): + """Replace every maybe_* in the enactment module with a recorder so we + can assert the exact sequence and dry-run flag of each side effect.""" + calls: list[dict] = [] + + def make(name): + def _stub(*args, dry_run=None, **kw): + calls.append({"name": name, "args": args, "kw": kw, "dry_run": dry_run}) + + return _stub + + for name in ( + "maybe_post_comment", + "maybe_close_pr", + "maybe_close_issue", + "maybe_add_label", + "maybe_remove_label", + ): + monkeypatch.setattr(enact_module, name, make(name)) + return calls + + +class TestApplyPRResult: + def _apply(self, enact_module, action, *, dry_run, comment="body", **extra): + return enact_module._apply_pr_result( + repo="o/r", + number=7, + result={"action": action, "comment": comment, **extra}, + dry_run=dry_run, + ) + + def test_noop_passing_does_nothing(self, enact_module, recorder): + r = self._apply(enact_module, "noop-passing", dry_run=True) + assert r["result"] == "noop" + assert recorder == [] + + def test_skip_internal_author_is_noop(self, enact_module, recorder): + r = self._apply(enact_module, "skip-internal-author", dry_run=True) + assert r["result"] == "noop" + assert recorder == [] + + def test_skip_no_llm_key_records_unavailable(self, enact_module, recorder): + r = self._apply(enact_module, "skip-no-llm-key", dry_run=True, error=None) + assert r["result"] == "noop-llm-unavailable" + assert recorder == [] + + def test_would_label_ready_posts_then_labels(self, enact_module, recorder): + r = self._apply(enact_module, "would-label-ready", dry_run=True) + assert r["result"] == "labeled-ready" + assert [c["name"] for c in recorder] == [ + "maybe_post_comment", + "maybe_add_label", + ] + # Both maybe_* calls must carry the same dry_run boolean. + assert all(c["dry_run"] is True for c in recorder) + + def test_would_close_real_run_posts_then_closes(self, enact_module, recorder): + r = self._apply(enact_module, "would-close", dry_run=False) + assert r["result"] == "closed" + assert [c["name"] for c in recorder] == [ + "maybe_post_comment", + "maybe_close_pr", + ] + assert all(c["dry_run"] is False for c in recorder) + + def test_would_remove_label_posts_and_removes(self, enact_module, recorder): + r = self._apply(enact_module, "would-remove-label", dry_run=True) + assert r["result"] == "label-removed-regressed" + names = [c["name"] for c in recorder] + assert names.count("maybe_remove_label") == 1 + assert names.count("maybe_post_comment") == 1 + + def test_within_grace_only_warns_no_close(self, enact_module, recorder): + r = self._apply(enact_module, "would-notify-within-grace", dry_run=False) + assert r["result"] == "warned-within-grace" + assert [c["name"] for c in recorder] == ["maybe_post_comment"] + # Crucially, NO maybe_close_pr — the contributor still has time. + assert not any(c["name"] == "maybe_close_pr" for c in recorder) + + def test_unknown_action_is_safe_noop(self, enact_module, recorder): + r = self._apply(enact_module, "future-action-we-dont-handle", dry_run=False) + assert r["result"] == "noop-unknown-action" + assert recorder == [] + + +class TestApplyIssueResult: + def _apply(self, enact_module, action, *, dry_run, verdict=None): + return enact_module._apply_issue_result( + repo="o/r", + number=42, + result={ + "action": action, + "verdict": verdict or {"missing": ["X"], "explanation": ""}, + }, + dry_run=dry_run, + ) + + def test_pass_llm_is_noop(self, enact_module, recorder): + r = self._apply(enact_module, "pass-llm", dry_run=True) + assert r["result"] == "noop" + assert recorder == [] + + def test_would_warn_grace_posts_warning(self, enact_module, recorder): + r = self._apply(enact_module, "would-warn-grace", dry_run=True) + assert r["result"] == "warned-within-grace" + assert [c["name"] for c in recorder] == ["maybe_post_comment"] + + def test_in_grace_already_warned_is_noop(self, enact_module, recorder): + r = self._apply(enact_module, "skip-in-grace-period", dry_run=False) + assert r["result"] == "noop-already-warned" + assert recorder == [] + + def test_would_close_real_posts_then_closes(self, enact_module, recorder): + r = self._apply(enact_module, "would-close", dry_run=False) + assert r["result"] == "closed" + names = [c["name"] for c in recorder] + assert names == ["maybe_post_comment", "maybe_close_issue"] + assert all(c["dry_run"] is False for c in recorder) + + +# --------------------------------------------------------------------------- +# _process_one — skip cases + + +class TestProcessOneSkips: + @pytest.fixture + def patched(self, enact_module, triage_module, monkeypatch): + def install(*, item): + monkeypatch.setattr(enact_module, "fetch_pr", lambda repo, n: item) + monkeypatch.setattr(enact_module, "fetch_issue", lambda repo, n: item) + monkeypatch.setattr(triage_module, "fetch_pr", lambda repo, n: item) + monkeypatch.setattr(triage_module, "fetch_issue", lambda repo, n: item) + + return install + + def test_closed_pr_is_skipped(self, enact_module, patched): + patched(item={"state": "closed", "user": {"login": "outside-dev"}}) + r = enact_module._process_one( + repo="o/r", + kind="pr", + number=7, + model="m", + dry_run=True, + current_time=dt.datetime.now(dt.timezone.utc), + ) + assert r["result"] == "skip-not-open" + + def test_internal_author_is_skipped(self, enact_module, patched): + patched( + item={ + "state": "open", + "user": {"login": "mateo-berri"}, + "author_association": "MEMBER", + } + ) + r = enact_module._process_one( + repo="o/r", + kind="pr", + number=7, + model="m", + dry_run=True, + current_time=dt.datetime.now(dt.timezone.utc), + ) + assert r["result"] == "skip-internal-author" + + +# --------------------------------------------------------------------------- +# run() — end-to-end sweep with stubbed evaluators + + +class TestRun: + @pytest.fixture + def env(self, enact_module, triage_module, monkeypatch): + posts: list[dict] = [] + + def record(name): + def _stub(*args, dry_run=None, **kw): + posts.append({"name": name, "args": args, "kw": kw, "dry_run": dry_run}) + + return _stub + + for name in ( + "maybe_post_comment", + "maybe_close_pr", + "maybe_close_issue", + "maybe_add_label", + "maybe_remove_label", + ): + monkeypatch.setattr(enact_module, name, record(name)) + + def list_open(repo, kind): + return [1, 2] if kind == "pr" else [101] + + monkeypatch.setattr(enact_module, "_list_open_numbers", list_open) + + def item(_repo, _n): + return { + "state": "open", + "user": {"login": "outside-dev"}, + "author_association": "NONE", + } + + monkeypatch.setattr(enact_module, "fetch_pr", item) + monkeypatch.setattr(enact_module, "fetch_issue", item) + monkeypatch.setattr(triage_module, "fetch_pr", item) + monkeypatch.setattr(triage_module, "fetch_issue", item) + + # Pretend review_gate / triage have already been called and pass + # back canned would-* verdicts that exercise both branches. + seen_now: list[dt.datetime] = [] + + def fake_evaluate_pr(*, repo, number, model, current_time, judge=None): + seen_now.append(current_time) + if number == 1: + return { + "action": "would-label-ready", + "comment": "ready!", + "passing": True, + } + return { + "action": "would-close", + "comment": "closing!", + "passing": False, + "verdict": {"verdict": "fail", "missing": ["X"]}, + } + + def fake_evaluate_issue(*, repo, number, model, current_time, judge=None): + seen_now.append(current_time) + return { + "action": "would-warn-grace", + "verdict": {"verdict": "fail", "missing": ["repro"]}, + } + + monkeypatch.setattr(enact_module, "_evaluate_pr", fake_evaluate_pr) + monkeypatch.setattr(enact_module, "_evaluate_issue", fake_evaluate_issue) + return posts, seen_now + + def test_dry_run_passes_dry_true_through_to_wrappers(self, enact_module, env): + posts, _ = env + clock = dt.datetime(2026, 6, 1, tzinfo=dt.timezone.utc) + enact_module.run(repo="o/r", close=False, model="m", current_time=clock) + assert posts, "dry-run should still call wrappers (they self-gate)" + assert all(p["dry_run"] is True for p in posts) + + def test_real_run_passes_dry_false_to_wrappers(self, enact_module, env): + posts, _ = env + clock = dt.datetime(2026, 6, 1, tzinfo=dt.timezone.utc) + enact_module.run(repo="o/r", close=True, model="m", current_time=clock) + assert all(p["dry_run"] is False for p in posts) + + def test_current_time_threads_through_to_evaluators(self, enact_module, env): + _, seen_now = env + clock = dt.datetime(2099, 1, 1, tzinfo=dt.timezone.utc) + enact_module.run(repo="o/r", close=False, model="m", current_time=clock) + # PRs and issue all evaluated against the same future clock. + assert seen_now and all(t == clock for t in seen_now) + + def test_kinds_filter(self, enact_module, env): + _, seen_now = env + clock = dt.datetime.now(dt.timezone.utc) + results = enact_module.run( + repo="o/r", + close=False, + model="m", + current_time=clock, + kinds=("issue",), + ) + assert {r["kind"] for r in results} == {"issue"} + + def test_only_numbers_restricts_sweep(self, enact_module, env): + _, _ = env + clock = dt.datetime.now(dt.timezone.utc) + results = enact_module.run( + repo="o/r", + close=False, + model="m", + current_time=clock, + only_numbers={"pr": [2]}, + ) + prs = [r for r in results if r["kind"] == "pr"] + assert [r["number"] for r in prs] == [2]