diff --git a/.github/scripts/close_low_quality_prs.py b/.github/scripts/close_low_quality_prs.py index 95335aec167..43141a38a61 100644 --- a/.github/scripts/close_low_quality_prs.py +++ b/.github/scripts/close_low_quality_prs.py @@ -35,7 +35,7 @@ import json import re import subprocess import sys -from typing import Any, Iterable +from typing import Iterable # Greptile's GitHub App appears as `greptile-apps[bot]` in REST API comments # and `greptile-apps` in `gh pr view --json` output. Accept either form. @@ -54,6 +54,12 @@ SCORE_PATTERN = re.compile( # exempt from auto-triage. INTERNAL_AUTHOR_ASSOCIATIONS = frozenset({"OWNER", "MEMBER", "COLLABORATOR"}) +# Default labels that exempt a PR from auto-close. Defined at module scope (not +# as a mutable argparse default) so that `--optout-label foo` REPLACES the +# defaults instead of appending to them — the argparse `action="append"` + +# `default=[...]` combination silently mutates the shared default list. +DEFAULT_OPTOUT_LABELS = ("do not close", "keep open", "wip") + def gh(*args: str) -> str: """Run a `gh` CLI command and return stdout. Raises on non-zero exit.""" @@ -66,11 +72,6 @@ def gh(*args: str) -> str: return result.stdout -def gh_json(*args: str) -> Any: - """Run a `gh` CLI command that emits JSON and return the parsed value.""" - return json.loads(gh(*args)) - - def fetch_open_prs(repo: str | None) -> list[dict]: """Fetch all open PRs (number, createdAt, isDraft, labels, author).""" repo_args = ["--repo", repo] if repo else [] @@ -297,10 +298,13 @@ def main() -> int: parser.add_argument( "--optout-label", action="append", - default=["do not close", "keep open", "wip"], + default=None, help=( - "Label(s) that exempt a PR from auto-close. " - "Repeat to add more. Case-insensitive." + "Label(s) that exempt a PR from auto-close. Repeat to add more. " + "Case-insensitive. When omitted, defaults to " + f"{list(DEFAULT_OPTOUT_LABELS)!r}; passing this flag REPLACES the " + "defaults (argparse `append` with a mutable default would append " + "instead, which we explicitly avoid)." ), ) parser.add_argument( @@ -334,7 +338,7 @@ def main() -> int: print(f"Found {len(prs)} open PRs.\n") now = dt.datetime.now(dt.timezone.utc) - optout_labels = set(args.optout_label) + optout_labels = set(args.optout_label or DEFAULT_OPTOUT_LABELS) closed = 0 summary = { diff --git a/tests/test_litellm/test_github_close_low_quality_prs.py b/tests/test_litellm/test_github_close_low_quality_prs.py index 0d63562ec0f..6d1ad50012e 100644 --- a/tests/test_litellm/test_github_close_low_quality_prs.py +++ b/tests/test_litellm/test_github_close_low_quality_prs.py @@ -292,6 +292,84 @@ class TestEvaluatePr: assert score is None +class TestMainOptoutLabelDefault: + """`--optout-label` must REPLACE the canonical defaults, not append.""" + + def _patch_no_op(self, closer_module, monkeypatch): + monkeypatch.setattr(closer_module, "fetch_open_prs", lambda repo: []) + # `optout_labels` is captured indirectly via evaluate_pr; sniff the + # set passed in by stubbing evaluate_pr. + captured: dict = {} + + def fake_evaluate(pr, now, min_age_days, min_score, repo, optout_labels): + captured["optout_labels"] = set(optout_labels) + return ("skip-draft", None, None) + + monkeypatch.setattr(closer_module, "evaluate_pr", fake_evaluate) + return captured + + def test_should_use_canonical_defaults_when_flag_omitted( + self, closer_module, monkeypatch + ): + captured = self._patch_no_op(closer_module, monkeypatch) + # No PRs -> capture won't fire; instead inject one synthetic PR via + # fetch_open_prs so evaluate_pr is invoked at least once. + monkeypatch.setattr( + closer_module, + "fetch_open_prs", + lambda repo: [ + { + "number": 1, + "title": "p", + "createdAt": "2026-05-10T00:00:00Z", + "isDraft": True, + "labels": [], + "author": {"login": "x"}, + } + ], + ) + monkeypatch.setattr(sys, "argv", ["close_low_quality_prs.py"]) + rc = closer_module.main() + assert rc == 0 + assert captured["optout_labels"] == set(closer_module.DEFAULT_OPTOUT_LABELS) + + def test_should_replace_defaults_when_flag_provided( + self, closer_module, monkeypatch + ): + captured = self._patch_no_op(closer_module, monkeypatch) + monkeypatch.setattr( + closer_module, + "fetch_open_prs", + lambda repo: [ + { + "number": 1, + "title": "p", + "createdAt": "2026-05-10T00:00:00Z", + "isDraft": True, + "labels": [], + "author": {"login": "x"}, + } + ], + ) + monkeypatch.setattr( + sys, + "argv", + [ + "close_low_quality_prs.py", + "--optout-label", + "hold", + "--optout-label", + "needs-discussion", + ], + ) + rc = closer_module.main() + assert rc == 0 + # Crucially, none of the canonical defaults leak in. + assert captured["optout_labels"] == {"hold", "needs-discussion"} + for default in closer_module.DEFAULT_OPTOUT_LABELS: + assert default not in captured["optout_labels"], default + + class TestHasOptoutLabel: def test_should_match_label_case_insensitively(self, closer_module): pr = {"labels": [{"name": "Do Not Close"}, {"name": "bug"}]}