diff --git a/.circleci/config.yml b/.circleci/config.yml index e8a8483781b..5e77729df29 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -1025,7 +1025,7 @@ jobs: name: Run tests command: | mkdir -p test-results - TEST_FILES=$(circleci tests glob "tests/agent_tests/**/test_*.py" | grep -v "^tests/agent_tests/local_only_agent_tests/") + TEST_FILES=$(circleci tests glob "tests/agent_tests/test_*.py") echo "$TEST_FILES" | circleci tests run \ --verbose \ --command="tr ' ' '\\n' | awk '/\\.py/ {print; next} {sub(/\\.[A-Z][^.]*$/, \"\"); gsub(/\\./, \"/\"); print \$0 \".py\"}' | xargs uv run --no-sync python -m pytest \ diff --git a/.github/ci-coverage-allowlist.yml b/.github/ci-coverage-allowlist.yml index 1423228e725..ccc21512e79 100644 --- a/.github/ci-coverage-allowlist.yml +++ b/.github/ci-coverage-allowlist.yml @@ -116,6 +116,14 @@ test_paths: - tests/load_tests/test_otel_load_test.py - tests/load_tests/test_vertex_embeddings_load_test.py - tests/load_tests/test_vertex_load_tests.py + - reason: >- + A local-only agent rig: test_a2a_completion_bridge.py needs a LangGraph server on + localhost:2024 and test_a2a.py drives a live A2A endpoint, so neither can run in a + pull request job. Until 2026-08-20 the CircleCI agent job hid them behind a grep -v + that this census could not see; the glob now excludes them structurally and this entry + is the decision on the record. Revisit when the A2A bridge gets a recorded-wire fixture + paths: + - tests/agent_tests/local_only_agent_tests - reason: >- Third-party integration tests that skip themselves without OCI configuration or sandbox credentials, neither of which a pull request job holds diff --git a/.github/workflows/auto_update_price_and_context_window_file.py b/.github/scripts/auto_update_price_and_context_window_file.py similarity index 100% rename from .github/workflows/auto_update_price_and_context_window_file.py rename to .github/scripts/auto_update_price_and_context_window_file.py diff --git a/.github/workflows/run_llm_translation_tests.py b/.github/scripts/run_llm_translation_tests.py similarity index 100% rename from .github/workflows/run_llm_translation_tests.py rename to .github/scripts/run_llm_translation_tests.py diff --git a/.github/scripts/triage_rollout_heads_up.py b/.github/scripts/triage_rollout_heads_up.py deleted file mode 100644 index a5dedb1c9e7..00000000000 --- a/.github/scripts/triage_rollout_heads_up.py +++ /dev/null @@ -1,557 +0,0 @@ -#!/usr/bin/env python3 -"""One-shot 7-day heads-up sweep for the Agent Shin rollout. - -Posts a friendly "the OSS triage bot kicks in next Monday" comment on every -open external PR/issue that currently *would* fail the new rubric — i.e., -every PR/issue Agent Shin would close once the rollout completes. The point -is to give contributors a full week to fix their description before the bot -ever takes a destructive action, so nobody is surprised by an auto-close. - -The script is designed to run **exactly once** at rollout, fired by a manual -``workflow_dispatch`` (``dry_run=false``) on the heads-up workflow. Re-runs -are safe: every comment is stamped with the hidden ``HEADS_UP_MARKER`` and -PRs/issues that already carry the marker are skipped. - -Dry-run vs. real run --------------------- -Defaults to dry-run. Passing ``--close`` flips into real mode. Every GitHub -mutation goes through ``_agent_shin_actions``, which has a one-line -``if dry_run: log else: do_it`` per call, so the only difference between a -dry-run preview and the real run is the call site that actually hits the -GitHub API. - -Local preview:: - - python3 .github/scripts/triage_rollout_heads_up.py --repo BerriAI/litellm - -Real run (the manual rollout dispatch uses this):: - - python3 .github/scripts/triage_rollout_heads_up.py --repo BerriAI/litellm --close -""" - -from __future__ import annotations - -import argparse -import datetime as dt -import json -import os -import sys -from pathlib import Path -from typing import Any - -# Make the sibling triage_with_llm + _agent_shin_actions importable when this -# script is invoked directly (the GitHub workflow does `python3 .github/scripts/...`). -_SCRIPTS_DIR = Path(__file__).resolve().parent -if str(_SCRIPTS_DIR) not in sys.path: - sys.path.insert(0, str(_SCRIPTS_DIR)) - -from _agent_shin_actions import maybe_post_comment # noqa: E402 -from agent_shin_shared import ( # noqa: E402 - AGENT_SHIN_DEFAULT_BOT_LOGIN, - ALLOWLIST_LOGINS, - list_open_items, -) -from triage_with_llm import ( # noqa: E402 - DEFAULT_MODEL, - call_llm_judge, - fetch_issue, - fetch_pr, - gh, - is_internal_contributor, - review_gate, - triage, -) - -# Hidden marker so re-runs skip PRs/issues we've already notified. Distinct from -# the within-grace / ready / regressed markers so it can't be confused with the -# steady-state lifecycle comments. -HEADS_UP_MARKER = "" - -# Placeholder until the litellm-docs PR ships. The rollout blog post explains -# the new rubric, the 7-day grace, and how to recover after an auto-close. -# TODO(docs): replace with the canonical URL once the litellm-docs PR merges. -ROLLOUT_BLOG_URL = "https://docs.litellm.ai/docs/agent_shin_triage_rollout" - -# Default cutoff is one week from "now". Computed at runtime so the wording -# stays correct even if the rollout is merged later than planned. The user can -# override with --close-on YYYY-MM-DD when running the script manually. -DEFAULT_GRACE_DAYS = 7 - -# The daily auto-close sweeps (close_low_quality_prs.yml at 09:00 UTC and -# review_gate.yml at 09:30 UTC) are what actually close a still-failing item, -# so the deadline we promise contributors has to name that wall-clock moment. -ACTIVATION_TIME_UTC = "09:00 UTC" - - -def _format_cutoff(cutoff: dt.date) -> str: - """Human-readable, timezone-explicit cutoff, e.g. ``Monday, June 1, 2026 - (09:00 UTC)`` — the moment a still-failing PR/issue gets closed.""" - return ( - f"{cutoff.strftime('%A, %B')} {cutoff.day}, {cutoff.year} " - f"({ACTIVATION_TIME_UTC})" - ) - - -def _rubric_section_pr() -> str: - return ( - "**Going forward, every external PR needs ONE of:**\n" - "\n" - "- A linked GitHub issue using a closing keyword: " - "`Fixes #1234`, `Closes #1234`, or `Resolves #1234`, OR\n" - "- All three of: a clear **problem description**, **expected vs. " - "actual behavior**, and **end-to-end QA proof** (at least one of a " - "short screen recording / video, before/after screenshots, or the " - "exact commands you ran with their real output; mocked or stubbed " - "runs don't count).\n" - "\n" - "PRs also need a **Greptile confidence score of 4/5 or higher** before " - "the bot will tag them `ready for review`. You can `@greptileai` to " - "request a fresh review at any time, including after the PR is closed." - ) - - -def _rubric_section_issue() -> str: - return ( - "**Going forward, every external issue needs:**\n" - "\n" - "- For **bug reports**: end-to-end evidence of the bug (at least one " - "of a screen recording / video, a screenshot, or the exact commands " - "you ran with their real output / traceback) plus expected vs. actual " - "behavior. Written steps with no run output don't count, and mocked " - "or stubbed runs don't count.\n" - "- For **feature requests**: a clear description of the proposed " - "feature plus a use case + concrete example (config, API call, UI " - "flow, or scenario showing what's blocked today)." - ) - - -def _description_only_note(kind: str) -> str: - noun = "PR" if kind == "pr" else "issue" - return ( - f"⚠️ **The requirements must live in the {noun} *description*, not in " - "comments.** Some PRs/issues collect 100+ comments from humans and " - "bots; reading the entire thread on every triage run would balloon " - "GitHub API usage (we'd start getting 429'd) and blow out the LLM " - "judge's context. The bot only reads the description, so anything " - "you add as a comment will be invisible to it." - ) - - -def _missing_section(verdict: dict, greptile_score: int | None) -> str: - """Bullet list of what's currently missing on this PR/issue. - - Combines the LLM judge's `missing` list (rubric items) with a Greptile - shortfall (for PRs) so the contributor sees one list of things to fix. - """ - missing = list(verdict.get("missing") or []) - if greptile_score is not None and greptile_score < 4: - missing.insert( - 0, - f"Greptile's most recent review scored this PR {greptile_score}/5 " - "(below the 4/5 bar Agent Shin will require).", - ) - if not missing: - return ( - "_The bot couldn't articulate a specific missing piece; see the " - "rubric link above and double-check the description includes all " - "of it before the rollout._" - ) - bullets = "\n".join(f"- {m}" for m in missing) - return f"**What this one is currently missing:**\n\n{bullets}" - - -def _recovery_section(kind: str) -> str: - if kind == "pr": - return ( - "**If the bot closes this PR after the rollout:** update the " - "description with the missing pieces, then either open a fresh " - "PR or comment `@agent-shin reconsider` on the closed PR. If " - "Greptile re-scores you at 4/5 or higher I'll reopen and tag " - "the PR `ready for review`. (`@greptileai` works on closed PRs " - "too; a fresh review is one of the signals that lifts you back " - "into the queue.) This is **not** us losing interest in your " - "change; far from it. We just need open PRs to be a list of " - "things a maintainer can act on, so we can get to yours faster." - ) - return ( - "**If the bot closes this issue after the rollout:** edit the issue " - "description to add the missing pieces, then comment `@agent-shin " - "reconsider` on the closed issue. I'll re-evaluate and, if the rubric " - "is met, reopen it. (GitHub doesn't let external authors reopen an " - "issue a maintainer or bot closed, so the comment is the reliable " - "path.) This is **not** us saying the bug isn't real or the request " - "isn't useful; it's so the remaining open issues are a list of things " - "a maintainer can act on." - ) - - -def format_heads_up_comment( - *, kind: str, verdict: dict, greptile_score: int | None, cutoff: dt.date -) -> str: - """Compose the friendly 7-day heads-up comment posted on a failing PR/issue.""" - noun = "PR" if kind == "pr" else "issue" - rubric = _rubric_section_pr() if kind == "pr" else _rubric_section_issue() - cutoff_str = _format_cutoff(cutoff) - explanation = (verdict.get("explanation") or "").strip() - explanation_block = ( - f"> _(The judge's note for this one: {explanation})_\n\n" if explanation else "" - ) - - return ( - "🚅 **Heads-up: we're turning on the OSS triage bot in " - f"{DEFAULT_GRACE_DAYS} days, on {cutoff_str}.**\n" - "\n" - "We're rolling out **Agent Shin**, an LLM-as-judge triage bot for " - f"external {noun}s. Once it's live, the bot reads each open " - f"{noun}'s description, scores it against a small rubric, and " - f"auto-closes any {noun} that's missing the basics, with a single " - f"comment explaining what's missing and how to recover. Full " - f"context: [Agent Shin rollout blog post]({ROLLOUT_BLOG_URL}).\n" - "\n" - f"{rubric}\n" - "\n" - f"{_description_only_note(kind)}\n" - "\n" - f"{_missing_section(verdict, greptile_score)}\n" - "\n" - f"{explanation_block}" - "**Timeline (you have a week):**\n" - "\n" - f"- We turn the bot on in {DEFAULT_GRACE_DAYS} days, on " - f"**{cutoff_str}**. You have until then to update this {noun}'s " - "description with the missing pieces above.\n" - f"- If this {noun} still fails the rubric at **{cutoff_str}**, " - "we'll close it.\n" - f"- From then on the bot runs daily, and every {noun} that fails " - "the rubric gets a **2-hour lifetime**: one warning comment, then " - "auto-close 2 hours later.\n" - "\n" - f"{_recovery_section(kind)}\n" - "\n" - f"{HEADS_UP_MARKER}" - ) - - -def _list_open_numbers(repo: str, kind: str) -> list[int]: - """Return every open PR or issue number in ``repo``. - - Delegates to ``list_open_items`` so the full backlog is fetched (no cap) - and the `gh {pr,issue} list` invocation stays in one shared place. ``gh - issue list`` would include PRs, but ``list_open_items`` uses the dedicated - command per kind, so the two never mix. - """ - return [ - item["number"] for item in list_open_items(kind, repo=repo, fields="number") - ] - - -def _has_heads_up_marker(item: dict) -> bool: - """Cheap fast-path: check the PR/issue body itself for the marker. - - The marker is appended to the *comment* we post, not the body, so this - will only fire if the body literally contains the marker text. We still - do the comment-marker check separately below; this body check just lets - us short-circuit for PRs/issues that quote the marker for any reason. - """ - body = item.get("body") or "" - return HEADS_UP_MARKER in body - - -def _comments_have_marker(repo: str, number: int) -> bool: - """True if the bot already posted a comment carrying the marker. - - Used for idempotency: a re-run skips items the previous run notified. - Filters by author (matching the sibling marker-checks in - ``triage_with_llm._has_marker`` and - ``agent_shin_shared.seconds_since_latest_marker_comment``) so a - contributor who quotes the heads-up via GitHub's "Quote reply" — which - preserves HTML comments in the raw markdown — can't trick the - idempotency check into silently skipping a real heads-up. - - Comments live on the unified issues endpoint regardless of whether the - item is a PR or an issue, so no ``kind`` argument is required here. - """ - expected_login = ( - os.environ.get("AGENT_SHIN_BOT_LOGIN") or AGENT_SHIN_DEFAULT_BOT_LOGIN - ).lower() - raw = gh( - "api", - "--paginate", - f"repos/{repo}/issues/{number}/comments?per_page=100", - ) - for line in raw.splitlines(): - line = line.strip() - if not line: - continue - try: - payload = json.loads(line) - except json.JSONDecodeError: - continue - comments = payload if isinstance(payload, list) else [payload] - for comment in comments: - author = ((comment.get("user") or {}).get("login") or "").lower() - if author != expected_login: - continue - if HEADS_UP_MARKER in (comment.get("body") or ""): - return True - return False - - -def _evaluate_pr(*, repo: str, number: int, model: str, judge: Any = None) -> dict: - """Run the future PR rubric (review_gate) in dry-run and return the result.""" - return review_gate( - repo=repo, - number=number, - close=False, # we only want the verdict, never act here - model=model, - judge=judge, - ) - - -def _evaluate_issue(*, repo: str, number: int, model: str, judge: Any = None) -> dict: - """Run the future issue rubric (triage kind='issue') in dry-run.""" - return triage( - repo=repo, - kind="issue", - number=number, - close=False, - model=model, - judge=judge, - ) - - -def _would_be_closed(kind: str, result: dict) -> bool: - """True if the future triage would auto-close this PR/issue based on the - rubric (regardless of grace-period gating). - - For PRs we trust ``review_gate``'s ``passing`` field — it combines the LLM - verdict and the Greptile score. For issues we read the LLM verdict - directly. Both fields are ``None``/missing on skip paths - (skip-internal-author, skip-llm-error, etc.) where the future bot would - NOT close the item — those return False. - """ - if kind == "pr": - passing = result.get("passing") - if passing is None: - return False # skipped — nothing for the heads-up to warn about - return passing is False - verdict = result.get("verdict") or {} - return (verdict.get("verdict") or "").lower() == "fail" - - -def _process_one( - *, - repo: str, - kind: str, - number: int, - model: str, - cutoff: dt.date, - dry_run: bool, - judge: Any = None, - skip_marker_check: bool = False, - allowlist: frozenset[str] = ALLOWLIST_LOGINS, -) -> dict: - """Evaluate one PR/issue and post a heads-up if it would be auto-closed. - - Returns a per-item dict for the summary table. - """ - base = {"kind": kind, "number": number} - fetcher = fetch_pr if kind == "pr" else fetch_issue - item = fetcher(repo, number) - - if (item.get("state") or "") != "open": - return {**base, "action": "skip-not-open"} - if allowlist: - login = (item.get("user") or {}).get("login") or "" - if login.lower() not in allowlist: - return {**base, "action": "skip-not-allowlisted"} - elif is_internal_contributor(item): - return {**base, "action": "skip-internal-author"} - if not skip_marker_check and _has_heads_up_marker(item): - return {**base, "action": "skip-already-marked-in-body"} - if not skip_marker_check and _comments_have_marker(repo, number): - return {**base, "action": "skip-already-notified"} - - if kind == "pr": - result = _evaluate_pr(repo=repo, number=number, model=model, judge=judge) - else: - result = _evaluate_issue(repo=repo, number=number, model=model, judge=judge) - - if not _would_be_closed(kind, result): - return {**base, "action": "skip-passing", "evaluator": result.get("action")} - - verdict = result.get("verdict") or {} - greptile_score = result.get("greptile_score") if kind == "pr" else None - comment = format_heads_up_comment( - kind=kind, verdict=verdict, greptile_score=greptile_score, cutoff=cutoff - ) - maybe_post_comment(repo, number, comment, dry_run=dry_run) - return { - **base, - "action": "heads-up-posted" if not dry_run else "would-post-heads-up", - "verdict": (verdict.get("verdict") or "").lower(), - "greptile_score": greptile_score, - } - - -def _print_summary(results: list[dict]) -> None: - """Tally per-action counts so a dry-run preview tells you at a glance how - many comments the real run would post.""" - counts: dict[str, int] = {} - for r in results: - counts[r["action"]] = counts.get(r["action"], 0) + 1 - print("\n=== rollout heads-up summary ===") - for action in sorted(counts): - print(f" {action:35s} {counts[action]}") - print(f" total {len(results)}") - - -def run( - *, - repo: str, - close: bool, - cutoff: dt.date, - model: str, - kinds: tuple[str, ...] = ("pr", "issue"), - judge: Any = None, - only_numbers: dict[str, list[int]] | None = None, - skip_marker_check: bool = False, -) -> list[dict]: - """Sweep ``repo`` and post heads-up comments. Returns the per-item results.""" - dry_run = not close - if dry_run: - print( - f"[DRY RUN] sweeping {repo}; --close not passed, no comments will be posted." - ) - else: - print(f"[REAL RUN] sweeping {repo}; comments WILL be posted.") - print(f"Cutoff date in comment body: {cutoff.isoformat()}") - - results: list[dict] = [] - for kind in kinds: - if only_numbers and kind in only_numbers: - numbers = list(only_numbers[kind]) - else: - numbers = _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, - cutoff=cutoff, - dry_run=dry_run, - judge=judge, - skip_marker_check=skip_marker_check, - ) - except ( - Exception - ) as exc: # noqa: BLE001 - per-item errors don't abort the sweep - result = { - "kind": kind, - "number": n, - "action": "error", - "error": str(exc), - } - print(f"!! {kind}#{n}: {exc}", file=sys.stderr) - print(f" {kind}#{n}: {result['action']}") - results.append(result) - _print_summary(results) - 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. Without this flag the script is in " - "dry-run mode and only logs what it would do." - ), - ) - parser.add_argument( - "--close-on", - type=dt.date.fromisoformat, - default=None, - help=( - "Cutoff date shown in the heads-up comment as the rollout date " - f"(default: today + {DEFAULT_GRACE_DAYS} days)." - ), - ) - 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).", - ) - parser.add_argument( - "--ignore-existing-marker", - action="store_true", - help=( - "Re-post on PRs/issues that already carry the heads-up marker. " - "Useful for testing the comment wording on a known PR." - ), - ) - args = parser.parse_args() - - cutoff = args.close_on or ( - dt.datetime.now(dt.timezone.utc).date() + dt.timedelta(days=DEFAULT_GRACE_DAYS) - ) - - 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 - - # The script must NOT hit the LLM in dry-run if no key is set — we still - # want a useful preview that says "skip-no-llm-key" for items that would - # have been judged. Production runs require OPENAI_API_KEY. - if args.close and not os.environ.get("OPENAI_API_KEY"): - parser.error("OPENAI_API_KEY must be set for --close (real-run) mode.") - - run( - repo=args.repo, - close=args.close, - cutoff=cutoff, - model=args.model, - kinds=kinds, - only_numbers=only or None, - skip_marker_check=args.ignore_existing_marker, - ) - return 0 - - -if __name__ == "__main__": - sys.exit(main()) diff --git a/.github/workflows/auto_update_price_and_context_window.yml b/.github/workflows/auto_update_price_and_context_window.yml index d391c0bd6ce..7e40a860ee9 100644 --- a/.github/workflows/auto_update_price_and_context_window.yml +++ b/.github/workflows/auto_update_price_and_context_window.yml @@ -23,7 +23,7 @@ jobs: version: "0.10.9" - name: Update JSON Data run: | - uv run --frozen --with 'aiohttp==3.13.3' python ".github/workflows/auto_update_price_and_context_window_file.py" + uv run --frozen --with 'aiohttp==3.13.3' python ".github/scripts/auto_update_price_and_context_window_file.py" - name: Regenerate JSON Schema run: | uv run --frozen python ci_cd/generate_model_prices_schema.py diff --git a/.github/workflows/triage_rollout_heads_up.yml b/.github/workflows/triage_rollout_heads_up.yml deleted file mode 100644 index 903960151e2..00000000000 --- a/.github/workflows/triage_rollout_heads_up.yml +++ /dev/null @@ -1,92 +0,0 @@ -name: Agent Shin — rollout heads-up (one-shot) - -# Fires the 7-day heads-up comment on every open external PR/issue that the -# new triage bot would auto-close. The real sweep is a deliberate one-shot: -# trigger it at rollout via a manual `workflow_dispatch` with `dry_run=false`. -# The script is idempotent (skips items that already carry the -# `` marker), so a re-run is harmless. -# -# The automatic push trigger runs DRY-RUN only, so merging the script to -# `litellm_internal_staging` never posts a comment; it just confirms the -# workflow is wired up. Posting real comments requires the manual dispatch, -# which is also the only trigger that exposes `OPENAI_API_KEY`. The heads-up -# is intentionally NOT gated on `AGENT_SHIN_ENABLED`: it has to warn -# contributors while that flag is still off, ahead of the flip that turns on -# auto-closing. -# -# The workflow is a thin shell over `.github/scripts/triage_rollout_heads_up.py`. -# Dry-run vs. real run differ in EXACTLY one CLI flag (`--close`), added only -# on a manual dispatch with `dry_run=false`. - -on: - push: - branches: - - litellm_internal_staging - paths: - # The presence of this script on staging IS the rollout merge marker. - # Editing the file later would re-fire the workflow; that's safe because - # the script skips PRs/issues that already have the heads-up marker. - - ".github/scripts/triage_rollout_heads_up.py" - workflow_dispatch: - inputs: - dry_run: - description: "Dry run (true = preview only, false = actually post comments)." - required: false - default: "true" - type: choice - options: - - "true" - - "false" - -permissions: - contents: read - issues: write - pull-requests: write - -jobs: - heads-up: - 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 --require-hashes -r .github/scripts/triage-requirements.txt - - - name: Run heads-up sweep - env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - # Only the manual dispatch (the real-run trigger) needs the LLM key. - # The automatic push trigger runs dry-run and never posts, so it gets - # no key. Mirrors the sibling triage workflows, which expose the key - # only on an enabled/dispatched run rather than unconditionally. - OPENAI_API_KEY: ${{ github.event_name == 'workflow_dispatch' && secrets.OPENAI_API_KEY || '' }} - OPENAI_BASE_URL: ${{ vars.OPENAI_BASE_URL }} - TRIAGE_MODEL: ${{ vars.TRIAGE_MODEL }} - # The real run is a deliberate manual dispatch with dry_run=false. - # Use the EXACT "false" comparison so any unexpected input value - # fail-closes to dry-run (mirrors the AGENT_SHIN_ENABLED pattern in - # the sibling workflows). The automatic push trigger always stays - # dry-run, so merging the script never posts. - DRY_RUN_INPUT: ${{ github.event.inputs.dry_run }} - run: | - set -euo pipefail - ARGS=(--repo "${{ github.repository }}") - if [ "${GITHUB_EVENT_NAME:-}" = "workflow_dispatch" ] && [ "${DRY_RUN_INPUT:-true}" = "false" ]; then - ARGS+=(--close) - echo "::notice::Manual rollout dispatch with dry_run=false -> heads-up comments WILL be posted." - elif [ "${GITHUB_EVENT_NAME:-}" = "workflow_dispatch" ]; then - echo "::notice::Manual dispatch in dry-run mode -> previewing only, no comments will be posted." - else - echo "::notice::Automatic push trigger -> dry-run preview only. Fire the real rollout sweep with a manual workflow_dispatch (dry_run=false)." - fi - python3 .github/scripts/triage_rollout_heads_up.py "${ARGS[@]}" diff --git a/Makefile b/Makefile index 5e5f7c80027..6c125268678 100644 --- a/Makefile +++ b/Makefile @@ -314,7 +314,7 @@ test-unit-helm: install-helm-unittest # LLM Translation testing targets test-llm-translation: install-test-deps @echo "Running LLM translation tests..." - @python .github/workflows/run_llm_translation_tests.py + @python .github/scripts/run_llm_translation_tests.py test-llm-translation-single: install-test-deps @echo "Running single LLM translation test file..." diff --git a/tests/scim_tests/scim_e2e_test.json b/tests/scim_tests/scim_e2e_test.json deleted file mode 100644 index bc5810762da..00000000000 --- a/tests/scim_tests/scim_e2e_test.json +++ /dev/null @@ -1,750 +0,0 @@ -{ - "version": "1.0", - "exported_at": 1715608731, - "name": "Okta SCIM 2.0 SPEC Test", - "description": "Basic tests to see if your SCIM server will work with Okta", - "trigger_url": "https://api.runscope.com/radar/37d9f10e-e250-4071-9cec-1fa30e56b42b/trigger", - "steps": [ - { - "step_type": "request", - "skipped": false, - "note": "Required Test: Test Users endpoint", - "auth": {}, - "multipart_form": [], - "headers": { - "Accept-Charset": [ - "utf-8" - ], - "Content-Type": [ - "application/scim+json; charset=utf-8" - ], - "Accept": [ - "application/scim+json" - ], - "Authorization": [ - "{{auth}}" - ], - "User-Agent": [ - "OKTA SCIM Integration" - ] - }, - "method": "GET", - "url": "{{SCIMBaseURL}}/Users?count=1&startIndex=1", - "assertions": [ - { - "comparison": "equal_number", - "source": "response_status", - "value": "200" - }, - { - "comparison": "not_empty", - "source": "response_json", - "value": null, - "property": "Resources" - }, - { - "comparison": "has_value", - "source": "response_json", - "value": "urn:ietf:params:scim:api:messages:2.0:ListResponse", - "property": "schemas" - }, - { - "comparison": "is_a_number", - "source": "response_json", - "value": null, - "property": "itemsPerPage" - }, - { - "comparison": "is_a_number", - "source": "response_json", - "value": null, - "property": "startIndex" - }, - { - "comparison": "is_a_number", - "source": "response_json", - "value": null, - "property": "totalResults" - }, - { - "comparison": "not_empty", - "source": "response_json", - "value": null, - "property": "Resources[0].id" - }, - { - "comparison": "not_empty", - "source": "response_json", - "value": null, - "property": "Resources[0].name.familyName" - }, - { - "comparison": "not_empty", - "source": "response_json", - "value": null, - "property": "Resources[0].name.givenName" - }, - { - "comparison": "not_empty", - "source": "response_json", - "value": null, - "property": "Resources[0].userName" - }, - { - "comparison": "not_empty", - "source": "response_json", - "value": null, - "property": "Resources[0].active" - }, - { - "comparison": "not_empty", - "source": "response_json", - "value": null, - "property": "Resources[0].emails[0].value" - } - ], - "variables": [ - { - "source": "response_json", - "name": "ISVUserid", - "property": "Resources[0].id" - } - ], - "scripts": [], - "before_scripts": [] - }, - { - "step_type": "pause", - "skipped": false, - "duration": 5 - }, - { - "step_type": "request", - "skipped": false, - "note": "Required Test: Get Users/{{id}} ", - "auth": {}, - "multipart_form": [], - "headers": { - "Accept-Charset": [ - "utf-8" - ], - "Content-Type": [ - "application/scim+json; charset=utf-8" - ], - "Accept": [ - "application/scim+json" - ], - "Authorization": [ - "{{auth}}" - ], - "User-Agent": [ - "OKTA SCIM Integration" - ] - }, - "method": "GET", - "url": "{{SCIMBaseURL}}/Users/{{ISVUserid}}", - "assertions": [ - { - "comparison": "equal_number", - "source": "response_status", - "value": "200" - }, - { - "comparison": "not_empty", - "source": "response_json", - "value": null, - "property": "id" - }, - { - "comparison": "not_empty", - "source": "response_json", - "value": null, - "property": "name.familyName" - }, - { - "comparison": "not_empty", - "source": "response_json", - "value": null, - "property": "name.givenName" - }, - { - "comparison": "not_empty", - "source": "response_json", - "value": null, - "property": "userName" - }, - { - "comparison": "not_empty", - "source": "response_json", - "value": null, - "property": "active" - }, - { - "comparison": "not_empty", - "source": "response_json", - "value": null, - "property": "emails[0].value" - }, - { - "comparison": "equal", - "source": "response_json", - "value": "{{ISVUserid}}", - "property": "id" - } - ], - "variables": [], - "scripts": [], - "before_scripts": [] - }, - { - "step_type": "pause", - "skipped": false, - "duration": 5 - }, - { - "step_type": "request", - "skipped": false, - "note": "Required Test: Test invalid User by username", - "auth": {}, - "multipart_form": [], - "headers": { - "Accept-Charset": [ - "utf-8" - ], - "Content-Type": [ - "application/scim+json; charset=utf-8" - ], - "Accept": [ - "application/scim+json" - ], - "Authorization": [ - "{{auth}}" - ], - "User-Agent": [ - "OKTA SCIM Integration" - ] - }, - "method": "GET", - "url": "{{SCIMBaseURL}}/Users?filter=userName eq \"{{InvalidUserEmail}}\"", - "assertions": [ - { - "comparison": "equal_number", - "source": "response_status", - "value": "200" - }, - { - "comparison": "has_value", - "source": "response_json", - "value": "urn:ietf:params:scim:api:messages:2.0:ListResponse", - "property": "schemas" - }, - { - "comparison": "equal", - "source": "response_json", - "value": "0", - "property": "totalResults" - } - ], - "variables": [], - "scripts": [], - "before_scripts": [] - }, - { - "step_type": "pause", - "skipped": false, - "duration": 5 - }, - { - "step_type": "request", - "skipped": false, - "note": "Required Test: Test invalid User by ID", - "auth": {}, - "multipart_form": [], - "headers": { - "Accept-Charset": [ - "utf-8" - ], - "Content-Type": [ - "application/scim+json; charset=utf-8" - ], - "Authorization": [ - "{{auth}}" - ], - "Accept": [ - "application/scim+json" - ], - "User-Agent": [ - "OKTA SCIM Integration" - ] - }, - "method": "GET", - "url": "{{SCIMBaseURL}}/Users/{{UserIdThatDoesNotExist}}", - "assertions": [ - { - "comparison": "equal_number", - "source": "response_status", - "value": "404" - }, - { - "comparison": "not_empty", - "source": "response_json", - "value": null, - "property": "detail" - }, - { - "comparison": "has_value", - "source": "response_json", - "value": "urn:ietf:params:scim:api:messages:2.0:Error", - "property": "schemas" - } - ], - "variables": [], - "scripts": [], - "before_scripts": [] - }, - { - "step_type": "pause", - "skipped": false, - "duration": 5 - }, - { - "step_type": "request", - "skipped": false, - "note": "Required Test: Make sure random user doesn't exist", - "auth": {}, - "multipart_form": [], - "headers": { - "Accept-Charset": [ - "utf-8" - ], - "Content-Type": [ - "application/scim+json; charset=utf-8" - ], - "Authorization": [ - "{{auth}}" - ], - "Accept": [ - "application/scim+json" - ], - "User-Agent": [ - "OKTA SCIM Integration" - ] - }, - "method": "GET", - "url": "{{SCIMBaseURL}}/Users?filter=userName eq \"{{randomEmail}}\"", - "assertions": [ - { - "comparison": "equal_number", - "source": "response_status", - "value": "200" - }, - { - "comparison": "equal_number", - "source": "response_json", - "value": "0", - "property": "totalResults" - }, - { - "comparison": "has_value", - "source": "response_json", - "value": "urn:ietf:params:scim:api:messages:2.0:ListResponse", - "property": "schemas" - } - ], - "variables": [], - "scripts": [], - "before_scripts": [] - }, - { - "step_type": "pause", - "skipped": false, - "duration": 5 - }, - { - "step_type": "request", - "skipped": false, - "note": "Required Test: Create Okta user with realistic values", - "auth": {}, - "body": "{\"schemas\":[\"urn:ietf:params:scim:schemas:core:2.0:User\"],\"userName\":\"{{randomUsername}}\",\"name\":{\"givenName\":\"{{randomGivenName}}\",\"familyName\":\"{{randomFamilyName}}\"},\"emails\":[{\"primary\":true,\"value\":\"{{randomEmail}}\",\"type\":\"work\"}],\"displayName\":\"{{randomGivenName}} {{randomFamilyName}}\",\"active\":true}", - "form": {}, - "multipart_form": [], - "binary_body": null, - "headers": { - "Content-Type": [ - "application/json" - ], - "Authorization": [ - "{{auth}}" - ], - "Accept": [ - "application/scim+json; charset=utf-8" - ] - }, - "method": "POST", - "url": "{{SCIMBaseURL}}/Users", - "assertions": [ - { - "comparison": "equal_number", - "source": "response_status", - "value": "201" - }, - { - "comparison": "equal", - "source": "response_json", - "value": "true", - "property": "active" - }, - { - "comparison": "not_empty", - "source": "response_json", - "value": null, - "property": "id" - }, - { - "comparison": "equal", - "source": "response_json", - "value": "{{randomFamilyName}}", - "property": "name.familyName" - }, - { - "comparison": "equal", - "source": "response_json", - "value": "{{randomGivenName}}", - "property": "name.givenName" - }, - { - "comparison": "contains", - "source": "response_json", - "value": "urn:ietf:params:scim:schemas:core:2.0:User", - "property": "schemas" - }, - { - "comparison": "equal", - "source": "response_json", - "value": "{{randomUsername}}", - "property": "userName" - } - ], - "variables": [ - { - "source": "response_json", - "name": "idUserOne", - "property": "id" - }, - { - "source": "response_json", - "name": "randomUserEmail", - "property": "emails[0].value" - } - ], - "scripts": [ - "" - ], - "before_scripts": [] - }, - { - "step_type": "pause", - "skipped": false, - "duration": 5 - }, - { - "step_type": "request", - "skipped": false, - "note": "Required Test: Verify that user was created", - "auth": {}, - "multipart_form": [], - "headers": { - "Accept-Charset": [ - "utf-8" - ], - "Content-Type": [ - "application/scim+json; charset=utf-8" - ], - "Authorization": [ - "{{auth}}" - ], - "Accept": [ - "application/scim+json" - ], - "User-Agent": [ - "OKTA SCIM Integration" - ] - }, - "method": "GET", - "url": "{{SCIMBaseURL}}/Users/{{idUserOne}}", - "assertions": [ - { - "comparison": "equal_number", - "source": "response_status", - "value": "200" - }, - { - "comparison": "equal", - "source": "response_json", - "value": "{{randomUsername}}", - "property": "userName" - }, - { - "comparison": "equal", - "source": "response_json", - "value": "{{randomFamilyName}}", - "property": "name.familyName" - }, - { - "comparison": "equal", - "source": "response_json", - "value": "{{randomGivenName}}", - "property": "name.givenName" - } - ], - "variables": [], - "scripts": [], - "before_scripts": [] - }, - { - "step_type": "pause", - "skipped": false, - "duration": 10 - }, - { - "step_type": "request", - "skipped": false, - "note": "Required Test: Expect failure when recreating user with same values", - "auth": {}, - "body": "{\"schemas\":[\"urn:ietf:params:scim:schemas:core:2.0:User\"],\"userName\":\"{{randomUsername}}\",\"name\":{\"givenName\":\"{{randomGivenName}}\",\"familyName\":\"{{randomFamilyName}}\"},\"emails\":[{\"primary\":true,\"value\":\"{{randomUsername}}\",\"type\":\"work\"}],\"displayName\":\"{{randomGivenName}} {{randomFamilyName}}\",\"active\":true}", - "form": {}, - "multipart_form": [], - "binary_body": null, - "headers": { - "Content-Type": [ - "application/json" - ], - "Authorization": [ - "{{auth}}" - ], - "Accept": [ - "application/scim+json; charset=utf-8" - ] - }, - "method": "POST", - "url": "{{SCIMBaseURL}}/Users", - "assertions": [ - { - "comparison": "equal_number", - "source": "response_status", - "value": "409" - } - ], - "variables": [], - "scripts": [], - "before_scripts": [] - }, - { - "step_type": "pause", - "skipped": false, - "duration": 5 - }, - { - "step_type": "request", - "skipped": false, - "note": "Required Test: Username Case Sensitivity Check", - "auth": {}, - "multipart_form": [], - "headers": { - "Accept-Charset": [ - "utf-8" - ], - "Content-Type": [ - "application/scim+json; charset=utf-8" - ], - "Authorization": [ - "{{auth}}" - ], - "Accept": [ - "application/scim+json" - ], - "User-Agent": [ - "OKTA SCIM Integration" - ] - }, - "method": "GET", - "url": "{{SCIMBaseURL}}/Users?filter=userName eq \"{{randomUsernameCaps}}\"", - "assertions": [ - { - "comparison": "equal_number", - "source": "response_status", - "value": "200" - } - ], - "variables": [], - "scripts": [], - "before_scripts": [] - }, - { - "step_type": "pause", - "skipped": false, - "duration": 5 - }, - { - "step_type": "request", - "skipped": false, - "note": "Optional Test: Verify Groups endpoint", - "auth": {}, - "multipart_form": [], - "headers": { - "Accept-Charset": [ - "utf-8" - ], - "Content-Type": [ - "application/scim+json; charset=utf-8" - ], - "Accept": [ - "application/scim+json" - ], - "Authorization": [ - "{{auth}}" - ], - "User-Agent": [ - "OKTA SCIM Integration" - ] - }, - "method": "GET", - "url": "{{SCIMBaseURL}}/Groups", - "assertions": [ - { - "comparison": "equal_number", - "source": "response_status", - "value": "200" - }, - { - "comparison": "is_less_than", - "source": "response_time", - "value": "600" - } - ], - "variables": [], - "scripts": [ - "var data = JSON.parse(response.body);\nvar max = data.totalResults;\nvar res = data.Resources;\nvar exists = false;\n\nif (max === 0)\n\tassert(\"nogroups\", \"No Groups found in the endpoint\");\nelse if (max >= 1 && Array.isArray(res)) {\n exists = true;\n assert.ok(exists, \"Resources is of type Array\");\n\tlog(exists);\n}" - ], - "before_scripts": [] - }, - { - "step_type": "pause", - "skipped": false, - "duration": 5 - }, - { - "step_type": "request", - "skipped": false, - "note": "Required Test: Check status 401", - "multipart_form": [], - "headers": { - "Accept": [ - "application/scim+json" - ], - "Accept-Charset": [ - "utf-8" - ], - "Authorization": [ - "non-token" - ], - "Content-Type": [ - "application/scim+json; charset=utf-8" - ], - "User-Agent": [ - "OKTA SCIM Integration" - ] - }, - "auth": {}, - "method": "GET", - "url": "{{SCIMBaseURL}}/Users?filter=userName eq \"{{randomUsernameCaps}}\"", - "assertions": [ - { - "comparison": "equal_number", - "source": "response_status", - "value": "401" - }, - { - "comparison": "not_empty", - "source": "response_json", - "value": null, - "property": "detail" - }, - { - "comparison": "equal", - "source": "response_json", - "value": "401", - "property": "status" - }, - { - "comparison": "has_value", - "source": "response_json", - "value": "urn:ietf:params:scim:api:messages:2.0:Error", - "property": "schemas" - } - ], - "variables": [], - "scripts": [], - "before_scripts": [] - }, - { - "step_type": "pause", - "skipped": false, - "duration": 5 - }, - { - "step_type": "request", - "skipped": false, - "note": "Required Test: Check status 404", - "multipart_form": [], - "headers": { - "Accept": [ - "application/scim+json" - ], - "Accept-Charset": [ - "utf-8" - ], - "Authorization": [ - "{{auth}}" - ], - "Content-Type": [ - "application/scim+json; charset=utf-8" - ], - "User-Agent": [ - "OKTA SCIM Integration" - ] - }, - "auth": {}, - "method": "GET", - "url": "{{SCIMBaseURL}}/Users/00919288221112222", - "assertions": [ - { - "comparison": "equal_number", - "source": "response_status", - "value": "404" - }, - { - "comparison": "not_empty", - "source": "response_json", - "value": null, - "property": "detail" - }, - { - "comparison": "equal", - "source": "response_json", - "value": "404", - "property": "status" - }, - { - "comparison": "has_value", - "source": "response_json", - "value": "urn:ietf:params:scim:api:messages:2.0:Error", - "property": "schemas" - } - ], - "variables": [], - "scripts": [], - "before_scripts": [] - } - ] - } \ No newline at end of file diff --git a/tests/test_litellm/test_github_triage_workflows.py b/tests/test_litellm/test_github_triage_workflows.py index ec6e9fc2381..ef3ab8d25da 100644 --- a/tests/test_litellm/test_github_triage_workflows.py +++ b/tests/test_litellm/test_github_triage_workflows.py @@ -62,7 +62,6 @@ DESTRUCTIVE_GATE_ENV: dict[str, str] = { LLM_CLIENT_INSTALLER_WORKFLOWS = ( "triage_issue_with_llm.yml", "triage_reconsider.yml", - "triage_rollout_heads_up.yml", ) PINNED_INSTALL = "--require-hashes -r .github/scripts/triage-requirements.txt" @@ -206,49 +205,6 @@ def test_triage_requirements_are_fully_hash_pinned() -> None: ) -def _heads_up_run_step() -> dict: - workflow = _load_workflow("triage_rollout_heads_up.yml") - for step in workflow["jobs"]["heads-up"]["steps"]: - if isinstance(step.get("run"), str) and "triage_rollout_heads_up.py" in step["run"]: - return step - raise AssertionError("no run step invokes triage_rollout_heads_up.py") - - -def test_rollout_heads_up_push_trigger_never_posts() -> None: - """Merging the heads-up script to staging must stay inert: the automatic - push trigger only ever runs dry-run. The real one-shot sweep is a - deliberate manual `workflow_dispatch` with `dry_run=false`, the sole path - that adds `--close`. - - This guards the "inert by default" invariant for the one workflow that is - intentionally not gated on AGENT_SHIN_ENABLED (it has to warn contributors - before that flag flips on). A regression to auto-`--close`-on-push would - post real comments on every push that touches the script. - """ - run = _heads_up_run_step()["run"] - assert '"${GITHUB_EVENT_NAME:-}" = "workflow_dispatch"' in run, ( - "the real (--close) run must be a manual workflow_dispatch, not the automatic push trigger" - ) - assert '"${DRY_RUN_INPUT:-true}" = "false"' in run, ( - "the real run must require the dry_run input to be the exact string 'false' (fail-safe); any other value stays dry-run" - ) - assert run.count("ARGS+=(--close)") == 1, ( - "--close must appear once, inside the manual real-run branch; a second occurrence means the push path posts real comments on merge" - ) - - -def test_rollout_heads_up_key_is_dispatch_gated() -> None: - """OPENAI_API_KEY is exposed only on the manual dispatch (the real-run - trigger), never unconditionally. The sibling triage workflows gate the key - the same way; an unconditional `secrets.OPENAI_API_KEY` here would hand the - key to the automatic push run, which must stay a no-op dry-run preview. - """ - key_expr = (_heads_up_run_step().get("env") or {}).get("OPENAI_API_KEY", "") - assert "github.event_name == 'workflow_dispatch'" in key_expr, ( - f"OPENAI_API_KEY must be gated on workflow_dispatch so the automatic push trigger gets no key; found: {key_expr!r}" - ) - - def _reconsider_steps() -> list[dict]: workflow = _load_workflow("triage_reconsider.yml") return workflow["jobs"]["reconsider"]["steps"] diff --git a/tests/test_litellm/test_triage_rollout_heads_up.py b/tests/test_litellm/test_triage_rollout_heads_up.py deleted file mode 100644 index 535590fd2d6..00000000000 --- a/tests/test_litellm/test_triage_rollout_heads_up.py +++ /dev/null @@ -1,612 +0,0 @@ -"""Unit tests for the one-shot 7-day heads-up sweep. - -Exercises: - - * The ``_agent_shin_actions`` dry-run wrappers — each ``maybe_*`` helper - must call the real underlying mutation iff ``dry_run=False``, and log to - stdout otherwise. - * ``triage_rollout_heads_up._would_be_closed`` — the predicate that - decides "would the future bot close this?" for both PRs and issues. - * ``triage_rollout_heads_up._process_one`` — the per-item processor: - skip when state != open, skip internal authors, skip already-notified - items, post heads-up on failing items, leave passing items alone. - * ``triage_rollout_heads_up.run`` — the sweep loop end-to-end, in both - dry-run and real modes, with the comment-posting injected so we never - talk to GitHub. - -Every test stubs out ``gh()`` and the GitHub mutations; nothing in this file -ever shells out. -""" - -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" - - -@pytest.fixture(scope="module") -def triage_module(): - """Load triage_with_llm under its canonical name so the sibling modules - can `from triage_with_llm import ...`.""" - spec = importlib.util.spec_from_file_location( - "triage_with_llm", _SCRIPTS_DIR / "triage_with_llm.py" - ) - assert spec and spec.loader - module = importlib.util.module_from_spec(spec) - sys.modules["triage_with_llm"] = module - spec.loader.exec_module(module) - return module - - -@pytest.fixture(scope="module") -def actions_module(triage_module): - spec = importlib.util.spec_from_file_location( - "_agent_shin_actions", _SCRIPTS_DIR / "_agent_shin_actions.py" - ) - assert spec and spec.loader - module = importlib.util.module_from_spec(spec) - sys.modules["_agent_shin_actions"] = module - spec.loader.exec_module(module) - return module - - -@pytest.fixture(scope="module") -def heads_up_module(triage_module, actions_module): - spec = importlib.util.spec_from_file_location( - "triage_rollout_heads_up", _SCRIPTS_DIR / "triage_rollout_heads_up.py" - ) - assert spec and spec.loader - module = importlib.util.module_from_spec(spec) - sys.modules["triage_rollout_heads_up"] = module - spec.loader.exec_module(module) - return module - - -# --------------------------------------------------------------------------- -# _agent_shin_actions: the dry-run wrappers - - -class TestActionsDryRun: - """Each maybe_* helper must NOT hit GitHub in dry-run, and MUST hit it - in real mode. The whole rollout's safety story rests on this.""" - - def test_maybe_post_comment_dry_run_logs_only( - self, actions_module, triage_module, monkeypatch, capsys - ): - called = [] - monkeypatch.setattr( - triage_module, - "post_comment", - lambda *a, **k: called.append((a, k)), - ) - actions_module.maybe_post_comment("o/r", 7, "hello", dry_run=True) - assert called == [] - assert "[DRY RUN] comment o/r#7" in capsys.readouterr().out - - def test_maybe_post_comment_real_run_calls_through( - self, actions_module, triage_module, monkeypatch - ): - called = [] - monkeypatch.setattr( - triage_module, - "post_comment", - lambda repo, n, body: called.append((repo, n, body)), - ) - actions_module.maybe_post_comment("o/r", 7, "hello", dry_run=False) - assert called == [("o/r", 7, "hello")] - - -# --------------------------------------------------------------------------- -# _would_be_closed predicate - - -class TestWouldBeClosed: - def test_pr_passing_returns_false(self, heads_up_module): - assert ( - heads_up_module._would_be_closed( - "pr", {"passing": True, "action": "noop-passing"} - ) - is False - ) - - def test_pr_failing_returns_true(self, heads_up_module): - assert ( - heads_up_module._would_be_closed( - "pr", - { - "passing": False, - "action": "would-close", - "verdict": {"verdict": "fail"}, - }, - ) - is True - ) - - def test_pr_skipped_returns_false(self, heads_up_module): - # passing is None for skip paths (internal-author, llm-error, etc.) - assert ( - heads_up_module._would_be_closed("pr", {"action": "skip-internal-author"}) - is False - ) - - def test_issue_pass_returns_false(self, heads_up_module): - assert ( - heads_up_module._would_be_closed( - "issue", {"action": "pass-llm", "verdict": {"verdict": "pass"}} - ) - is False - ) - - def test_issue_fail_returns_true(self, heads_up_module): - assert ( - heads_up_module._would_be_closed( - "issue", {"action": "would-close", "verdict": {"verdict": "fail"}} - ) - is True - ) - - def test_issue_missing_verdict_returns_false(self, heads_up_module): - # Skip paths don't surface a verdict; treat as "won't close". - assert ( - heads_up_module._would_be_closed("issue", {"action": "skip-not-open"}) - is False - ) - - -# --------------------------------------------------------------------------- -# Comment formatter — wording sanity checks - - -class TestHeadsUpCommentBody: - def test_pr_comment_contains_cutoff_rubric_marker(self, heads_up_module): - body = heads_up_module.format_heads_up_comment( - kind="pr", - verdict={"verdict": "fail", "missing": ["QA proof"], "explanation": "thin"}, - greptile_score=3, - cutoff=dt.date(2026, 6, 1), - ) - assert "Monday, June 1, 2026" in body # cutoff readable - assert "09:00 UTC" in body # deadline is timezone-explicit - assert "we'll close it" in body # hard deadline, not a passive notice - assert "2-hour lifetime" in body # post-rollout steady state - assert "Greptile" in body and "3/5" in body # specific shortfall - assert "QA proof" in body # missing piece surfaced - assert "PR *description*" in body # description-only note - assert heads_up_module.HEADS_UP_MARKER in body # idempotency marker - - def test_issue_comment_uses_reconsider_recovery_path(self, heads_up_module): - # OSS authors can't reopen an issue the bot closed (read access only - # lets them reopen issues they closed themselves), so the heads-up - # recovery path is `@agent-shin reconsider`, not self-reopen. - body = heads_up_module.format_heads_up_comment( - kind="issue", - verdict={"verdict": "fail", "missing": ["repro"], "explanation": ""}, - greptile_score=None, - cutoff=dt.date(2026, 6, 1), - ) - assert "@agent-shin reconsider" in body - assert heads_up_module.HEADS_UP_MARKER in body - - def test_empty_missing_uses_fallback_copy(self, heads_up_module): - body = heads_up_module.format_heads_up_comment( - kind="pr", - verdict={"verdict": "fail", "missing": [], "explanation": ""}, - greptile_score=None, - cutoff=dt.date(2026, 6, 1), - ) - assert "couldn't articulate" in body - # Make sure the fallback didn't leave us with a broken sentence. - assert "specific missing piece" in body - - -# --------------------------------------------------------------------------- -# _process_one — per-item dispatch - - -def _stub_fetchers(heads_up_module, triage_module, *, item): - """Monkeypatch fetch_pr and fetch_issue (both in triage_with_llm and the - re-imported names in heads_up_module) to return `item`.""" - return [ - (triage_module, "fetch_pr", lambda repo, n: item), - (triage_module, "fetch_issue", lambda repo, n: item), - (heads_up_module, "fetch_pr", lambda repo, n: item), - (heads_up_module, "fetch_issue", lambda repo, n: item), - ] - - -class TestProcessOne: - """Per-item processing: the right skip reason fires for each scenario, - and the heads-up only goes out when the rubric is genuinely failing.""" - - @pytest.fixture - def patch_env(self, heads_up_module, triage_module, monkeypatch): - """Helper that returns a callable to install a PR/issue body, suppress - marker checks, and stub the comment poster.""" - posts = [] - monkeypatch.setattr( - heads_up_module, - "maybe_post_comment", - lambda repo, n, body, *, dry_run: posts.append((repo, n, body, dry_run)), - ) - monkeypatch.setattr(heads_up_module, "_has_heads_up_marker", lambda item: False) - monkeypatch.setattr( - heads_up_module, "_comments_have_marker", lambda repo, n: False - ) - - def _install(item): - for mod, name, fn in _stub_fetchers( - heads_up_module, triage_module, item=item - ): - monkeypatch.setattr(mod, name, fn) - - return _install, posts - - def test_skip_closed_pr(self, heads_up_module, patch_env): - install, posts = patch_env - install( - {"state": "closed", "user": {"login": "ext"}, "author_association": "NONE"} - ) - r = heads_up_module._process_one( - repo="o/r", - kind="pr", - number=7, - model="m", - cutoff=dt.date(2026, 6, 1), - dry_run=True, - ) - assert r["action"] == "skip-not-open" - assert posts == [] - - def test_skip_internal_pr(self, heads_up_module, patch_env): - install, posts = patch_env - install( - { - "state": "open", - "user": {"login": "krrishdholakia"}, - "author_association": "MEMBER", - "body": "", - "labels": [], - "created_at": "2026-05-25T00:00:00Z", - } - ) - r = heads_up_module._process_one( - repo="o/r", - kind="pr", - number=7, - model="m", - cutoff=dt.date(2026, 6, 1), - dry_run=True, - allowlist=frozenset(), - ) - assert r["action"] == "skip-internal-author" - assert posts == [] - - def test_skip_passing_pr(self, heads_up_module, patch_env, monkeypatch): - install, posts = patch_env - install( - { - "state": "open", - "user": {"login": "mateo-berri"}, - "author_association": "NONE", - "body": "Fixes #123 — clean fix with a passing rubric.", - "labels": [], - "created_at": "2026-05-25T00:00:00Z", - } - ) - monkeypatch.setattr( - heads_up_module, - "_evaluate_pr", - lambda **kwargs: { - "action": "noop-passing", - "passing": True, - "verdict": {"verdict": "pass"}, - "greptile_score": 5, - }, - ) - r = heads_up_module._process_one( - repo="o/r", - kind="pr", - number=7, - model="m", - cutoff=dt.date(2026, 6, 1), - dry_run=True, - ) - assert r["action"] == "skip-passing" - assert posts == [] - - def test_failing_pr_posts_heads_up_dry_run( - self, heads_up_module, patch_env, monkeypatch, capsys - ): - install, posts = patch_env - install( - { - "state": "open", - "user": {"login": "mateo-berri"}, - "author_association": "NONE", - "body": "thin", - "labels": [], - "created_at": "2026-05-25T00:00:00Z", - } - ) - monkeypatch.setattr( - heads_up_module, - "_evaluate_pr", - lambda **kwargs: { - "action": "would-close", - "passing": False, - "verdict": { - "verdict": "fail", - "missing": ["QA proof"], - "explanation": "PR body is one line.", - }, - "greptile_score": 3, - }, - ) - r = heads_up_module._process_one( - repo="o/r", - kind="pr", - number=7, - model="m", - cutoff=dt.date(2026, 6, 1), - dry_run=True, - ) - assert r["action"] == "would-post-heads-up" - assert posts == [("o/r", 7, posts[0][2], True)] # tuple shape preserved - assert "QA proof" in posts[0][2] - assert heads_up_module.HEADS_UP_MARKER in posts[0][2] - - def test_failing_issue_posts_heads_up_real_run( - self, heads_up_module, patch_env, monkeypatch - ): - install, posts = patch_env - install( - { - "state": "open", - "user": {"login": "mateo-berri"}, - "author_association": "NONE", - "body": "X is broken", - "labels": [], - "created_at": "2026-05-25T00:00:00Z", - } - ) - monkeypatch.setattr( - heads_up_module, - "_evaluate_issue", - lambda **kwargs: { - "action": "would-close", - "verdict": { - "verdict": "fail", - "missing": ["reproduction"], - "explanation": "too thin", - }, - }, - ) - r = heads_up_module._process_one( - repo="o/r", - kind="issue", - number=42, - model="m", - cutoff=dt.date(2026, 6, 1), - dry_run=False, - ) - assert r["action"] == "heads-up-posted" - assert len(posts) == 1 - _, n, _, dry = posts[0] - assert n == 42 and dry is False - - def test_already_notified_is_skipped(self, heads_up_module, patch_env, monkeypatch): - install, posts = patch_env - install( - { - "state": "open", - "user": {"login": "mateo-berri"}, - "author_association": "NONE", - "body": "thin", - "labels": [], - "created_at": "2026-05-25T00:00:00Z", - } - ) - # Override the marker check for this scenario only. - monkeypatch.setattr( - heads_up_module, "_comments_have_marker", lambda repo, n: True - ) - r = heads_up_module._process_one( - repo="o/r", - kind="pr", - number=7, - model="m", - cutoff=dt.date(2026, 6, 1), - dry_run=True, - ) - assert r["action"] == "skip-already-notified" - assert posts == [] - - def test_ignore_existing_marker_forces_post( - self, heads_up_module, patch_env, monkeypatch - ): - install, posts = patch_env - install( - { - "state": "open", - "user": {"login": "mateo-berri"}, - "author_association": "NONE", - "body": "thin", - "labels": [], - "created_at": "2026-05-25T00:00:00Z", - } - ) - monkeypatch.setattr( - heads_up_module, "_comments_have_marker", lambda repo, n: True - ) - monkeypatch.setattr( - heads_up_module, - "_evaluate_pr", - lambda **kwargs: { - "action": "would-close", - "passing": False, - "verdict": {"verdict": "fail", "missing": ["X"], "explanation": ""}, - "greptile_score": None, - }, - ) - r = heads_up_module._process_one( - repo="o/r", - kind="pr", - number=7, - model="m", - cutoff=dt.date(2026, 6, 1), - dry_run=True, - skip_marker_check=True, - ) - assert r["action"] == "would-post-heads-up" - - -# --------------------------------------------------------------------------- -# run() — sweep loop - - -class TestRun: - """End-to-end the sweep loop with a tiny fake repo: 1 passing PR, 1 - failing PR, 1 passing issue, 1 failing issue.""" - - @pytest.fixture - def configured(self, heads_up_module, triage_module, monkeypatch): - posts = [] - monkeypatch.setattr( - heads_up_module, - "maybe_post_comment", - lambda repo, n, body, *, dry_run: posts.append((n, dry_run, body)), - ) - monkeypatch.setattr(heads_up_module, "_has_heads_up_marker", lambda item: False) - monkeypatch.setattr( - heads_up_module, "_comments_have_marker", lambda repo, n: False - ) - - def fake_list(repo, kind): - return [1, 2] if kind == "pr" else [101, 102] - - monkeypatch.setattr(heads_up_module, "_list_open_numbers", fake_list) - - def make_item(login="mateo-berri"): - return { - "state": "open", - "user": {"login": login}, - "author_association": "NONE", - "body": "thin", - "labels": [], - "created_at": "2026-05-25T00:00:00Z", - } - - monkeypatch.setattr(heads_up_module, "fetch_pr", lambda repo, n: make_item()) - monkeypatch.setattr(heads_up_module, "fetch_issue", lambda repo, n: make_item()) - monkeypatch.setattr(triage_module, "fetch_pr", lambda repo, n: make_item()) - monkeypatch.setattr(triage_module, "fetch_issue", lambda repo, n: make_item()) - - def pr_eval(*, number, **kwargs): - if number == 1: - return { - "action": "noop-passing", - "passing": True, - "verdict": {"verdict": "pass"}, - } - return { - "action": "would-close", - "passing": False, - "verdict": {"verdict": "fail", "missing": ["m"], "explanation": ""}, - "greptile_score": 2, - } - - def issue_eval(*, number, **kwargs): - if number == 101: - return {"action": "pass-llm", "verdict": {"verdict": "pass"}} - return { - "action": "would-close", - "verdict": {"verdict": "fail", "missing": ["repro"], "explanation": ""}, - } - - monkeypatch.setattr(heads_up_module, "_evaluate_pr", pr_eval) - monkeypatch.setattr(heads_up_module, "_evaluate_issue", issue_eval) - return posts - - def test_dry_run_posts_nothing_but_logs_both_would_posts( - self, heads_up_module, configured, capsys - ): - results = heads_up_module.run( - repo="o/r", - close=False, - cutoff=dt.date(2026, 6, 1), - model="m", - ) - actions = [r["action"] for r in results] - assert actions.count("would-post-heads-up") == 2 - assert actions.count("skip-passing") == 2 - assert all(dry for _, dry, _ in configured) # every post was dry-run - - def test_real_run_posts_two_comments(self, heads_up_module, configured): - results = heads_up_module.run( - repo="o/r", - close=True, - cutoff=dt.date(2026, 6, 1), - model="m", - ) - assert sum(1 for r in results if r["action"] == "heads-up-posted") == 2 - # Two real-run posts: one failing PR (#2), one failing issue (#102). - real_posts = [n for n, dry, _ in configured if dry is False] - assert sorted(real_posts) == [2, 102] - - def test_kinds_filter_skips_issues(self, heads_up_module, configured): - results = heads_up_module.run( - repo="o/r", - close=False, - cutoff=dt.date(2026, 6, 1), - model="m", - kinds=("pr",), - ) - assert {r["kind"] for r in results} == {"pr"} - - def test_only_numbers_restricts_sweep(self, heads_up_module, configured): - results = heads_up_module.run( - repo="o/r", - close=False, - cutoff=dt.date(2026, 6, 1), - model="m", - only_numbers={"pr": [2], "issue": [101]}, - ) - assert sorted((r["kind"], r["number"]) for r in results) == [ - ("issue", 101), - ("pr", 2), - ] - - -class TestListOpenNumbersNoCap: - """`_list_open_numbers` must sweep the WHOLE backlog, not a capped page. - - Regression guard: the rollout sweep is one-shot, so any item it misses - here never gets a heads-up before the bot starts auto-closing. - """ - - def test_delegates_to_list_open_items_with_no_cap( - self, heads_up_module, monkeypatch - ): - import agent_shin_shared - - captured: dict = {} - - def fake_gh(*args): - captured["args"] = args - return '[{"number": 5}, {"number": 9}]' - - monkeypatch.setattr(agent_shin_shared, "gh", fake_gh) - numbers = heads_up_module._list_open_numbers("o/r", "issue") - assert numbers == [5, 9] - args = captured["args"] - assert args[0] == "issue" - assert args[args.index("--limit") + 1] == str( - agent_shin_shared.GH_LIST_ALL_LIMIT - ) - assert "1000" not in args