From 185cf78bf553cf050e708a0bf9c4d8b627a19672 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 12:02:04 +0000 Subject: [PATCH 1/2] CI: skip PR re-test after a clean main merge Merging main into a PR branch re-ran the whole PR tier on a diff that had already passed, though the merge queue re-tests every entry on the latest main before it lands. During post-landing bursts about a quarter of full PR runs were these re-syncs (#1373). clippy now runs scripts/ci-reuse-pr-head.py on pull_request. When the head is a two-parent merge, the PR's own diff matches its previous head hunk for hunk (line numbers ignored, like git patch-id), and CI passed on that head with clippy actually running, it sets pr_unchanged=true. The build and test jobs then skip; clippy, the cheap preflight jobs and hosted-e2e still run. Conflict resolutions, merges of other branches, binary or oversized diffs and any API error run everything. Refs #1373 Assisted-by: Claude Code:claude-opus-5-5 --- .github/workflows/ci.yml | 40 +++++-- scripts/ci-reuse-pr-head.py | 115 +++++++++++++++++++++ scripts/tests/test_ci_reuse_pr_head.py | 138 +++++++++++++++++++++++++ scripts/tests/test_ci_scheduling.py | 24 +++++ 4 files changed, 307 insertions(+), 10 deletions(-) create mode 100644 scripts/ci-reuse-pr-head.py create mode 100644 scripts/tests/test_ci_reuse_pr_head.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 20caf9438..3f013d306 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -72,6 +72,11 @@ concurrency: # runs (test-release, e2e-full, yarn-berry-full, cargo-vex-matrix-full) and # hosted-e2e still run. API errors, missing evidence and direct pushes run # everything. +# MERGE-MAIN REUSE: a pull_request push that only merged the base branch, +# with the PR's own diff unchanged and the previous head green +# (scripts/ci-reuse-pr-head.py, run by clippy), runs only clippy, the cheap +# preflight jobs and hosted-e2e. The merge queue re-tests the PR on the +# latest main before it lands. jobs: # Required independently of ci-ok so the merge queue sees a compile/lint # failure immediately. Expensive jobs also depend on this preflight. @@ -84,6 +89,7 @@ jobs: actions: read outputs: reuse: ${{ steps.merge-queue.outputs.reuse }} + pr_unchanged: ${{ steps.pr-head.outputs.unchanged }} steps: - name: Checkout uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 @@ -99,6 +105,20 @@ jobs: GH_TOKEN: ${{ github.token }} run: python3 scripts/ci-reuse-merge-group.py + # A PR push that only merged the base branch, with the PR's own diff + # unchanged and the previous head green, skips the test jobs: the + # merge queue re-tests every entry on the latest main anyway (#1373). + # This job and hosted-e2e still run. + - name: Check for an unchanged PR diff + id: pr-head + if: github.event_name == 'pull_request' + env: + GH_TOKEN: ${{ github.token }} + PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }} + PR_BASE_REF: ${{ github.base_ref }} + PR_HEAD_REPOSITORY: ${{ github.event.pull_request.head.repo.full_name }} + run: python3 scripts/ci-reuse-pr-head.py + - name: Install Rust # rustup is pre-installed on GitHub-hosted runners. `toolchain # install` with no name reads rust-toolchain.toml in the repo root, @@ -133,7 +153,7 @@ jobs: # The napi addon is only ever loaded by Node, so cargo's own tests never # exercise its JS loader or the engine/provider boundary. node-addon: - if: github.event.pull_request.draft != true + if: (github.event.pull_request.draft != true) && needs.clippy.outputs.pr_unchanged != 'true' needs: clippy runs-on: ${{ vars.DISABLE_DEPOT_RUNNERS == 'true' && 'ubuntu-latest' || 'depot-ubuntu-24.04-4' }} timeout-minutes: 30 @@ -299,7 +319,7 @@ jobs: bash scripts/release-lint.sh --sync-only test: - if: (github.event.pull_request.draft != true) && (vars.CI_SCOPE == 'full' || github.event_name == 'schedule' || github.event_name == 'workflow_dispatch') + if: ((github.event.pull_request.draft != true) && (vars.CI_SCOPE == 'full' || github.event_name == 'schedule' || github.event_name == 'workflow_dispatch')) && needs.clippy.outputs.pr_unchanged != 'true' needs: clippy # No ubuntu-latest leg: `coverage` runs this same `cargo test # --workspace` (debug, default features, plus Go and vexctl) on ubuntu, @@ -489,7 +509,7 @@ jobs: test-release: # Each PR and main push runs the complete release-mode suite. The merge # queue already skips this job because each constituent PR ran it. - if: (github.event.pull_request.draft != true && github.event_name != 'merge_group') && (vars.CI_SCOPE == 'full' || github.event_name == 'schedule' || github.event_name == 'workflow_dispatch') + if: ((github.event.pull_request.draft != true && github.event_name != 'merge_group') && (vars.CI_SCOPE == 'full' || github.event_name == 'schedule' || github.event_name == 'workflow_dispatch')) && needs.clippy.outputs.pr_unchanged != 'true' needs: clippy runs-on: ${{ vars.DISABLE_DEPOT_RUNNERS == 'true' && 'ubuntu-latest' || 'depot-ubuntu-24.04-16' }} strategy: @@ -535,7 +555,7 @@ jobs: run: python3 scripts/ci-test-shard.py "$TEST_SHARD" "$TEST_SHARD_COUNT" --locked --profile ci-release coverage: - if: github.event.pull_request.draft != true + if: (github.event.pull_request.draft != true) && needs.clippy.outputs.pr_unchanged != 'true' needs: clippy # Code coverage via cargo-llvm-cov (LLVM source-based instrumentation). # Reports as a markdown table in the job summary and uploads the raw @@ -662,7 +682,7 @@ jobs: # Dockerfile.base compiles the full-LTO release binary inside Docker, with # no cache. Build it once per run and hand the image to every docker leg. docker-base: - if: (github.event.pull_request.draft != true && needs.clippy.outputs.reuse != 'true') && (vars.CI_SCOPE == 'full' || github.event_name == 'schedule' || github.event_name == 'workflow_dispatch' || github.head_ref == 'release/v5-prerelease') + if: ((github.event.pull_request.draft != true && needs.clippy.outputs.reuse != 'true') && (vars.CI_SCOPE == 'full' || github.event_name == 'schedule' || github.event_name == 'workflow_dispatch' || github.head_ref == 'release/v5-prerelease')) && needs.clippy.outputs.pr_unchanged != 'true' needs: clippy runs-on: ${{ vars.DISABLE_DEPOT_RUNNERS == 'true' && 'ubuntu-22.04' || 'depot-ubuntu-22.04-4' }} timeout-minutes: 30 @@ -952,7 +972,7 @@ jobs: # test binaries directly from the same checkout path, so `CARGO_BIN_EXE_*` # and `CARGO_MANIFEST_DIR` resolve as they did under `cargo test`. e2e-build: - if: github.event.pull_request.draft != true + if: (github.event.pull_request.draft != true) && needs.clippy.outputs.pr_unchanged != 'true' needs: clippy outputs: reuse: ${{ needs.clippy.outputs.reuse }} @@ -1018,7 +1038,7 @@ jobs: # Lean scope still compiles every test target on Windows in the merge # queue (one job): lean pull_request runs skip Windows, and a cfg(unix) # slip only shows up here (#1324). Its consumers stay full-scope only. - if: (github.event.pull_request.draft != true) && (vars.CI_SCOPE == 'full' || github.event_name == 'merge_group' || github.event_name == 'schedule' || github.event_name == 'workflow_dispatch') + if: ((github.event.pull_request.draft != true) && (vars.CI_SCOPE == 'full' || github.event_name == 'merge_group' || github.event_name == 'schedule' || github.event_name == 'workflow_dispatch')) && needs.clippy.outputs.pr_unchanged != 'true' needs: clippy outputs: reuse: ${{ needs.clippy.outputs.reuse }} @@ -2113,7 +2133,7 @@ jobs: # Dropping that `needs` also dropped the draft skip it inherited, so # gate on draft here directly (push/merge_group/schedule still run). # Only wait for the clippy preflight. - if: github.event.pull_request.draft != true && needs.clippy.outputs.reuse != 'true' + if: (github.event.pull_request.draft != true && needs.clippy.outputs.reuse != 'true') && needs.clippy.outputs.pr_unchanged != 'true' needs: clippy runs-on: ${{ vars.DISABLE_DEPOT_RUNNERS == 'true' && 'ubuntu-latest' || 'depot-ubuntu-24.04-4' }} timeout-minutes: 40 @@ -2157,7 +2177,7 @@ jobs: # Dropping that `needs` also dropped the draft skip it inherited, so # gate on draft here directly (push/merge_group/schedule still run). # Only wait for the clippy preflight. - if: github.event.pull_request.draft != true && needs.clippy.outputs.reuse != 'true' + if: (github.event.pull_request.draft != true && needs.clippy.outputs.reuse != 'true') && needs.clippy.outputs.pr_unchanged != 'true' needs: clippy strategy: fail-fast: false @@ -2394,7 +2414,7 @@ jobs: # Dropping that `needs` also dropped the draft skip it inherited, so # gate on draft here directly (push/merge_group/schedule still run). # Only wait for the clippy preflight. - if: (github.event.pull_request.draft != true) && (vars.CI_SCOPE == 'full' || github.event_name == 'schedule' || github.event_name == 'workflow_dispatch') + if: ((github.event.pull_request.draft != true) && (vars.CI_SCOPE == 'full' || github.event_name == 'schedule' || github.event_name == 'workflow_dispatch')) && needs.clippy.outputs.pr_unchanged != 'true' needs: clippy runs-on: ${{ vars.DISABLE_DEPOT_RUNNERS == 'true' && 'ubuntu-latest' || 'depot-ubuntu-24.04-4' }} timeout-minutes: 30 diff --git a/scripts/ci-reuse-pr-head.py b/scripts/ci-reuse-pr-head.py new file mode 100644 index 000000000..7020a2104 --- /dev/null +++ b/scripts/ci-reuse-pr-head.py @@ -0,0 +1,115 @@ +#!/usr/bin/env python3 +"""Skip re-testing a PR push that only merged the base branch. + +With a merge queue, a PR never has to be up to date with main: the queue +re-tests every entry on top of the latest main before it lands. Merging main +into a PR branch therefore re-runs the whole PR tier on a diff that already +passed. This reports `unchanged=true` only when all of these hold: + +- the PR head is a two-parent merge commit; +- the PR's own diff (merge-base with the base branch to head) is the same, + hunk for hunk, as the diff of its first parent, the previous head; +- this workflow completed successfully on that previous head, for the same + head repository, and its clippy job ran (a draft run skips everything). + +A conflict resolution changes a hunk, and a merge of anything other than the +base branch adds files or hunks, so both run the full tier. Any API error, +oversized or binary diff, or missing evidence also runs the full tier. +""" + +import json +import os +import re +import subprocess +from pathlib import Path +from urllib.parse import quote + +# The compare API lists at most 300 files and leaves out the patch of a +# binary or very large file. Either way the diff cannot be proven unchanged. +MAX_FILES = 300 +HUNK_HEADER = re.compile(r"^@@ -\d+(?:,\d+)? \+\d+(?:,\d+)? @@", re.MULTILINE) + + +def api(endpoint): + result = subprocess.run(["gh", "api", endpoint], check=True, capture_output=True, + text=True, timeout=15) + return json.loads(result.stdout) + + +def diff_signature(compare): + """The PR diff with line numbers dropped, like `git patch-id --stable`.""" + files = compare["files"] + if not files or len(files) >= MAX_FILES: + return None + signature = [] + for entry in files: + patch = entry.get("patch") + if patch is None: + return None + signature.append((entry["filename"], entry.get("previous_filename"), entry["status"], + HUNK_HEADER.sub("@@", patch))) + return sorted(signature, key=repr) + + +def passed_run(runs, jobs_of, head_sha, head_repository): + for run in runs["workflow_runs"]: + if (run.get("event") == "pull_request" + and run.get("head_sha") == head_sha + and run.get("status") == "completed" + and run.get("conclusion") == "success" + and run.get("path") == ".github/workflows/ci.yml" + and run.get("head_repository", {}).get("full_name") == head_repository): + # The run's success already covers ci-ok. A draft run also ends in + # success with every job skipped, so clippy must have run. It is + # the first job, so it is on the first page even of a full run. + jobs = {job.get("name"): job.get("conclusion") for job in jobs_of(run["id"])["jobs"]} + if jobs.get("clippy") == "success": + return run["id"] + return None + + +def reusable_run(env): + if env.get("GITHUB_EVENT_NAME") != "pull_request": + return None + try: + repository = env["GITHUB_REPOSITORY"] + head, base = env["PR_HEAD_SHA"], env["PR_BASE_REF"] + head_repository = env["PR_HEAD_REPOSITORY"] + parents = api(f"repos/{repository}/commits/{head}")["parents"] + if len(parents) != 2: + return None + previous = parents[0]["sha"] + # Three-dot compares diff from the merge-base, which is what the PR + # shows. per_page=1 keeps the commit list short; files stay complete. + branch = quote(base, safe="") + now = diff_signature(api(f"repos/{repository}/compare/{branch}...{head}?per_page=1")) + before = diff_signature(api(f"repos/{repository}/compare/{branch}...{previous}?per_page=1")) + if now is None or now != before: + return None + runs = api(f"repos/{repository}/actions/workflows/ci.yml/runs" + f"?event=pull_request&head_sha={previous}&per_page=20") + return passed_run(runs, lambda run_id: api( + f"repos/{repository}/actions/runs/{run_id}/jobs?filter=latest&per_page=100"), + previous, head_repository) + except (OSError, subprocess.SubprocessError, ValueError, KeyError, TypeError, + AttributeError, IndexError) as error: + # Do not print response bodies or stderr, which may contain credentials. + print(f"::notice::Could not compare with the previous PR head ({type(error).__name__}); " + "running all checks.") + return None + + +def main(): + run = reusable_run(os.environ) + with Path(os.environ["GITHUB_OUTPUT"]).open("a", encoding="utf-8") as output: + output.write(f"unchanged={'true' if run is not None else 'false'}\n") + if run is not None: + print(f"This push only merged {os.environ['PR_BASE_REF']}; the PR diff is unchanged and " + f"CI run {run} passed the previous head. The merge queue re-tests it on the " + "latest base, so only the preflight runs here.") + else: + print("Not a clean base-branch merge onto a passing head; running all checks.") + + +if __name__ == "__main__": + main() diff --git a/scripts/tests/test_ci_reuse_pr_head.py b/scripts/tests/test_ci_reuse_pr_head.py new file mode 100644 index 000000000..96e980686 --- /dev/null +++ b/scripts/tests/test_ci_reuse_pr_head.py @@ -0,0 +1,138 @@ +"""Only a clean base-branch merge onto a head that passed CI may skip tests.""" + +import importlib.util +import json +import subprocess +import tempfile +import unittest +from pathlib import Path +from unittest.mock import patch + +ROOT = Path(__file__).parents[2] +spec = importlib.util.spec_from_file_location("reuse_pr", ROOT / "scripts/ci-reuse-pr-head.py") +reuse = importlib.util.module_from_spec(spec) +spec.loader.exec_module(reuse) + +REPO = "SocketDev/socket-patch" +HEAD, PREVIOUS, MAIN_TIP = "a" * 40, "b" * 40, "c" * 40 + + +def file(name="src/lib.rs", patch_text="@@ -10,2 +10,3 @@ fn main() {\n a\n+b\n c", **extra): + return dict({"filename": name, "status": "modified", "patch": patch_text}, **extra) + + +class PrHeadReuse(unittest.TestCase): + env = {"GITHUB_EVENT_NAME": "pull_request", "GITHUB_REPOSITORY": REPO, + "PR_HEAD_SHA": HEAD, "PR_BASE_REF": "main", "PR_HEAD_REPOSITORY": REPO} + pr_run = {"id": 77, "event": "pull_request", "head_sha": PREVIOUS, "status": "completed", + "conclusion": "success", "path": ".github/workflows/ci.yml", + "head_repository": {"full_name": REPO}} + jobs = [{"name": "clippy", "conclusion": "success"}, {"name": "ci-ok", "conclusion": "success"}, + {"name": "e2e-build (ubuntu-latest)", "conclusion": "success"}] + + def responses(self, parents=(PREVIOUS, MAIN_TIP), now=None, before=None, runs=None, jobs=None): + """Fake `gh api` keyed on the endpoint, recording every call.""" + now = [file()] if now is None else now + before = [file(patch_text="@@ -4,2 +4,3 @@ fn main() {\n a\n+b\n c")] if before is None else before + table = { + f"repos/{REPO}/commits/{HEAD}": {"parents": [{"sha": p} for p in parents]}, + f"repos/{REPO}/compare/main...{HEAD}?per_page=1": {"files": now}, + f"repos/{REPO}/compare/main...{PREVIOUS}?per_page=1": {"files": before}, + f"repos/{REPO}/actions/workflows/ci.yml/runs?event=pull_request&head_sha={PREVIOUS}&per_page=20": + {"workflow_runs": [self.pr_run] if runs is None else runs}, + f"repos/{REPO}/actions/runs/77/jobs?filter=latest&per_page=100": + {"jobs": self.jobs if jobs is None else jobs}, + } + calls = [] + + def run(argv, **kwargs): + self.assertEqual(argv[:2], ["gh", "api"]) + self.assertLessEqual(kwargs["timeout"], 15) + calls.append(argv[2]) + return subprocess.CompletedProcess(argv, 0, stdout=json.dumps(table[argv[2]])) + return run, calls + + def reuse_with(self, env=None, **kwargs): + run, calls = self.responses(**kwargs) + with patch.object(reuse.subprocess, "run", side_effect=run): + return reuse.reusable_run(self.env if env is None else env), calls + + def test_clean_merge_of_main_onto_a_passing_head_is_reused(self): + # main moved the PR's hunk down six lines; the change itself is equal. + run, calls = self.reuse_with() + self.assertEqual(run, 77) + self.assertEqual(len(calls), 5) + + def test_ordinary_pushes_cost_one_request_and_run_everything(self): + for parents in ((PREVIOUS,), (PREVIOUS, MAIN_TIP, "d" * 40)): + with self.subTest(parents=len(parents)): + run, calls = self.reuse_with(parents=parents) + self.assertIsNone(run) + self.assertEqual(len(calls), 1) + + def test_a_changed_pr_diff_runs_everything(self): + cases = { + "conflict resolution edits a hunk": [file(patch_text="@@ -10,2 +10,3 @@\n a\n+B\n c")], + "merge brings in another file": [file(), file("src/other.rs")], + "file renamed": [file(previous_filename="src/old.rs", status="renamed")], + "binary or oversized patch": [{"filename": "src/lib.rs", "status": "modified"}], + "empty PR diff": [], + "too many files": [file(f"f{i}") for i in range(reuse.MAX_FILES)], + } + for name, now in cases.items(): + with self.subTest(name): + run, _ = self.reuse_with(now=now) + self.assertIsNone(run) + + def test_previous_head_without_a_full_passing_run_runs_everything(self): + mismatches = [ + {"head_sha": "e" * 40}, {"event": "push"}, {"event": "merge_group"}, + {"path": ".github/workflows/other.yml"}, {"status": "in_progress"}, + {"conclusion": "failure"}, {"conclusion": "cancelled"}, {"conclusion": None}, + {"head_repository": {"full_name": "fork/socket-patch"}}, + ] + for change in mismatches: + with self.subTest(change=change): + run, _ = self.reuse_with(runs=[dict(self.pr_run, **change)]) + self.assertIsNone(run) + with self.subTest("no run"): + self.assertIsNone(self.reuse_with(runs=[])[0]) + # A draft run passes with every job skipped; it tested nothing. + for jobs in ([{"name": "clippy", "conclusion": "skipped"}, + {"name": "ci-ok", "conclusion": "success"}], + [{"name": "clippy", "conclusion": "failure"}], + []): + with self.subTest(jobs=jobs): + self.assertIsNone(self.reuse_with(jobs=jobs)[0]) + + def test_other_events_never_query_or_skip(self): + for event in ("push", "merge_group", "schedule", "workflow_dispatch"): + with self.subTest(event=event), patch.object(reuse.subprocess, "run") as call: + self.assertIsNone(reuse.reusable_run(dict(self.env, GITHUB_EVENT_NAME=event))) + call.assert_not_called() + + def test_api_failure_timeout_and_bad_payload_run_everything(self): + for error in (FileNotFoundError(), subprocess.CalledProcessError(1, "gh"), + subprocess.TimeoutExpired("gh", 15)): + with self.subTest(error=error), patch.object(reuse.subprocess, "run", side_effect=error): + self.assertIsNone(reuse.reusable_run(self.env)) + for text in ("not json", "{}", '{"parents": null}', '{"parents": [null, null]}'): + with self.subTest(text=text), patch.object(reuse.subprocess, "run", return_value= + subprocess.CompletedProcess([], 0, stdout=text)): + self.assertIsNone(reuse.reusable_run(self.env)) + with self.subTest("missing env"), patch.object(reuse.subprocess, "run") as call: + self.assertIsNone(reuse.reusable_run({"GITHUB_EVENT_NAME": "pull_request"})) + call.assert_not_called() + + def test_main_writes_the_output(self): + with tempfile.TemporaryDirectory() as tmp: + output = Path(tmp) / "output" + run, _ = self.responses() + with patch.object(reuse.subprocess, "run", side_effect=run), \ + patch.dict(reuse.os.environ, dict(self.env, GITHUB_OUTPUT=str(output)), clear=True): + reuse.main() + self.assertEqual(output.read_text(encoding="utf-8"), "unchanged=true\n") + + +if __name__ == "__main__": + unittest.main() diff --git a/scripts/tests/test_ci_scheduling.py b/scripts/tests/test_ci_scheduling.py index 67427b5b4..552ad0343 100644 --- a/scripts/tests/test_ci_scheduling.py +++ b/scripts/tests/test_ci_scheduling.py @@ -114,6 +114,30 @@ def test_reused_push_keeps_cache_writers_and_unqueued_jobs(self): self.assertIn("reuse: ${{ steps.merge-queue.outputs.reuse }}", clippy) self.assertIn("python3 scripts/ci-reuse-merge-group.py", clippy) + def test_unchanged_pr_merge_skips_every_test_job_but_hosted_e2e(self): + # A PR push that only merged main keeps clippy and the cheap preflight + # jobs; everything that builds or tests skips, directly or through + # e2e-build. hosted-e2e is never scoped. + gated = {"node-addon", "test", "test-release", "coverage", "docker-base", "e2e-build", + "e2e-build-windows", "yarn-classic-matrix", "yarn-berry-e2e", "cargo-old-toolchains"} + for job in JOBS: + condition = next((line for line in JOBS[job] if line.startswith(" if:")), "") + with self.subTest(job=job): + self.assertEqual("needs.clippy.outputs.pr_unchanged != 'true'" in condition, + job in gated) + if job in gated: + self.assertTrue(condition.endswith(") && needs.clippy.outputs.pr_unchanged != 'true'")) + pr_tier = {job for job in JOBS if "clippy" in ancestors(job)} - {"hosted-e2e"} + for job in sorted(pr_tier - gated): + with self.subTest(job=job): + # Ungated jobs either never run on a lean PR or wait on a gated build. + self.assertTrue(ancestors(job) & gated or "pull_request" in + next(line for line in JOBS[job] if line.startswith(" if:")), job) + clippy = "\n".join(JOBS["clippy"]) + self.assertIn("pr_unchanged: ${{ steps.pr-head.outputs.unchanged }}", clippy) + self.assertIn("python3 scripts/ci-reuse-pr-head.py", clippy) + self.assertIn("if: github.event_name == 'pull_request'", clippy) + def test_row_reader_preserves_both_os_siblings(self): jobs = reader.jobs("""jobs: example: From 941e1c4ea4c189b1d8d1512b391a0b103f89fdc7 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 12:07:14 +0000 Subject: [PATCH 2/2] CI: reuse only on PRs the merge queue re-tests Review of the first cut found gaps where a skipped PR job would never run again before landing: - PRs into branches other than main have no merge queue, so they now always run everything. - The release/v5-prerelease PR runs a full tier the queue skips, so it never reuses either. - test-release is excluded from merge_group runs, so it now runs on an unchanged merge-main push too. The script also checks for a green previous run before fetching the two compare payloads, since most merge pushes follow a cancelled run. Assisted-by: Claude Code:claude-opus-5-5 --- .github/workflows/ci.yml | 16 ++++++++------ scripts/ci-reuse-pr-head.py | 25 +++++++++++----------- scripts/tests/test_ci_reuse_pr_head.py | 20 ++++++++++++------ scripts/tests/test_ci_scheduling.py | 29 +++++++++++++++++--------- 4 files changed, 56 insertions(+), 34 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3f013d306..ae5057855 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -75,8 +75,9 @@ concurrency: # MERGE-MAIN REUSE: a pull_request push that only merged the base branch, # with the PR's own diff unchanged and the previous head green # (scripts/ci-reuse-pr-head.py, run by clippy), runs only clippy, the cheap -# preflight jobs and hosted-e2e. The merge queue re-tests the PR on the -# latest main before it lands. +# preflight jobs, test-release and hosted-e2e. The merge queue re-tests the +# PR on the latest main before it lands; PRs into other branches and the +# release/v5-prerelease PR never reuse. jobs: # Required independently of ci-ok so the merge queue sees a compile/lint # failure immediately. Expensive jobs also depend on this preflight. @@ -108,10 +109,12 @@ jobs: # A PR push that only merged the base branch, with the PR's own diff # unchanged and the previous head green, skips the test jobs: the # merge queue re-tests every entry on the latest main anyway (#1373). - # This job and hosted-e2e still run. + # This job and hosted-e2e still run. Only PRs into main (the only + # queue), and never the v5 integration PR, whose full tier the queue + # does not run. - name: Check for an unchanged PR diff id: pr-head - if: github.event_name == 'pull_request' + if: github.event_name == 'pull_request' && github.base_ref == 'main' && github.head_ref != 'release/v5-prerelease' env: GH_TOKEN: ${{ github.token }} PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }} @@ -508,8 +511,9 @@ jobs: test-release: # Each PR and main push runs the complete release-mode suite. The merge - # queue already skips this job because each constituent PR ran it. - if: ((github.event.pull_request.draft != true && github.event_name != 'merge_group') && (vars.CI_SCOPE == 'full' || github.event_name == 'schedule' || github.event_name == 'workflow_dispatch')) && needs.clippy.outputs.pr_unchanged != 'true' + # queue already skips this job because each constituent PR ran it, so + # an unchanged merge-main PR push (pr_unchanged) still runs it too. + if: (github.event.pull_request.draft != true && github.event_name != 'merge_group') && (vars.CI_SCOPE == 'full' || github.event_name == 'schedule' || github.event_name == 'workflow_dispatch') needs: clippy runs-on: ${{ vars.DISABLE_DEPOT_RUNNERS == 'true' && 'ubuntu-latest' || 'depot-ubuntu-24.04-16' }} strategy: diff --git a/scripts/ci-reuse-pr-head.py b/scripts/ci-reuse-pr-head.py index 7020a2104..859bbc6a3 100644 --- a/scripts/ci-reuse-pr-head.py +++ b/scripts/ci-reuse-pr-head.py @@ -6,6 +6,7 @@ into a PR branch therefore re-runs the whole PR tier on a diff that already passed. This reports `unchanged=true` only when all of these hold: +- the PR targets main, the only branch with a merge queue; - the PR head is a two-parent merge commit; - the PR's own diff (merge-base with the base branch to head) is the same, hunk for hunk, as the diff of its first parent, the previous head; @@ -22,7 +23,6 @@ import re import subprocess from pathlib import Path -from urllib.parse import quote # The compare API lists at most 300 files and leaves out the patch of a # binary or very large file. Either way the diff cannot be proven unchanged. @@ -69,28 +69,29 @@ def passed_run(runs, jobs_of, head_sha, head_repository): def reusable_run(env): - if env.get("GITHUB_EVENT_NAME") != "pull_request": + # Only main has a merge queue to re-test the PR before it lands. + if env.get("GITHUB_EVENT_NAME") != "pull_request" or env.get("PR_BASE_REF") != "main": return None try: - repository = env["GITHUB_REPOSITORY"] - head, base = env["PR_HEAD_SHA"], env["PR_BASE_REF"] + repository, head = env["GITHUB_REPOSITORY"], env["PR_HEAD_SHA"] head_repository = env["PR_HEAD_REPOSITORY"] parents = api(f"repos/{repository}/commits/{head}")["parents"] if len(parents) != 2: return None previous = parents[0]["sha"] - # Three-dot compares diff from the merge-base, which is what the PR - # shows. per_page=1 keeps the commit list short; files stay complete. - branch = quote(base, safe="") - now = diff_signature(api(f"repos/{repository}/compare/{branch}...{head}?per_page=1")) - before = diff_signature(api(f"repos/{repository}/compare/{branch}...{previous}?per_page=1")) - if now is None or now != before: - return None + # The cheap evidence first: most merge pushes follow a cancelled run. runs = api(f"repos/{repository}/actions/workflows/ci.yml/runs" f"?event=pull_request&head_sha={previous}&per_page=20") - return passed_run(runs, lambda run_id: api( + run = passed_run(runs, lambda run_id: api( f"repos/{repository}/actions/runs/{run_id}/jobs?filter=latest&per_page=100"), previous, head_repository) + if run is None: + return None + # Three-dot compares diff from the merge-base, which is what the PR + # shows. per_page=1 keeps the commit list short; files stay complete. + now = diff_signature(api(f"repos/{repository}/compare/main...{head}?per_page=1")) + before = diff_signature(api(f"repos/{repository}/compare/main...{previous}?per_page=1")) + return run if now is not None and now == before else None except (OSError, subprocess.SubprocessError, ValueError, KeyError, TypeError, AttributeError, IndexError) as error: # Do not print response bodies or stderr, which may contain credentials. diff --git a/scripts/tests/test_ci_reuse_pr_head.py b/scripts/tests/test_ci_reuse_pr_head.py index 96e980686..da22bd47a 100644 --- a/scripts/tests/test_ci_reuse_pr_head.py +++ b/scripts/tests/test_ci_reuse_pr_head.py @@ -96,7 +96,10 @@ def test_previous_head_without_a_full_passing_run_runs_everything(self): run, _ = self.reuse_with(runs=[dict(self.pr_run, **change)]) self.assertIsNone(run) with self.subTest("no run"): - self.assertIsNone(self.reuse_with(runs=[])[0]) + run, calls = self.reuse_with(runs=[]) + self.assertIsNone(run) + # No green previous run: the large compare payloads are never fetched. + self.assertFalse(any("/compare/" in call for call in calls)) # A draft run passes with every job skipped; it tested nothing. for jobs in ([{"name": "clippy", "conclusion": "skipped"}, {"name": "ci-ok", "conclusion": "success"}], @@ -105,10 +108,14 @@ def test_previous_head_without_a_full_passing_run_runs_everything(self): with self.subTest(jobs=jobs): self.assertIsNone(self.reuse_with(jobs=jobs)[0]) - def test_other_events_never_query_or_skip(self): - for event in ("push", "merge_group", "schedule", "workflow_dispatch"): - with self.subTest(event=event), patch.object(reuse.subprocess, "run") as call: - self.assertIsNone(reuse.reusable_run(dict(self.env, GITHUB_EVENT_NAME=event))) + def test_other_events_and_bases_never_query_or_skip(self): + # Only main has a merge queue to re-test the PR before it lands. + changes = [{"GITHUB_EVENT_NAME": e} for e in + ("push", "merge_group", "schedule", "workflow_dispatch")] + changes += [{"PR_BASE_REF": b} for b in ("release/v5-prerelease", "v4", "")] + for change in changes: + with self.subTest(change=change), patch.object(reuse.subprocess, "run") as call: + self.assertIsNone(reuse.reusable_run(dict(self.env, **change))) call.assert_not_called() def test_api_failure_timeout_and_bad_payload_run_everything(self): @@ -121,7 +128,8 @@ def test_api_failure_timeout_and_bad_payload_run_everything(self): subprocess.CompletedProcess([], 0, stdout=text)): self.assertIsNone(reuse.reusable_run(self.env)) with self.subTest("missing env"), patch.object(reuse.subprocess, "run") as call: - self.assertIsNone(reuse.reusable_run({"GITHUB_EVENT_NAME": "pull_request"})) + self.assertIsNone(reuse.reusable_run({"GITHUB_EVENT_NAME": "pull_request", + "PR_BASE_REF": "main"})) call.assert_not_called() def test_main_writes_the_output(self): diff --git a/scripts/tests/test_ci_scheduling.py b/scripts/tests/test_ci_scheduling.py index 552ad0343..74a28c030 100644 --- a/scripts/tests/test_ci_scheduling.py +++ b/scripts/tests/test_ci_scheduling.py @@ -114,12 +114,18 @@ def test_reused_push_keeps_cache_writers_and_unqueued_jobs(self): self.assertIn("reuse: ${{ steps.merge-queue.outputs.reuse }}", clippy) self.assertIn("python3 scripts/ci-reuse-merge-group.py", clippy) - def test_unchanged_pr_merge_skips_every_test_job_but_hosted_e2e(self): + def test_unchanged_pr_merge_skips_only_what_the_queue_re_tests(self): # A PR push that only merged main keeps clippy and the cheap preflight # jobs; everything that builds or tests skips, directly or through - # e2e-build. hosted-e2e is never scoped. - gated = {"node-addon", "test", "test-release", "coverage", "docker-base", "e2e-build", - "e2e-build-windows", "yarn-classic-matrix", "yarn-berry-e2e", "cargo-old-toolchains"} + # e2e-build, because the merge queue runs it again on the latest + # main. test-release is not in the queue and hosted-e2e is never + # scoped, so both still run. + gated = {"node-addon", "test", "coverage", "docker-base", "e2e-build", + "e2e-build-windows", "yarn-classic-matrix", "yarn-berry-e2e", + "cargo-old-toolchains"} + never_on_pr = " if: (github.event_name != 'pull_request'" + v5_pr_only = (" if: (github.event_name != 'pull_request' && github.event_name != 'merge_group')" + " || github.head_ref == 'release/v5-prerelease'") for job in JOBS: condition = next((line for line in JOBS[job] if line.startswith(" if:")), "") with self.subTest(job=job): @@ -127,16 +133,19 @@ def test_unchanged_pr_merge_skips_every_test_job_but_hosted_e2e(self): job in gated) if job in gated: self.assertTrue(condition.endswith(") && needs.clippy.outputs.pr_unchanged != 'true'")) - pr_tier = {job for job in JOBS if "clippy" in ancestors(job)} - {"hosted-e2e"} - for job in sorted(pr_tier - gated): + pr_tier = {job for job in JOBS if "clippy" in ancestors(job)} + for job in sorted(pr_tier - gated - {"hosted-e2e", "test-release"}): + condition = next(line for line in JOBS[job] if line.startswith(" if:")) with self.subTest(job=job): - # Ungated jobs either never run on a lean PR or wait on a gated build. - self.assertTrue(ancestors(job) & gated or "pull_request" in - next(line for line in JOBS[job] if line.startswith(" if:")), job) + # Ungated jobs wait on a gated build, never run on a PR, or run + # only on the v5 integration PR, which never reuses (below). + self.assertTrue(ancestors(job) & gated or condition.startswith(never_on_pr) + and (condition == v5_pr_only or "head_ref" not in condition), job) clippy = "\n".join(JOBS["clippy"]) self.assertIn("pr_unchanged: ${{ steps.pr-head.outputs.unchanged }}", clippy) self.assertIn("python3 scripts/ci-reuse-pr-head.py", clippy) - self.assertIn("if: github.event_name == 'pull_request'", clippy) + self.assertIn("if: github.event_name == 'pull_request' && github.base_ref == 'main'" + " && github.head_ref != 'release/v5-prerelease'", clippy) def test_row_reader_preserves_both_os_siblings(self): jobs = reader.jobs("""jobs: