diff --git a/.github/workflows/ci-cla-quota-rerun.yml b/.github/workflows/ci-cla-quota-rerun.yml index e9eb61c0d549..be61b8461137 100644 --- a/.github/workflows/ci-cla-quota-rerun.yml +++ b/.github/workflows/ci-cla-quota-rerun.yml @@ -8,14 +8,23 @@ name: CLA quota re-run # on open pull requests into main and feat-cmux-next, at their current heads. # A re-run changes no policy: the same trusted workflow runs again. # +# A pass starts when a CLA run fails, and when one of this janitor's re-runs +# completes (attempt above 1), so a backlog drains pass by pass. The cron is +# only the fallback: GitHub fired it late or not at all (12:35Z to 14:19Z on +# 2026-10-26, while #25466 sat blocked for 80 minutes). The job reads nothing +# from the triggering run; it checks out this file's own branch. +# # The budget resets hourly and push-triggered workflows drain it again within -# minutes, so when the reset is under 15 minutes away a pass sleeps until just +# minutes, so when the reset is under 40 minutes away a pass sleeps until just # after it and runs then: one planned pass per reset, not a loop. Cost: free # rate_limit reads of GITHUB_TOKEN; reads and re-runs on the glaeda-route # App's own budget. Its own workflow and concurrency group (moved out of # ci-queue-janitor.yml), so a pass waiting for the reset never holds the # macOS sweep. on: + workflow_run: + workflows: ["CLA Assistant", "CLA policy guard"] + types: [completed] schedule: - cron: "*/10 * * * *" workflow_dispatch: @@ -28,17 +37,29 @@ on: permissions: {} -# One pass at a time; a pass waiting for the reset is never cancelled. -concurrency: - group: ci-cla-quota-rerun - cancel-in-progress: false +env: + # The files behind the workflow_run names above (tests/test_ci_workflow_run_sources.py). + SOURCE_WORKFLOW_PATHS: >- + .github/workflows/cla.yml + .github/workflows/cla-policy-guard.yml jobs: cla-quota-rerun: name: Re-run CLA checks the spent budget failed - if: vars.GLAEDA_ROUTE_APP_ID != '' + # From a CLA run: only a pull request's failure, or the completion of a re-run + # (this janitor's), starts a job; comment-triggered and first-attempt passes do not. + if: >- + vars.GLAEDA_ROUTE_APP_ID != '' && (github.event_name != 'workflow_run' || + (github.event.workflow_run.event == 'pull_request_target' && + (github.event.workflow_run.conclusion == 'failure' || github.event.workflow_run.run_attempt > 1))) runs-on: ${{ github.repository_owner != 'manaflow-ai' && 'ubuntu-24.04' || vars.LINUX_RUNNER || 'blacksmith-4vcpu-ubuntu-2404' }} - timeout-minutes: 22 + timeout-minutes: 47 + # One pass at a time; a pass waiting for the reset is never cancelled. Passes + # that arrive meanwhile collapse into one pending pass. Job level, so the many + # CLA runs that start no job (comments) never take the pending slot. + concurrency: + group: ci-cla-quota-rerun + cancel-in-progress: false continue-on-error: true permissions: contents: read @@ -66,10 +87,10 @@ jobs: sparse-checkout: scripts/ci/cla_quota_rerun.py sparse-checkout-cone-mode: false - # Waits at most WAIT_FOR_RESET (15 minutes) for a near reset, then one pass. + # Waits at most WAIT_FOR_RESET (40 minutes) for a near reset, then one pass. - name: Re-run CLA checks the spent budget failed if: steps.glaeda-token.outputs.token != '' - timeout-minutes: 19 + timeout-minutes: 44 env: GH_TOKEN: ${{ steps.glaeda-token.outputs.token }} RATE_TOKEN: ${{ github.token }} diff --git a/.github/workflows/ci-guards.yml b/.github/workflows/ci-guards.yml index 7c29dd326e8f..1b3a7e5a09b2 100644 --- a/.github/workflows/ci-guards.yml +++ b/.github/workflows/ci-guards.yml @@ -619,6 +619,10 @@ jobs: if: ${{ matrix.group == 'ci' }} run: python3 tests/test_ci_workflow_run_sources.py + - name: Validate the CLA quota re-run janitor + if: ${{ matrix.group == 'ci' }} + run: python3 tests/test_cla_quota_rerun.py + - name: Validate shallow complexity comparison fetching if: ${{ matrix.group == 'ci' }} run: python3 tests/test_ci_fetch_complexity_base.py diff --git a/scripts/ci/cla_quota_rerun.py b/scripts/ci/cla_quota_rerun.py index b5ddd40889b2..a147fd0bfee4 100644 --- a/scripts/ci/cla_quota_rerun.py +++ b/scripts/ci/cla_quota_rerun.py @@ -7,7 +7,9 @@ until someone re-runs them by hand, often while the budget is still spent. Both files are a trusted control plane: a change to either needs a trusted maintainer's review (scripts/ci/validate-cla-policy.rb), so the retry lives here instead. ci-cla-quota-rerun.yml -runs this every 10 minutes: +runs this when a CLA run fails (and when one of this janitor's re-runs completes, so a +backlog drains pass by pass), and every 10 minutes as a fallback: GitHub fires that cron +late or not at all (12:35Z to 14:19Z on 2026-10-26, while #25466 sat blocked for 80 min). 1. One `rate_limit` read of the workflow token's budget (free). Under the floor, a re-run now would fail the same way. When that budget resets within WAIT_FOR_RESET, the pass @@ -23,11 +25,15 @@ job's log says the API rate limit was exceeded. A CLA policy guard run reports every error, a spent budget included, as "rejected the proposed policy" (validate-cla-policy.rb rescues PolicyError and hides the reason), so a guard failure is re-run only with - evidence of an API failure: its own log shows a 403 or rate limit before the reject, - or a CLA Assistant job, on any pull request, logged "API rate limit exceeded for - installation" within - GUARD_WINDOW of it (the budget is the repository's, shared by both). A reject with - neither is never re-run: it is reported as a possible real policy rejection. + evidence of an API failure: its own log shows a 403 or rate limit before the reject; + or its pull request changes none of the paths the guard validates (POLICY_PATHS), so + it proposes no policy and a failed read is the likely reject (a retry heuristic, not + proof: a stale event base or an old policy with no usable test merge is rejected too, + which is why it holds only while the run is below NO_POLICY_MAX_ATTEMPT); or a CLA Assistant job, on any pull + request, logged "API rate limit exceeded for installation" within GUARD_WINDOW of it + (the budget is the repository's, shared by both). None of these needs the response's + x-ratelimit headers or a particular 403 wording. A reject with none of them is never + re-run: it is reported as a possible real policy rejection. A re-run changes no policy: the same trusted workflow runs again on the same head. Reads and re-runs use GH_TOKEN (the janitor's App token, its own budget); RATE_TOKEN is the workflow token @@ -55,13 +61,28 @@ # What a failed GitHub API read looks like in a log. "403" alone matches hashes and ids. API_FAILURE = re.compile(r"rate limit|\bHTTP 403\b|\b403 Forbidden\b|\(HTTP 403\)", re.IGNORECASE) GUARD_REJECT = "rejected the proposed policy" +# Every path validate-cla-policy.rb reads as policy (CLA_POLICY_PATHS and GUARD_PATHS there; +# tests/test_cla_quota_rerun.py keeps the two lists equal). A pull request that changes none +# of them proposes no policy: the guard passes it unless a GitHub read fails. +POLICY_PATHS = frozenset({ + ".github/workflows/cla.yml", "CLA.md", ".github/scripts/rerun-failed-cla.sh", + ".github/scripts/refresh-cla-check.sh", ".github/workflows/cla-policy-guard.yml", + "scripts/ci/validate-cla-policy.rb", +}) +# A no-policy pull request's guard is re-run on this evidence while its attempt is below +# this: a branch far behind a policy change on main can still be rejected for real. +NO_POLICY_MAX_ATTEMPT = 3 +MAX_FILE_PAGES = 3 # 300 changed files; a larger pull request gets no evidence this way GUARD_WINDOW = dt.timedelta(minutes=15) BASES = ("main", "feat-cmux-next") MAX_PULL_PAGES = 5 FLOOR_SHARE = 0.1 FLOOR_MIN = 500 RERUN_COST = 40 # GITHUB_TOKEN calls a CLA Assistant or CLA policy guard re-run spends, generously -WAIT_FOR_RESET = dt.timedelta(minutes=15) # under the cron's observed 25-40 minute gaps +# A pass that finds the budget spent waits for a reset this close (the job's timeout covers +# it, and the App token, minted at the start, lives an hour). Rejects start passes, so the +# last reject before a reset leaves one pass waiting for it. +WAIT_FOR_RESET = dt.timedelta(minutes=40) RESET_GRACE = dt.timedelta(seconds=20) MAX_AGE = dt.timedelta(hours=24) MAX_ATTEMPT = 20 @@ -108,7 +129,9 @@ def open_heads(gh: GitHub, log: Callable[[str], None]) -> dict[str, set[int]]: heads: dict[str, set[int]] = {} for base in BASES: for page in range(1, MAX_PULL_PAGES + 1): - pulls = gh.request("GET", f"/repos/{gh.repo}/pulls?state=open&base={base}&per_page=100&page={page}") or [] + # Most recently updated first: a fresh reject's pull request was just pushed or edited. + pulls = gh.request("GET", f"/repos/{gh.repo}/pulls?state=open&base={base}&per_page=100" + f"&sort=updated&direction=desc&page={page}") or [] for pull in pulls: if (pull.get("base") or {}).get("ref") == base and (pull.get("head") or {}).get("sha"): heads.setdefault(pull["head"]["sha"], set()).add(int(pull.get("number") or 0)) @@ -185,6 +208,19 @@ def guard_api_failure(log: str) -> bool: return bool(API_FAILURE.search(before)) +def proposes_no_policy(gh: GitHub, number: int) -> bool: + """The pull request changes, adds, removes or renames none of POLICY_PATHS. False when + that is not known (over MAX_FILE_PAGES of files).""" + for page in range(1, MAX_FILE_PAGES + 1): + files = gh.request("GET", f"/repos/{gh.repo}/pulls/{number}/files?per_page=100&page={page}") or [] + for item in files: + if {item.get("filename"), item.get("previous_filename")} & POLICY_PATHS: + return False + if len(files) < 100: + return True + return False + + def rotated(runs: list[dict[str, Any]], now: dt.datetime) -> list[dict[str, Any]]: """Oldest first, starting at a slot that moves every sweep, so a bounded scan reaches every candidate in turn and genuine failures cannot hold the read budget.""" @@ -197,8 +233,9 @@ def rotated(runs: list[dict[str, Any]], now: dt.datetime) -> list[dict[str, Any] def plan(gh: GitHub, now: dt.datetime, log: Callable[[str], None], limit: int = MAX_RERUNS) -> list[dict[str, Any]]: """The runs to re-run: CLA Assistant failures whose log shows the rate limit, and CLA - policy guard failures with evidence of an API failure (guard_api_failure, or a - rate-limited CLA Assistant job within GUARD_WINDOW). Log reads are bounded by + policy guard failures with evidence of an API failure (guard_api_failure, a pull request + that proposes no policy, or a rate-limited CLA Assistant job within GUARD_WINDOW). Log + and file reads are bounded by MAX_LOG_READS and MAX_GUARD_READS and rotate, so genuine failures cannot hold the read budget.""" heads = open_heads(gh, log) assistant_runs = workflow_runs(gh, "cla.yml") @@ -236,6 +273,9 @@ def plan(gh: GitHub, now: dt.datetime, log: Callable[[str], None], limit: int = text, finished = failed_job(gh, run) near = lambda stamp: finished is not None and abs(stamp - finished) <= GUARD_WINDOW # noqa: E731 evidence = guard_api_failure(text) or any(near(stamp) for stamp in limited_at) + if not evidence and int(run.get("run_attempt") or 1) < NO_POLICY_MAX_ATTEMPT and reads < MAX_GUARD_READS: + reads += 1 + evidence = proposes_no_policy(gh, run["number"]) if not evidence and finished is not None: for other in extra: stamp = when(other.get("updated_at")) @@ -253,7 +293,8 @@ def plan(gh: GitHub, now: dt.datetime, log: Callable[[str], None], limit: int = chosen.append(run) else: log(f"::warning title=CLA policy guard reject left alone::run {run['id']} for #{run['number']} at " - f"{run['head_sha'][:12]} shows no API failure and no rate-limited CLA Assistant within " + f"{run['head_sha'][:12]} changes CLA policy (or was re-run on that evidence already) and " + f"shows no API failure and no rate-limited CLA Assistant within " f"{int(GUARD_WINDOW.total_seconds() // 60)} min; possibly a real policy rejection, not re-run") return chosen diff --git a/tests/test_cla_quota_rerun.py b/tests/test_cla_quota_rerun.py index 24f1c8fcbc68..38f0c5397b15 100644 --- a/tests/test_cla_quota_rerun.py +++ b/tests/test_cla_quota_rerun.py @@ -5,6 +5,7 @@ import datetime as dt import io +import re import sys import unittest from contextlib import redirect_stdout @@ -15,6 +16,7 @@ sys.path.insert(0, str(ROOT / "scripts" / "ci")) import cla_quota_rerun as rerun # noqa: E402 +import yaml # noqa: E402 NOW = dt.datetime(2026, 10, 26, 4, 30, tzinfo=dt.timezone.utc) LIMITED_LOG = "##[error]Could not complete the CLA check: ... API rate limit exceeded for installation." @@ -25,6 +27,8 @@ GUARD_403_LOG = ("gh: API rate limit exceeded for installation ID 1 (HTTP 403)\n" "##[error]CLA policy validation rejected the proposed policy\n") GUARD = "CLA policy guard" +POLICY_CHANGE = [{"filename": ".github/workflows/cla.yml"}] +NO_POLICY = [{"filename": "Sources/App.swift"}, {"filename": "tests/test_app.py"}] def run(run_id, head, *, number=1, attempt=1, name="CLA", updated="2026-10-26T04:00:00Z", conclusion="failure"): @@ -38,9 +42,11 @@ def pull(number, head, base="main"): class FakeGitHub: - def __init__(self, assistant, guard, logs, *, pulls=None, finished=None, remaining=9000, reset=None): + def __init__(self, assistant, guard, logs, *, pulls=None, finished=None, remaining=9000, reset=None, files=None): # `remaining` may be a list: one value per rate_limit read, the last one repeating. + # `files`: pull request number -> its changed files (default: one that changes CLA policy). self.assistant, self.guard, self.logs = assistant, guard, logs + self.files = files or {} self.remaining = remaining if isinstance(remaining, list) else [remaining] self.reset = reset or NOW + dt.timedelta(hours=1) # By default every run's head is an open pull request into main. @@ -57,6 +63,11 @@ def request(self, method, path, raw=False): if method == "POST": self.posts.append(path) return {} + if "/files?" in path: + number = int(path.split("/pulls/")[1].split("/")[0]) + page = int(path.rsplit("page=", 1)[1]) + changed = self.files.get(number, POLICY_CHANGE) + return changed[(page - 1) * 100:page * 100] if path.startswith("/repos/o/r/pulls?state=open"): base = path.split("base=")[1].split("&")[0] return [p for p in self.pulls if p["base"]["ref"] == base] if path.endswith("page=1") else [] @@ -106,7 +117,7 @@ def test_a_reset_that_leaves_the_budget_low_stops_after_one_more_read(self): self.assertEqual(fake.calls, [("GET", "/rate_limit")] * 2) def test_a_far_reset_is_not_waited_for(self): - fake = FakeGitHub([run(1, "a" * 40)], [], {1: LIMITED_LOG}, remaining=0, reset=NOW + dt.timedelta(minutes=40)) + fake = FakeGitHub([run(1, "a" * 40)], [], {1: LIMITED_LOG}, remaining=0, reset=NOW + dt.timedelta(minutes=50)) slept = [] self.sweep(fake, slept=slept) self.assertEqual(slept, []) @@ -178,8 +189,9 @@ def test_pull_requests_are_read_per_base(self): fake = FakeGitHub([], [], {}) self.sweep(fake) reads = [path for _, path in fake.calls if "/pulls?" in path] - self.assertEqual(reads, ["/repos/o/r/pulls?state=open&base=main&per_page=100&page=1", - "/repos/o/r/pulls?state=open&base=feat-cmux-next&per_page=100&page=1"]) + self.assertEqual(reads, [ + "/repos/o/r/pulls?state=open&base=main&per_page=100&sort=updated&direction=desc&page=1", + "/repos/o/r/pulls?state=open&base=feat-cmux-next&per_page=100&sort=updated&direction=desc&page=1"]) def test_a_secondary_rate_limit_elsewhere_is_not_evidence_for_a_guard(self): guard_head, other_head = "b" * 40, "c" * 40 @@ -265,6 +277,65 @@ def test_genuine_failures_cannot_hold_the_read_budget(self): reached.update(fake.posts) self.assertIn(rerun_path(99), reached) + def test_a_guard_reject_on_a_pull_request_that_proposes_no_policy_is_rerun(self): + # No x-ratelimit header, no 403 wording and no CLA Assistant failure needed: a pull + # request that changes no policy path passes the guard unless a GitHub read fails. + fake = FakeGitHub([], [run(2, "b" * 40, number=7, name=GUARD)], {2: GUARD_LOG}, files={7: NO_POLICY}) + self.sweep(fake) + self.assertEqual(fake.posts, [rerun_path(2)]) + self.assertIn(("GET", "/repos/o/r/pulls/7/files?per_page=100&page=1"), fake.calls) + + def test_no_policy_evidence_stops_after_early_attempts(self): + fake = FakeGitHub([], [run(2, "b" * 40, number=7, attempt=rerun.NO_POLICY_MAX_ATTEMPT, name=GUARD)], + {2: GUARD_LOG}, files={7: NO_POLICY}) + self.assertIn("possibly a real policy rejection", self.sweep(fake)) + self.assertEqual(fake.posts, []) + + def test_a_pull_request_that_touches_any_policy_path_is_not_evidence(self): + for changed in ([{"filename": path} for path in sorted(rerun.POLICY_PATHS)] + + [{"filename": "docs/cla.md", "previous_filename": "CLA.md"}]): + fake = FakeGitHub([], [run(2, "b" * 40, number=7, name=GUARD)], {2: GUARD_LOG}, + files={7: NO_POLICY + [changed]}) + self.sweep(fake) + self.assertEqual(fake.posts, [], changed) + + def test_a_pull_request_too_large_to_list_is_not_evidence(self): + many = [{"filename": f"f{i}.txt"} for i in range(100 * rerun.MAX_FILE_PAGES)] + fake = FakeGitHub([], [run(2, "b" * 40, number=7, name=GUARD)], {2: GUARD_LOG}, files={7: many}) + self.sweep(fake) + self.assertEqual(fake.posts, []) + + def test_policy_paths_match_the_guard(self): + # The guard's CLA_POLICY_PATHS and GUARD_PATHS, read from its source. + source = (ROOT / "scripts" / "ci" / "validate-cla-policy.rb").read_text(encoding="utf-8") + constants = dict(re.findall(r'^([A-Z0-9_]+_PATH) = "([^"]+)"\.freeze$', source, re.MULTILINE)) + listed = re.search(r"^CLA_POLICY_PATHS = \[(.*?)\]", source, re.MULTILINE | re.DOTALL).group(1) + guard = re.search(r"^GUARD_PATHS = \[(.*?)\]", source, re.MULTILINE | re.DOTALL).group(1) + names = re.findall(r"[A-Z0-9_]+_PATH\b", listed + guard) + self.assertEqual(len(names), 6) + self.assertEqual({constants[name] for name in names}, set(rerun.POLICY_PATHS)) + + def test_the_workflow_runs_after_each_cla_failure_and_each_rerun(self): + workflow = yaml.safe_load((ROOT / ".github" / "workflows" / "ci-cla-quota-rerun.yml").read_text()) + on = workflow.get("on", workflow.get(True)) + self.assertEqual(sorted(on["workflow_run"]["workflows"]), ["CLA Assistant", "CLA policy guard"]) + self.assertEqual(on["workflow_run"]["types"], ["completed"]) + self.assertIn("schedule", on) + job = workflow["jobs"]["cla-quota-rerun"] + for term in ("github.event.workflow_run.event == 'pull_request_target'", + "github.event.workflow_run.conclusion == 'failure'", "github.event.workflow_run.run_attempt > 1"): + self.assertIn(term, job["if"]) + # Job level: CLA runs that start no job must never displace a pending pass. + self.assertNotIn("concurrency", workflow) + self.assertEqual(job["concurrency"], {"group": "ci-cla-quota-rerun", "cancel-in-progress": False}) + # The pass may wait WAIT_FOR_RESET, and the job outlives it. + wait = rerun.WAIT_FOR_RESET.total_seconds() / 60 + step = next(s for s in job["steps"] if "cla_quota_rerun.py" in str(s.get("run", ""))) + self.assertGreater(step["timeout-minutes"], wait) + self.assertGreater(job["timeout-minutes"], step["timeout-minutes"]) + self.assertNotIn("pull_request_target", on) + self.assertEqual(workflow["permissions"], {}) + def test_dry_run_writes_nothing(self): fake = FakeGitHub([run(1, "a" * 40)], [], {1: LIMITED_LOG}) self.assertIn("would re-run", self.sweep(fake, dry=True))