diff --git a/.github/workflows/run-ci-command.yml b/.github/workflows/run-ci-command.yml index b0764bfa06..d333677271 100644 --- a/.github/workflows/run-ci-command.yml +++ b/.github/workflows/run-ci-command.yml @@ -39,6 +39,7 @@ jobs: BUILDKITE_ORGANIZATION: vllm BUILDKITE_PIPELINE: ci CI_TRUSTED_USERS: ${{ vars.CI_TRUSTED_USERS }} + GH_BRANCH_UPDATE_TOKEN: ${{ secrets.CI_BRANCH_UPDATE_TOKEN }} GH_TOKEN: ${{ github.token }} - name: Report failure on the PR if: failure() diff --git a/.github/workflows/scripts/run_ci_command.py b/.github/workflows/scripts/run_ci_command.py index 537efa83c4..b6ef1c3ae8 100644 --- a/.github/workflows/scripts/run_ci_command.py +++ b/.github/workflows/scripts/run_ci_command.py @@ -62,6 +62,8 @@ CI_COMMAND_PREFIXES = frozenset({"/ci", "/amd-ci"}) CI_AUTHORIZED_COMMENT_MARKER = "" READY_LABELS = {"ready", "ready-run-all-tests"} TRUSTED_PERMISSIONS = {"admin", "maintain", "write"} +BRANCH_UPDATE_POLL_SECONDS = 3 +BRANCH_UPDATE_POLL_ATTEMPTS = 40 ACTIVE_BUILD_STATES = { "blocked", "creating", @@ -259,6 +261,13 @@ class GitHubClient: raise ApiError(None, "GitHub returned an invalid commits-behind count.") return behind + def update_branch(self, number: int, expected_head_sha: str) -> None: + self._request( + self._repo_path(f"/pulls/{number}/update-branch"), + body={"expected_head_sha": expected_head_sha}, + method="PUT", + ) + def list_pulls_for_commit(self, commit: str) -> list[dict[str, Any]]: commit = urllib.parse.quote(commit, safe="") response = self._request(self._repo_path(f"/commits/{commit}/pulls")) @@ -778,10 +787,11 @@ def notify_authorized( ( f"✅ @{author}, CI is now available for this PR.\n\n" "- `/ci run` starts upstream CI; `/amd-ci run` starts AMD CI only.\n" - "- Your branch must contain every commit currently on its upstream " - "target branch. Merge or rebase onto the latest target branch, then " - "rerun the command. Append `--allow-stale` to a run command to test " - "an outdated branch at your own risk.\n" + "- Run commands test your branch with every commit currently on its " + "upstream target branch. If it is behind, the bot merges the target " + "branch into it first (pull before pushing again). Append " + "`--allow-stale` to a run command to test an outdated branch as is, " + "at your own risk.\n" "- `/ci retry` retries failed jobs in the CI build for the current " "PR head. If the current head has no CI build, it starts a new CI " "build for the current head containing only jobs that failed in " @@ -829,10 +839,55 @@ def resolve_workflow_run_pr( return find_matching_pr(github.list_pulls_for_commit(head_sha)) +def merge_base_into_pr( + github: GitHubClient, + branch_updater: GitHubClient, + pr: Mapping[str, Any], + command: str, + lag: str, +) -> tuple[dict[str, Any], str]: + base_ref = pr["base"]["ref"] + + def merge_failed(reason: str) -> CiPreparationError: + return CiPreparationError( + f"{lag} The bot could not merge upstream `{base_ref}` into this " + f"branch ({reason}). No new CI build was started. Either merge or " + f"rebase onto the latest `{base_ref}` yourself and rerun `{command}`, " + f"or use `{command}{ALLOW_STALE_SUFFIX}` to test this branch as is." + ) + + try: + branch_updater.update_branch(pr["number"], pr["head"]["sha"]) + for _ in range(BRANCH_UPDATE_POLL_ATTEMPTS): + time.sleep(BRANCH_UPDATE_POLL_SECONDS) + current_pr = github.get_pr(pr["number"]) + if current_pr["head"]["sha"] != pr["head"]["sha"]: + break + else: + raise merge_failed("the merge commit did not appear in time") + if ( + current_pr["state"] != "open" + or current_pr["base"]["ref"] != base_ref + or github.get_commits_behind_base(base_ref, current_pr["head"]["sha"]) + ): + raise merge_failed("the PR changed during the merge") + except ApiError as error: + raise merge_failed(str(error)) from error + + note = ( + f"ℹ️ {lag} Merged upstream `{base_ref}` into this branch as " + f"`{current_pr['head']['sha'][:12]}` before starting CI. " + "Pull the branch before pushing more changes." + ) + print(note) + return current_pr, note + + def prepare_pr_for_ci( github: GitHubClient, pr: Mapping[str, Any], command: str, + branch_updater: GitHubClient | None = None, ) -> tuple[dict[str, Any], str]: base_ref = pr["base"]["ref"] no_action = "No new CI build was started." @@ -868,6 +923,8 @@ def prepare_pr_for_ci( print(warning) return current_pr, warning + if behind and branch_updater is not None: + return merge_base_into_pr(github, branch_updater, current_pr, command, lag) if behind: raise CiPreparationError( f"{lag} Your branch must contain every commit currently on upstream " @@ -887,6 +944,8 @@ def handle_run_ci( command: str, github: GitHubClient, pr: Mapping[str, Any], + branch_updater: GitHubClient | None = None, + allow_repeat_run: bool = False, ) -> str: ci_name = ci_name_for_command(command) duplicate_builds = buildkite.list_builds( @@ -913,7 +972,19 @@ def handle_run_ci( f"{ci_name} is already running for this commit: {active_build['web_url']}" ) - current_pr, warning = prepare_pr_for_ci(github, pr, command) + tested_build = select_latest_build( + [build for build in current_builds if build.get("state") != "canceled"], + pr["number"], + ) + if tested_build and not allow_repeat_run: + raise CiPreparationError( + f"{ci_name} already ran on this commit: {tested_build['web_url']}. " + "No new CI build was started. Push new changes to run CI again, use " + f"`{retry_command_for_command(command)}` to rerun failed jobs, or ask " + "a reviewer with write access to rerun it." + ) + + current_pr, warning = prepare_pr_for_ci(github, pr, command, branch_updater) build = buildkite.create_build( create_build_payload( @@ -1120,6 +1191,7 @@ def run( github: GitHubClient, buildkite: BuildkiteClient, trusted_users_value: str = "", + branch_updater: GitHubClient | None = None, ) -> None: command = parse_command(event["comment"]["body"]) if not command or "pull_request" not in event["issue"]: @@ -1182,6 +1254,8 @@ def run( command=command, github=github, pr=pr, + branch_updater=branch_updater, + allow_repeat_run=is_trusted_permission(permission), ) elif command in RETRY_COMMANDS: message = handle_retry_failed( @@ -1263,11 +1337,15 @@ def main() -> None: os.environ.get("BUILDKITE_ORGANIZATION", "vllm"), pipeline, ) + branch_update_token = os.environ.get("GH_BRANCH_UPDATE_TOKEN", "") run( event, github, buildkite, os.environ.get("CI_TRUSTED_USERS", ""), + GitHubClient(branch_update_token, os.environ["GITHUB_REPOSITORY"]) + if branch_update_token + else None, ) diff --git a/.github/workflows/scripts/test_run_ci_command.py b/.github/workflows/scripts/test_run_ci_command.py index 623b412476..2061ff895e 100644 --- a/.github/workflows/scripts/test_run_ci_command.py +++ b/.github/workflows/scripts/test_run_ci_command.py @@ -144,6 +144,23 @@ class FakeGitHub: self.comments.append(body) +class FakeBranchUpdater: + def __init__(self, github: FakeGitHub, error: ApiError | None = None) -> None: + self.calls: list[tuple[int, str]] = [] + self.error = error + self.github = github + + def update_branch(self, number: int, expected_head_sha: str) -> None: + self.calls.append((number, expected_head_sha)) + if self.error: + raise self.error + self.github.prs[number] = { + **self.github.prs[number], + "head": {**self.github.prs[number]["head"], "sha": "merge-commit-sha"}, + } + self.github.behind = 0 + + class FakeBuildkite: def __init__( self, @@ -616,6 +633,64 @@ class RunCiCommandTest(unittest.TestCase): ) self.assertIn(command, github.comments[0]) + def test_run_merges_base_into_outdated_branch_before_building(self) -> None: + github = FakeGitHub(behind=5) + updater = FakeBranchUpdater(github) + buildkite = FakeBuildkite([[], []]) + + with patch("run_ci_command.time.sleep"): + run(make_event(COMMAND_RUN_CI), github, buildkite, "", updater) + + self.assertEqual(updater.calls, [(42, "0123456789abcdef")]) + self.assertEqual(buildkite.created_builds[0]["commit"], "merge-commit-sha") + self.assertTrue(github.comments[0].startswith("✅ ")) + self.assertIn("5 commits behind", github.comments[0]) + self.assertIn("Merged upstream `main`", github.comments[0]) + + def test_only_committers_can_rerun_a_commit_that_already_ran_ci(self) -> None: + finished_build = { + "created_at": "2026-07-28T01:00:00Z", + "pull_request": {"id": 42}, + "state": "failed", + "web_url": "https://buildkite.example/builds/122", + } + for actor, permission, command, allowed in ( + ("author", "read", COMMAND_RUN_CI, False), + ("author", "read", f"{COMMAND_RUN_CI} --allow-stale", False), + ("reviewer", "write", COMMAND_RUN_CI, True), + ): + with self.subTest(actor=actor, command=command): + github = FakeGitHub( + behind=5, + permission=permission, + pr=make_pr(labels=[{"name": "ready"}]), + ) + updater = FakeBranchUpdater(github) + buildkite = FakeBuildkite([[], [finished_build]]) + + with patch("run_ci_command.time.sleep"): + run(make_event(command, actor), github, buildkite, "", updater) + + self.assertEqual(bool(buildkite.created_builds), allowed) + if not allowed: + self.assertEqual(updater.calls, []) + self.assertIn("CI already ran on this commit", github.comments[0]) + self.assertIn("`/ci retry`", github.comments[0]) + + def test_run_reports_failed_merge_without_building(self) -> None: + github = FakeGitHub(behind=5) + updater = FakeBranchUpdater(github, error=ApiError(422, "merge conflict")) + buildkite = FakeBuildkite([[], []]) + + run(make_event(COMMAND_RUN_CI), github, buildkite, "", updater) + + self.assertEqual(buildkite.created_builds, []) + self.assertTrue(github.comments[0].startswith("❌ ")) + self.assertIn("could not merge upstream `main`", github.comments[0]) + self.assertIn("merge conflict", github.comments[0]) + self.assertIn("No new CI build was started.", github.comments[0]) + self.assertIn("`/ci run --allow-stale`", github.comments[0]) + def test_allow_stale_preserves_author_permissions_and_run_configuration( self, ) -> None: diff --git a/docs/contributing/README.md b/docs/contributing/README.md index 5acb8d9809..c36bdfe171 100644 --- a/docs/contributing/README.md +++ b/docs/contributing/README.md @@ -329,9 +329,15 @@ review process: in the upstream repository. Your branch must contain every commit currently on that branch: **zero commits behind**. This also applies to PRs targeting release branches or another PR's branch. -- If the PR is behind, the bot reports the count without creating a new build. - Merge or rebase onto the latest target branch, then rerun the command. If the - target branch advances in the meantime, update your branch and try again. +- If the PR is behind, the bot merges the target branch into your PR branch + (like the **Update branch** button) and runs CI on the resulting merge commit. + Pull the branch before pushing more changes. This needs **Allow edits by + maintainers** enabled on fork PRs. If the merge conflicts or is not allowed, + the bot reports the count without creating a new build; merge or rebase onto + the latest target branch yourself, then rerun the command. +- PR authors without write access cannot start another full run (including + `--allow-stale`) on a commit that already has a CI build. Push new changes, + use `/ci retry` to rerun failed jobs, or ask a reviewer with write access. - To run CI on an outdated branch at your own risk, append `--allow-stale` to any run command, for example `/ci run --allow-stale` or `/amd-ci run all --allow-stale`. The same authorization requirements apply. @@ -339,9 +345,8 @@ review process: failures. Before merging, merge or rebase onto the latest target branch and rerun CI without `--allow-stale` on the latest PR commit. - Retrying or cancelling builds does not check branch freshness. -- These commands do not modify your branch or enforce merge requirements. - Direct Buildkite launches are outside this workflow. No additional token is - needed. +- Apart from that merge, these commands do not modify your branch or enforce + merge requirements. Direct Buildkite launches are outside this workflow. Maintainers must configure required Buildkite status checks on the latest PR commit and enable GitHub's