From 0babb1fe41cb0d8c359360eccfcda3dc16609219 Mon Sep 17 00:00:00 2001 From: Meni Yakove Date: Mon, 20 Oct 2025 13:04:24 +0300 Subject: [PATCH] feat: add configurable auto-verification for cherry-picked PRs Added configuration option to control automatic verification of cherry-picked PRs, allowing users to disable auto-verification while maintaining backward compatibility (default: true). Configuration: - New `auto-verify-cherry-picked-prs` boolean config (default: true) - Available as global config and per-repository override - Cherry-picked PRs are detected via CHERRY_PICKED_LABEL_PREFIX label Changes: - Added auto_verify_cherry_picked_prs property to GithubWebhook class - Modified _process_verified_for_update_or_new_pull_request() to check: * If PR is cherry-picked (has CherryPicked label) * If auto-verify is disabled for cherry-picks * Skips auto-verification and sets status to queued if disabled - Added CHERRY_PICKED_LABEL_PREFIX import to pull_request_handler - Updated schema with new boolean field (global + repo level) - Updated example configs with usage examples - Added 2 comprehensive tests for both enabled/disabled scenarios - Updated README with cherry-pick configuration documentation - Updated schema validator to include new boolean field Backward Compatibility: - Default is true (maintains current behavior) - Cherry-picked PRs continue to be auto-verified by default - Users can opt-out per repository or globally Testing: - test_process_verified_cherry_picked_pr_auto_verify_enabled: Verifies default behavior - test_process_verified_cherry_picked_pr_auto_verify_disabled: Verifies disabled behavior - All 49 pull request handler tests pass --- README.md | 10 +++++ examples/.github-webhook-server.yaml | 3 ++ examples/config.yaml | 4 ++ webhook_server/config/schema.yaml | 8 ++++ webhook_server/libs/github_api.py | 3 ++ webhook_server/libs/pull_request_handler.py | 14 ++++++ webhook_server/tests/manifests/config.yaml | 2 + .../tests/test_pull_request_handler.py | 45 +++++++++++++++++++ webhook_server/tests/test_schema_validator.py | 10 ++++- 9 files changed, 97 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 0c18064d..7d9fc3c7 100644 --- a/README.md +++ b/README.md @@ -236,6 +236,8 @@ auto-verified-and-merged-users: - "renovate[bot]" - "dependabot[bot]" +auto-verify-cherry-picked-prs: true # Auto-verify cherry-picked PRs (default: true) + # Global PR Size Labels (optional) pr-size-thresholds: Tiny: @@ -1151,11 +1153,19 @@ Users can interact with the webhook server through GitHub comments on pull reque ### Cherry-pick Commands +Cherry-picked PRs can be automatically verified or require manual verification depending on your configuration. + | Command | Description | Example | | ------------------------------ | -------------------------------- | ------------------------ | | `/cherry-pick branch` | Cherry-pick to single branch | `/cherry-pick develop` | | `/cherry-pick branch1 branch2` | Cherry-pick to multiple branches | `/cherry-pick v1.0 v2.0` | +**Configuration**: Control auto-verification of cherry-picked PRs: +```yaml +auto-verify-cherry-picked-prs: true # Default: true (auto-verify) + # Set to false to require manual verification +``` + ### Label Commands | Command | Description | Example | diff --git a/examples/.github-webhook-server.yaml b/examples/.github-webhook-server.yaml index 44b1023a..2b373e2d 100644 --- a/examples/.github-webhook-server.yaml +++ b/examples/.github-webhook-server.yaml @@ -66,6 +66,9 @@ auto-verified-and-merged-users: - "dependabot[bot]" - "trusted-user" +# Auto-verify cherry-picked PRs (default: true) +auto-verify-cherry-picked-prs: false # Set to false to require manual verification for cherry-picked PRs + # Repository-specific GitHub tokens github-tokens: - ghp_your_repository_specific_token_here diff --git a/examples/config.yaml b/examples/config.yaml index 2966d59b..dca84256 100644 --- a/examples/config.yaml +++ b/examples/config.yaml @@ -27,6 +27,8 @@ auto-verified-and-merged-users: - "renovate[bot]" - "pre-commit-ci[bot]" +auto-verify-cherry-picked-prs: true # Default: true - automatically verify cherry-picked PRs. Set to false to require manual verification. + create-issue-for-new-pr: true # Global default: create tracking issues for new PRs # Global PR size label configuration (optional) @@ -101,6 +103,8 @@ repositories: auto-verified-and-merged-users: # override auto verified users per repository - "my[bot]" + auto-verify-cherry-picked-prs: false # Disable auto-verification for cherry-picked PRs in this repository + github-tokens: # override GitHub tokens per repository - - diff --git a/webhook_server/config/schema.yaml b/webhook_server/config/schema.yaml index 444a1ea6..56147a94 100644 --- a/webhook_server/config/schema.yaml +++ b/webhook_server/config/schema.yaml @@ -66,6 +66,10 @@ properties: type: array items: type: string + auto-verify-cherry-picked-prs: + type: boolean + description: Automatically verify cherry-picked PRs (default is true for backward compatibility) + default: true create-issue-for-new-pr: type: boolean description: Create a tracking issue for new pull requests (global default) @@ -211,6 +215,10 @@ properties: items: type: string description: Users whose PRs are automatically verified and merged + auto-verify-cherry-picked-prs: + type: boolean + description: Automatically verify cherry-picked PRs for this repository (default is true) + default: true github-tokens: type: array items: diff --git a/webhook_server/libs/github_api.py b/webhook_server/libs/github_api.py index 14951cfb..b23ce068 100644 --- a/webhook_server/libs/github_api.py +++ b/webhook_server/libs/github_api.py @@ -242,6 +242,9 @@ def _repo_data_from_config(self, repository_config: dict[str, Any]) -> None: self.auto_verified_and_merged_users: list[str] = self.config.get_value( value="auto-verified-and-merged-users", return_on_none=[], extra_dict=repository_config ) + self.auto_verify_cherry_picked_prs: bool = self.config.get_value( + value="auto-verify-cherry-picked-prs", return_on_none=True, extra_dict=repository_config + ) self.can_be_merged_required_labels = self.config.get_value( value="can-be-merged-required-labels", return_on_none=[], extra_dict=repository_config ) diff --git a/webhook_server/libs/pull_request_handler.py b/webhook_server/libs/pull_request_handler.py index b5d1c3db..34151e28 100644 --- a/webhook_server/libs/pull_request_handler.py +++ b/webhook_server/libs/pull_request_handler.py @@ -18,6 +18,7 @@ CAN_BE_MERGED_STR, CHANGED_REQUESTED_BY_LABEL_PREFIX, CHERRY_PICK_LABEL_PREFIX, + CHERRY_PICKED_LABEL_PREFIX, COMMENTED_BY_LABEL_PREFIX, CONVENTIONAL_TITLE_STR, FAILURE_STR, @@ -577,6 +578,19 @@ async def _process_verified_for_update_or_new_pull_request(self, pull_request: P if not self.github_webhook.verified_job: return + # Check if this is a cherry-picked PR + labels = await asyncio.to_thread(lambda: list(pull_request.labels)) + is_cherry_picked = any(label.name == CHERRY_PICKED_LABEL_PREFIX for label in labels) + + # If it's a cherry-picked PR and auto-verify is disabled for cherry-picks, skip auto-verification + if is_cherry_picked and not self.github_webhook.auto_verify_cherry_picked_prs: + self.logger.info( + f"{self.log_prefix} Cherry-picked PR detected and auto-verify-cherry-picked-prs is disabled, " + "skipping auto-verification" + ) + await self.check_run_handler.set_verify_check_queued() + return + if self.github_webhook.parent_committer in self.github_webhook.auto_verified_and_merged_users: self.logger.info( f"{self.log_prefix} Committer {self.github_webhook.parent_committer} is part of {self.github_webhook.auto_verified_and_merged_users}" diff --git a/webhook_server/tests/manifests/config.yaml b/webhook_server/tests/manifests/config.yaml index e00f459b..da1c14bb 100644 --- a/webhook_server/tests/manifests/config.yaml +++ b/webhook_server/tests/manifests/config.yaml @@ -23,6 +23,8 @@ auto-verified-and-merged-users: - "renovate[bot]" - "pre-commit-ci[bot]" +auto-verify-cherry-picked-prs: true + repositories: test-repo: name: my-org/test-repo diff --git a/webhook_server/tests/test_pull_request_handler.py b/webhook_server/tests/test_pull_request_handler.py index b6392291..e4cbebdd 100644 --- a/webhook_server/tests/test_pull_request_handler.py +++ b/webhook_server/tests/test_pull_request_handler.py @@ -1,5 +1,6 @@ import pytest from unittest.mock import AsyncMock, Mock, patch +from github.PullRequest import PullRequest from webhook_server.libs.pull_request_handler import PullRequestHandler from webhook_server.utils.constants import ( @@ -501,6 +502,50 @@ async def test_process_verified_for_update_or_new_pull_request_not_auto_verified mock_add_label.assert_not_called() mock_success.assert_not_called() + @pytest.mark.asyncio + async def test_process_verified_cherry_picked_pr_auto_verify_enabled( + self, pull_request_handler: PullRequestHandler + ) -> None: + """Test cherry-picked PR with auto-verify enabled (default behavior).""" + from webhook_server.utils.constants import CHERRY_PICKED_LABEL_PREFIX + + mock_pull_request = Mock(spec=PullRequest) + mock_label = Mock() + mock_label.name = CHERRY_PICKED_LABEL_PREFIX + mock_pull_request.labels = [mock_label] + + with ( + patch.object(pull_request_handler.github_webhook, "auto_verify_cherry_picked_prs", True), + patch.object(pull_request_handler.labels_handler, "_add_label") as mock_add_label, + patch.object(pull_request_handler.check_run_handler, "set_verify_check_success") as mock_set_success, + ): + await pull_request_handler._process_verified_for_update_or_new_pull_request(mock_pull_request) + # Should auto-verify since auto_verify_cherry_picked_prs is True and user is in auto_verified list + mock_add_label.assert_called_once() + mock_set_success.assert_called_once() + + @pytest.mark.asyncio + async def test_process_verified_cherry_picked_pr_auto_verify_disabled( + self, pull_request_handler: PullRequestHandler + ) -> None: + """Test cherry-picked PR with auto-verify disabled.""" + from webhook_server.utils.constants import CHERRY_PICKED_LABEL_PREFIX + + mock_pull_request = Mock(spec=PullRequest) + mock_label = Mock() + mock_label.name = CHERRY_PICKED_LABEL_PREFIX + mock_pull_request.labels = [mock_label] + + with ( + patch.object(pull_request_handler.github_webhook, "auto_verify_cherry_picked_prs", False), + patch.object(pull_request_handler.labels_handler, "_add_label") as mock_add_label, + patch.object(pull_request_handler.check_run_handler, "set_verify_check_queued") as mock_set_queued, + ): + await pull_request_handler._process_verified_for_update_or_new_pull_request(mock_pull_request) + # Should NOT auto-verify since auto_verify_cherry_picked_prs is False + mock_add_label.assert_not_called() + mock_set_queued.assert_called_once() + @pytest.mark.asyncio async def test_add_pull_request_owner_as_assingee( self, pull_request_handler: PullRequestHandler, mock_pull_request: Mock diff --git a/webhook_server/tests/test_schema_validator.py b/webhook_server/tests/test_schema_validator.py index be6c2778..4260b061 100644 --- a/webhook_server/tests/test_schema_validator.py +++ b/webhook_server/tests/test_schema_validator.py @@ -67,7 +67,13 @@ def _validate_root_fields(self, config: dict[str, Any]) -> None: self.errors.append(f"Field '{field}' must be an integer") # Boolean fields - boolean_fields = ["verify-github-ips", "verify-cloudflare-ips", "disable-ssl-warnings", "mask-sensitive-data"] + boolean_fields = [ + "verify-github-ips", + "verify-cloudflare-ips", + "disable-ssl-warnings", + "mask-sensitive-data", + "auto-verify-cherry-picked-prs", + ] for field in boolean_fields: if field in config and not isinstance(config[field], bool): self.errors.append(f"Field '{field}' must be a boolean") @@ -160,7 +166,7 @@ def _validate_single_repository(self, repo_name: str, repo_config: Any) -> None: self.errors.append(f"Repository '{repo_name}' field '{field}' must be a string") # Boolean fields - boolean_fields = ["verified-job", "pre-commit", "mask-sensitive-data"] + boolean_fields = ["verified-job", "pre-commit", "mask-sensitive-data", "auto-verify-cherry-picked-prs"] for field in boolean_fields: if field in repo_config and not isinstance(repo_config[field], bool): self.errors.append(f"Repository '{repo_name}' field '{field}' must be a boolean")