diff --git a/.github/workflows/notify-ci-authorized.yml b/.github/workflows/notify-ci-authorized.yml new file mode 100644 index 0000000000..30ae33c376 --- /dev/null +++ b/.github/workflows/notify-ci-authorized.yml @@ -0,0 +1,49 @@ +name: Notify CI authorization + +on: + pull_request_target: + types: [labeled] + workflow_run: + workflows: [Record CI approval] + types: [completed] + +concurrency: + group: >- + notify-ci-authorized-${{ + github.event.pull_request.number || + github.event.workflow_run.pull_requests[0].number + }} + cancel-in-progress: false + +permissions: + contents: read + issues: write + pull-requests: read + +jobs: + notify: + if: >- + (github.event_name == 'pull_request_target' && + github.event.action == 'labeled' && + (github.event.label.name == 'ready' || + github.event.label.name == 'ready-run-all-tests')) || + (github.event_name == 'workflow_run' && + github.event.workflow_run.event == 'pull_request_review' && + github.event.workflow_run.conclusion == 'success' && + github.event.workflow_run.pull_requests[0]) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + ref: ${{ github.event.repository.default_branch }} + persist-credentials: false + - uses: astral-sh/setup-uv@37802adc94f370d6bfd71619e3f0bf239e1f3b78 # v7.6.0 + with: + python-version: "3.12" + - name: Notify PR author that CI is available + run: >- + uv run --no-project --python 3.12 + .github/workflows/scripts/run_ci_command.py + env: + CI_TRUSTED_USERS: ${{ vars.CI_TRUSTED_USERS }} + GH_TOKEN: ${{ github.token }} diff --git a/.github/workflows/record-ci-approval.yml b/.github/workflows/record-ci-approval.yml new file mode 100644 index 0000000000..f668b23029 --- /dev/null +++ b/.github/workflows/record-ci-approval.yml @@ -0,0 +1,14 @@ +name: Record CI approval + +on: + pull_request_review: + types: [submitted] + +permissions: {} + +jobs: + approved: + if: github.event.review.state == 'approved' + runs-on: ubuntu-latest + steps: + - run: echo "Approval recorded" diff --git a/.github/workflows/scripts/run_ci_command.py b/.github/workflows/scripts/run_ci_command.py index 52458b23fb..1dcf962325 100644 --- a/.github/workflows/scripts/run_ci_command.py +++ b/.github/workflows/scripts/run_ci_command.py @@ -12,6 +12,7 @@ from typing import Any COMMAND_RUN_CI = "/ci run" COMMAND_RETRY_FAILED = "/ci retry" +CI_AUTHORIZED_COMMENT_MARKER = "" READY_LABELS = {"ready", "ready-run-all-tests"} TRUSTED_PERMISSIONS = {"admin", "maintain", "write"} ACTIVE_BUILD_STATES = { @@ -175,6 +176,9 @@ class GitHubClient: def list_reviews(self, number: int) -> list[dict[str, Any]]: return self._paginate(self._repo_path(f"/pulls/{number}/reviews")) + def list_issue_comments(self, number: int) -> list[dict[str, Any]]: + return self._paginate(self._repo_path(f"/issues/{number}/comments")) + def list_reactions(self, comment_id: int) -> list[dict[str, Any]]: return self._paginate( self._repo_path(f"/issues/comments/{comment_id}/reactions") @@ -484,12 +488,90 @@ def add_reaction_safely( print(f"Could not add {content} reaction: {error}", file=sys.stderr) -def is_already_handled(github: GitHubClient, comment_id: int) -> bool: +def command_comment_marker(comment_id: int) -> str: + return f"" + + +def has_bot_comment_marker( + github: GitHubClient, + issue_number: int, + marker: str, +) -> bool: return any( + marker in str(comment.get("body", "")) + and (comment.get("user") or {}).get("login") == "github-actions[bot]" + for comment in github.list_issue_comments(issue_number) + ) + + +def is_already_handled( + github: GitHubClient, + issue_number: int, + comment_id: int, +) -> bool: + terminal_reaction = any( reaction.get("content") in {"rocket", "-1"} and (reaction.get("user") or {}).get("login") == "github-actions[bot]" for reaction in github.list_reactions(comment_id) ) + if terminal_reaction: + return True + return has_bot_comment_marker( + github, + issue_number, + command_comment_marker(comment_id), + ) + + +def notify_authorized( + event: Mapping[str, Any], + github: GitHubClient, + trusted_users_value: str = "", +) -> None: + pr = event["pull_request"] + if pr["state"] != "open" or pr["draft"]: + return + + trusted_users = parse_trusted_users(trusted_users_value) + author = pr["user"]["login"] + author_permission = github.get_permission(author) + if ( + is_trusted_permission(author_permission) + or author.casefold() in trusted_users + or has_bot_comment_marker( + github, + pr["number"], + CI_AUTHORIZED_COMMENT_MARKER, + ) + ): + return + + if "label" in event: + if ( + event.get("action") != "labeled" + or event["label"]["name"] not in READY_LABELS + or has_trusted_approval(github, pr["number"], trusted_users) + ): + return + elif "review" in event: + if ( + event.get("action") != "submitted" + or str(event["review"].get("state", "")).casefold() != "approved" + or has_ready_label(pr) + or not has_trusted_approval(github, pr["number"], trusted_users) + ): + return + else: + return + + github.add_comment( + pr["number"], + ( + f"✅ @{author}, CI is now available for this PR. Comment `/ci run` " + "to run full CI or `/ci retry` to retry failed jobs.\n\n" + f"{CI_AUTHORIZED_COMMENT_MARKER}" + ), + ) def handle_run_ci( @@ -553,8 +635,6 @@ def handle_retry_failed( metadata = build.get("meta_data") or {} if str(metadata.get("github-comment-id")) == str(comment_id): return f"CI was already requested by this comment: {build['web_url']}" - if not build.get("finished_at") or is_active_build(build): - return f"CI is still running for this commit: {build['web_url']}" retried = buildkite.retry_failed_jobs(build["number"], RETRY_STATES) if retried["retried_jobs_count"] == 0: @@ -652,7 +732,7 @@ def run( comment_id = event["comment"]["id"] actor = event["comment"]["user"]["login"] - if is_already_handled(github, comment_id): + if is_already_handled(github, issue_number, comment_id): print(f"Comment {comment_id} was already handled.") return add_reaction_safely(github, comment_id, "eyes") @@ -661,7 +741,11 @@ def run( pr = github.get_pr(issue_number) permission = github.get_permission(actor) if pr["state"] != "open": - github.add_comment(issue_number, "CI commands require an open PR.") + github.add_comment( + issue_number, + "❌ CI commands require an open PR.\n\n" + f"{command_comment_marker(comment_id)}", + ) return trusted_users = parse_trusted_users(trusted_users_value) @@ -685,8 +769,10 @@ def run( trusted_users=trusted_users, ) if not allowed: - add_reaction_safely(github, comment_id, "-1") - github.add_comment(issue_number, f"@{actor}, {reason}") + github.add_comment( + issue_number, + f"❌ @{actor}, {reason}\n\n{command_comment_marker(comment_id)}", + ) return print(f"Authorized @{actor}: {reason}") @@ -707,7 +793,7 @@ def run( pr=pr, ) add_reaction_safely(github, comment_id, "rocket") - github.add_comment(issue_number, message) + github.add_comment(issue_number, f"✅ {message}") except Exception: add_reaction_safely(github, comment_id, "confused") raise @@ -718,13 +804,37 @@ def main() -> None: with open(event_path, encoding="utf-8") as event_file: event = json.load(event_file) - if not parse_command(event["comment"]["body"]): - return - github = GitHubClient( os.environ.get("GH_TOKEN", ""), os.environ["GITHUB_REPOSITORY"], ) + event_name = os.environ.get("GITHUB_EVENT_NAME", "issue_comment") + if event_name == "pull_request_target": + notify_authorized( + event, + github, + os.environ.get("CI_TRUSTED_USERS", ""), + ) + return + if event_name == "workflow_run": + pull_requests = event["workflow_run"].get("pull_requests") or [] + if not pull_requests: + return + pr = github.get_pr(int(pull_requests[0]["number"])) + notify_authorized( + { + "action": "submitted", + "pull_request": pr, + "review": {"state": "approved"}, + }, + github, + os.environ.get("CI_TRUSTED_USERS", ""), + ) + return + + if not parse_command(event["comment"]["body"]): + return + buildkite = BuildkiteClient( os.environ.get("BUILDKITE_API_TOKEN", ""), os.environ.get("BUILDKITE_ORGANIZATION", "vllm"), diff --git a/.github/workflows/scripts/test_run_ci_command.py b/.github/workflows/scripts/test_run_ci_command.py index e8c58991b2..9ad6922aec 100644 --- a/.github/workflows/scripts/test_run_ci_command.py +++ b/.github/workflows/scripts/test_run_ci_command.py @@ -6,6 +6,7 @@ import unittest from typing import Any from run_ci_command import ( + CI_AUTHORIZED_COMMENT_MARKER, COMMAND_RETRY_FAILED, COMMAND_RUN_CI, RETRY_STATES, @@ -15,6 +16,7 @@ from run_ci_command import ( has_trusted_approval, is_active_build, is_build_for_pr, + notify_authorized, parse_command, parse_trusted_users, run, @@ -63,8 +65,9 @@ class FakeGitHub: pr: dict[str, Any] | None = None, review_decision: str = "REVIEW_REQUIRED", reviews: list[dict[str, Any]] | None = None, + comments: list[str] | None = None, ) -> None: - self.comments: list[str] = [] + self.comments = comments or [] self.permission = permission self.permissions = permissions or {} self.pr = pr or make_pr() @@ -84,6 +87,15 @@ class FakeGitHub: def list_reviews(self, number: int) -> list[dict[str, Any]]: return self.reviews + def list_issue_comments(self, number: int) -> list[dict[str, Any]]: + return [ + { + "body": body, + "user": {"login": "github-actions[bot]"}, + } + for body in self.comments + ] + def list_reactions(self, comment_id: int) -> list[dict[str, Any]]: return [] @@ -303,7 +315,7 @@ class RunCiCommandTest(unittest.TestCase): }, ) - def test_ci_run_dispatches_build_with_current_pr_metadata(self) -> None: + def test_write_reviewer_runs_ci_without_delegation(self) -> None: github = FakeGitHub() buildkite = FakeBuildkite([[], []]) run(make_event(COMMAND_RUN_CI), github, buildkite) @@ -314,6 +326,7 @@ class RunCiCommandTest(unittest.TestCase): "PR #42 /ci run by @reviewer", ) self.assertEqual(github.reactions, ["eyes", "rocket"]) + self.assertTrue(github.comments[0].startswith("✅ ")) self.assertIn("Buildkite CI #123", github.comments[0]) def test_unapproved_authors_are_denied_without_buildkite(self) -> None: @@ -326,10 +339,171 @@ class RunCiCommandTest(unittest.TestCase): run(make_event(COMMAND_RUN_CI, "author"), github, buildkite) self.assertEqual(buildkite.list_calls, []) - self.assertEqual(github.reactions, ["eyes", "-1"]) + self.assertEqual(github.reactions, ["eyes"]) + self.assertTrue(github.comments[0].startswith("❌ ")) self.assertIn("approve the PR", github.comments[0]) - def test_ci_retry_uses_latest_current_sha_build(self) -> None: + run(make_event(COMMAND_RUN_CI, "author"), github, buildkite) + self.assertEqual(len(github.comments), 1) + + def test_untrusted_approval_cannot_launch_ci(self) -> None: + github = FakeGitHub( + permission="read", + pr=make_pr(), + review_decision="APPROVED", + reviews=[ + { + "state": "APPROVED", + "user": {"login": "untrusted-reviewer"}, + } + ], + ) + buildkite = FakeBuildkite() + + run(make_event(COMMAND_RUN_CI, "author"), github, buildkite) + + self.assertEqual(buildkite.list_calls, []) + self.assertEqual(github.reactions, ["eyes"]) + self.assertTrue(github.comments[0].startswith("❌ ")) + + def test_ready_label_notifies_author_once(self) -> None: + pr = make_pr(labels=[{"name": "ready"}]) + event = { + "action": "labeled", + "label": {"name": "ready"}, + "pull_request": pr, + } + github = FakeGitHub(permission="read", pr=pr) + + notify_authorized(event, github) + notify_authorized(event, github) + + self.assertEqual(len(github.comments), 1) + self.assertTrue(github.comments[0].startswith("✅ @author")) + self.assertIn("`/ci run`", github.comments[0]) + self.assertIn("`/ci retry`", github.comments[0]) + self.assertIn(CI_AUTHORIZED_COMMENT_MARKER, github.comments[0]) + + def test_ready_label_does_not_notify_after_trusted_approval(self) -> None: + pr = make_pr(labels=[{"name": "ready"}]) + event = { + "action": "labeled", + "label": {"name": "ready"}, + "pull_request": pr, + } + github = FakeGitHub( + permission="read", + permissions={"reviewer": "write"}, + pr=pr, + review_decision="APPROVED", + reviews=[ + { + "state": "APPROVED", + "user": {"login": "reviewer"}, + } + ], + ) + + notify_authorized(event, github) + + self.assertEqual(github.comments, []) + + def test_trusted_approval_notifies_author_once(self) -> None: + event = { + "action": "submitted", + "pull_request": make_pr(), + "review": {"state": "approved"}, + } + github = FakeGitHub( + permission="read", + permissions={"reviewer": "write"}, + review_decision="APPROVED", + reviews=[ + { + "state": "APPROVED", + "user": {"login": "reviewer"}, + } + ], + ) + + notify_authorized(event, github) + notify_authorized(event, github) + + self.assertEqual(len(github.comments), 1) + self.assertIn("@author", github.comments[0]) + + def test_approval_does_not_notify_when_ready_label_exists(self) -> None: + pr = make_pr(labels=[{"name": "ready"}]) + event = { + "action": "submitted", + "pull_request": pr, + "review": {"state": "approved"}, + } + github = FakeGitHub( + permission="read", + permissions={"reviewer": "write"}, + pr=pr, + review_decision="APPROVED", + reviews=[ + { + "state": "APPROVED", + "user": {"login": "reviewer"}, + } + ], + ) + + notify_authorized(event, github) + + self.assertEqual(github.comments, []) + + def test_untrusted_approval_does_not_notify_author(self) -> None: + event = { + "action": "submitted", + "pull_request": make_pr(), + "review": {"state": "approved"}, + } + github = FakeGitHub( + permission="read", + review_decision="APPROVED", + reviews=[ + { + "state": "APPROVED", + "user": {"login": "reviewer"}, + } + ], + ) + + notify_authorized(event, github) + + self.assertEqual(github.comments, []) + + def test_notification_skips_authors_who_already_have_write(self) -> None: + pr = make_pr(labels=[{"name": "ready"}]) + event = { + "action": "labeled", + "label": {"name": "ready"}, + "pull_request": pr, + } + github = FakeGitHub(permission="write", pr=pr) + + notify_authorized(event, github) + + self.assertEqual(github.comments, []) + + def test_notification_skips_draft_prs(self) -> None: + pr = make_pr(draft=True, labels=[{"name": "ready"}]) + event = { + "action": "labeled", + "label": {"name": "ready"}, + "pull_request": pr, + } + github = FakeGitHub(permission="read", pr=pr) + + notify_authorized(event, github) + + self.assertEqual(github.comments, []) + + def test_ci_retry_retries_failed_jobs_while_build_is_running(self) -> None: github = FakeGitHub( permission="read", pr=make_pr(labels=[{"name": "ready"}]), @@ -339,10 +513,9 @@ class RunCiCommandTest(unittest.TestCase): [ { "created_at": "2026-07-28T01:00:00Z", - "finished_at": "2026-07-28T02:00:00Z", "number": 123, "pull_request": {"id": 42}, - "state": "failed", + "state": "failing", "web_url": "https://buildkite.example/builds/123", } ]