feat: AI-powered conflict resolution for auto cherry-pick#1025
Merged
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (15)
WalkthroughAdds AI-driven cherry-pick conflict resolution: new config Changes
Sequence Diagram(s)sequenceDiagram
participant Event as Webhook Event
participant PRHandler as PR Handler
participant Runner as Runner Handler
participant AICLI as AI CLI
participant GitHub as GitHub API
participant Comment as PR Comments
Event->>PRHandler: Cherry-pick PR created / webhook event
PRHandler->>PRHandler: Check labels for AI_RESOLVED_CONFLICTS_LABEL
alt AI-resolved label present
PRHandler->>PRHandler: Remove VERIFIED label, queue verification (skip auto-verify)
PRHandler-->>Event: Return early
else proceed
PRHandler->>Runner: Trigger cherry-pick workflow
end
Runner->>Runner: Attempt git cherry-pick
alt Conflicts detected
Runner->>AICLI: Invoke AI CLI with upstream-first prompt & timeout
alt AI resolves conflicts
AICLI-->>Runner: Resolved files
Runner->>Runner: Stage files & git cherry-pick --continue
Runner->>GitHub: Push branch, create PR (prefer App token)
Runner->>GitHub: Add AI_RESOLVED_CONFLICTS_LABEL
Runner->>Comment: Post AI resolution comment (manual verification required)
else AI fails
AICLI-->>Runner: Error
Runner->>Comment: Post manual cherry-pick instructions
end
else No conflicts
Runner->>GitHub: Push branch, create PR, apply standard labels/auto-verify
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 unit tests (beta)
📝 Coding Plan for PR comments
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. |
ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan Review Summary by QodoAdd AI-powered conflict resolution for cherry-pick operations
WalkthroughsDescription• Add AI-powered conflict resolution for cherry-pick operations - Detects cherry-pick conflicts and attempts automatic resolution via AI CLI - Uses upstream-first priority when resolving conflicting changes - Falls back to manual instructions if AI resolution fails or is disabled • Prevent auto-verification of AI-resolved cherry-picks - New ai-resolved-conflicts label marks PRs with AI-resolved conflicts - AI-resolved cherry-picks skip auto-verification regardless of configuration • Add configuration schema and constants for the new feature - New resolve-cherry-pick-conflicts-with-ai boolean in ai-features config - New AI_RESOLVED_CONFLICTS_LABEL constant with orange color • Comprehensive test coverage for all resolution scenarios - Tests for successful AI resolution, AI failure, and disabled feature cases - Tests for auto-verify precedence and schema validation Diagramflowchart LR
CP["Cherry-pick<br/>command"] -->|Success| Push["Push branch"]
CP -->|Conflict| AICheck{"AI resolution<br/>enabled?"}
AICheck -->|No| Manual["Post manual<br/>instructions"]
AICheck -->|Yes| AIResolve["Call AI CLI<br/>with upstream-first"]
AIResolve -->|Success| Stage["git add +<br/>cherry-pick --continue"]
AIResolve -->|Failure| Manual
Stage -->|Success| Push
Stage -->|Failure| Manual
Push --> CreatePR["Create PR with<br/>labels"]
CreatePR -->|AI resolved| AILabel["Add ai-resolved-conflicts<br/>label"]
AILabel --> Comment["Post AI resolution<br/>comment"]
CreatePR -->|No conflicts| StandardComment["Post standard<br/>comment"]
Comment --> Skip["Skip auto-verify"]
File Changes1. webhook_server/utils/constants.py
|
Code Review by Qodo
1. Token exposed to AI
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
myakove
force-pushed
the
feat/issue-1022-ai-cherry-pick-conflict-resolution
branch
from
98d283e to
fc23fdf
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
examples/config.yaml (1)
128-132:⚠️ Potential issue | 🟠 MajorHIGH: Keep this example opt-in.
The schema/default for this flag is
false, but the sample turns it on. Anyone copyingexamples/config.yamlnow opts into AI-driven conflict resolution by default, which changes behavior for a user-facing config example.Suggested change
- resolve-cherry-pick-conflicts-with-ai: true # Use AI to resolve cherry-pick merge conflicts (default: false) + resolve-cherry-pick-conflicts-with-ai: false # Opt in explicitly if you want AI to resolve cherry-pick merge conflictsAs per coding guidelines: Maintain backward compatibility for user-facing configuration files (
config.yaml,.github-webhook-server.yaml) and webhook payload handling following GitHub webhook spec.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/config.yaml` around lines 128 - 132, The example turns the resolve-cherry-pick-conflicts-with-ai flag on by default; change the ai-features sample so resolve-cherry-pick-conflicts-with-ai is set to false (matching the schema/default) and update its inline comment to indicate this feature is opt-in and defaults to false, referencing the ai-features section and the resolve-cherry-pick-conflicts-with-ai key so users copying the example won't be unintentionally opted into AI-driven conflict resolution.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@docs/superpowers/specs/2026-03-12-ai-cherry-pick-conflict-resolution-design.md`:
- Line 173: The design doc's label check is inaccurate: the implementation in
pull_request_handler uses a prefix match for dynamic labels (e.g.,
"CherryPicked-from-main") rather than exact equality. Update the doc's example
expression to reflect the actual logic by replacing the equality check with a
prefix check using CHERRY_PICKED_LABEL (e.g., is_cherry_picked =
any(label.name.startswith(CHERRY_PICKED_LABEL) for label in labels)) and add a
short note that labels may be suffixed with source branches; reference the
symbols is_cherry_picked and CHERRY_PICKED_LABEL so readers know this matches
the real behavior.
In `@webhook_server/libs/handlers/labels_handler.py`:
- Around line 104-105: Replace the two separate startswith checks with a single
tuple-based startswith call: detect when label starts with either
CHERRY_PICK_LABEL_PREFIX or CHERRY_PICKED_LABEL by using
label.startswith((CHERRY_PICK_LABEL_PREFIX, CHERRY_PICKED_LABEL)) and then
return "cherry-pick" in enabled_labels; update the check in labels_handler (the
branch that references label, CHERRY_PICK_LABEL_PREFIX, CHERRY_PICKED_LABEL, and
enabled_labels) to use this combined startswith pattern.
In `@webhook_server/libs/handlers/pull_request_handler.py`:
- Around line 1080-1085: The AI-resolved early-return path leaves a stale
verified label; update the branch where is_ai_resolved is computed (using
AI_RESOLVED_CONFLICTS_LABEL) to explicitly remove/reset the VERIFIED_LABEL_STR
state before calling await self.check_run_handler.set_check_queued(...) and
returning (e.g., call the same label-removal/reset routine used in the normal
reset path or invoke the method that clears the verified label on the PR), and
add a regression test for the synchronize/reprocess path to assert that a
manually-applied verified label is removed when an AI-resolved cherry-pick is
reprocessed.
In `@webhook_server/libs/handlers/runner_handler.py`:
- Around line 952-977: The PR is being created without the
AI_RESOLVED_CONFLICTS_LABEL causing a race where the PR opened webhook may fire
before the label is applied; update gh_pr_command construction to include the
label atomically by adding the --label
{shlex.quote(AI_RESOLVED_CONFLICTS_LABEL)} flag so the label is set at creation
(affecting the gh_pr_command used by run_command), and continue to pass the
token via env; additionally, wrap all direct PyGithub property accesses in
asyncio.to_thread to avoid blocking (e.g., replace direct uses of
cherry_pick_pr.number and pull_request.title with await
asyncio.to_thread(lambda: cherry_pick_pr.number) and await
asyncio.to_thread(lambda: pull_request.title) respectively).
- Around line 1031-1033: The code is directly accessing blocking PyGithub
properties (cherry_pick_pr.number and pull_request.title) in an async handler;
wrap those attribute reads in asyncio.to_thread() to avoid blocking the event
loop. Locate usages of cherry_pick_pr.number and pull_request.title in the
handler and replace direct access with await asyncio.to_thread(lambda:
cherry_pick_pr.number) and await asyncio.to_thread(lambda: pull_request.title)
(or a small helper like read_prop(obj, "attr") run in to_thread) so logging and
any conditional logic use the awaited, non-blocking values consistently with the
existing pattern used elsewhere in the function.
In `@webhook_server/tests/test_config_schema.py`:
- Around line 820-842: Add an explicit negative test for the
resolve-cherry-pick-conflicts-with-ai flag: in the existing test function
test_ai_features_resolve_cherry_pick_conflicts_with_ai (and the similar
repository-level test around 844-866), create a config where "ai-features" sets
"resolve-cherry-pick-conflicts-with-ai": False, load it via Config() (use
Config().root_data["ai-features"]) and assert that the flag is strictly False;
mirror the setup/teardown and environment monkeypatching used in the True case
so schema regressions treating the key as truthy-only will be caught.
In `@webhook_server/tests/test_pull_request_handler.py`:
- Around line 861-866: Replace the positional boolean replacement in the three
patch.object calls that target
pull_request_handler.github_webhook.auto_verify_cherry_picked_prs so they use
the explicit keyword argument new=True instead of passing True positionally;
locate the three occurrences in the test file (the patch.object calls around the
blocks that include pull_request_handler.github_webhook, _add_label,
set_check_queued, set_check_success) and change the third argument from True to
new=True to satisfy Ruff's FBT003 rule.
In `@webhook_server/tests/test_runner_handler.py`:
- Line 1285: Replace the brittle assertion that checks
mocks.to_thread.call_count with assertions that the specific offloaded
operations were invoked: verify that mocks.to_thread was called for get_pull,
add_to_labels, and create_review_request (e.g., inspect
mocks.to_thread.call_args_list for calls that wrap those functions) so the test
asserts required behavior rather than the total call count; make the same change
for the other occurrence flagged in the comment.
In `@webhook_server/utils/github_repository_settings.py`:
- Around line 457-461: Replace the bare except Exception logging call so the
exception traceback is captured automatically: in the except block that
currently calls LOGGER.error for the GitHub App installation token retrieval
(the block referencing repository_name and the message about making sure the app
is installed), call LOGGER.exception(...) with the same descriptive message
instead of LOGGER.error (or LOGGER.error(..., exc_info=True)); this will include
the stack trace and preserve the existing text about the app installation.
---
Outside diff comments:
In `@examples/config.yaml`:
- Around line 128-132: The example turns the
resolve-cherry-pick-conflicts-with-ai flag on by default; change the ai-features
sample so resolve-cherry-pick-conflicts-with-ai is set to false (matching the
schema/default) and update its inline comment to indicate this feature is opt-in
and defaults to false, referencing the ai-features section and the
resolve-cherry-pick-conflicts-with-ai key so users copying the example won't be
unintentionally opted into AI-driven conflict resolution.
🪄 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: a24e9940-2a20-41d0-932a-1992147ffd93
📒 Files selected for processing (14)
docs/superpowers/plans/2026-03-12-ai-cherry-pick-conflict-resolution.mddocs/superpowers/specs/2026-03-12-ai-cherry-pick-conflict-resolution-design.mdexamples/.github-webhook-server.yamlexamples/config.yamlwebhook_server/config/schema.yamlwebhook_server/libs/handlers/labels_handler.pywebhook_server/libs/handlers/pull_request_handler.pywebhook_server/libs/handlers/runner_handler.pywebhook_server/tests/test_config_schema.pywebhook_server/tests/test_github_repository_settings.pywebhook_server/tests/test_pull_request_handler.pywebhook_server/tests/test_runner_handler.pywebhook_server/utils/constants.pywebhook_server/utils/github_repository_settings.py
myakove-bot
added
lgtm-coderabbitai[bot]
commented-qodo-code-review[bot]
and removed
changes-requested-coderabbitai[bot]
labels
myakove
force-pushed
the
feat/issue-1022-ai-cherry-pick-conflict-resolution
branch
from
fc23fdf to
53c23ca
Compare
Collaborator
Author
|
/retest all |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
webhook_server/tests/test_pull_request_handler.py (1)
820-827:⚠️ Potential issue | 🟡 MinorUse
assert_awaited_*for these AsyncMock assertions to catch missing awaits.These mocks are
AsyncMockinstances and the implementation properly awaits all four methods. However,assert_called_*()only verifies that a coroutine was created—not that it was awaited. A bug that forgetsawaiton any of these calls would silently pass these tests. Useassert_awaited_*()instead:Required changes
- mock_add_label.assert_called_once() - mock_set_success.assert_called_once_with(name=VERIFIED_LABEL_STR) + mock_add_label.assert_awaited_once() + mock_set_success.assert_awaited_once_with(name=VERIFIED_LABEL_STR)- mock_add_label.assert_not_called() - mock_set_queued.assert_called_once_with(name=VERIFIED_LABEL_STR) + mock_add_label.assert_not_awaited() + mock_set_queued.assert_awaited_once_with(name=VERIFIED_LABEL_STR)- mock_remove_label.assert_called_once_with(pull_request=mock_pull_request, label=VERIFIED_LABEL_STR) - mock_set_queued.assert_called_once_with(name=VERIFIED_LABEL_STR) - mock_set_success.assert_not_called() + mock_remove_label.assert_awaited_once_with(pull_request=mock_pull_request, label=VERIFIED_LABEL_STR) + mock_set_queued.assert_awaited_once_with(name=VERIFIED_LABEL_STR) + mock_set_success.assert_not_awaited()- mock_add_label.assert_not_called() - mock_remove_label.assert_called_once_with(pull_request=mock_pull_request, label=VERIFIED_LABEL_STR) - mock_set_queued.assert_called_once_with(name=VERIFIED_LABEL_STR) - mock_set_success.assert_not_called() + mock_add_label.assert_not_awaited() + mock_remove_label.assert_awaited_once_with(pull_request=mock_pull_request, label=VERIFIED_LABEL_STR) + mock_set_queued.assert_awaited_once_with(name=VERIFIED_LABEL_STR) + mock_set_success.assert_not_awaited()Also applies to: 861-872, 887-900
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@webhook_server/tests/test_pull_request_handler.py` around lines 820 - 827, The test currently uses assert_called_once()/assert_called_once_with() on AsyncMock objects (mock_add_label, mock_set_success) after calling pull_request_handler._process_verified_for_update_or_new_pull_request, which only verifies a coroutine was created but not awaited; replace those assertions with the AsyncMock-specific await-aware assertions (e.g., assert_awaited_once() / assert_awaited_once_with()) for mock_add_label and mock_set_success (and apply the same change to the other test blocks at the noted ranges) so the tests fail if the code stops awaiting these async calls.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/superpowers/plans/2026-03-12-ai-cherry-pick-conflict-resolution.md`:
- Around line 47-79: The plan and schema snippet are outdated: instead of adding
a boolean `resolve-cherry-pick-conflicts-with-ai` to the `ai-features` $defs,
update `webhook_server/config/schema.yaml` so the new property is an object with
`enabled` (boolean, default false) and `timeout-minutes` (integer, default as
used in shipped code) under `$defs` → `ai-features` (insert after
`conventional-title` and before `required:`), and adjust the procedural steps in
the plan to match the current workflow (stage changes with `git add -u` and
reference that PR labeling is applied via the PyGithub-based code path rather
than expecting `gh pr create` to apply labels).
In `@webhook_server/libs/handlers/runner_handler.py`:
- Around line 702-710: The code assumes cherry_pick_ai_config is a mapping and
calls .get(), which raises AttributeError when the YAML value is a scalar
(bool); update the logic around ai_config and cherry_pick_ai_config in
runner_handler.py to first inspect types (use isinstance(cherry_pick_ai_config,
dict)), treat a boolean True as equivalent to {"enabled": True}, and only call
.get("enabled") when cherry_pick_ai_config is a dict; apply the same
normalization at the other occurrence referenced around line 739 so scalar YAML
values like resolve-cherry-pick-conflicts-with-ai: true remain
backward-compatible and do not cause attribute errors.
In `@webhook_server/tests/test_config_schema.py`:
- Around line 820-823: The test function
test_ai_features_resolve_cherry_pick_conflicts_with_ai (and the other test with
a boolean positional) uses a boolean positional parameter `enabled: bool` which
triggers Ruff FBT001; make the boolean parameter keyword-only by adding a bare
`*` before `enabled` in the function signature (e.g., change the signature to
include `*, enabled: bool`) so pytest.parametrize still works while satisfying
the linter.
In `@webhook_server/tests/test_runner_handler.py`:
- Around line 1581-1589: The test test_cherry_pick_ai_feature_disabled_fallback
currently omits the resolve-cherry-pick-conflicts-with-ai setting so it
exercises the "missing config" path; update the test's
runner_handler.github_webhook.ai_features to include
"resolve-cherry-pick-conflicts-with-ai": {"enabled": False} (or alternatively
rename the test to reflect the missing-config scenario) so
RunnerHandler._resolve_cherry_pick_with_ai() actually exercises the
enabled:false branch.
In `@webhook_server/utils/constants.py`:
- Line 25: The new constant AI_RESOLVED_CONFLICTS_LABEL was added but not mapped
into LABEL_CATEGORY_MAP so LabelsHandler.is_label_enabled() treats it as unknown
and allows it even when its category is disabled; update LABEL_CATEGORY_MAP to
include AI_RESOLVED_CONFLICTS_LABEL mapped to the appropriate category (e.g.,
the same category as cherry-pick) so the label gating logic in
LabelsHandler.is_label_enabled() will respect labels.enabled-labels; also ensure
any other related static labels declared around the same area (the labels at the
other entries mentioned) are likewise categorized.
In `@webhook_server/utils/github_repository_settings.py`:
- Around line 442-461: Move local configuration reads and parsing out of the
broad try/except: open and read the PEM file
(config_.data_dir/webhook-server.private-key.pem), extract github_app_id from
config_.root_data, and split repository_name with repository_name.split("/",
maxsplit=1) outside any exception handler so
FileNotFoundError/KeyError/ValueError propagate. Wrap only the PyGithub API
calls (creating Auth.AppAuth / GithubIntegration, calling
GithubIntegration.get_repo_installation and get_access_token) in a try/except
that catches GithubException (or the specific PyGithub exception type) and
returns None for the “app not installed” case; do not catch Exception broadly
and do not suppress local config errors. Ensure identifiers referenced are
Auth.AppAuth, GithubIntegration, get_repo_installation, get_access_token,
installation, and LOGGER.
---
Outside diff comments:
In `@webhook_server/tests/test_pull_request_handler.py`:
- Around line 820-827: The test currently uses
assert_called_once()/assert_called_once_with() on AsyncMock objects
(mock_add_label, mock_set_success) after calling
pull_request_handler._process_verified_for_update_or_new_pull_request, which
only verifies a coroutine was created but not awaited; replace those assertions
with the AsyncMock-specific await-aware assertions (e.g., assert_awaited_once()
/ assert_awaited_once_with()) for mock_add_label and mock_set_success (and apply
the same change to the other test blocks at the noted ranges) so the tests fail
if the code stops awaiting these async calls.
🪄 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: e35bf456-7b1f-4e1a-b0dc-583f37b07834
📒 Files selected for processing (15)
README.mddocs/superpowers/plans/2026-03-12-ai-cherry-pick-conflict-resolution.mddocs/superpowers/specs/2026-03-12-ai-cherry-pick-conflict-resolution-design.mdexamples/.github-webhook-server.yamlexamples/config.yamlwebhook_server/config/schema.yamlwebhook_server/libs/handlers/labels_handler.pywebhook_server/libs/handlers/pull_request_handler.pywebhook_server/libs/handlers/runner_handler.pywebhook_server/tests/test_config_schema.pywebhook_server/tests/test_github_repository_settings.pywebhook_server/tests/test_pull_request_handler.pywebhook_server/tests/test_runner_handler.pywebhook_server/utils/constants.pywebhook_server/utils/github_repository_settings.py
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (3)
webhook_server/utils/github_repository_settings.py (1)
450-459:⚠️ Potential issue | 🟠 MajorHIGH: Only convert the not-installed case to
None.Line 454 currently collapses every
GithubExceptionfrom bothget_repo_installation()andget_access_token()into the same “app is not installed” fallback. That hides bad app credentials, permission errors, rate limiting, and transient GitHub failures, so callers take the manual-fallback path for real outages instead of failing fast.🔧 Narrow the fallback to the optional 404/not-installed path
try: installation = app_instance.get_repo_installation(owner=owner, repo=repo) access_token = app_instance.get_access_token(installation.id) return access_token.token - except GithubException: - LOGGER.exception( - f"Failed to get GitHub App installation token for {repository_name}, " - f"make sure the app is installed (https://github.com/apps/manage-repositories-app)" - ) + except UnknownObjectException: + LOGGER.warning(f"GitHub App is not installed for {repository_name}") return None + except GithubException: + LOGGER.exception(f"Failed to get GitHub App installation token for {repository_name}") + raiseAs per coding guidelines: "Do NOT return fake defaults to hide missing data - always fail-fast with exceptions (ValueError, KeyError) when required data is unavailable."
For PyGithub 2.4.0, which exception/status does `GithubIntegration.get_repo_installation(owner, repo)` raise when a GitHub App is not installed on a repository, and which other `GithubException` subclasses/statuses can `GithubIntegration.get_access_token(installation_id)` raise?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@webhook_server/utils/github_repository_settings.py` around lines 450 - 459, The current broad except GithubException hides real errors; change the logic so you only return None when get_repo_installation(owner, repo) raises a GithubException with a 404/not-installed status (inspect e.status or e.status == 404), otherwise re-raise or let the exception propagate (and log it) so callers fail-fast; do not catch exceptions from app_instance.get_access_token(installation.id) into the not-installed fallback—allow those errors to bubble up or be handled separately. Ensure you reference get_repo_installation, get_access_token, installation, and repository_name to locate and adjust the specific try/except behavior.webhook_server/libs/handlers/pull_request_handler.py (1)
1088-1097:⚠️ Potential issue | 🟠 MajorHIGH: Clear
verifiedin the normal cherry-pick skip path.Now that Line 1088 matches real labels like
CherryPicked-from-main, theauto-verify-cherry-picked-prs: falsebranch finally executes. If someone previously applied/verified, synchronize/reprocess returns here and leaves that stale verification on the new head. Verification has to be recomputed per commit, so reset it before queueing the check.🔧 Mirror the AI-resolved reset behavior here too
if is_cherry_picked and not self.github_webhook.auto_verify_cherry_picked_prs: self.logger.info( f"{self.log_prefix} Cherry-picked PR detected and auto-verify-cherry-picked-prs is disabled, " "skipping auto-verification" ) + await self.labels_handler._remove_label(pull_request=pull_request, label=VERIFIED_LABEL_STR) await self.check_run_handler.set_check_queued(name=VERIFIED_LABEL_STR) return🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@webhook_server/libs/handlers/pull_request_handler.py` around lines 1088 - 1097, The cherry-pick skip path currently returns while leaving any previous verification state on the PR head; before calling self.check_run_handler.set_check_queued(name=VERIFIED_LABEL_STR) in the is_cherry_picked and not self.github_webhook.auto_verify_cherry_picked_prs branch, explicitly reset/clear the verification state for VERIFIED_LABEL_STR so verification is recomputed for the new commit (e.g., remove or reset any stored "verified" flag/check-run status via the check_run_handler or label removal API) and then queue the check; update the block around is_cherry_picked, CHERRY_PICKED_LABEL, self.github_webhook.auto_verify_cherry_picked_prs, self.logger.info and self.check_run_handler.set_check_queued to perform the clear/reset before returning.webhook_server/tests/test_pull_request_handler.py (1)
840-848:⚠️ Potential issue | 🟡 MinorLOW: Keep the last
patch.object()boolean explicit.This touched hunk still has one positional boolean replacement on Line 841. Ruff already flagged the same pattern elsewhere in this file, so leaving this
Falsepositional keeps the lint issue alive.Suggested change
- patch.object(pull_request_handler.github_webhook, "auto_verify_cherry_picked_prs", False), + patch.object(pull_request_handler.github_webhook, "auto_verify_cherry_picked_prs", new=False),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@webhook_server/tests/test_pull_request_handler.py` around lines 840 - 848, The test uses a positional boolean in the last patch.object call which triggers the linter; change the patch.object(...) for pull_request_handler.github_webhook.auto_verify_cherry_picked_prs to use the explicit keyword argument (e.g. new=False) instead of the positional False so the call is patch.object(pull_request_handler.github_webhook, "auto_verify_cherry_picked_prs", new=False), leaving the rest of the context manager and assertions unchanged; this targets the patch on auto_verify_cherry_picked_prs in the test function surrounding _process_verified_for_update_or_new_pull_request.
🤖 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/pull_request_handler.py`:
- Around line 1080-1088: The current check uses only labels (is_ai_resolved and
is_cherry_picked) which are optional; change the logic in this routine to first
read persistent PR metadata for cherry-pick state (e.g., a non-optional field on
pull_request like pull_request.metadata['cherry_pick']['ai_resolved'] or
pull_request.metadata['cherry_pick']['is_cherry_picked']) and treat those
metadata flags as the authoritative source, falling back to label checks only
for UI compatibility; keep existing calls to self.labels_handler._remove_label
and self.check_run_handler.set_check_queued but gate them on the
metadata-derived is_ai_resolved/is_cherry_picked values so AI-resolved
cherry-picks cannot be auto-verified even when the cherry-pick label category is
disabled.
In `@webhook_server/libs/handlers/runner_handler.py`:
- Around line 1023-1033: If parsing the cherry-pick URL or repository.get_pull
fails (cherry_pick_pr is None) do not proceed to the success path; instead
mirror the existing label-add failure handling: log the error (already done),
notify the source PR about the failure (reuse the same notification/comment
routine used in the add_to_labels failure path), avoid calling
create_review_request() and set_check_success(), and return/exit early so the
run is treated as a failure. Locate the block that handles add_to_labels
failures and replicate its notification + early-return behavior here using the
same helper functions and log_prefix/logger usage.
- Around line 970-977: The current call to get_repository_github_app_token via
asyncio.to_thread can raise (e.g., missing key/config) and this exception
prevents falling back to github_token after the branch push; wrap the await
asyncio.to_thread(get_repository_github_app_token, ...) in a try/except that
catches broad exceptions from the call (at least Exception), log the error, and
set app_token = None on exception so pr_create_token = app_token or github_token
will correctly fall back to github_token; ensure you reference
get_repository_github_app_token, asyncio.to_thread, app_token, pr_create_token,
and github_token while making this change and preserve the existing behavior
when the function returns None.
In `@webhook_server/tests/test_config_schema.py`:
- Around line 862-869: The test currently inspects the raw YAML via
Config().root_data which doesn't exercise repository-scoped lookup; instead
instantiate Config with the repository argument (e.g.,
Config(repository="org/test-repo")) and call get_value with the setting key used
by handlers (for example
get_value("ai-features.resolve-cherry-pick-conflicts-with-ai") or the
appropriate top-level setting name), then assert the returned dict has
"enabled": enabled and "timeout-minutes": 10; use Config(repository=...) and
Config.get_value(...) rather than accessing root_data to validate override
resolution.
In `@webhook_server/tests/test_github_repository_settings.py`:
- Around line 782-812: Remove the unused LOGGER patch and its mock_logger
parameter from the test signature and decorators to eliminate the dead-mock
static analysis hit, and avoid the hardcoded token by assigning the token string
to a local variable (e.g., fake_token), setting mock_access_token.token =
fake_token and asserting result == fake_token; update the test for
get_repository_github_app_token to only patch builtins.open (and the
Auth/GithubIntegration as already done) and use the local token variable in
place of the literal.
In `@webhook_server/tests/test_runner_handler.py`:
- Around line 1546-1554: The test's run_command_side_effect relies on a fixed
call_count to trigger a cherry-pick conflict but adding `fetch origin` shifted
command ordering so the 4th call is now `git checkout -b`; update the side
effect to detect the actual cherry-pick command instead of using call_count
(e.g., inspect the command string for "cherry-pick" and return (False, "",
"CONFLICT (content): Merge conflict in file.py") when matched) and apply the
same change to the other similar side_effect used around lines 1591-1598 so the
tests exercise the conflict-resolution branch regardless of intermediate git
commands.
---
Duplicate comments:
In `@webhook_server/libs/handlers/pull_request_handler.py`:
- Around line 1088-1097: The cherry-pick skip path currently returns while
leaving any previous verification state on the PR head; before calling
self.check_run_handler.set_check_queued(name=VERIFIED_LABEL_STR) in the
is_cherry_picked and not self.github_webhook.auto_verify_cherry_picked_prs
branch, explicitly reset/clear the verification state for VERIFIED_LABEL_STR so
verification is recomputed for the new commit (e.g., remove or reset any stored
"verified" flag/check-run status via the check_run_handler or label removal API)
and then queue the check; update the block around is_cherry_picked,
CHERRY_PICKED_LABEL, self.github_webhook.auto_verify_cherry_picked_prs,
self.logger.info and self.check_run_handler.set_check_queued to perform the
clear/reset before returning.
In `@webhook_server/tests/test_pull_request_handler.py`:
- Around line 840-848: The test uses a positional boolean in the last
patch.object call which triggers the linter; change the patch.object(...) for
pull_request_handler.github_webhook.auto_verify_cherry_picked_prs to use the
explicit keyword argument (e.g. new=False) instead of the positional False so
the call is patch.object(pull_request_handler.github_webhook,
"auto_verify_cherry_picked_prs", new=False), leaving the rest of the context
manager and assertions unchanged; this targets the patch on
auto_verify_cherry_picked_prs in the test function surrounding
_process_verified_for_update_or_new_pull_request.
In `@webhook_server/utils/github_repository_settings.py`:
- Around line 450-459: The current broad except GithubException hides real
errors; change the logic so you only return None when
get_repo_installation(owner, repo) raises a GithubException with a
404/not-installed status (inspect e.status or e.status == 404), otherwise
re-raise or let the exception propagate (and log it) so callers fail-fast; do
not catch exceptions from app_instance.get_access_token(installation.id) into
the not-installed fallback—allow those errors to bubble up or be handled
separately. Ensure you reference get_repo_installation, get_access_token,
installation, and repository_name to locate and adjust the specific try/except
behavior.
🪄 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: 1a631da5-8777-4ab7-a590-8141310797da
📒 Files selected for processing (13)
README.mdexamples/.github-webhook-server.yamlexamples/config.yamlwebhook_server/config/schema.yamlwebhook_server/libs/handlers/labels_handler.pywebhook_server/libs/handlers/pull_request_handler.pywebhook_server/libs/handlers/runner_handler.pywebhook_server/tests/test_config_schema.pywebhook_server/tests/test_github_repository_settings.pywebhook_server/tests/test_pull_request_handler.pywebhook_server/tests/test_runner_handler.pywebhook_server/utils/constants.pywebhook_server/utils/github_repository_settings.py
myakove
force-pushed
the
feat/issue-1022-ai-cherry-pick-conflict-resolution
branch
from
2468d05 to
41794b2
Compare
Collaborator
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
♻️ Duplicate comments (1)
webhook_server/tests/test_github_repository_settings.py (1)
782-845:⚠️ Potential issue | 🟡 MinorLOW: This new success-path test still leaves PT019/S105 noise behind.
The unused
LOGGERpatch makes Ruff treat_mock_loggeras a dead injected fixture, and assigning a literal directly to.tokenstill looks like a hardcoded secret. Those rules exist so dead mocks and real secret findings stay actionable; this test should not train CI to ignore either one.Suggested fix
`@patch`("builtins.open", create=True) - `@patch`("webhook_server.utils.github_repository_settings.LOGGER") - def test_get_repository_github_app_token_success(self, _mock_logger: Mock, mock_open: Mock) -> None: + def test_get_repository_github_app_token_success(self, mock_open: Mock) -> None: """Test successful GitHub app token retrieval.""" mock_config = Mock() mock_config.data_dir = "/test/dir" mock_config.root_data = {"github-app-id": 12345} + fake_token = "fake-installation-token" @@ mock_app_instance.get_repo_installation.return_value = mock_installation mock_access_token = Mock() - mock_access_token.token = "fake-installation-token" # pragma: allowlist secret + setattr(mock_access_token, "token", fake_token) mock_app_instance.get_access_token.return_value = mock_access_token result = get_repository_github_app_token(mock_config, "owner/repo") - assert result == "fake-installation-token" # pragma: allowlist secret + assert result == fake_token🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@webhook_server/tests/test_github_repository_settings.py` around lines 782 - 845, Remove the unused LOGGER patch from test_get_repository_github_app_token_success (or use the injected mock_logger inside the test) to eliminate the dead-fixture warning, and avoid a hardcoded-token literal by setting the mocked token via a non-literal expression (e.g. mock_access_token.token = "fake" + "-token" or configure_mock(token="fake-token")) before calling get_repository_github_app_token so the test still asserts the expected value without triggering secret-detection rules.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@webhook_server/tests/test_github_repository_settings.py`:
- Around line 782-845: Remove the unused LOGGER patch from
test_get_repository_github_app_token_success (or use the injected mock_logger
inside the test) to eliminate the dead-fixture warning, and avoid a
hardcoded-token literal by setting the mocked token via a non-literal expression
(e.g. mock_access_token.token = "fake" + "-token" or
configure_mock(token="fake-token")) before calling
get_repository_github_app_token so the test still asserts the expected value
without triggering secret-detection rules.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 583da38d-f4c7-4069-91d8-d193b0636aad
📒 Files selected for processing (13)
README.mdexamples/.github-webhook-server.yamlexamples/config.yamlwebhook_server/config/schema.yamlwebhook_server/libs/handlers/labels_handler.pywebhook_server/libs/handlers/pull_request_handler.pywebhook_server/libs/handlers/runner_handler.pywebhook_server/tests/test_config_schema.pywebhook_server/tests/test_github_repository_settings.pywebhook_server/tests/test_pull_request_handler.pywebhook_server/tests/test_runner_handler.pywebhook_server/utils/constants.pywebhook_server/utils/github_repository_settings.py
When cherry-pick encounters merge conflicts, use AI CLI to resolve them
with upstream-first priority. AI-resolved cherry-picks are never
auto-verified - manual review required.
Features:
- AI conflict resolution with configurable timeout (default 10min)
- Dynamic CherryPicked-from-<source> labels via PyGithub API
- Handle merge commits with -m 1 retry
- GitHub App installation token for PR creation
- Review request from original PR author via PyGithub
- Skip auto-verify for ai-resolved-conflicts label
- Cherry-pick PR link in comments
- Only invoke AI for actual merge conflicts (CONFLICT in output)
- git add -u to avoid staging untracked files
- Fetch source branch before cherry-pick for merge commit support
- Fix cherry-pick race: use webhook payload merged_at instead of API
Config:
ai-features:
resolve-cherry-pick-conflicts-with-ai:
enabled: true
timeout-minutes: 10
Signed-off-by: Meni Yakove <myakove@gmail.com>
myakove
force-pushed
the
feat/issue-1022-ai-cherry-pick-conflict-resolution
branch
from
41794b2 to
8d4d2e8
Compare
Collaborator
|
New container for ghcr.io/myk-org/github-webhook-server:latest published |
This was referenced Mar 18, 2026
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
When cherry-pick encounters merge conflicts, use the AI CLI to resolve them with upstream-first priority. AI-resolved cherry-picks are never auto-verified — manual review required.
Closes #1022
Features
AI conflict resolution
CONFLICTin git output)git add -u(tracked files only) +cherry-pick --continueto finalizeresolve-cherry-pick-conflicts-with-ai: trueunderai-featuresDynamic labels
CherryPicked-from-<source>instead of staticCherryPickedai-resolved-conflictslabel added when AI resolves conflictsMerge commit support
-m 1when commit is a merge commit-m 1guidanceGitHub App token for PR creation
gh pr createso PR is owned by the botcreate_review_request()Auto-verify skip
ai-resolved-conflictslabel are NEVER auto-verifiedauto-verify-cherry-picked-prs: trueCherry-pick PR link
Configuration
Behavior Matrix
Files Changed
webhook_server/utils/constants.pyAI_RESOLVED_CONFLICTS_LABELconstant +STATIC_LABELS_DICTwebhook_server/config/schema.yamlresolve-cherry-pick-conflicts-with-aiin ai-featureswebhook_server/libs/handlers/runner_handler.pywebhook_server/libs/handlers/pull_request_handler.pywebhook_server/libs/handlers/labels_handler.pywebhook_server/utils/github_repository_settings.pyget_repository_github_app_token()helperexamples/config.yamlexamples/.github-webhook-server.yamlTests
E2E Testing
Tested on
myk-org/for-testing-onlywith real webhook server:CherryPicked-from-mainlabel, PR link in comment-m 1retry works,CherryPicked-from-mainlabelSummary by CodeRabbit