@@ -137,8 +137,9 @@ def cherry_pick_commits(commits, source, branch_name, repo_path=None):
async def run_review_agent(mr_iid, branch_name, comments, remote, target='master',
- repo_path=None):
- """Run the Claude agent to handle MR comments
+ needs_rebase=False, has_conflicts=False,
+ mr_description='', repo_path=None):
+ """Run the Claude agent to handle MR comments and/or rebase
Args:
mr_iid (int): Merge request IID
@@ -146,6 +147,9 @@ async def run_review_agent(mr_iid, branch_name, comments, remote, target='master
comments (list): List of comment dicts with 'author', 'body' keys
remote (str): Git remote name
target (str): Target branch for rebase operations
+ needs_rebase (bool): Whether the MR needs rebasing
+ has_conflicts (bool): Whether the MR has merge conflicts
+ mr_description (str): MR description with context from previous work
repo_path (str): Path to repository (defaults to current directory)
Returns:
@@ -157,35 +161,75 @@ async def run_review_agent(mr_iid, branch_name, comments, remote, target='master
if repo_path is None:
repo_path = os.getcwd()
- # Format comments for the prompt
- comment_text = '\n'.join(
- f'- [{c.author}]: {c.body}'
- for c in comments
- )
+ # Build the prompt based on what needs to be done
+ tasks = []
+ if needs_rebase:
+ if has_conflicts:
+ tasks.append("rebase and resolve merge conflicts")
+ else:
+ tasks.append("rebase onto latest target branch")
+ if comments:
+ tasks.append(f"address {len(comments)} review comment(s)")
+
+ task_desc = " and ".join(tasks)
+
+ # Include MR description for context from previous work
+ context_section = ""
+ if mr_description:
+ context_section = f"""
+Context from MR description (includes previous work done on this MR):
- prompt = f"""Review comments on merge request !{mr_iid} (branch: {branch_name}):
+{mr_description}
+
+Use this context to understand what was done previously and respond appropriately.
+"""
+
+ # Format comments for the prompt (if any)
+ comment_section = ""
+ if comments:
+ comment_text = '\n'.join(
+ f'- [{c.author}]: {c.body}'
+ for c in comments
+ )
+ comment_section = f"""
+Review comments to address:
{comment_text}
+"""
+
+ # Build rebase instructions
+ rebase_section = ""
+ if needs_rebase:
+ rebase_section = f"""
+Rebase instructions:
+- The MR is behind the target branch and needs rebasing
+- Use: git rebase --keep-empty {remote}/{target}
+- This preserves empty merge commits which are important for tracking
+- If there are conflicts, try to resolve them automatically
+- For complex conflicts that cannot be resolved, describe them and abort
+"""
+ prompt = f"""Task for merge request !{mr_iid} (branch: {branch_name}): {task_desc}
+{context_section}{comment_section}{rebase_section}
Steps to follow:
1. Checkout the branch: git checkout {branch_name}
-2. Read and understand each comment
-3. For each actionable comment:
- - Make the requested changes to the code
- - Amend the relevant commit or create a fixup commit
+2. {'Rebase onto ' + remote + '/' + target + ' first' if needs_rebase else 'Read and understand each comment'}
+3. {'After rebase, address any review comments' if needs_rebase and comments else 'For each actionable comment:' if comments else 'Verify the rebase completed successfully'}
+ {'- Make the requested changes to the code' if comments else ''}
+ {'- Amend the relevant commit or create a fixup commit' if comments else ''}
4. Run 'crosfw sandbox -L' to verify the build
5. Create a local branch with suffix '-v2' (or increment: -v3, -v4, etc.)
6. Force push to the ORIGINAL remote branch to update the MR:
- git push --force-with-lease {remote} HEAD:{branch_name}
-7. Report what changes were made and what reply should be posted to the MR
+ git push -o ci.skip --force-with-lease {remote} HEAD:{branch_name}
+ (ci.skip prevents a duplicate pipeline; the MR pipeline will run automatically)
+7. Report what was done and what reply should be posted to the MR
Important:
-- Keep changes minimal and focused on addressing the comments
+- Keep changes minimal and focused
- If a comment is unclear or cannot be addressed, note this in your report
- Local branch: {branch_name}-v2 (or -v3, -v4 etc.)
- Remote push: always to '{branch_name}' to update the existing MR
-- If rebasing is requested, use: git rebase --keep-empty {remote}/{target}
- This preserves empty merge commits which are important for tracking
+- Always use -o ci.skip when pushing to avoid duplicate pipelines
"""
options = ClaudeAgentOptions(
@@ -193,7 +237,7 @@ Important:
cwd=repo_path,
)
- tout.info(f'Starting Claude agent to handle {len(comments)} comment(s)...')
+ tout.info(f'Starting Claude agent to {task_desc}...')
tout.info('')
conversation_log = []
@@ -211,7 +255,8 @@ Important:
def handle_mr_comments(mr_iid, branch_name, comments, remote, target='master',
- repo_path=None):
+ needs_rebase=False, has_conflicts=False,
+ mr_description='', repo_path=None):
"""Synchronous wrapper for running the review agent
Args:
@@ -220,6 +265,9 @@ def handle_mr_comments(mr_iid, branch_name, comments, remote, target='master',
comments (list): List of comment dicts
remote (str): Git remote name
target (str): Target branch for rebase operations
+ needs_rebase (bool): Whether the MR needs rebasing
+ has_conflicts (bool): Whether the MR has merge conflicts
+ mr_description (str): MR description with context from previous work
repo_path (str): Path to repository (defaults to current directory)
Returns:
@@ -227,4 +275,5 @@ def handle_mr_comments(mr_iid, branch_name, comments, remote, target='master',
conversation_log is the agent's output text
"""
return asyncio.run(run_review_agent(mr_iid, branch_name, comments, remote,
- target, repo_path))
+ target, needs_rebase, has_conflicts,
+ mr_description, repo_path))
@@ -663,6 +663,9 @@ def process_mr_reviews(remote, mrs, dbs, target='master'):
Returns:
int: Number of MRs with comments processed
"""
+ # Save current branch to restore later
+ original_branch = run_git(['rev-parse', '--abbrev-ref', 'HEAD'])
+
# Fetch to get latest remote state (needed for rebase)
tout.info(f'Fetching {remote}...')
run_git(['fetch', remote])
@@ -673,7 +676,7 @@ def process_mr_reviews(remote, mrs, dbs, target='master'):
mr_iid = merge_req.iid
comments = gitlab_api.get_mr_comments(remote, mr_iid)
if comments is None:
- continue
+ comments = []
# Filter to unresolved comments that haven't been processed
unresolved = []
@@ -683,21 +686,35 @@ def process_mr_reviews(remote, mrs, dbs, target='master'):
if dbs.comment_is_processed(mr_iid, com.id):
continue
unresolved.append(com)
- if not unresolved:
+
+ # Check if rebase is needed
+ needs_rebase = merge_req.needs_rebase or merge_req.has_conflicts
+
+ # Skip if no comments and no rebase needed
+ if not unresolved and not needs_rebase:
continue
tout.info('')
- tout.info(f"MR !{mr_iid} has {len(unresolved)} new comment(s):")
- for comment in unresolved:
- tout.info(f' [{comment.author}]: {comment.body[:80]}...')
+ if needs_rebase:
+ if merge_req.has_conflicts:
+ tout.info(f"MR !{mr_iid} has merge conflicts - rebasing...")
+ else:
+ tout.info(f"MR !{mr_iid} needs rebase...")
+ if unresolved:
+ tout.info(f"MR !{mr_iid} has {len(unresolved)} new comment(s):")
+ for comment in unresolved:
+ tout.info(f' [{comment.author}]: {comment.body[:80]}...')
- # Run agent to handle comments
+ # Run agent to handle comments and/or rebase
success, conversation_log = agent.handle_mr_comments(
mr_iid,
merge_req.source_branch,
unresolved,
remote,
target,
+ needs_rebase=needs_rebase,
+ has_conflicts=merge_req.has_conflicts,
+ mr_description=merge_req.description,
)
if success:
@@ -726,6 +743,11 @@ def process_mr_reviews(remote, mrs, dbs, target='master'):
tout.error(f"Failed to handle comments for MR !{mr_iid}")
processed += 1
+ # Restore original branch
+ if processed:
+ tout.info(f'Returning to {original_branch}')
+ run_git(['checkout', original_branch])
+
return processed
@@ -1917,6 +1917,69 @@ class TestProcessMrReviewsCommentTracking(unittest.TestCase):
dbs.close()
+ def test_rebase_without_comments(self):
+ """Test that MRs needing rebase trigger agent even without comments."""
+ with terminal.capture():
+ dbs = database.Database(self.db_path)
+ dbs.start()
+
+ # MR needs rebase but has no comments
+ mrs = [gitlab_api.PickmanMr(
+ iid=100,
+ title='[pickman] Test MR',
+ source_branch='cherry-test',
+ description='Test',
+ web_url='https://gitlab.com/mr/100',
+ has_conflicts=False,
+ needs_rebase=True,
+ )]
+
+ with mock.patch.object(control, 'run_git'):
+ with mock.patch.object(gitlab_api, 'get_mr_comments',
+ return_value=[]):
+ with mock.patch.object(agent, 'handle_mr_comments',
+ return_value=(True, 'Rebased')) as mock_agent:
+ with mock.patch.object(gitlab_api, 'update_mr_description'):
+ with mock.patch.object(control, 'update_history_with_review'):
+ control.process_mr_reviews('ci', mrs, dbs)
+
+ # Agent should be called with needs_rebase=True
+ mock_agent.assert_called_once()
+ call_kwargs = mock_agent.call_args[1]
+ self.assertTrue(call_kwargs.get('needs_rebase'))
+ self.assertFalse(call_kwargs.get('has_conflicts'))
+
+ dbs.close()
+
+ def test_skips_mr_no_rebase_no_comments(self):
+ """Test that MRs without rebase need or comments are skipped."""
+ with terminal.capture():
+ dbs = database.Database(self.db_path)
+ dbs.start()
+
+ # MR has no comments and doesn't need rebase
+ mrs = [gitlab_api.PickmanMr(
+ iid=100,
+ title='[pickman] Test MR',
+ source_branch='cherry-test',
+ description='Test',
+ web_url='https://gitlab.com/mr/100',
+ has_conflicts=False,
+ needs_rebase=False,
+ )]
+
+ with mock.patch.object(control, 'run_git'):
+ with mock.patch.object(gitlab_api, 'get_mr_comments',
+ return_value=[]):
+ with mock.patch.object(agent, 'handle_mr_comments',
+ return_value=(True, 'Done')) as mock_agent:
+ control.process_mr_reviews('ci', mrs, dbs)
+
+ # Agent should NOT be called
+ mock_agent.assert_not_called()
+
+ dbs.close()
+
class TestParsePoll(unittest.TestCase):
"""Tests for poll command argument parsing."""
@@ -28,9 +28,11 @@ except ImportError:
# Merge request info returned by get_pickman_mrs()
+# Use defaults for new fields so existing code doesn't break
PickmanMr = namedtuple('PickmanMr', [
- 'iid', 'title', 'web_url', 'source_branch', 'description'
-])
+ 'iid', 'title', 'web_url', 'source_branch', 'description',
+ 'has_conflicts', 'needs_rebase'
+], defaults=[False, False])
# Comment info returned by get_mr_comments()
MrComment = namedtuple('MrComment', [
@@ -232,12 +234,22 @@ def get_pickman_mrs(remote, state='opened'):
pickman_mrs = []
for merge_req in mrs:
if '[pickman]' in merge_req.title:
+ # Check merge status - detailed_merge_status is newer API
+ detailed_status = getattr(merge_req, 'detailed_merge_status', '')
+ needs_rebase = detailed_status == 'need_rebase'
+ # Also check diverged_commits_count as fallback
+ if not needs_rebase:
+ diverged = getattr(merge_req, 'diverged_commits_count', 0)
+ needs_rebase = diverged and diverged > 0
+
pickman_mrs.append(PickmanMr(
iid=merge_req.iid,
title=merge_req.title,
web_url=merge_req.web_url,
source_branch=merge_req.source_branch,
description=merge_req.description or '',
+ has_conflicts=getattr(merge_req, 'has_conflicts', False),
+ needs_rebase=needs_rebase,
))
return pickman_mrs
except gitlab.exceptions.GitlabError as exc: