fix: improve AI cherry-pick conflict resolution for modify/delete conflicts#1046
Merged
fix: improve AI cherry-pick conflict resolution for modify/delete conflicts#1046
Conversation
…flicts 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
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughRefactors AI-driven cherry-pick conflict resolution prompt to explicit per-conflict-type guidance and staging instructions; removes the automatic Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment |
Collaborator
|
Report bugs in Issues Welcome! 🎉This pull request will be automatically processed with the following features: 🔄 Automatic Actions
📋 Available CommandsPR Status Management
Review & Approval
Testing & Validation
Container Operations
Cherry-pick Operations
Label Management
✅ Merge RequirementsThis PR will be automatically approved when the following conditions are met:
📊 Review ProcessApprovers and ReviewersApprovers:
Reviewers:
Available Labels
💡 Tips
For more information, please refer to the project documentation or contact the maintainers. |
Collaborator
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
webhook_server/libs/handlers/runner_handler.py (1)
802-810:⚠️ Potential issue | 🟠 MajorHIGH: Missing empty cherry-pick recovery blocks valid operations.
When the target branch already contains equivalent changes,
git cherry-pick --continuereports "The previous cherry-pick is now empty, possibly due to conflict resolution"—a legitimate condition, not an error. The current code returnsFalseimmediately, failing valid cherry-picks that need--allow-emptyto complete.The web search confirms
git commit --allow-empty -C CHERRY_PICK_HEADis the standard, supported approach to finish the cherry-pick while preserving the original commit message and author. This should be applied whencherry-pick --continuefails with an empty result.Suggested fix
if not rc: - self.logger.error(f"{self.log_prefix} cherry-pick --continue failed after AI resolution: {err}") - return False + if "previous cherry-pick is now empty" in err.lower(): + rc, _, err = await run_command( + command=f"{git_cmd} commit --allow-empty -C CHERRY_PICK_HEAD", + log_prefix=self.log_prefix, + redact_secrets=[github_token], + mask_sensitive=self.github_webhook.mask_sensitive, + ) + if rc: + self.logger.info( + f"{self.log_prefix} Cherry-pick became empty after AI resolution; preserved it with --allow-empty" + ) + return True + self.logger.error(f"{self.log_prefix} cherry-pick --continue failed after AI resolution: {err}") + return False🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@webhook_server/libs/handlers/runner_handler.py` around lines 802 - 810, The failure handler after calling run_command for "cherry-pick --continue" treats any non-zero rc as fatal; change it to detect the empty-cherry-pick message ("The previous cherry-pick is now empty" / "possibly due to conflict resolution") and, in that case, run a recovery command using git commit --allow-empty -C CHERRY_PICK_HEAD (reusing git_cmd, github_token, and mask_sensitive parameters passed to run_command) to finalize the cherry-pick and return success; otherwise keep the existing error logging via self.logger.error and return False. Ensure the recovery path logs a clear info/debug message and still uses run_command with redact_secrets and mask_sensitive like the original call.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@webhook_server/libs/handlers/runner_handler.py`:
- Around line 753-756: The current conflict-resolution branch that outputs the
message fragment "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'..." must not unconditionally resurrect deleted files;
instead detect the specific conflict pattern "deleted in HEAD and modified in"
and treat the intent to delete in HEAD as authoritative: do not auto-create or
stage the file, mark the conflict resolved by removing the file (use 'git rm' or
equivalent in the code path) or leave it deleted and record resolution, and
update the user-facing message to reflect that deletion is preserved; locate the
code that emits that string and the associated resolution actions in
runner_handler.py and change the logic to prefer deletion for that exact
conflict pattern rather than always keeping the file.
---
Outside diff comments:
In `@webhook_server/libs/handlers/runner_handler.py`:
- Around line 802-810: The failure handler after calling run_command for
"cherry-pick --continue" treats any non-zero rc as fatal; change it to detect
the empty-cherry-pick message ("The previous cherry-pick is now empty" /
"possibly due to conflict resolution") and, in that case, run a recovery command
using git commit --allow-empty -C CHERRY_PICK_HEAD (reusing git_cmd,
github_token, and mask_sensitive parameters passed to run_command) to finalize
the cherry-pick and return success; otherwise keep the existing error logging
via self.logger.error and return False. Ensure the recovery path logs a clear
info/debug message and still uses run_command with redact_secrets and
mask_sensitive like the original call.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a5b11842-561e-45df-baa8-4246d3b12543
📒 Files selected for processing (2)
webhook_server/libs/handlers/runner_handler.pywebhook_server/tests/test_runner_handler.py
myakove-bot
added
commented-myakove
lgtm-coderabbitai[bot]
and removed
changes-requested-coderabbitai[bot]
labels
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.
Collaborator
|
New container for ghcr.io/myk-org/github-webhook-server:latest published |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
--allow-emptyfallback — empty cherry-picks now fall back to manual instructions since they indicate incorrect AI resolutionContext
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.shdoesn'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
test_cherry_pick_ai_resolves_conflictsverifies prompt includes modify/delete guidancetest_cherry_pick_ai_empty_result_falls_back_to_manualverifies empty cherry-picks fail correctlyCloses #1043
Summary by CodeRabbit
Improvements
Bug Fixes
Tests