fix(cherry-pick): prevent label duplication on merged PRs

[myakove]

When cherry-picking to multiple branches on a merged PR, labels were being added N times (once per branch) inside the cherry-pick loop, resulting in N×N total label additions.

Fixed by moving label addition outside the cherry-pick loop, ensuring labels are added exactly once per target branch.

Also added:

• Comprehensive test for multiple branches scenario
• Docstring explaining why labels are added for both unmerged and merged PRs
• Labels serve as audit trail for completed cherry-picks on merged PRs

Test coverage:

• test_process_cherry_pick_command_merged_pr_multiple_branches validates fix
• All 5 cherry-pick tests passing

Summary by CodeRabbit

Release Notes

• Bug Fixes

  • Improved consistency in cherry-pick branch label tracking and application.

• Tests

  • Extended test coverage for multi-branch cherry-pick operations.

[coderabbitai]

Walkthrough

The PR refactors cherry-pick label handling in the issue comment handler. Labels are now computed upfront from exit target branches and applied uniformly via a dedicated post-processing loop, regardless of PR merge status. A comprehensive docstring documents expected behavior for both unmerged and merged PR scenarios.

Changes

Cohort / File(s)	Change Summary
Implementation refactoring webhook_server/libs/handlers/issue_comment_handler.py	Adds comprehensive docstring to process_cherry_pick_command. Refactors label preparation by computing cp_labels upfront from exit target branches. Centralizes label creation to a single dedicated loop that executes after branch validation and merge status checks, ensuring uniform label application regardless of merge state. Removes inlined label creation from the "PR not merged" branch.
Test updates webhook_server/tests/test_issue_comment_handler.py	Extends test_process_cherry_pick_command_merged_pr to validate _add_label invocation for single-branch case. Adds new test test_process_cherry_pick_command_merged_pr_multiple_branches to verify cherry-pick and label application (cherry-pick-branch1, cherry-pick-branch2, cherry-pick-branch3) when multiple branches are provided on merged PRs.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Focus on understanding the refactored label application flow in process_cherry_pick_command to ensure uniform behavior across merge states
• Verify the post-processing label loop correctly handles both single and multiple branch scenarios
• Confirm test assertions accurately reflect the new centralized label application logic

Possibly related issues

• fix(cherry-pick): prevent label duplication on merged PRs - 902 #903: Addresses the same cherry-pick label handling behavior for merged PRs—centralizing label addition to prevent duplication and ensure labels are applied once per target branch.

Suggested labels

size/L, branch-main, can-be-merged

Suggested reviewers

• rnetser

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix(cherry-pick): prevent label duplication on merged PRs' is specific and directly related to the main change: refactoring label addition logic outside the cherry-pick loop to prevent N×N label additions.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch add-cherrypick-label-on-merged-pr

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b779108 and 67aecff.

📒 Files selected for processing (2)

• webhook_server/libs/handlers/issue_comment_handler.py (3 hunks)
• webhook_server/tests/test_issue_comment_handler.py (1 hunks)

🧰 Additional context used

🧠 Learnings (3)

📓 Common learnings

Learnt from: myakove
Repo: myk-org/github-webhook-server PR: 612
File: webhook_server_container/libs/github_api.py:2089-2100
Timestamp: 2024-10-29T08:09:57.157Z
Learning: In `webhook_server_container/libs/github_api.py`, when the function `_keep_approved_by_approvers_after_rebase` is called, existing approval labels have already been cleared after pushing new changes, so there's no need to check for existing approvals within this function.

Learnt from: myakove
Repo: myk-org/github-webhook-server PR: 612
File: webhook_server_container/libs/github_api.py:925-926
Timestamp: 2024-10-29T10:42:50.163Z
Learning: In `webhook_server_container/libs/github_api.py`, the method `self._keep_approved_by_approvers_after_rebase()` must be called after removing labels when synchronizing a pull request. Therefore, it should be placed outside the `ThreadPoolExecutor` to ensure it runs sequentially after label removal.

📚 Learning: 2024-10-29T10:42:50.163Z

Learnt from: myakove
Repo: myk-org/github-webhook-server PR: 612
File: webhook_server_container/libs/github_api.py:925-926
Timestamp: 2024-10-29T10:42:50.163Z
Learning: In `webhook_server_container/libs/github_api.py`, the method `self._keep_approved_by_approvers_after_rebase()` must be called after removing labels when synchronizing a pull request. Therefore, it should be placed outside the `ThreadPoolExecutor` to ensure it runs sequentially after label removal.

Applied to files:

• webhook_server/libs/handlers/issue_comment_handler.py

📚 Learning: 2024-10-29T08:09:57.157Z

Learnt from: myakove
Repo: myk-org/github-webhook-server PR: 612
File: webhook_server_container/libs/github_api.py:2089-2100
Timestamp: 2024-10-29T08:09:57.157Z
Learning: In `webhook_server_container/libs/github_api.py`, when the function `_keep_approved_by_approvers_after_rebase` is called, existing approval labels have already been cleared after pushing new changes, so there's no need to check for existing approvals within this function.

Applied to files:

• webhook_server/tests/test_issue_comment_handler.py

🧬 Code graph analysis (2)

webhook_server/libs/handlers/issue_comment_handler.py (1)

webhook_server/libs/handlers/labels_handler.py (1)

• _add_label (90-144)

webhook_server/tests/test_issue_comment_handler.py (1)

webhook_server/libs/handlers/issue_comment_handler.py (1)

• process_cherry_pick_command (329-404)

🔇 Additional comments (2)

webhook_server/libs/handlers/issue_comment_handler.py (1)

382-405: Centralized label loop removes the duplication

Moving cp_labels construction and the _add_label loop outside the per-branch cherry-pick iteration ensures each branch label is applied exactly once even when multiple branches are requested. This directly fixes the N×N explosion we saw in merged PR scenarios. Nicely done.

webhook_server/tests/test_issue_comment_handler.py (1)

660-717: Great coverage on multi-branch merged cherry-picks

The new test verifies both the per-branch cherry_pick invocations and that each cherry-pick/<branch> label lands only once. That regression guard will keep the duplication bug from sneaking back.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[myakove-bot]

Report bugs in Issues

Welcome! 🎉

This pull request will be automatically processed with the following features:

🔄 Automatic Actions

• Reviewer Assignment: Reviewers are automatically assigned based on the "
  "OWNERS file in the repository root
  "
  "* Size Labeling: PR size labels (XS, S, M, L, XL, XXL) are "
  "automatically applied based on changes
  "
  f"* Issue Creation: A tracking issue is created for this PR and will be closed when the PR is merged or closed
  "
  "* Pre-commit Checks: pre-commit runs "
  "automatically if .pre-commit-config.yaml exists
  "
• Branch Labeling: Branch-specific labels are applied to track the target branch
• Auto-verification: Auto-verified users have their PRs automatically marked as verified

📋 Available Commands

PR Status Management

• /wip - Mark PR as work in progress (adds WIP: prefix to title)
• /wip cancel - Remove work in progress status
• /hold - Block PR merging (approvers only)
• /hold cancel - Unblock PR merging
• /verified - Mark PR as verified
• /verified cancel - Remove verification status
• /reprocess - Trigger complete PR workflow reprocessing (useful if webhook failed or configuration changed)

Review & Approval

• /lgtm - Approve changes (looks good to me)
• /approve - Approve PR (approvers only)
• /automerge - Enable automatic merging when all requirements are met (maintainers and approvers only)
• /assign-reviewers - Assign reviewers based on OWNERS file
• /assign-reviewer @username - Assign specific reviewer
• /check-can-merge - Check if PR meets merge requirements

Testing & Validation

• /retest tox - Run Python test suite with tox
• /retest build-container - Rebuild and test container image
• /retest python-module-install - Test Python package installation
• /retest pre-commit - Run pre-commit hooks and checks
• /retest conventional-title - Validate commit message format
• /retest all - Run all available tests

Container Operations

• /build-and-push-container - Build and push container image (tagged with PR number)
  • Supports additional build arguments: /build-and-push-container --build-arg KEY=value

Cherry-pick Operations

• /cherry-pick <branch> - Schedule cherry-pick to target branch when PR is merged
  • Multiple branches: /cherry-pick branch1 branch2 branch3

Label Management

• /<label-name> - Add a label to the PR
• /<label-name> cancel - Remove a label from the PR

✅ Merge Requirements

This PR will be automatically approved when the following conditions are met:

1. Approval: /approve from at least one approver
2. LGTM Count: Minimum 1 /lgtm from reviewers
3. Status Checks: All required status checks must pass
4. No Blockers: No WIP, hold, or conflict labels
5. Verified: PR must be marked as verified (if verification is enabled)

📊 Review Process

Approvers and Reviewers

Approvers:

• myakove
• rnetser

Reviewers:

• myakove
• rnetser

Available Labels

• hold
• verified
• wip
• lgtm
• approve
• automerge

💡 Tips

• WIP Status: Use /wip when your PR is not ready for review
• Verification: The verified label is automatically removed on each new commit
• Cherry-picking: Cherry-pick labels are processed when the PR is merged
• Container Builds: Container images are automatically tagged with the PR number
• Permission Levels: Some commands require approver permissions
• Auto-verified Users: Certain users have automatic verification and merge privileges

For more information, please refer to the project documentation or contact the maintainers.

[myakove]

/verified
/approve

[myakove-bot]

New container for ghcr.io/myk-org/github-webhook-server:latest published