From f2258d71e18f73efe7b4a5f8c0ef423c283e7010 Mon Sep 17 00:00:00 2001 From: Meni Yakove Date: Wed, 18 Mar 2026 13:35:59 +0200 Subject: [PATCH 1/2] fix: improve AI cherry-pick conflict resolution for modify/delete conflicts The AI prompt incorrectly told the AI to prefer the target branch version for incompatible changes. For modify/delete conflicts (file doesn't exist on target branch), this caused the AI to delete the file, resulting in an empty cherry-pick with no changes. Updated the prompt with explicit rules: - For modify/delete conflicts, KEEP the file (it's new to the target) - Removed --allow-empty fallback since empty cherry-picks indicate wrong AI resolution, not a valid state Closes #1043 --- .../libs/handlers/runner_handler.py | 35 +++++++---------- webhook_server/tests/test_runner_handler.py | 39 ++++++++----------- 2 files changed, 30 insertions(+), 44 deletions(-) diff --git a/webhook_server/libs/handlers/runner_handler.py b/webhook_server/libs/handlers/runner_handler.py index 45781b79..77feb119 100644 --- a/webhook_server/libs/handlers/runner_handler.py +++ b/webhook_server/libs/handlers/runner_handler.py @@ -745,12 +745,17 @@ async def _resolve_cherry_pick_with_ai( prompt = ( "You are in a git repository with cherry-pick merge conflicts. " - "The conflicted files contain git conflict markers (<<<<<<< HEAD, =======, >>>>>>>). " - "Resolve ALL conflicts in ALL files. " - "Priority: the target branch (HEAD/upstream) changes are the baseline. " - "Adapt the cherry-picked changes to fit the target branch's codebase. " - "If changes are incompatible, prefer the target branch version. " - "After resolving, ensure the code compiles/is syntactically valid." + "Resolve ALL conflicts in ALL files.\n\n" + "Rules:\n" + "1. For files with conflict markers (<<<<<<< HEAD, =======, >>>>>>>): " + "the target branch (HEAD) is the baseline. " + "Adapt the cherry-picked changes to fit the target branch's codebase.\n" + "2. For modify/delete conflicts (file 'deleted in HEAD and modified in ...'): " + "the file does NOT exist on the target branch yet. " + "KEEP the file — use 'git add' to stage it. " + "This is a cherry-pick bringing a new/modified file to the target branch.\n" + "3. After resolving, stage all changes with 'git add' and " + "ensure the code is syntactically valid." ) self.logger.info(f"{self.log_prefix} Attempting AI conflict resolution with {ai_provider}/{ai_model}") @@ -801,22 +806,8 @@ async def _resolve_cherry_pick_with_ai( mask_sensitive=self.github_webhook.mask_sensitive, ) if not rc: - if "cherry-pick is now empty" in err: - self.logger.info( - f"{self.log_prefix} Cherry-pick is empty after AI resolution, committing with --allow-empty" - ) - rc_empty, _, err_empty = await run_command( - command=f"{git_cmd} -c core.editor=true commit --allow-empty -C CHERRY_PICK_HEAD", - log_prefix=self.log_prefix, - redact_secrets=[github_token], - mask_sensitive=self.github_webhook.mask_sensitive, - ) - if not rc_empty: - self.logger.error(f"{self.log_prefix} Failed to commit empty cherry-pick: {err_empty}") - return False - else: - self.logger.error(f"{self.log_prefix} cherry-pick --continue failed after AI resolution: {err}") - return False + self.logger.error(f"{self.log_prefix} cherry-pick --continue failed after AI resolution: {err}") + return False else: if err_check and "needed a single revision" not in err_check.lower(): self.logger.error(f"{self.log_prefix} Unexpected CHERRY_PICK_HEAD check error: {err_check}") diff --git a/webhook_server/tests/test_runner_handler.py b/webhook_server/tests/test_runner_handler.py index 56e2e6aa..b3e605b3 100644 --- a/webhook_server/tests/test_runner_handler.py +++ b/webhook_server/tests/test_runner_handler.py @@ -1502,6 +1502,11 @@ async def run_command_side_effect(command: str, **kwargs: Any) -> tuple[bool, st await runner_handler.cherry_pick(mock_pull_request, "main") mock_set_success.assert_called_once() mock_ai_cli.assert_called_once() + # Verify prompt includes modify/delete guidance + ai_prompt = str(mock_ai_cli.call_args) + assert "modify/delete" in ai_prompt, ( + "AI prompt should include modify/delete conflict guidance" + ) # Verify AI comment was posted comment_calls = mock_pull_request.create_issue_comment.call_args_list ai_comment = any( @@ -1599,10 +1604,10 @@ async def run_command_side_effect(command: str, **kwargs: Any) -> tuple[bool, st assert "ai-resolved-conflicts" in gh_cmd_str @pytest.mark.asyncio - async def test_cherry_pick_ai_resolves_empty_cherry_pick( + async def test_cherry_pick_ai_empty_result_falls_back_to_manual( self, runner_handler: RunnerHandler, mock_pull_request: Mock ) -> None: - """Cherry-pick conflict resolved by AI results in empty commit — committed with --allow-empty.""" + """Cherry-pick conflict resolved by AI but result is empty — falls back to manual instructions.""" runner_handler.github_webhook.ai_features = { "ai-provider": "claude", "ai-model": "sonnet", @@ -1624,13 +1629,11 @@ async def run_command_side_effect(command: str, **kwargs: Any) -> tuple[bool, st "The previous cherry-pick is now empty, possibly due to conflict resolution.\n" "If you wish to commit it anyway, use:\n\n git commit --allow-empty\n", ) - if "gh pr create" in command: - return (True, "https://github.com/test-org/test-repo/pull/99", "") return (True, "success", "") with patch.object(runner_handler, "is_branch_exists", new=AsyncMock(return_value=Mock())): with patch.object(runner_handler.check_run_handler, "set_check_in_progress"): - with patch.object(runner_handler.check_run_handler, "set_check_success") as mock_set_success: + with patch.object(runner_handler.check_run_handler, "set_check_failure") as mock_set_failure: with patch.object(runner_handler, "_checkout_worktree") as mock_checkout: mock_checkout.return_value = AsyncMock() mock_checkout.return_value.__aenter__ = AsyncMock( @@ -1654,29 +1657,21 @@ async def run_command_side_effect(command: str, **kwargs: Any) -> tuple[bool, st return_value=None, ): await runner_handler.cherry_pick(mock_pull_request, "main") - mock_set_success.assert_called_once() + mock_set_failure.assert_called() mock_ai_cli.assert_called_once() - # Verify --allow-empty commit was called + # Verify --allow-empty was NOT called allow_empty_calls = [ c for c in mock_run_cmd.call_args_list if "commit --allow-empty" in str(c) ] - assert allow_empty_calls, ( - "git commit --allow-empty should be called for empty cherry-pick" - ) - # Verify AI comment was posted + assert not allow_empty_calls, "git commit --allow-empty should NOT be called" + # Verify manual cherry-pick comment was posted comment_calls = mock_pull_request.create_issue_comment.call_args_list - ai_comment = any( - "Cherry-pick conflicts were resolved by AI" in str(c) for c in comment_calls + manual_comment = any( + "Manual cherry-pick is needed" in str(c) for c in comment_calls + ) + assert manual_comment, ( + f"Expected manual cherry-pick comment, got: {comment_calls}" ) - assert ai_comment, f"Expected AI comment, got: {comment_calls}" - # Verify labels are in gh pr create command - gh_cmd_calls = [ - c for c in mock_run_cmd.call_args_list if "gh pr create" in str(c) - ] - assert gh_cmd_calls, "gh pr create not called" - gh_cmd_str = str(gh_cmd_calls[-1]) - assert "--label" in gh_cmd_str - assert "ai-resolved-conflicts" in gh_cmd_str @pytest.mark.asyncio async def test_cherry_pick_ai_fails_fallback(self, runner_handler: RunnerHandler, mock_pull_request: Mock) -> None: From 037b327f3b2e90ca9a8e7930b3576f790e015f06 Mon Sep 17 00:00:00 2001 From: Meni Yakove Date: Wed, 18 Mar 2026 14:47:05 +0200 Subject: [PATCH 2/2] fix: use scenario-based AI prompt for cherry-pick conflict resolution Rewrite the AI prompt with clear natural language instructions for each conflict type instead of vague rules. The AI now understands when to keep vs remove files in modify/delete conflicts based on whether the cherry-pick is introducing or backporting the file. --- .../libs/handlers/runner_handler.py | 23 +++++++++++-------- webhook_server/tests/test_runner_handler.py | 6 ++--- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/webhook_server/libs/handlers/runner_handler.py b/webhook_server/libs/handlers/runner_handler.py index 77feb119..ff51acef 100644 --- a/webhook_server/libs/handlers/runner_handler.py +++ b/webhook_server/libs/handlers/runner_handler.py @@ -746,16 +746,19 @@ async def _resolve_cherry_pick_with_ai( prompt = ( "You are in a git repository with cherry-pick merge conflicts. " "Resolve ALL conflicts in ALL files.\n\n" - "Rules:\n" - "1. For files with conflict markers (<<<<<<< HEAD, =======, >>>>>>>): " - "the target branch (HEAD) is the baseline. " - "Adapt the cherry-picked changes to fit the target branch's codebase.\n" - "2. For modify/delete conflicts (file 'deleted in HEAD and modified in ...'): " - "the file does NOT exist on the target branch yet. " - "KEEP the file — use 'git add' to stage it. " - "This is a cherry-pick bringing a new/modified file to the target branch.\n" - "3. After resolving, stage all changes with 'git add' and " - "ensure the code is syntactically valid." + "How to handle each conflict type:\n" + "- Standard conflict markers (<<<<<<< HEAD, =======, >>>>>>>): " + "HEAD is the target branch. Adapt the cherry-picked changes to fit " + "the target branch code.\n" + "- File 'deleted in HEAD and modified in ': This means the file " + "does not exist on the target branch. If the cherry-pick is introducing " + "this file to the target branch, keep the file and 'git add' it. " + "If the file was intentionally removed from the target branch and the " + "changes are not relevant, 'git rm' it.\n" + "- File 'added in both' or 'renamed': Merge the content, keeping both " + "sides' intent.\n\n" + "After resolving all conflicts, stage everything with 'git add' and " + "make sure the result is syntactically valid." ) self.logger.info(f"{self.log_prefix} Attempting AI conflict resolution with {ai_provider}/{ai_model}") diff --git a/webhook_server/tests/test_runner_handler.py b/webhook_server/tests/test_runner_handler.py index b3e605b3..b15dce9c 100644 --- a/webhook_server/tests/test_runner_handler.py +++ b/webhook_server/tests/test_runner_handler.py @@ -1502,10 +1502,10 @@ async def run_command_side_effect(command: str, **kwargs: Any) -> tuple[bool, st await runner_handler.cherry_pick(mock_pull_request, "main") mock_set_success.assert_called_once() mock_ai_cli.assert_called_once() - # Verify prompt includes modify/delete guidance + # Verify prompt includes delete/modify conflict guidance ai_prompt = str(mock_ai_cli.call_args) - assert "modify/delete" in ai_prompt, ( - "AI prompt should include modify/delete conflict guidance" + assert "deleted in HEAD and modified in" in ai_prompt, ( + "AI prompt should include delete/modify conflict guidance" ) # Verify AI comment was posted comment_calls = mock_pull_request.create_issue_comment.call_args_list