From patchwork Sat Apr 4 21:29:00 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 2139 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=1775338363; bh=r4MTBAYDliBwfvSWA0oyTY2BHKPZ5EyzD7i/dyuoWoY=; 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=cWiucJm4xKnVVNxGEMd+FBK2+nFOylly5IOegzByk/UQNptBHqILO6HoL8ANsPZwl 41rCZvAgPbkP6W/5FrtoHUFsiTQtsFLCnPBGtwgR431ehh/zO/6QOEIabocbS+FoDW tagi9juft4Zvkxd3F4cV3qWbFoQ+Wu0qX3gDO9Ww4b5X9lgYgSx3uH5YLLMXgPcwto hdQrKjsy8yt/kwQSBWbkr8p9Y1Ykfqw4MtVsCd6qnG2sBN/zXqGy05IhO1ERXq7HAj 81pm74JR3X8RyDBr9w6fn9rxGNX3QCZyN/XrqaBVvnEE/ectD+cotT7UDjcFIV8rLh QtcPqwNpe5y7g== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 80B1A68F53 for ; Sat, 4 Apr 2026 15:32:43 -0600 (MDT) 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 vel_zPQIyc8c for ; Sat, 4 Apr 2026 15:32:43 -0600 (MDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1775338363; bh=r4MTBAYDliBwfvSWA0oyTY2BHKPZ5EyzD7i/dyuoWoY=; 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=cWiucJm4xKnVVNxGEMd+FBK2+nFOylly5IOegzByk/UQNptBHqILO6HoL8ANsPZwl 41rCZvAgPbkP6W/5FrtoHUFsiTQtsFLCnPBGtwgR431ehh/zO/6QOEIabocbS+FoDW tagi9juft4Zvkxd3F4cV3qWbFoQ+Wu0qX3gDO9Ww4b5X9lgYgSx3uH5YLLMXgPcwto hdQrKjsy8yt/kwQSBWbkr8p9Y1Ykfqw4MtVsCd6qnG2sBN/zXqGy05IhO1ERXq7HAj 81pm74JR3X8RyDBr9w6fn9rxGNX3QCZyN/XrqaBVvnEE/ectD+cotT7UDjcFIV8rLh QtcPqwNpe5y7g== Received: from mail.u-boot.org (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 70D266833B for ; Sat, 4 Apr 2026 15:32:43 -0600 (MDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1775338361; bh=XBi64aflibyE9PadyeJS+JEQXlSm0R8dSX8RzGR7YZk=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=vOJPAD9eIp79ny2pS/6nO3kys7dKZ3nEdduxPe8J/uJNOchSdIqXFppBjLPHxl+yN PpwgeURSWBy9cqIKtWsoOrOkNeVul5LVAatD9LxD1rcOQrnm55si+bb0mPI/nB/aQv t7vB1FqC/xnHRBwp07kKghfCyug3+2uaV/AmumersLK5pqqxDJM4b8cypYI3C5sZlo 6PO9d/+yhIWRCo2l9OKQAJsnaVzgYq0/CmOQRsG3DtixFj09q7aR2rMv8oTmXx9AO0 uFnhU9Yl6OHtZaWcdbHRrqgPBfChsausKQvGq3XsdtahVIIFW7znfRvN9IzZn4tZ4y 0asdfok9F//5w== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 228695FC8F; Sat, 4 Apr 2026 15:32:41 -0600 (MDT) 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 qzK1K0x507wt; Sat, 4 Apr 2026 15:32:41 -0600 (MDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1775338357; bh=+HeiXPh3IfLY7+4ZkXlQUt2/V1U3sNp4UP92/n/Ukgs=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=vfr2Qk978ofOCaK7/qo0c6V79CJL3zH8AB6hTUaAZwdeyyBflbgza5akDVzu6au/r fz+aI32wV+e1GABmjIVA+OxWuksNb/tprqJ+CODOu+kqFanlPsRanJ71wNqzGf2Vfk gtrneAQWlskYzA2PJZW0JJWt8thk1XAaSljaewcDLUK6dQkapCgC3n8tym14gV+xk4 xfOQxkPmnP+TOQ6d3VM1YiMoZ4iPMYoK9WKAO2XL+mxkU/Zb+4YpCGtD2HrqfA7zs6 j9x3pjl5yAdXAKE9ef+0h0Y+Y7ozgla1eXR7eOI1K3oGUKcoEBoDFQ3SbvcnDZcKFJ Hky4fNtIxiMvg== Received: from u-boot.org (unknown [73.34.74.121]) by mail.u-boot.org (Postfix) with ESMTPSA id 28F515E7B4; Sat, 4 Apr 2026 15:32:37 -0600 (MDT) From: Simon Glass To: U-Boot Concept Date: Sat, 4 Apr 2026 15:29:00 -0600 Message-ID: <20260404213020.372253-25-sjg@u-boot.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20260404213020.372253-1-sjg@u-boot.org> References: <20260404213020.372253-1-sjg@u-boot.org> MIME-Version: 1.0 Message-ID-Hash: 543MSE3HYZCI6QPAQA4FZDJ2A3JC4MZQ X-Message-ID-Hash: 543MSE3HYZCI6QPAQA4FZDJ2A3JC4MZQ 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 X-Mailman-Version: 3.3.10 Precedence: list Subject: [Concept] [PATCH 24/37] patman: Add 'review' command to download and apply patches 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 a new top-level 'patman review' command (aliases: 'r', 'rev') for AI-assisted review of other people's patches from patchwork. This is the first stage which handles downloading and applying the patches to a local branch. The command accepts a patchwork series link (-l) or a title search (-t) and: - Fetches series metadata from the patchwork API - Auto-creates database records (series with source='review', ser_ver, pcommit entries) to track the review - Detects previously reviewed series by link or by name, so new versions are added under the same series record - Downloads the series mbox from patchwork - Uses a Claude agent to apply the patches via 'git am', handling any conflicts that arise - Creates a branch named 'review' where N is the series ID Later stages will add AI-assisted review of each patch and Gmail draft creation. Signed-off-by: Simon Glass --- tools/patman/cmdline.py | 43 +++++ tools/patman/control.py | 34 ++++ tools/patman/review.py | 372 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 449 insertions(+) create mode 100644 tools/patman/review.py diff --git a/tools/patman/cmdline.py b/tools/patman/cmdline.py index 3c12200949f..edefa778446 100644 --- a/tools/patman/cmdline.py +++ b/tools/patman/cmdline.py @@ -25,6 +25,7 @@ ALIASES = { 'series': ['s', 'ser'], 'status': ['st'], 'patchwork': ['pw'], + 'review': ['r', 'rev'], 'upstream': ['us'], 'workflow': ['wf'], @@ -521,6 +522,46 @@ def add_workflow_subparser(subparsers): return workflow +def add_review_subparser(subparsers): + """Add the 'review' subparser + + Args: + subparsers (argparse action): Subparser parent + + Return: + ArgumentParser: review subparser + """ + review = subparsers.add_parser( + 'review', aliases=ALIASES['review'], + help='AI-powered review of a patchwork series') + review.add_argument( + '-l', '--link', type=str, dest='pw_link', + help='Patchwork series link/ID number') + review.add_argument( + '-t', '--title', type=str, + help='Search for series by cover-letter title') + review.add_argument( + '-n', '--dry-run', action='store_true', dest='dry_run', + default=False, + help='Show what would be done without creating drafts') + review.add_argument( + '--create-drafts', action='store_true', + help='Create Gmail draft emails for each review') + review.add_argument( + '--no-cover', action='store_true', + help='Skip reviewing the cover letter') + review.add_argument( + '--reviewer', type=str, default=None, + help="Override reviewer identity (format: 'Name ')") + review.add_argument( + '-U', '--upstream', type=str, default=None, + help='Upstream name (for patchwork URL lookup)') + review.add_argument( + '--apply-only', action='store_true', + help='Only download and apply patches, skip AI review') + return review + + def setup_parser(): """Set up command-line parser @@ -560,6 +601,7 @@ def setup_parser(): subparsers = parser.add_subparsers(dest='cmd') add_send_subparser(subparsers) patchwork = add_patchwork_subparser(subparsers) + review = add_review_subparser(subparsers) series = add_series_subparser(subparsers) add_status_subparser(subparsers) upstream = add_upstream_subparser(subparsers) @@ -573,6 +615,7 @@ def setup_parser(): parsers = { 'main': parser, + 'review': review, 'series': series, 'patchwork': patchwork, 'upstream': upstream, diff --git a/tools/patman/control.py b/tools/patman/control.py index d82b80ce807..3ce9736d6ba 100644 --- a/tools/patman/control.py +++ b/tools/patman/control.py @@ -19,6 +19,7 @@ except ImportError: from patman import cseries from patman import patchstream +from patman import review as review_mod from patman import send from patman import settings from patman import status @@ -396,6 +397,37 @@ def do_workflow(args, test_db=None): cser.close_database() +def do_review(args, test_db=None, pwork=None, cser=None): + """Process the 'review' command + + Sets up patchwork and cseries, then delegates to + review.do_review(). + + Args: + args (Namespace): Arguments to process + test_db (str or None): Directory containing the test + database, None to use the normal one + pwork (Patchwork): Patchwork object to use, or None to + create one + cser (Cseries): Cseries object to use, or None to create + one + """ + if not cser: + cser = cseries.Cseries(test_db) + try: + cser.open_database() + + ups = args.upstream + if not ups: + ups = cser.db.upstream_get_default() + pwork = _setup_patchwork( + cser, pwork, ups, args.patchwork_url) + + return review_mod.do_review(args, pwork, cser) + finally: + cser.close_database() + + # pylint: disable=R0912 def do_patman(args, test_db=None, pwork=None, cser=None): """Process a patman command @@ -445,6 +477,8 @@ def do_patman(args, test_db=None, pwork=None, cser=None): upstream(args, test_db) elif args.cmd == 'patchwork': patchwork(args, test_db, pwork) + elif args.cmd == 'review': + do_review(args, test_db, pwork, cser) elif args.cmd == 'workflow': do_workflow(args, test_db) except Exception as exc: # pylint: disable=W0718 diff --git a/tools/patman/review.py b/tools/patman/review.py new file mode 100644 index 00000000000..d3eac86f8a1 --- /dev/null +++ b/tools/patman/review.py @@ -0,0 +1,372 @@ +# SPDX-License-Identifier: GPL-2.0+ +# +# Copyright 2025 Canonical Ltd. +# Written by Simon Glass +# + +"""AI-powered patch review using Claude. + +Fetches patches from patchwork, applies them to a local branch using +a Claude agent, and (in later stages) reviews each patch and creates +Gmail draft replies. +""" + +import asyncio +import os +import tempfile + +import aiohttp + +from patman.database import Pcommit +from u_boot_pylib import gitutil +from u_boot_pylib import tout + + +async def fetch_mbox(pwork_url, link): + """Download the series mbox file from patchwork + + Args: + pwork_url (str): Patchwork server URL, e.g. + 'https://patchwork.ozlabs.org' + link (str): Patchwork series link/ID + + Returns: + str: Path to the downloaded mbox file + + Raises: + ValueError: if the download fails + """ + url = f'{pwork_url}/series/{link}/mbox/' + tout.info(f'Downloading mbox from {url}') + + mbox_path = os.path.join(tempfile.gettempdir(), + f'patman_review_{link}.mbox') + async with aiohttp.ClientSession() as client: + async with client.get(url) as response: + if response.status != 200: + raise ValueError( + f'Failed to download mbox from {url}: ' + f'HTTP {response.status}') + content = await response.read() + if not content: + raise ValueError(f'Empty mbox downloaded from {url}') + + with open(mbox_path, 'wb') as fh: + fh.write(content) + tout.info(f'Downloaded {len(content)} bytes to {mbox_path}') + return mbox_path + + +def _build_apply_prompt(mbox_path, branch_name, upstream_branch): + """Build the Claude agent prompt for applying patches + + The agent will create a branch and apply the mbox using git am, + handling any conflicts that arise. + + Args: + mbox_path (str): Path to the downloaded mbox file + branch_name (str): Name for the new branch + upstream_branch (str): Branch to start from (e.g. 'origin/master') + + Returns: + str: The prompt text for the agent + """ + return f"""Apply a patch series from a patchwork mbox file to a new \ +git branch. + +TASK: +1. Create a new branch called '{branch_name}' from '{upstream_branch}': + git checkout -b {branch_name} {upstream_branch} + +2. Apply the patches from the mbox file: + git am {mbox_path} + +3. If git am fails with conflicts: + - Inspect the conflict with 'git diff' and 'git status' + - Try to resolve the conflicts sensibly + - Run 'git add' on resolved files and 'git am --continue' + - If a patch simply cannot be applied (e.g. already applied or + completely incompatible), use 'git am --skip' and note which + patch was skipped + +4. After all patches are applied (or skipped), run: + git log --oneline {upstream_branch}..HEAD + +5. Report the result: + - How many patches were applied successfully + - Which patches (if any) were skipped and why + - The branch name: {branch_name} + +IMPORTANT: +- Do NOT modify the patches before applying +- Do NOT use 'git am --abort' unless all attempts to resolve fail +- If you skip a patch, continue with the remaining patches +- The mbox file is at: {mbox_path} +""" + + +async def apply_series(pwork_url, link, branch_name, upstream_branch, + repo_path): + """Download and apply a patch series to a new local branch + + Uses the Claude agent to handle the git am process, including + conflict resolution. + + Args: + pwork_url (str): Patchwork server URL + link (str): Patchwork series link/ID + branch_name (str): Name for the new branch + upstream_branch (str): Branch to base from + repo_path (str): Path to the git repository + + Returns: + tuple: (success, branch_name) where success is bool and + branch_name is the name of the created branch + """ + # pylint: disable=C0415,E0611 + from u_boot_pylib.claude import (check_available, run_agent_collect, + MAX_BUFFER_SIZE) + + if not check_available(): + return False, None + + from u_boot_pylib.claude import ClaudeAgentOptions + + # Download the mbox + mbox_path = await fetch_mbox(pwork_url, link) + + # Build the prompt and run the agent + prompt = _build_apply_prompt(mbox_path, branch_name, upstream_branch) + options = ClaudeAgentOptions( + allowed_tools=['Bash', 'Read', 'Grep'], + cwd=repo_path, + max_buffer_size=MAX_BUFFER_SIZE, + ) + + tout.info(f'Applying series to branch {branch_name}...') + success, _ = await run_agent_collect(prompt, options) + + # Clean up the mbox file + try: + os.unlink(mbox_path) + except OSError: + pass + + return success, branch_name + + +def apply_series_sync(pwork_url, link, branch_name, upstream_branch, + repo_path): + """Synchronous wrapper for apply_series() + + Args: + pwork_url (str): Patchwork server URL + link (str): Patchwork series link/ID + branch_name (str): Name for the new branch + upstream_branch (str): Branch to base from + repo_path (str): Path to the git repository + + Returns: + tuple: (success, branch_name) + """ + return asyncio.run( + apply_series(pwork_url, link, branch_name, upstream_branch, + repo_path)) + + +def search_series(pwork, title): + """Search patchwork for a series by cover-letter title + + Queries the patchwork API and returns the link for the best match. + If multiple matches are found, shows them and picks the most recent. + + Args: + pwork (Patchwork): Configured patchwork instance + title (str): Title text to search for + + Returns: + str: Patchwork series link/ID + + Raises: + ValueError: if no matching series is found + """ + async def _query(): + if not pwork.proj_id: + raise ValueError( + 'Patchwork project not configured; use ' + "'patman patchwork set-project' or provide -l ") + async with aiohttp.ClientSession() as client: + # pylint: disable=W0212 + return await pwork._query_series(client, title) + + loop = asyncio.get_event_loop() + results = loop.run_until_complete(_query()) + + if not results: + raise ValueError(f"No series found matching '{title}'") + + if len(results) == 1: + match = results[0] + tout.notice(f"Found: {match['name']} (v{match['version']}, " + f"link {match['id']})") + return str(match['id']) + + tout.notice(f"Found {len(results)} matching series:") + for i, match in enumerate(results): + tout.notice(f" {i + 1}. [{match['id']}] {match['name']} " + f"(v{match['version']}, {match['date']})") + + best = max(results, + key=lambda r: (r.get('version', 0), r.get('date', ''))) + tout.notice(f"Using most recent: {best['name']} (link {best['id']})") + return str(best['id']) + + +def _clean_series_name(name): + """Strip the [U-Boot,v2,0/4] prefix from a series name + + Args: + name (str): Raw series name from patchwork + + Returns: + str: Cleaned name + """ + if name.startswith('['): + bracket_end = name.find(']') + if bracket_end != -1: + return name[bracket_end + 1:].strip() + return name + + +# pylint: disable=R0914 +def _register_series(cser, clean_name, version, link, series_data): + """Register a series in the database for review tracking + + Creates or finds the series record, adds a ser_ver entry and + pcommit records for each patch. + + Args: + cser (Cseries): Open cseries instance + clean_name (str): Cleaned series name + version (int): Series version number + link (str): Patchwork series link/ID + series_data (dict): Series data from patchwork + + Returns: + tuple: (series_id, svid) or None if already reviewed + """ + existing = cser.db.series_find_by_link(link) + if existing: + series_id, db_name, db_version, svid = existing + tout.notice( + f"Already reviewed: '{db_name}' v{db_version}") + return None + + prev = cser.db.series_find_review_by_name(clean_name) + if prev: + series_id, db_name, prev_version = prev + tout.notice(f"Previously reviewed '{db_name}' " + f"v{prev_version}; adding v{version}") + else: + desc = series_data.get('cover_letter', {}) + desc = desc.get('name', '') if desc else '' + series_id = cser.db.series_add(clean_name, desc) + cser.db.series_set_source(series_id, 'review') + + svid = cser.db.ser_ver_add(series_id, version, + link=str(link)) + + patches = series_data.get('patches', []) + pcommits = [] + for i, patch in enumerate(patches): + pcommits.append(Pcommit( + idnum=None, seq=i, + subject=patch.get('name', ''), + svid=svid, change_id=None, state=None, + patch_id=patch.get('id'), num_comments=0)) + if pcommits: + cser.db.pcommit_add_list(svid, pcommits) + + cser.commit() + tout.notice( + f"Added series '{clean_name}' v{version} to database") + return series_id, svid + + +# pylint: disable=R0914 +def do_review(args, pwork, cser): + """Run the review command + + Downloads patches from patchwork, applies them to a local + branch using a Claude agent, and (in later stages) reviews + each patch. + + Args: + args (Namespace): Command-line arguments + pwork (Patchwork): Configured patchwork instance + cser (Cseries): Open cseries instance + """ + if not args.pw_link and not args.title: + raise ValueError( + "Please provide -l or -t to " + "identify the series") + + link = args.pw_link + if not link: + link = search_series(pwork, args.title) + + # Fetch series metadata from patchwork + async def _fetch(): + async with aiohttp.ClientSession() as client: + return await pwork.get_series(client, link) + + loop = asyncio.get_event_loop() + series_data = loop.run_until_complete(_fetch()) + + series_name = series_data.get('name', f'series-{link}') + version = series_data.get('version', 1) + patch_count = series_data.get('received_total', 0) + clean_name = _clean_series_name(series_name) + + tout.notice(f"Series: {clean_name}") + tout.notice( + f"Version: {version}, Patches: {patch_count}") + + result = _register_series( + cser, clean_name, version, link, series_data) + if result is None: + return 0 + series_id, _ = result + + # Determine the upstream branch for applying patches + ups = args.upstream + if not ups: + ups = cser.db.upstream_get_default() + if ups: + upstream_branch = f'{ups}/master' + else: + upstream_branch = 'origin/master' + + # Download and apply the patches + branch_name = f'review{series_id}' + repo_path = gitutil.get_top_level() + success, branch_name = apply_series_sync( + pwork.url, link, branch_name, upstream_branch, + repo_path) + + if not success: + tout.error('Failed to apply patches to branch') + return 1 + tout.notice(f'Patches applied to branch: {branch_name}') + + if args.apply_only: + tout.notice('Apply-only mode; skipping review') + return 0 + + # TODO: Stage 2 - AI review of each patch + # TODO: Stage 3 - Gmail draft creation + tout.notice( + 'Patch application complete. ' + 'AI review not yet implemented.') + + return 0