Skip to content

Commit 5a4bdd3

Browse files
authored
fix: improve AI cherry-pick conflict resolution for modify/delete conflicts (#1046)
## Summary - Updated AI conflict resolution prompt with explicit rules for modify/delete conflicts - For modify/delete conflicts (file "deleted in HEAD"), the AI now keeps the file instead of deleting it - Removed `--allow-empty` fallback — empty cherry-picks now fall back to manual instructions since they indicate incorrect AI resolution ## Context Cherry-pick of PR RedHatQE/mtv-api-tests#349 to v2.10 resulted in an empty PR (#352) because the AI deleted the file (following the old prompt's "prefer target branch version" instruction). The file `tools/bm-dns-setup.sh` doesn't exist on v2.10 — git reports this as "deleted in HEAD" but it's actually a new file being brought to the target branch. ## Test plan - [x] Updated `test_cherry_pick_ai_resolves_conflicts` verifies prompt includes modify/delete guidance - [x] Updated `test_cherry_pick_ai_empty_result_falls_back_to_manual` verifies empty cherry-picks fail correctly - [x] Full test suite passes (1436 tests, 90.28% coverage) Closes #1043 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Improvements** * Updated AI conflict-resolution behavior to handle modify/delete conflicts more explicitly and simplify post-resolution instructions. * **Bug Fixes** * Changed empty cherry-pick handling to report failure instead of creating empty commits or attempting auto-fallbacks. * **Tests** * Updated tests to expect a manual fallback path and to verify the AI prompt includes delete/modify conflict guidance. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent f392601 commit 5a4bdd3

2 files changed

Lines changed: 33 additions & 44 deletions

File tree

webhook_server/libs/handlers/runner_handler.py

Lines changed: 16 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -745,12 +745,20 @@ async def _resolve_cherry_pick_with_ai(
745745

746746
prompt = (
747747
"You are in a git repository with cherry-pick merge conflicts. "
748-
"The conflicted files contain git conflict markers (<<<<<<< HEAD, =======, >>>>>>>). "
749-
"Resolve ALL conflicts in ALL files. "
750-
"Priority: the target branch (HEAD/upstream) changes are the baseline. "
751-
"Adapt the cherry-picked changes to fit the target branch's codebase. "
752-
"If changes are incompatible, prefer the target branch version. "
753-
"After resolving, ensure the code compiles/is syntactically valid."
748+
"Resolve ALL conflicts in ALL files.\n\n"
749+
"How to handle each conflict type:\n"
750+
"- Standard conflict markers (<<<<<<< HEAD, =======, >>>>>>>): "
751+
"HEAD is the target branch. Adapt the cherry-picked changes to fit "
752+
"the target branch code.\n"
753+
"- File 'deleted in HEAD and modified in <commit>': This means the file "
754+
"does not exist on the target branch. If the cherry-pick is introducing "
755+
"this file to the target branch, keep the file and 'git add' it. "
756+
"If the file was intentionally removed from the target branch and the "
757+
"changes are not relevant, 'git rm' it.\n"
758+
"- File 'added in both' or 'renamed': Merge the content, keeping both "
759+
"sides' intent.\n\n"
760+
"After resolving all conflicts, stage everything with 'git add' and "
761+
"make sure the result is syntactically valid."
754762
)
755763

756764
self.logger.info(f"{self.log_prefix} Attempting AI conflict resolution with {ai_provider}/{ai_model}")
@@ -801,22 +809,8 @@ async def _resolve_cherry_pick_with_ai(
801809
mask_sensitive=self.github_webhook.mask_sensitive,
802810
)
803811
if not rc:
804-
if "cherry-pick is now empty" in err:
805-
self.logger.info(
806-
f"{self.log_prefix} Cherry-pick is empty after AI resolution, committing with --allow-empty"
807-
)
808-
rc_empty, _, err_empty = await run_command(
809-
command=f"{git_cmd} -c core.editor=true commit --allow-empty -C CHERRY_PICK_HEAD",
810-
log_prefix=self.log_prefix,
811-
redact_secrets=[github_token],
812-
mask_sensitive=self.github_webhook.mask_sensitive,
813-
)
814-
if not rc_empty:
815-
self.logger.error(f"{self.log_prefix} Failed to commit empty cherry-pick: {err_empty}")
816-
return False
817-
else:
818-
self.logger.error(f"{self.log_prefix} cherry-pick --continue failed after AI resolution: {err}")
819-
return False
812+
self.logger.error(f"{self.log_prefix} cherry-pick --continue failed after AI resolution: {err}")
813+
return False
820814
else:
821815
if err_check and "needed a single revision" not in err_check.lower():
822816
self.logger.error(f"{self.log_prefix} Unexpected CHERRY_PICK_HEAD check error: {err_check}")

webhook_server/tests/test_runner_handler.py

Lines changed: 17 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1502,6 +1502,11 @@ async def run_command_side_effect(command: str, **kwargs: Any) -> tuple[bool, st
15021502
await runner_handler.cherry_pick(mock_pull_request, "main")
15031503
mock_set_success.assert_called_once()
15041504
mock_ai_cli.assert_called_once()
1505+
# Verify prompt includes delete/modify conflict guidance
1506+
ai_prompt = str(mock_ai_cli.call_args)
1507+
assert "deleted in HEAD and modified in" in ai_prompt, (
1508+
"AI prompt should include delete/modify conflict guidance"
1509+
)
15051510
# Verify AI comment was posted
15061511
comment_calls = mock_pull_request.create_issue_comment.call_args_list
15071512
ai_comment = any(
@@ -1599,10 +1604,10 @@ async def run_command_side_effect(command: str, **kwargs: Any) -> tuple[bool, st
15991604
assert "ai-resolved-conflicts" in gh_cmd_str
16001605

16011606
@pytest.mark.asyncio
1602-
async def test_cherry_pick_ai_resolves_empty_cherry_pick(
1607+
async def test_cherry_pick_ai_empty_result_falls_back_to_manual(
16031608
self, runner_handler: RunnerHandler, mock_pull_request: Mock
16041609
) -> None:
1605-
"""Cherry-pick conflict resolved by AI results in empty commit — committed with --allow-empty."""
1610+
"""Cherry-pick conflict resolved by AI but result is empty — falls back to manual instructions."""
16061611
runner_handler.github_webhook.ai_features = {
16071612
"ai-provider": "claude",
16081613
"ai-model": "sonnet",
@@ -1624,13 +1629,11 @@ async def run_command_side_effect(command: str, **kwargs: Any) -> tuple[bool, st
16241629
"The previous cherry-pick is now empty, possibly due to conflict resolution.\n"
16251630
"If you wish to commit it anyway, use:\n\n git commit --allow-empty\n",
16261631
)
1627-
if "gh pr create" in command:
1628-
return (True, "https://github.com/test-org/test-repo/pull/99", "")
16291632
return (True, "success", "")
16301633

16311634
with patch.object(runner_handler, "is_branch_exists", new=AsyncMock(return_value=Mock())):
16321635
with patch.object(runner_handler.check_run_handler, "set_check_in_progress"):
1633-
with patch.object(runner_handler.check_run_handler, "set_check_success") as mock_set_success:
1636+
with patch.object(runner_handler.check_run_handler, "set_check_failure") as mock_set_failure:
16341637
with patch.object(runner_handler, "_checkout_worktree") as mock_checkout:
16351638
mock_checkout.return_value = AsyncMock()
16361639
mock_checkout.return_value.__aenter__ = AsyncMock(
@@ -1654,29 +1657,21 @@ async def run_command_side_effect(command: str, **kwargs: Any) -> tuple[bool, st
16541657
return_value=None,
16551658
):
16561659
await runner_handler.cherry_pick(mock_pull_request, "main")
1657-
mock_set_success.assert_called_once()
1660+
mock_set_failure.assert_called()
16581661
mock_ai_cli.assert_called_once()
1659-
# Verify --allow-empty commit was called
1662+
# Verify --allow-empty was NOT called
16601663
allow_empty_calls = [
16611664
c for c in mock_run_cmd.call_args_list if "commit --allow-empty" in str(c)
16621665
]
1663-
assert allow_empty_calls, (
1664-
"git commit --allow-empty should be called for empty cherry-pick"
1665-
)
1666-
# Verify AI comment was posted
1666+
assert not allow_empty_calls, "git commit --allow-empty should NOT be called"
1667+
# Verify manual cherry-pick comment was posted
16671668
comment_calls = mock_pull_request.create_issue_comment.call_args_list
1668-
ai_comment = any(
1669-
"Cherry-pick conflicts were resolved by AI" in str(c) for c in comment_calls
1669+
manual_comment = any(
1670+
"Manual cherry-pick is needed" in str(c) for c in comment_calls
1671+
)
1672+
assert manual_comment, (
1673+
f"Expected manual cherry-pick comment, got: {comment_calls}"
16701674
)
1671-
assert ai_comment, f"Expected AI comment, got: {comment_calls}"
1672-
# Verify labels are in gh pr create command
1673-
gh_cmd_calls = [
1674-
c for c in mock_run_cmd.call_args_list if "gh pr create" in str(c)
1675-
]
1676-
assert gh_cmd_calls, "gh pr create not called"
1677-
gh_cmd_str = str(gh_cmd_calls[-1])
1678-
assert "--label" in gh_cmd_str
1679-
assert "ai-resolved-conflicts" in gh_cmd_str
16801675

16811676
@pytest.mark.asyncio
16821677
async def test_cherry_pick_ai_fails_fallback(self, runner_handler: RunnerHandler, mock_pull_request: Mock) -> None:

0 commit comments

Comments
 (0)