@@ -80,6 +80,18 @@ After successfully applying commits, update the database to record progress::
This updates the last cherry-picked commit for the source branch, so subsequent
``next-set`` and ``apply`` commands will start from the new position.
+To check open MRs for comments and address them::
+
+ ./tools/pickman/pickman review
+
+This lists open pickman MRs (those with ``[pickman]`` in the title), checks each
+for unresolved comments, and uses a Claude agent to address them. The agent will
+make code changes based on the feedback and push an updated branch.
+
+Options for the review command:
+
+- ``-r, --remote``: Git remote (default: ci)
+
Requirements
------------
@@ -57,6 +57,11 @@ def parse_args(argv):
help='Show next set of commits to cherry-pick')
next_set.add_argument('source', help='Source branch name')
+ review_cmd = subparsers.add_parser('review',
+ help='Check open MRs and handle comments')
+ review_cmd.add_argument('-r', '--remote', default='ci',
+ help='Git remote (default: ci)')
+
subparsers.add_parser('test', help='Run tests')
return parser.parse_args(argv)
@@ -132,3 +132,90 @@ def cherry_pick_commits(commits, source, branch_name, repo_path=None):
"""
return asyncio.run(run(commits, source, branch_name,
repo_path))
+
+
+async def run_review_agent(mr_iid, branch_name, comments, remote, repo_path=None):
+ """Run the Claude agent to handle MR comments
+
+ Args:
+ mr_iid (int): Merge request IID
+ branch_name (str): Source branch name
+ comments (list): List of comment dicts with 'author', 'body' keys
+ remote (str): Git remote name
+ repo_path (str): Path to repository (defaults to current directory)
+
+ Returns:
+ bool: True on success, False on failure
+ """
+ if not check_available():
+ return False
+
+ 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
+ )
+
+ prompt = f"""Review comments on merge request !{mr_iid} (branch: {branch_name}):
+
+{comment_text}
+
+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
+4. Run 'buildman -L --board sandbox -w -o /tmp/pickman' to verify the build
+5. Create a new branch with suffix '-v2' (or increment existing version)
+6. Push the new branch: git push {remote} <new-branch-name>
+7. Report what changes were made and what reply should be posted to the MR
+
+Important:
+- Keep changes minimal and focused on addressing the comments
+- If a comment is unclear or cannot be addressed, note this in your report
+- Do not force push to the original branch
+- The new branch name should be: {branch_name}-v2 (or -v3, -v4 etc if needed)
+"""
+
+ options = ClaudeAgentOptions(
+ allowed_tools=['Bash', 'Read', 'Grep', 'Edit', 'Write'],
+ cwd=repo_path,
+ )
+
+ tout.info(f'Starting Claude agent to handle {len(comments)} comment(s)...')
+ tout.info('')
+
+ conversation_log = []
+ try:
+ async for message in query(prompt=prompt, options=options):
+ if hasattr(message, 'content'):
+ for block in message.content:
+ if hasattr(block, 'text'):
+ print(block.text)
+ conversation_log.append(block.text)
+ return True, '\n\n'.join(conversation_log)
+ except (RuntimeError, ValueError, OSError) as exc:
+ tout.error(f'Agent failed: {exc}')
+ return False, '\n\n'.join(conversation_log)
+
+
+def handle_mr_comments(mr_iid, branch_name, comments, remote, repo_path=None):
+ """Synchronous wrapper for running the review agent
+
+ Args:
+ mr_iid (int): Merge request IID
+ branch_name (str): Source branch name
+ comments (list): List of comment dicts
+ remote (str): Git remote name
+ repo_path (str): Path to repository (defaults to current directory)
+
+ Returns:
+ tuple: (success, conversation_log) where success is bool and
+ conversation_log is the agent's output text
+ """
+ return asyncio.run(run_review_agent(mr_iid, branch_name, comments, remote,
+ repo_path))
@@ -436,6 +436,82 @@ def do_commit_source(args, dbs):
return 0
+def process_mr_reviews(remote, mrs):
+ """Process review comments on open MRs
+
+ Checks each MR for unresolved comments and uses Claude agent to address
+ them.
+
+ Args:
+ remote (str): Remote name
+ mrs (list): List of MR dicts from get_open_pickman_mrs()
+
+ Returns:
+ int: Number of MRs with comments processed
+ """
+ processed = 0
+
+ for merge_req in mrs:
+ comments = gitlab_api.get_mr_comments(remote, merge_req['iid'])
+ if comments is None:
+ continue
+
+ # Filter to unresolved comments
+ unresolved = [c for c in comments if not c.get('resolved', True)]
+ if not unresolved:
+ continue
+
+ tout.info('')
+ tout.info(f"MR !{merge_req['iid']} has {len(unresolved)} comment(s):")
+ for comment in unresolved:
+ tout.info(f" [{comment['author']}]: {comment['body'][:80]}...")
+
+ # Run agent to handle comments
+ success, _ = agent.handle_mr_comments(
+ merge_req['iid'],
+ merge_req['source_branch'],
+ unresolved,
+ remote,
+ )
+ if not success:
+ tout.error(f"Failed to handle comments for MR !{merge_req['iid']}")
+ processed += 1
+
+ return processed
+
+
+def do_review(args, dbs): # pylint: disable=unused-argument
+ """Check open pickman MRs and handle comments
+
+ Lists open MRs created by pickman, checks for human comments, and uses
+ Claude agent to address them.
+
+ Args:
+ args (Namespace): Parsed arguments with 'remote' attribute
+ dbs (Database): Database instance
+
+ Returns:
+ int: 0 on success, 1 on failure
+ """
+ remote = args.remote
+
+ # Get open pickman MRs
+ mrs = gitlab_api.get_open_pickman_mrs(remote)
+ if mrs is None:
+ return 1
+
+ if not mrs:
+ tout.info('No open pickman MRs found')
+ return 0
+
+ tout.info(f'Found {len(mrs)} open pickman MR(s):')
+ for merge_req in mrs:
+ tout.info(f" !{merge_req['iid']}: {merge_req['title']}")
+
+ process_mr_reviews(remote, mrs)
+
+ return 0
+
def do_test(args, dbs): # pylint: disable=unused-argument
"""Run tests for this module.
@@ -463,6 +539,7 @@ COMMANDS = {
'compare': do_compare,
'list-sources': do_list_sources,
'next-set': do_next_set,
+ 'review': do_review,
'test': do_test,
}
@@ -1105,5 +1105,43 @@ class TestParseApplyWithPush(unittest.TestCase):
self.assertEqual(args.target, 'main')
+class TestParseReview(unittest.TestCase):
+ """Tests for review command argument parsing."""
+
+ def test_parse_review_defaults(self):
+ """Test parsing review command with defaults."""
+ args = pickman.parse_args(['review'])
+ self.assertEqual(args.cmd, 'review')
+ self.assertEqual(args.remote, 'ci')
+
+ def test_parse_review_with_remote(self):
+ """Test parsing review command with custom remote."""
+ args = pickman.parse_args(['review', '-r', 'origin'])
+ self.assertEqual(args.cmd, 'review')
+ self.assertEqual(args.remote, 'origin')
+
+
+class TestReview(unittest.TestCase):
+ """Tests for review command."""
+
+ def test_review_no_mrs(self):
+ """Test review when no open MRs found."""
+ with mock.patch.object(gitlab_api, 'get_open_pickman_mrs',
+ return_value=[]):
+ args = argparse.Namespace(cmd='review', remote='ci')
+ ret = control.do_review(args, None)
+
+ self.assertEqual(ret, 0)
+
+ def test_review_gitlab_error(self):
+ """Test review when GitLab API returns error."""
+ with mock.patch.object(gitlab_api, 'get_open_pickman_mrs',
+ return_value=None):
+ args = argparse.Namespace(cmd='review', remote='ci')
+ ret = control.do_review(args, None)
+
+ self.assertEqual(ret, 1)
+
+
if __name__ == '__main__':
unittest.main()
@@ -152,6 +152,136 @@ def create_mr(host, proj_path, source, target, title, desc=''):
return None
+def get_open_pickman_mrs(remote):
+ """Get open merge requests created by pickman
+
+ Args:
+ remote (str): Remote name
+
+ Returns:
+ list: List of dicts with 'iid', 'title', 'web_url', 'source_branch' keys,
+ or None on failure
+ """
+ if not check_available():
+ return None
+
+ token = get_token()
+ if not token:
+ tout.error('GITLAB_TOKEN environment variable not set')
+ return None
+
+ remote_url = get_remote_url(remote)
+ host, proj_path = parse_url(remote_url)
+
+ if not host or not proj_path:
+ tout.error(f"Could not parse GitLab URL from remote '{remote}'")
+ return None
+
+ try:
+ glab = gitlab.Gitlab(f'https://{host}', private_token=token)
+ project = glab.projects.get(proj_path)
+
+ mrs = project.mergerequests.list(state='opened', get_all=True)
+ pickman_mrs = []
+ for merge_req in mrs:
+ if '[pickman]' in merge_req.title:
+ pickman_mrs.append({
+ 'iid': merge_req.iid,
+ 'title': merge_req.title,
+ 'web_url': merge_req.web_url,
+ 'source_branch': merge_req.source_branch,
+ })
+ return pickman_mrs
+ except gitlab.exceptions.GitlabError as exc:
+ tout.error(f'GitLab API error: {exc}')
+ return None
+
+
+def get_mr_comments(remote, mr_iid):
+ """Get human comments on a merge request (excluding bot/system notes)
+
+ Args:
+ remote (str): Remote name
+ mr_iid (int): Merge request IID
+
+ Returns:
+ list: List of dicts with 'id', 'author', 'body', 'created_at',
+ 'resolvable', 'resolved' keys, or None on failure
+ """
+ if not check_available():
+ return None
+
+ token = get_token()
+ if not token:
+ tout.error('GITLAB_TOKEN environment variable not set')
+ return None
+
+ remote_url = get_remote_url(remote)
+ host, proj_path = parse_url(remote_url)
+
+ if not host or not proj_path:
+ return None
+
+ try:
+ glab = gitlab.Gitlab(f'https://{host}', private_token=token)
+ project = glab.projects.get(proj_path)
+ merge_req = project.mergerequests.get(mr_iid)
+
+ comments = []
+ for note in merge_req.notes.list(get_all=True):
+ # Skip system notes (merge status, etc.)
+ if note.system:
+ continue
+ comments.append({
+ 'id': note.id,
+ 'author': note.author['username'],
+ 'body': note.body,
+ 'created_at': note.created_at,
+ 'resolvable': getattr(note, 'resolvable', False),
+ 'resolved': getattr(note, 'resolved', False),
+ })
+ return comments
+ except gitlab.exceptions.GitlabError as exc:
+ tout.error(f'GitLab API error: {exc}')
+ return None
+
+
+def reply_to_mr(remote, mr_iid, message):
+ """Post a reply to a merge request
+
+ Args:
+ remote (str): Remote name
+ mr_iid (int): Merge request IID
+ message (str): Reply message
+
+ Returns:
+ bool: True on success
+ """
+ if not check_available():
+ return False
+
+ token = get_token()
+ if not token:
+ tout.error('GITLAB_TOKEN environment variable not set')
+ return False
+
+ remote_url = get_remote_url(remote)
+ host, proj_path = parse_url(remote_url)
+
+ if not host or not proj_path:
+ return False
+
+ try:
+ glab = gitlab.Gitlab(f'https://{host}', private_token=token)
+ project = glab.projects.get(proj_path)
+ merge_req = project.mergerequests.get(mr_iid)
+ merge_req.notes.create({'body': message})
+ return True
+ except gitlab.exceptions.GitlabError as exc:
+ tout.error(f'GitLab API error: {exc}')
+ return False
+
+
def push_and_create_mr(remote, branch, target, title, desc=''):
"""Push a branch and create a merge request