Skip to content

Commit f1d6e8b

Browse files
authored
fix: handle cherry-pick PR assignee failure gracefully (#1051)
## Summary - Remove `--assignee` flag from `gh pr create` command in cherry-pick flow - Add post-creation assignee logic via PyGithub `add_to_assignees()` - If PR author cannot be assigned (not a collaborator), fall back to first root approver from OWNERS file - Assignment failure no longer blocks cherry-pick PR creation ## Root Cause When cherry-picking a PR, the `gh pr create --assignee {pr_author}` command would fail entirely if the PR author wasn't a collaborator on the target repo (e.g., fork contributors). Error: `could not assign user: 'username' not found` Triggered by: RedHatQE/mtv-api-tests#371 Closes #1050 ## Test plan - [x] Updated `test_cherry_pick_assigns_pr_author` to verify post-creation assignment - [x] Updated `test_cherry_pick_requested_by_uses_pr_owner` to verify no `--assignee` in command - [x] Added `test_cherry_pick_assignee_fallback_to_approver` for fallback path - [x] All 139 runner handler tests pass <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **Bug Fixes** * Cherry-pick pull requests now attempt to assign ownership to the original PR author after creation, with automatic fallback to the first root approver from the OWNERS configuration if the primary assignment fails. * Improved error handling and logging for pull request assignment failures during cherry-pick operations. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent 1c99d3e commit f1d6e8b

2 files changed

Lines changed: 68 additions & 6 deletions

File tree

webhook_server/libs/handlers/runner_handler.py

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -852,7 +852,6 @@ async def cherry_pick(
852852

853853
async with self._checkout_worktree(pull_request=pull_request) as (success, worktree_path, out, err):
854854
git_cmd = f"git --work-tree={worktree_path} --git-dir={worktree_path}/.git"
855-
assignee_flag = f" --assignee {shlex.quote(pr_author)}" if assign_to_pr_owner else ""
856855
pr_title = f"{CHERRY_PICKED_LABEL}: [{target_branch}] {commit_msg_striped}"
857856
pr_body = (
858857
f"Cherry-pick from `{source_branch}` branch, original PR: {pull_request_url}, PR owner: {pr_author}"
@@ -1007,7 +1006,6 @@ async def cherry_pick(
10071006
f"gh pr create --repo {shlex.quote(repo_full_name)}"
10081007
f" --base {shlex.quote(target_branch)}"
10091008
f" --head {shlex.quote(new_branch_name)}"
1010-
f"{assignee_flag}"
10111009
f"{label_flags}"
10121010
f" --title {shlex.quote(pr_title)}"
10131011
f" --body {shlex.quote(pr_body)}"
@@ -1083,6 +1081,37 @@ async def cherry_pick(
10831081
cherry_pick_pr = None
10841082

10851083
if cherry_pick_pr:
1084+
# Assign the PR to the original author (or fallback approver)
1085+
if assign_to_pr_owner:
1086+
try:
1087+
await asyncio.to_thread(cherry_pick_pr.add_to_assignees, pr_author)
1088+
self.logger.info(
1089+
f"{self.log_prefix} Assigned {pr_author} to cherry-pick PR #{cherry_pick_pr.number}"
1090+
)
1091+
except Exception:
1092+
self.logger.debug(
1093+
f"{self.log_prefix} Could not assign {pr_author} to cherry-pick PR"
1094+
" (may not be a collaborator), trying fallback"
1095+
)
1096+
try:
1097+
fallback_approvers = self.owners_file_handler.root_approvers
1098+
if fallback_approvers:
1099+
await asyncio.to_thread(cherry_pick_pr.add_to_assignees, fallback_approvers[0])
1100+
self.logger.info(
1101+
f"{self.log_prefix} Assigned fallback approver"
1102+
f" {fallback_approvers[0]} to cherry-pick PR #{cherry_pick_pr.number}"
1103+
)
1104+
else:
1105+
self.logger.warning(
1106+
f"{self.log_prefix} No fallback approvers found in OWNERS file"
1107+
f" for cherry-pick PR #{cherry_pick_pr.number}"
1108+
)
1109+
except Exception:
1110+
self.logger.exception(
1111+
f"{self.log_prefix} Could not assign any user"
1112+
f" to cherry-pick PR #{cherry_pick_pr.number}"
1113+
)
1114+
10861115
# Add labels to the created PR via PyGithub (auto-creates labels if needed)
10871116
try:
10881117
labels_to_add = [cherry_picked_label]

webhook_server/tests/test_runner_handler.py

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1311,16 +1311,49 @@ async def _run_command_side_effect(command: str, **kwargs: Any) -> tuple[bool, s
13111311

13121312
@pytest.mark.asyncio
13131313
async def test_cherry_pick_assigns_pr_author(self, runner_handler: RunnerHandler, mock_pull_request: Mock) -> None:
1314-
"""Test cherry_pick assigns to PR author, not the cherry-pick requester."""
1314+
"""Test cherry_pick assigns to PR author via add_to_assignees after PR creation."""
13151315
async with self.cherry_pick_setup(runner_handler, mock_pull_request) as mocks:
13161316
await runner_handler.cherry_pick(mock_pull_request, "main")
13171317
mocks.set_progress.assert_called_once()
13181318
mocks.set_success.assert_called_once()
13191319
mocks.comment.assert_called_once()
1320+
# Verify --assignee is NOT in the gh pr create command
13201321
last_cmd = mocks.run_cmd.call_args_list[-1]
13211322
gh_command = last_cmd.kwargs.get("command", last_cmd.args[0] if last_cmd.args else "")
1322-
assert "--assignee" in gh_command
1323-
assert "test-pr-author" in gh_command
1323+
assert "--assignee" not in gh_command
1324+
# Verify add_to_assignees was called with the PR author
1325+
cherry_pick_pr = runner_handler.repository.get_pull.return_value
1326+
cherry_pick_pr.add_to_assignees.assert_called_once_with("test-pr-author")
1327+
1328+
@pytest.mark.asyncio
1329+
async def test_cherry_pick_assignee_fallback_to_approver(
1330+
self, runner_handler: RunnerHandler, mock_pull_request: Mock
1331+
) -> None:
1332+
"""Test cherry_pick falls back to OWNERS approver when PR author cannot be assigned."""
1333+
async with self.cherry_pick_setup(runner_handler, mock_pull_request) as mocks:
1334+
# Make add_to_assignees fail for PR author, succeed for approver
1335+
cherry_pick_pr_mock = runner_handler.repository.get_pull.return_value
1336+
call_count = 0
1337+
1338+
def side_effect(*args: Any) -> None:
1339+
nonlocal call_count
1340+
call_count += 1
1341+
if call_count == 1:
1342+
raise Exception("could not assign user")
1343+
1344+
cherry_pick_pr_mock.add_to_assignees = Mock(side_effect=side_effect)
1345+
1346+
# Set up owners_file_handler with root approvers
1347+
mock_owners = Mock()
1348+
mock_owners.root_approvers = ["maintainer-1", "maintainer-2"]
1349+
runner_handler.owners_file_handler = mock_owners
1350+
1351+
await runner_handler.cherry_pick(mock_pull_request, "main")
1352+
mocks.set_success.assert_called_once()
1353+
# Verify add_to_assignees was called twice: first with author (fails), then with approver
1354+
assert cherry_pick_pr_mock.add_to_assignees.call_count == 2
1355+
cherry_pick_pr_mock.add_to_assignees.assert_any_call("test-pr-author")
1356+
cherry_pick_pr_mock.add_to_assignees.assert_any_call("maintainer-1")
13241357

13251358
@pytest.mark.asyncio
13261359
async def test_cherry_pick_requested_by_uses_pr_owner(
@@ -1337,7 +1370,7 @@ async def test_cherry_pick_requested_by_uses_pr_owner(
13371370
assert "Cherry-pick from" in gh_command
13381371
assert mock_pull_request.html_url in gh_command
13391372
assert "test-pr-author" in gh_command
1340-
assert "--assignee" in gh_command
1373+
assert "--assignee" not in gh_command
13411374

13421375
@pytest.mark.asyncio
13431376
async def test_checkout_worktree_branch_already_checked_out(

0 commit comments

Comments
 (0)