From patchwork Wed Dec 17 19:28:26 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 989 Return-Path: X-Original-To: u-boot-concept@u-boot.org Delivered-To: u-boot-concept@u-boot.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1765999729; bh=hnYRsqEby/ELmO1HYBLayT2g3feA8FjSM+4ZJXXApiU=; h=From:To:Date:In-Reply-To:References:CC:Subject:List-Id: List-Archive:List-Help:List-Owner:List-Post:List-Subscribe: List-Unsubscribe:From; b=XL/ivPC/IbeFub4/tBmwVD++V94T3VG2MY9EapQlX/2qoy6K7CWjbYcnoruWFyouX FVmUCNJX3TrzrpDaZUDZqYQT+us2NQ/TWsoTFZFhQM1zmQLuiKlsts+X3lDmrFqa2w oOGx10hLMoQMmnSNngnJR30yyDHMvENDNTUSn4wXRlWxBaG9cOWuZyROJ4PTkpE1zt 2iM6T4Y5kzAvLFJB+EeaaYxCaZGu9NX9qPLNUUIlKPWv4JvBwGIAiUrYjUXvZOXcHM XTwkav9hYjvTWLefEUMI2SmuEnkfVksxSCZM4pAOXFKTK15VjMMKeAVnI4kPDGcHQO eun6i/35zG0Tg== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id B9B7B68C2F for ; Wed, 17 Dec 2025 12:28:49 -0700 (MST) X-Virus-Scanned: Debian amavis at Received: from mail.u-boot.org ([127.0.0.1]) by localhost (mail.u-boot.org [127.0.0.1]) (amavis, port 10024) with ESMTP id O1C-IGCS0LXK for ; Wed, 17 Dec 2025 12:28:49 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1765999729; bh=hnYRsqEby/ELmO1HYBLayT2g3feA8FjSM+4ZJXXApiU=; h=From:To:Date:In-Reply-To:References:CC:Subject:List-Id: List-Archive:List-Help:List-Owner:List-Post:List-Subscribe: List-Unsubscribe:From; b=XL/ivPC/IbeFub4/tBmwVD++V94T3VG2MY9EapQlX/2qoy6K7CWjbYcnoruWFyouX FVmUCNJX3TrzrpDaZUDZqYQT+us2NQ/TWsoTFZFhQM1zmQLuiKlsts+X3lDmrFqa2w oOGx10hLMoQMmnSNngnJR30yyDHMvENDNTUSn4wXRlWxBaG9cOWuZyROJ4PTkpE1zt 2iM6T4Y5kzAvLFJB+EeaaYxCaZGu9NX9qPLNUUIlKPWv4JvBwGIAiUrYjUXvZOXcHM XTwkav9hYjvTWLefEUMI2SmuEnkfVksxSCZM4pAOXFKTK15VjMMKeAVnI4kPDGcHQO eun6i/35zG0Tg== Received: from mail.u-boot.org (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id A8C7068B5C for ; Wed, 17 Dec 2025 12:28:49 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1765999727; bh=ld2HF7K1GRRkABheQZlP8l/SUR4G5cTohOVD5ENQH2A=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=P70895iqnL0Ke+cahdel2BoS+0A4PWXs+nkNbDHX2S6Kw/x3im9SpiULjDZWGrwYK vgb1xBrXw2vE2nL16Za2MKGv15Yyuc55jEodiBjUiq3YObP9FJX5Wem7/YF25yQGQr EagcbPVGcNvQvFjhdUVh0i5oKFqp5+iIvwsTtPTKFJWNKhtAjwuiwqa3PYldpiX1D1 nCTUp6+jmU95QOhVm6QT8jeakmDqyPEN9ZC6ySW74X58TdioCLP2PMMdrepgXPfN3X q9CpJ5tpR4TYhPmXfO53JQVZqo2cX1mXfreLR31yCrHww0nMucfD7/1XtEmH1IWb4N 6vVmvADSV626g== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 3163E68B26; Wed, 17 Dec 2025 12:28:47 -0700 (MST) X-Virus-Scanned: Debian amavis at Received: from mail.u-boot.org ([127.0.0.1]) by localhost (mail.u-boot.org [127.0.0.1]) (amavis, port 10026) with ESMTP id FMCIYz_BMvbR; Wed, 17 Dec 2025 12:28:47 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1765999722; bh=1agbvU/+9owXDkrkWRH2BNZtWb+TmPYnNQDvv/dNxQU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=KVJgmI+Xnm731T8HMNGOWvPaSh8+RLBCy9jep3RV//DVikTbHZictY51ltUq1L24r 5/4ieDh7UxNXwe0PlgxhOss+j4JfB6md9xLvm7+Do/6OBkCXi+GxCkk+uPmQRCkiMU FQIv1bhT23DCf8qJW8kS9oylZRpPzQ5G3e24qCb2qrPE1yTGYgsaaOb7ODpvvTxkp6 irhFIAJbaoR9UA1qITLkaXDSyyYgvKoMYQ/JTKvAIt9ru2dH8wsts5JtQr+HIbQBgj QcLPcVolnt62ikAIASzGuu67p1c3R8PLGjqResFGeRfylgBSVmzO1JdB0ervZib6V1 7qPa10KPhvabw== Received: from u-boot.org (unknown [73.34.74.121]) by mail.u-boot.org (Postfix) with ESMTPSA id B25246897B; Wed, 17 Dec 2025 12:28:41 -0700 (MST) From: Simon Glass To: U-Boot Concept Date: Wed, 17 Dec 2025 12:28:26 -0700 Message-ID: <20251217192831.1308453-2-sjg@u-boot.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20251217192831.1308453-1-sjg@u-boot.org> References: <20251217192831.1308453-1-sjg@u-boot.org> MIME-Version: 1.0 Message-ID-Hash: HDU5RXTHCIJVQCGD55EM5B4XX7A7SRZJ X-Message-ID-Hash: HDU5RXTHCIJVQCGD55EM5B4XX7A7SRZJ X-MailFrom: sjg@u-boot.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: Simon Glass , Claude X-Mailman-Version: 3.3.10 Precedence: list Subject: [Concept] [PATCH v2 1/2] pickman: Add git fetch before MR operations List-Id: Discussion and patches related to U-Boot Concept Archived-At: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: From: Simon Glass Add git fetch to ensure pickman uses the latest remote state: - Before creating a new MR, fetch the remote first - Before processing review comments, fetch the remote This ensures that when creating MRs, the base branch (ci/master) is up to date, and when the agent handles review comments that request a rebase, it has the latest commits available. Also update the agent prompt to use the configured remote/target for rebase operations instead of a hardcoded value. Co-developed-by: Claude Signed-off-by: Simon Glass --- (no changes since v1) tools/pickman/agent.py | 12 ++++++++---- tools/pickman/control.py | 14 ++++++++++++-- tools/pickman/ftest.py | 15 ++++++++------- 3 files changed, 28 insertions(+), 13 deletions(-) diff --git a/tools/pickman/agent.py b/tools/pickman/agent.py index 26f4ac3afd9..bb922b03b13 100644 --- a/tools/pickman/agent.py +++ b/tools/pickman/agent.py @@ -136,7 +136,8 @@ def cherry_pick_commits(commits, source, branch_name, repo_path=None): repo_path)) -async def run_review_agent(mr_iid, branch_name, comments, remote, 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 Args: @@ -144,6 +145,7 @@ async def run_review_agent(mr_iid, branch_name, comments, remote, repo_path=None branch_name (str): Source branch name comments (list): List of comment dicts with 'author', 'body' keys remote (str): Git remote name + target (str): Target branch for rebase operations repo_path (str): Path to repository (defaults to current directory) Returns: @@ -182,7 +184,7 @@ Important: - 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 +- If rebasing is requested, use: git rebase --keep-empty {remote}/{target} This preserves empty merge commits which are important for tracking """ @@ -208,7 +210,8 @@ Important: return False, '\n\n'.join(conversation_log) -def handle_mr_comments(mr_iid, branch_name, comments, remote, repo_path=None): +def handle_mr_comments(mr_iid, branch_name, comments, remote, target='master', + repo_path=None): """Synchronous wrapper for running the review agent Args: @@ -216,6 +219,7 @@ def handle_mr_comments(mr_iid, branch_name, comments, remote, repo_path=None): branch_name (str): Source branch name comments (list): List of comment dicts remote (str): Git remote name + target (str): Target branch for rebase operations repo_path (str): Path to repository (defaults to current directory) Returns: @@ -223,4 +227,4 @@ def handle_mr_comments(mr_iid, branch_name, comments, remote, repo_path=None): conversation_log is the agent's output text """ return asyncio.run(run_review_agent(mr_iid, branch_name, comments, remote, - repo_path)) + target, repo_path)) diff --git a/tools/pickman/control.py b/tools/pickman/control.py index b07ef703ba2..dec2c68883f 100644 --- a/tools/pickman/control.py +++ b/tools/pickman/control.py @@ -648,7 +648,7 @@ def do_commit_source(args, dbs): return 0 -def process_mr_reviews(remote, mrs, dbs): +def process_mr_reviews(remote, mrs, dbs, target='master'): """Process review comments on open MRs Checks each MR for unresolved comments and uses Claude agent to address @@ -658,10 +658,15 @@ def process_mr_reviews(remote, mrs, dbs): remote (str): Remote name mrs (list): List of MR dicts from get_open_pickman_mrs() dbs (Database): Database instance for tracking processed comments + target (str): Target branch for rebase operations Returns: int: Number of MRs with comments processed """ + # Fetch to get latest remote state (needed for rebase) + tout.info(f'Fetching {remote}...') + run_git(['fetch', remote]) + processed = 0 for merge_req in mrs: @@ -692,6 +697,7 @@ def process_mr_reviews(remote, mrs, dbs): merge_req.source_branch, unresolved, remote, + target, ) if success: @@ -911,13 +917,17 @@ def do_step(args, dbs): tout.info(f" !{merge_req.iid}: {merge_req.title}") # Process any review comments on open MRs - process_mr_reviews(remote, mrs, dbs) + process_mr_reviews(remote, mrs, dbs, args.target) tout.info('') tout.info('Not creating new MR while others are pending') return 0 # No pending MRs, run apply with push + # First fetch to get latest remote state + tout.info(f'Fetching {remote}...') + run_git(['fetch', remote]) + tout.info('No pending pickman MRs, creating new one...') args.push = True args.branch = None # Let do_apply generate branch name diff --git a/tools/pickman/ftest.py b/tools/pickman/ftest.py index 769813122fb..9d7786541c3 100644 --- a/tools/pickman/ftest.py +++ b/tools/pickman/ftest.py @@ -1900,13 +1900,14 @@ class TestProcessMrReviewsCommentTracking(unittest.TestCase): resolved=False), ] - with mock.patch.object(gitlab_api, 'get_mr_comments', - return_value=comments): - with mock.patch.object(agent, 'handle_mr_comments', - return_value=(True, 'Done')) 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) + with mock.patch.object(control, 'run_git'): + with mock.patch.object(gitlab_api, 'get_mr_comments', + return_value=comments): + with mock.patch.object(agent, 'handle_mr_comments', + return_value=(True, 'Done')) 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 only receive the new comment call_args = mock_agent.call_args