fix: prevent duplicate cherry-pick PRs via label check (#1055)

## Summary

- Check if cherry-pick label already exists on the PR before processing
- Skip branches whose `cherry-pick-{branch}` label is already present
- Post comment telling user to remove the label to re-trigger
- Works for both merged and unmerged PRs
- Early return when no valid target branches (avoids unnecessary API
calls)

Triggered by duplicate cherry-pick PRs on
RedHatQE/mtv-api-tests#369

Closes #1054

## Test plan

- [x] `test_process_cherry_pick_command_skips_already_cherry_picked` -
partial skip (merged PR)
- [x] `test_process_cherry_pick_command_all_already_cherry_picked` -
full skip (merged PR)
- [x]
`test_process_cherry_pick_command_skips_already_cherry_picked_unmerged`
- partial skip (unmerged PR)
- [x] All existing cherry-pick tests pass (9 total)
- [x] All 200 tests pass

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Bug Fixes**
* Cherry-pick operations now skip branches that are already labeled,
post a comment listing skipped branches with re-trigger instructions,
and return early if no valid targets remain.
  * Label naming/formatting standardized for consistency.
* Per-branch handling ensures failed label additions skip that branch
without affecting others.

* **Tests**
* Added idempotency and skip-case tests covering merged/unmerged PRs and
label-add failures.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Author: myakove

webhook_server/libs/handlers/issue_comment_handler.py

@@ -391,7 +391,7 @@ async def process_cherry_pick_command(
         """Process cherry-pick command for pull requests.
 
         This method handles cherry-pick requests for both unmerged and merged PRs.
-        Cherry-pick labels (cherry-pick/<branch-name>) are added in BOTH scenarios:
+        Cherry-pick labels (cherry-pick-<branch-name>) are added in BOTH scenarios:
 
         **Unmerged PRs:**
         - Labels indicate which branches need cherry-picking after the PR is merged
@@ -411,12 +411,17 @@ async def process_cherry_pick_command(
 
         Example:
             # Unmerged PR: /cherry-pick v1.0 v2.0
-            # - Adds labels: cherry-pick/v1.0, cherry-pick/v2.0
+            # - Adds labels: cherry-pick-v1.0, cherry-pick-v2.0
             # - Posts comment explaining labels will trigger auto cherry-pick on merge
 
             # Merged PR: /cherry-pick v1.0 v2.0
+            # - Adds labels: cherry-pick-v1.0, cherry-pick-v2.0 as idempotency markers
             # - Executes cherry-pick to v1.0 and v2.0 immediately
-            # - Adds labels: cherry-pick/v1.0, cherry-pick/v2.0 to track completion
+            # - Remove the label to retry a failed cherry-pick
+
+        Duplicate prevention:
+            If a cherry-pick label already exists for a target branch, that branch is
+            skipped. To re-trigger a cherry-pick, remove the label and run the command again.
         """
         _target_branches: list[str] = command_args.split()
         _exits_target_branches: set[str] = set()
@@ -438,29 +443,63 @@ async def process_cherry_pick_command(
             self.logger.info(f"{self.log_prefix} {_non_exits_target_branches_msg}")
             await asyncio.to_thread(pull_request.create_issue_comment, _non_exits_target_branches_msg)
 
-        cp_labels: list[str] = [
-            f"{CHERRY_PICK_LABEL_PREFIX}{_target_branch}" for _target_branch in _exits_target_branches
-        ]
+        if not _exits_target_branches:
+            return
+
+        # Filter out branches that already have cherry-pick labels
+        existing_labels = {label.name for label in await asyncio.to_thread(lambda: list(pull_request.labels))}
+        _already_cherry_picked: list[str] = []
+        _branches_to_process: set[str] = set()
+
+        for _branch in _exits_target_branches:
+            label_name = f"{CHERRY_PICK_LABEL_PREFIX}{_branch}"
+            if label_name in existing_labels:
+                _already_cherry_picked.append(_branch)
+            else:
+                _branches_to_process.add(_branch)
+
+        if _already_cherry_picked:
+            already_msg = ", ".join(f"`{b}`" for b in _already_cherry_picked)
+            await asyncio.to_thread(
+                pull_request.create_issue_comment,
+                f"Cherry-pick label already present for: {already_msg}\n"
+                "To re-trigger, remove the cherry-pick label(s) and run the command again.",
+            )
+
+        if not _branches_to_process:
+            return
 
-        if _exits_target_branches:
-            if not self.hook_data["issue"].get("pull_request", {}).get("merged_at"):
-                info_msg: str = f"""
+        cp_labels: list[str] = [f"{CHERRY_PICK_LABEL_PREFIX}{_branch}" for _branch in _branches_to_process]
+
+        if not self.hook_data["issue"].get("pull_request", {}).get("merged_at"):
+            info_msg: str = f"""
 Cherry-pick requested for PR: `{pull_request.title}` by user `{reviewed_user}`
-Adding label/s `{" ".join([_cp_label for _cp_label in cp_labels])}` for automatic cheery-pick once the PR is merged
+Adding label/s `{" ".join(cp_labels)}` for automatic cherry-pick once the PR is merged
 """
 
-                self.logger.info(f"{self.log_prefix} {info_msg}")
-                await asyncio.to_thread(pull_request.create_issue_comment, info_msg)
-            else:
-                for _exits_target_branch in _exits_target_branches:
-                    await self.runner_handler.cherry_pick(
-                        pull_request=pull_request,
-                        target_branch=_exits_target_branch,
-                        assign_to_pr_owner=self.github_webhook.cherry_pick_assign_to_pr_author,
+            self.logger.info(f"{self.log_prefix} {info_msg}")
+            await asyncio.to_thread(pull_request.create_issue_comment, info_msg)
+        else:
+            for _branch in _branches_to_process:
+                label_added = await self.labels_handler._add_label(
+                    pull_request=pull_request,
+                    label=f"{CHERRY_PICK_LABEL_PREFIX}{_branch}",
+                )
+                if not label_added:
+                    self.logger.info(
+                        f"{self.log_prefix} Skipping cherry-pick for {_branch},"
+                        " label already exists or could not be added"
                     )
+                    continue
+                await self.runner_handler.cherry_pick(
+                    pull_request=pull_request,
+                    target_branch=_branch,
+                    assign_to_pr_owner=self.github_webhook.cherry_pick_assign_to_pr_author,
+                )
+            return
 
-            for _cp_label in cp_labels:
-                await self.labels_handler._add_label(pull_request=pull_request, label=_cp_label)
+        for _cp_label in cp_labels:
+            await self.labels_handler._add_label(pull_request=pull_request, label=_cp_label)
 
     async def process_retest_command(
         self, pull_request: PullRequest, command_args: str, reviewed_user: str, automerge: bool = False

webhook_server/tests/test_issue_comment_handler.py

@@ -8,6 +8,7 @@
 from webhook_server.utils.constants import (
     APPROVE_STR,
     BUILD_AND_PUSH_CONTAINER_STR,
+    CHERRY_PICK_LABEL_PREFIX,
     COMMAND_ASSIGN_REVIEWER_STR,
     COMMAND_ASSIGN_REVIEWERS_STR,
     COMMAND_CHECK_CAN_MERGE_STR,
@@ -794,6 +795,7 @@ async def test_process_cherry_pick_command_existing_branches(
         """Test processing cherry pick command with existing branches."""
         mock_pull_request = Mock()
         mock_pull_request.title = "Test PR"
+        mock_pull_request.labels = []
         # Patch is_merged as a method
         with patch.object(mock_pull_request, "is_merged", new=Mock(return_value=False)):
             with patch.object(issue_comment_handler.repository, "get_branch") as mock_get_branch:
@@ -820,6 +822,7 @@ async def test_process_cherry_pick_command_non_existing_branches(
     ) -> None:
         """Test processing cherry pick command with non-existing branches."""
         mock_pull_request = Mock()
+        mock_pull_request.labels = []
 
         with patch.object(issue_comment_handler.repository, "get_branch", side_effect=Exception("Branch not found")):
             with patch.object(mock_pull_request, "create_issue_comment") as mock_comment:
@@ -834,6 +837,7 @@ async def test_process_cherry_pick_command_non_existing_branches(
     async def test_process_cherry_pick_command_merged_pr(self, issue_comment_handler: IssueCommentHandler) -> None:
         """Test processing cherry pick command for merged PR."""
         mock_pull_request = Mock()
+        mock_pull_request.labels = []
         # Set merged_at in hook_data to simulate a merged PR at comment time
         issue_comment_handler.hook_data["issue"]["pull_request"] = {"merged_at": "2026-01-01T00:00:00Z"}
         with patch.object(issue_comment_handler.repository, "get_branch"):
@@ -846,6 +850,7 @@ async def test_process_cherry_pick_command_merged_pr(self, issue_comment_handler
                     issue_comment_handler.labels_handler,
                     "_add_label",
                     new_callable=AsyncMock,
+                    return_value=True,
                 ) as mock_add_label:
                     await issue_comment_handler.process_cherry_pick_command(
                         pull_request=mock_pull_request,
@@ -859,7 +864,7 @@ async def test_process_cherry_pick_command_merged_pr(self, issue_comment_handler
                     )
                     mock_add_label.assert_called_once_with(
                         pull_request=mock_pull_request,
-                        label="cherry-pick-branch1",
+                        label=f"{CHERRY_PICK_LABEL_PREFIX}branch1",
                     )
 
     @pytest.mark.asyncio
@@ -875,6 +880,7 @@ async def test_process_cherry_pick_command_merged_pr_multiple_branches(
         """
         mock_pull_request = Mock()
         mock_pull_request.title = "Test PR"
+        mock_pull_request.labels = []
 
         # Set merged_at in hook_data to simulate a merged PR at comment time
         issue_comment_handler.hook_data["issue"]["pull_request"] = {"merged_at": "2026-01-01T00:00:00Z"}
@@ -888,6 +894,7 @@ async def test_process_cherry_pick_command_merged_pr_multiple_branches(
                     issue_comment_handler.labels_handler,
                     "_add_label",
                     new_callable=AsyncMock,
+                    return_value=True,
                 ) as mock_add_label:
                     # Execute cherry-pick command with multiple branches
                     await issue_comment_handler.process_cherry_pick_command(
@@ -916,9 +923,15 @@ async def test_process_cherry_pick_command_merged_pr_multiple_branches(
 
                     # Verify labels were added exactly once for each branch (not duplicated)
                     assert mock_add_label.call_count == 3
-                    mock_add_label.assert_any_call(pull_request=mock_pull_request, label="cherry-pick-branch1")
-                    mock_add_label.assert_any_call(pull_request=mock_pull_request, label="cherry-pick-branch2")
-                    mock_add_label.assert_any_call(pull_request=mock_pull_request, label="cherry-pick-branch3")
+                    mock_add_label.assert_any_call(
+                        pull_request=mock_pull_request, label=f"{CHERRY_PICK_LABEL_PREFIX}branch1"
+                    )
+                    mock_add_label.assert_any_call(
+                        pull_request=mock_pull_request, label=f"{CHERRY_PICK_LABEL_PREFIX}branch2"
+                    )
+                    mock_add_label.assert_any_call(
+                        pull_request=mock_pull_request, label=f"{CHERRY_PICK_LABEL_PREFIX}branch3"
+                    )
 
     @pytest.mark.asyncio
     async def test_process_cherry_pick_command_merged_pr_assign_disabled(
@@ -927,6 +940,7 @@ async def test_process_cherry_pick_command_merged_pr_assign_disabled(
         """Test cherry-pick on merged PR passes assign_to_pr_owner=False when config disabled."""
         issue_comment_handler.github_webhook.cherry_pick_assign_to_pr_author = False
         mock_pull_request = Mock()
+        mock_pull_request.labels = []
         # Set merged_at in hook_data to simulate a merged PR at comment time
         issue_comment_handler.hook_data["issue"]["pull_request"] = {"merged_at": "2026-01-01T00:00:00Z"}
         with patch.object(issue_comment_handler.repository, "get_branch"):
@@ -939,6 +953,7 @@ async def test_process_cherry_pick_command_merged_pr_assign_disabled(
                     issue_comment_handler.labels_handler,
                     "_add_label",
                     new_callable=AsyncMock,
+                    return_value=True,
                 ):
                     await issue_comment_handler.process_cherry_pick_command(
                         pull_request=mock_pull_request,
@@ -951,6 +966,181 @@ async def test_process_cherry_pick_command_merged_pr_assign_disabled(
                         assign_to_pr_owner=False,
                     )
 
+    @pytest.mark.asyncio
+    async def test_process_cherry_pick_command_skips_already_cherry_picked(
+        self, issue_comment_handler: IssueCommentHandler
+    ) -> None:
+        """Test cherry-pick skips branches whose cherry-pick label already exists on the PR."""
+        mock_pull_request = Mock()
+        mock_pull_request.title = "Test PR"
+
+        # Simulate existing cherry-pick label for branch1
+        existing_label = Mock()
+        existing_label.name = f"{CHERRY_PICK_LABEL_PREFIX}branch1"
+        mock_pull_request.labels = [existing_label]
+
+        # Set merged_at in hook_data to simulate a merged PR at comment time
+        issue_comment_handler.hook_data["issue"]["pull_request"] = {"merged_at": "2026-01-01T00:00:00Z"}
+        with (
+            patch.object(issue_comment_handler.repository, "get_branch"),
+            patch.object(
+                issue_comment_handler.runner_handler,
+                "cherry_pick",
+                new_callable=AsyncMock,
+            ) as mock_cherry_pick,
+            patch.object(
+                issue_comment_handler.labels_handler,
+                "_add_label",
+                new_callable=AsyncMock,
+                return_value=True,
+            ) as mock_add_label,
+            patch.object(mock_pull_request, "create_issue_comment") as mock_comment,
+        ):
+            await issue_comment_handler.process_cherry_pick_command(
+                pull_request=mock_pull_request,
+                command_args="branch1 branch2",
+                reviewed_user="test-user",
+            )
+
+            # branch1 should be skipped (label already exists), branch2 should proceed
+            mock_cherry_pick.assert_called_once_with(
+                pull_request=mock_pull_request,
+                target_branch="branch2",
+                assign_to_pr_owner=True,
+            )
+
+            # Only branch2 label should be added
+            mock_add_label.assert_called_once_with(
+                pull_request=mock_pull_request,
+                label=f"{CHERRY_PICK_LABEL_PREFIX}branch2",
+            )
+
+            # Verify "already present" comment was posted for branch1
+            assert any(
+                "already present" in call.args[0] and "branch1" in call.args[0] for call in mock_comment.call_args_list
+            ), f"Expected 'already present' comment for branch1, got: {mock_comment.call_args_list}"
+
+    @pytest.mark.asyncio
+    async def test_process_cherry_pick_command_all_already_cherry_picked(
+        self, issue_comment_handler: IssueCommentHandler
+    ) -> None:
+        """Test cherry-pick does nothing when all branches already have cherry-pick labels."""
+        mock_pull_request = Mock()
+        mock_pull_request.title = "Test PR"
+
+        # All branches already cherry-picked
+        label1 = Mock()
+        label1.name = f"{CHERRY_PICK_LABEL_PREFIX}branch1"
+        label2 = Mock()
+        label2.name = f"{CHERRY_PICK_LABEL_PREFIX}branch2"
+        mock_pull_request.labels = [label1, label2]
+
+        issue_comment_handler.hook_data["issue"]["pull_request"] = {"merged_at": "2026-01-01T00:00:00Z"}
+        with (
+            patch.object(issue_comment_handler.repository, "get_branch"),
+            patch.object(
+                issue_comment_handler.runner_handler,
+                "cherry_pick",
+                new_callable=AsyncMock,
+            ) as mock_cherry_pick,
+            patch.object(
+                issue_comment_handler.labels_handler,
+                "_add_label",
+                new_callable=AsyncMock,
+            ) as mock_add_label,
+            patch.object(mock_pull_request, "create_issue_comment") as mock_comment,
+        ):
+            await issue_comment_handler.process_cherry_pick_command(
+                pull_request=mock_pull_request,
+                command_args="branch1 branch2",
+                reviewed_user="test-user",
+            )
+
+            # No cherry-picks should be triggered
+            mock_cherry_pick.assert_not_called()
+
+            # No labels should be added
+            mock_add_label.assert_not_called()
+
+            # "already present" comment should be posted
+            assert any("already present" in call.args[0] for call in mock_comment.call_args_list), (
+                f"Expected 'already present' comment, got: {mock_comment.call_args_list}"
+            )
+
+    @pytest.mark.asyncio
+    async def test_process_cherry_pick_command_skips_when_add_label_fails(
+        self, issue_comment_handler: IssueCommentHandler
+    ) -> None:
+        """Test cherry_pick is skipped when _add_label returns False (TOCTOU protection)."""
+        mock_pull_request = Mock()
+        mock_pull_request.title = "Test PR"
+        mock_pull_request.labels = []  # No existing labels in snapshot
+
+        issue_comment_handler.hook_data["issue"]["pull_request"] = {"merged_at": "2024-01-01T00:00:00Z"}
+
+        with (
+            patch.object(issue_comment_handler.repository, "get_branch"),
+            # _add_label returns False (label was added by another webhook in the meantime)
+            patch.object(
+                issue_comment_handler.labels_handler,
+                "_add_label",
+                new_callable=AsyncMock,
+                return_value=False,
+            ),
+            patch.object(
+                issue_comment_handler.runner_handler,
+                "cherry_pick",
+                new_callable=AsyncMock,
+            ) as mock_cherry_pick,
+        ):
+            await issue_comment_handler.process_cherry_pick_command(
+                pull_request=mock_pull_request,
+                command_args="branch1",
+                reviewed_user="test-user",
+            )
+            mock_cherry_pick.assert_not_called()
+
+    @pytest.mark.asyncio
+    async def test_process_cherry_pick_command_skips_already_cherry_picked_unmerged(
+        self, issue_comment_handler: IssueCommentHandler
+    ) -> None:
+        """Test cherry-pick label check works for unmerged PRs too."""
+        mock_pull_request = Mock()
+        mock_pull_request.title = "Test PR"
+
+        # Simulate existing cherry-pick label for branch1
+        existing_label = Mock()
+        existing_label.name = f"{CHERRY_PICK_LABEL_PREFIX}branch1"
+        mock_pull_request.labels = [existing_label]
+
+        # Set merged_at to None to simulate an unmerged PR
+        issue_comment_handler.hook_data["issue"]["pull_request"] = {"merged_at": None}
+        with (
+            patch.object(issue_comment_handler.repository, "get_branch"),
+            patch.object(
+                issue_comment_handler.labels_handler,
+                "_add_label",
+                new_callable=AsyncMock,
+            ) as mock_add_label,
+            patch.object(mock_pull_request, "create_issue_comment") as mock_comment,
+        ):
+            await issue_comment_handler.process_cherry_pick_command(
+                pull_request=mock_pull_request,
+                command_args="branch1 branch2",
+                reviewed_user="test-user",
+            )
+
+            # Verify "already present" comment was posted for branch1
+            assert any(
+                "already present" in call.args[0] and "branch1" in call.args[0] for call in mock_comment.call_args_list
+            ), f"Expected 'already present' comment for branch1, got: {mock_comment.call_args_list}"
+
+            # Only branch2 label should be added (not branch1)
+            mock_add_label.assert_called_once_with(
+                pull_request=mock_pull_request,
+                label=f"{CHERRY_PICK_LABEL_PREFIX}branch2",
+            )
+
     @pytest.mark.asyncio
     async def test_process_retest_command_no_target_tests(self, issue_comment_handler: IssueCommentHandler) -> None:
         """Test processing retest command with no target tests."""