[Concept,15/16] pickman: Process subtree updates even at max MRs

Message ID 20260222154303.2851319-16-sjg@u-boot.org
State New
Headers
Series pickman: Support monitoring and fixing pipeline failures |

Commit Message

Simon Glass Feb. 22, 2026, 3:42 p.m. UTC
  From: Simon Glass <simon.glass@canonical.com>

When do_step() finds the number of open MRs is at the maximum, it
returns immediately without processing subtree updates or advancing
past fully-processed merges. These actions don't create MRs (subtree
updates push to the target branch directly), so they should happen
regardless. Currently the user must wait for the next poll interval
and for an MR slot to open before subtree updates are applied.

Move the _prepare_get_commits() call to before the max_mrs check in
do_step(), so subtree updates and source-position advances happen
unconditionally. Add an optional 'info' parameter to do_apply() and
prepare_apply() so the already-fetched result can be passed through,
avoiding a redundant call when below the max MR limit.

Co-developed-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>
---

 tools/pickman/control.py | 27 +++++++++++----
 tools/pickman/ftest.py   | 72 ++++++++++++++++++++++++++--------------
 2 files changed, 67 insertions(+), 32 deletions(-)
  

Patch

diff --git a/tools/pickman/control.py b/tools/pickman/control.py
index f4d4a43c292..a42927f991d 100644
--- a/tools/pickman/control.py
+++ b/tools/pickman/control.py
@@ -1663,7 +1663,7 @@  def _prepare_get_commits(dbs, source, args):
         return info, None
 
 
-def prepare_apply(dbs, source, branch, args=None):
+def prepare_apply(dbs, source, branch, args=None, info=None):
     """Prepare for applying commits from a source branch
 
     Get the next commits, set up the branch name and prints info about
@@ -1676,15 +1676,18 @@  def prepare_apply(dbs, source, branch, args=None):
         branch (str): Branch name to use, or None to auto-generate
         args (Namespace): Parsed arguments with 'push', 'remote', 'target'
             (needed for subtree updates)
+        info (NextCommitsInfo): Pre-fetched commit info from
+            _prepare_get_commits(), or None to fetch it here
 
     Returns:
         tuple: (ApplyInfo, return_code) where ApplyInfo is set if there are
             commits to apply, or None with return_code indicating the result
             (0 for no commits, 1 for error)
     """
-    info, ret = _prepare_get_commits(dbs, source, args)
-    if ret is not None:
-        return None, ret
+    if info is None:
+        info, ret = _prepare_get_commits(dbs, source, args)
+        if ret is not None:
+            return None, ret
 
     commits = info.commits
 
@@ -1889,18 +1892,20 @@  def execute_apply(dbs, source, commits, branch_name, args, advance_to=None):  #
     return ret, success, conv_log
 
 
-def do_apply(args, dbs):
+def do_apply(args, dbs, info=None):
     """Apply the next set of commits using Claude agent
 
     Args:
         args (Namespace): Parsed arguments with 'source' and 'branch' attributes
         dbs (Database): Database instance
+        info (NextCommitsInfo): Pre-fetched commit info from
+            _prepare_get_commits(), or None to fetch during prepare
 
     Returns:
         int: 0 on success, 1 on failure
     """
     source = args.source
-    info, ret = prepare_apply(dbs, source, args.branch, args)
+    info, ret = prepare_apply(dbs, source, args.branch, args, info=info)
     if not info:
         return ret
 
@@ -2851,6 +2856,14 @@  def do_step(args, dbs):
             process_pipeline_failures(remote, active_mrs, dbs,
                                       args.target, args.fix_retries)
 
+    # Process subtree updates and advance past fully-processed merges
+    # regardless of MR count, since these don't create MRs
+    info, ret = _prepare_get_commits(dbs, source, args)
+    if ret is not None:
+        if ret:
+            return ret
+        return 0
+
     # Only block new MR creation if we've reached the max allowed open MRs
     max_mrs = args.max_mrs
     if len(active_mrs) >= max_mrs:
@@ -2869,7 +2882,7 @@  def do_step(args, dbs):
         tout.info('No pending pickman MRs, creating new one...')
     args.push = True
     args.branch = None  # Let do_apply generate branch name
-    return do_apply(args, dbs)
+    return do_apply(args, dbs, info=info)
 
 
 def do_poll(args, dbs):
diff --git a/tools/pickman/ftest.py b/tools/pickman/ftest.py
index af26cbb4229..ced2c79ac87 100644
--- a/tools/pickman/ftest.py
+++ b/tools/pickman/ftest.py
@@ -2061,16 +2061,21 @@  class TestStep(unittest.TestCase):
             source_branch='cherry-test',
             description='Test',
         )
+        mock_info = control.NextCommitsInfo(
+            commits=['fake'], merge_found=True, advance_to='abc123')
         with mock.patch.object(control, 'run_git'):
             with mock.patch.object(gitlab, 'get_merged_pickman_mrs',
                                    return_value=[]):
                 with mock.patch.object(gitlab, 'get_open_pickman_mrs',
                                        return_value=[mock_mr]):
-                    args = argparse.Namespace(cmd='step', source='us/next',
-                                              remote='ci', target='master',
-                                              max_mrs=1, fix_retries=3)
-                    with terminal.capture():
-                        ret = control.do_step(args, None)
+                    with mock.patch.object(
+                            control, '_prepare_get_commits',
+                            return_value=(mock_info, None)):
+                        args = argparse.Namespace(
+                            cmd='step', source='us/next', remote='ci',
+                            target='master', max_mrs=1, fix_retries=3)
+                        with terminal.capture():
+                            ret = control.do_step(args, None)
 
         self.assertEqual(ret, 0)
 
@@ -2109,18 +2114,23 @@  class TestStep(unittest.TestCase):
             source_branch='cherry-test',
             description='Test',
         )
+        mock_info = control.NextCommitsInfo(
+            commits=['fake'], merge_found=True, advance_to='abc123')
         with mock.patch.object(control, 'run_git'):
             with mock.patch.object(gitlab, 'get_merged_pickman_mrs',
                                    return_value=[]):
                 with mock.patch.object(gitlab, 'get_open_pickman_mrs',
                                        return_value=[mock_mr]):
-                    with mock.patch.object(control, 'do_apply',
-                                           return_value=0) as mock_apply:
-                        args = argparse.Namespace(cmd='step', source='us/next',
-                                                  remote='ci', target='master',
-                                                  max_mrs=5, fix_retries=3)
-                        with terminal.capture():
-                            ret = control.do_step(args, None)
+                    with mock.patch.object(
+                            control, '_prepare_get_commits',
+                            return_value=(mock_info, None)):
+                        with mock.patch.object(control, 'do_apply',
+                                               return_value=0) as mock_apply:
+                            args = argparse.Namespace(
+                                cmd='step', source='us/next', remote='ci',
+                                target='master', max_mrs=5, fix_retries=3)
+                            with terminal.capture():
+                                ret = control.do_step(args, None)
 
         # With 1 open MR and max_mrs=5, it should try to create a new one
         self.assertEqual(ret, 0)
@@ -2138,17 +2148,23 @@  class TestStep(unittest.TestCase):
             )
             for i in range(3)
         ]
+        mock_info = control.NextCommitsInfo(
+            commits=['fake'], merge_found=True, advance_to='abc123')
         with mock.patch.object(control, 'run_git'):
             with mock.patch.object(gitlab, 'get_merged_pickman_mrs',
                                    return_value=[]):
                 with mock.patch.object(gitlab, 'get_open_pickman_mrs',
                                        return_value=mock_mrs):
-                    with mock.patch.object(control, 'do_apply') as mock_apply:
-                        args = argparse.Namespace(cmd='step', source='us/next',
-                                                  remote='ci', target='master',
-                                                  max_mrs=3, fix_retries=3)
-                        with terminal.capture():
-                            ret = control.do_step(args, None)
+                    with mock.patch.object(
+                            control, '_prepare_get_commits',
+                            return_value=(mock_info, None)):
+                        with mock.patch.object(control, 'do_apply') \
+                                as mock_apply:
+                            args = argparse.Namespace(
+                                cmd='step', source='us/next', remote='ci',
+                                target='master', max_mrs=3, fix_retries=3)
+                            with terminal.capture():
+                                ret = control.do_step(args, None)
 
         # With 3 open MRs and max_mrs=3, should not create new MR
         self.assertEqual(ret, 0)
@@ -6120,19 +6136,25 @@  class TestProcessPipelineFailures(unittest.TestCase):
             pipeline_status='failed',
             pipeline_id=100,
         )
+        mock_info = control.NextCommitsInfo(
+            commits=['fake'], merge_found=True, advance_to='abc123')
         with mock.patch.object(control, 'run_git'):
             with mock.patch.object(gitlab, 'get_merged_pickman_mrs',
                                    return_value=[]):
                 with mock.patch.object(gitlab, 'get_open_pickman_mrs',
                                        return_value=[mock_mr]):
                     with mock.patch.object(
-                            control, 'process_pipeline_failures') as mock_ppf:
-                        args = argparse.Namespace(
-                            cmd='step', source='us/next',
-                            remote='ci', target='master',
-                            max_mrs=1, fix_retries=0)
-                        with terminal.capture():
-                            control.do_step(args, None)
+                            control, '_prepare_get_commits',
+                            return_value=(mock_info, None)):
+                        with mock.patch.object(
+                                control,
+                                'process_pipeline_failures') as mock_ppf:
+                            args = argparse.Namespace(
+                                cmd='step', source='us/next',
+                                remote='ci', target='master',
+                                max_mrs=1, fix_retries=0)
+                            with terminal.capture():
+                                control.do_step(args, None)
 
         mock_ppf.assert_not_called()