From: Simon Glass <sjg@chromium.org> When pickman processes a subtree update via update-subtree.sh, merge conflicts are common for large DTS updates. Previously, any failure caused an immediate merge abort and error return. Detect whether a failed subtree pull left a merge in progress by checking for MERGE_HEAD. When conflicts exist, invoke the Claude agent to resolve them (preferring the upstream version), following the same pattern used for cherry-pick conflict resolution. If the agent cannot resolve the conflicts, abort the merge and return failure as before. Add SUBTREE_OK/FAIL/CONFLICT return codes to _subtree_run_update() so the caller can distinguish between merge conflicts and other failures. Add build_subtree_conflict_prompt(), run_subtree_conflict_agent(), and resolve_subtree_conflicts() to agent.py for the actual resolution. Signed-off-by: Simon Glass <sjg@chromium.org> --- tools/pickman/agent.py | 93 ++++++++++++ tools/pickman/control.py | 100 +++++++++---- tools/pickman/ftest.py | 298 ++++++++++++++++++++++++++++++++++++--- 3 files changed, 444 insertions(+), 47 deletions(-) diff --git a/tools/pickman/agent.py b/tools/pickman/agent.py index ec5c1b0df75..023b65b5d36 100644 --- a/tools/pickman/agent.py +++ b/tools/pickman/agent.py @@ -713,3 +713,96 @@ def fix_pipeline(mr_iid, branch_name, failed_jobs, remote, target='master', return asyncio.run(run_pipeline_fix_agent( mr_iid, branch_name, failed_jobs, remote, target, mr_description, attempt, repo_path)) + + +def build_subtree_conflict_prompt(name, tag, subtree_path): + """Build a prompt for resolving subtree merge conflicts + + Args: + name (str): Subtree name ('dts', 'mbedtls', 'lwip') + tag (str): Tag being pulled (e.g. 'v6.15-dts') + subtree_path (str): Path to the subtree (e.g. 'dts/upstream') + + Returns: + str: The prompt for the agent + """ + return f"""Resolve merge conflicts from a subtree pull. + +Context: A 'git subtree pull --prefix {subtree_path}' for the '{name}' \ +subtree (tag {tag}) has produced merge conflicts. Git is currently in a \ +merge state with MERGE_HEAD set. + +Resolution strategy - prefer the upstream version: +1. Run 'git status' to see conflicted files +2. For each conflicted file: + - If the file exists in MERGE_HEAD (content conflict or add/add): + git checkout MERGE_HEAD -- <file> + - If the file is a modify/delete conflict (deleted upstream): + git rm <file> +3. After resolving all conflicts, stage everything: + git add -A +4. Complete the merge: + git commit --no-edit +5. Verify with 'git status' that the working tree is clean + +If you cannot resolve the conflicts, abort with: + git merge --abort + +Important: +- Always prefer the upstream (MERGE_HEAD) version of conflicted files +- The subtree path is '{subtree_path}' - most conflicts will be there +- Do NOT modify files outside the subtree path +- Do NOT use 'git merge --continue'; use 'git commit --no-edit' +""" + + +async def run_subtree_conflict_agent(name, tag, subtree_path, + repo_path=None): + """Run the Claude agent to resolve subtree merge conflicts + + Args: + name (str): Subtree name ('dts', 'mbedtls', 'lwip') + tag (str): Tag being pulled (e.g. 'v6.15-dts') + subtree_path (str): Path to the subtree (e.g. 'dts/upstream') + 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 + """ + if not check_available(): + return False, '' + + if repo_path is None: + repo_path = os.getcwd() + + prompt = build_subtree_conflict_prompt(name, tag, subtree_path) + + options = ClaudeAgentOptions( + allowed_tools=['Bash', 'Read', 'Grep'], + cwd=repo_path, + max_buffer_size=MAX_BUFFER_SIZE, + ) + + tout.info(f'Starting Claude agent to resolve {name} subtree ' + f'conflicts...') + tout.info('') + + return await run_agent_collect(prompt, options) + + +def resolve_subtree_conflicts(name, tag, subtree_path, repo_path=None): + """Synchronous wrapper for running the subtree conflict agent + + Args: + name (str): Subtree name ('dts', 'mbedtls', 'lwip') + tag (str): Tag being pulled (e.g. 'v6.15-dts') + subtree_path (str): Path to the subtree (e.g. 'dts/upstream') + 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_subtree_conflict_agent(name, tag, subtree_path, repo_path)) diff --git a/tools/pickman/control.py b/tools/pickman/control.py index 815ac574093..ed6825c2333 100644 --- a/tools/pickman/control.py +++ b/tools/pickman/control.py @@ -58,6 +58,11 @@ SUBTREE_NAMES = { 'lib/lwip/lwip': 'lwip', } +# Return codes for _subtree_run_update() +SUBTREE_OK = 0 +SUBTREE_FAIL = 1 +SUBTREE_CONFLICT = 2 + # Named tuple for commit info Commit = namedtuple('Commit', ['hash', 'chash', 'subject', 'date']) @@ -1514,14 +1519,30 @@ def push_mr(args, branch_name, title, description): return bool(mr_url) +def _is_merge_in_progress(): + """Check if a git merge is currently in progress. + + Returns: + bool: True if MERGE_HEAD exists (merge in progress), False otherwise + """ + try: + run_git(['rev-parse', '--verify', 'MERGE_HEAD']) + return True + except Exception: # pylint: disable=broad-except + return False + + def _subtree_run_update(name, tag): """Run update-subtree.sh to pull a subtree update. - On failure, aborts any in-progress merge to clean up the working - tree. + On failure, checks whether a merge is in progress (indicating + conflicts). If so, returns SUBTREE_CONFLICT with the merge state + intact so the caller can invoke the agent. Otherwise aborts any + in-progress merge and returns SUBTREE_FAIL. Returns: - int: 0 on success, 1 on failure + int: SUBTREE_OK on success, SUBTREE_CONFLICT on merge conflicts, + SUBTREE_FAIL on other failures """ try: result = command.run_one( @@ -1534,16 +1555,18 @@ def _subtree_run_update(name, tag): f'{result.return_code})') if result.stderr: tout.error(result.stderr) + if _is_merge_in_progress(): + return SUBTREE_CONFLICT try: run_git(['merge', '--abort']) except Exception: # pylint: disable=broad-except pass - return 1 + return SUBTREE_FAIL except Exception as exc: # pylint: disable=broad-except tout.error(f'Subtree update failed: {exc}') - return 1 + return SUBTREE_FAIL - return 0 + return SUBTREE_OK def _subtree_record(dbs, source, squash_hash, merge_hash): @@ -1565,7 +1588,8 @@ def _subtree_record(dbs, source, squash_hash, merge_hash): f'{merge_hash[:12]}') -def apply_subtree_update(dbs, source, name, tag, merge_hash, args): # pylint: disable=too-many-arguments +def apply_subtree_update(dbs, source, name, tag, merge_hash, remote, # pylint: disable=too-many-arguments + target, push=True): """Apply a subtree update on the target branch Runs tools/update-subtree.sh to pull the subtree update, then @@ -1577,13 +1601,13 @@ def apply_subtree_update(dbs, source, name, tag, merge_hash, args): # pylint: d name (str): Subtree name ('dts', 'mbedtls', 'lwip') tag (str): Tag to pull (e.g. 'v6.15-dts') merge_hash (str): Hash of the subtree merge commit to advance past - args (Namespace): Parsed arguments with 'push', 'remote', 'target' + remote (str): Git remote name (e.g. 'ci') + target (str): Target branch name (e.g. 'master') + push (bool): Whether to push the result to the remote Returns: int: 0 on success, 1 on failure """ - target = args.target - tout.info(f'Applying subtree update: {name} -> {tag}') # Get the squash commit (second parent of the merge) @@ -1599,20 +1623,34 @@ def apply_subtree_update(dbs, source, name, tag, merge_hash, args): # pylint: d # Bare name may be ambiguous when multiple remotes have it try: run_git(['checkout', '-b', target, - f'{args.remote}/{target}']) + f'{remote}/{target}']) except command.CommandExc: tout.error(f'Could not checkout {target}') return 1 ret = _subtree_run_update(name, tag) - if ret: - return ret + if ret == SUBTREE_FAIL: + return 1 + if ret == SUBTREE_CONFLICT: + # Resolve via reverse lookup of subtree path + subtree_path = next( + (p for p, n in SUBTREE_NAMES.items() if n == name), None) + tout.info('Merge conflicts detected, invoking agent...') + success, _ = agent.resolve_subtree_conflicts( + name, tag, subtree_path) + if not success: + tout.error('Agent could not resolve subtree conflicts') + try: + run_git(['merge', '--abort']) + except command.CommandExc: + pass + return 1 - if args.push: + if push: try: - gitlab_api.push_branch(args.remote, target, skip_ci=True) - tout.info(f'Pushed {target} to {args.remote}') - except Exception as exc: # pylint: disable=broad-except + gitlab_api.push_branch(remote, target, skip_ci=True) + tout.info(f'Pushed {target} to {remote}') + except command.CommandExc as exc: tout.error(f'Failed to push {target}: {exc}') return 1 @@ -1621,7 +1659,7 @@ def apply_subtree_update(dbs, source, name, tag, merge_hash, args): # pylint: d return 0 -def _prepare_get_commits(dbs, source, args): +def _prepare_get_commits(dbs, source, remote, target): """Get the next commits to apply, handling subtrees and skips. Fetch the next batch of commits from the source. If a subtree @@ -1631,7 +1669,9 @@ def _prepare_get_commits(dbs, source, args): Args: dbs (Database): Database instance source (str): Source branch name - args (Namespace): Parsed arguments (needed for subtree updates) + remote (str): Git remote name (e.g. 'ci'), or None to skip + subtree updates + target (str): Target branch name (e.g. 'master') Returns: tuple: (NextCommitsInfo, return_code) where return_code is None @@ -1646,11 +1686,12 @@ def _prepare_get_commits(dbs, source, args): if info.subtree_update: name, tag = info.subtree_update tout.info(f'Subtree update needed: {name} -> {tag}') - if not args: - tout.error('Cannot apply subtree update without args') + if not remote: + tout.error('Cannot apply subtree update without remote') return None, 1 ret = apply_subtree_update(dbs, source, name, tag, - info.advance_to, args) + info.advance_to, remote, + target) if ret: return None, ret continue @@ -1668,7 +1709,8 @@ def _prepare_get_commits(dbs, source, args): return info, None -def prepare_apply(dbs, source, branch, args=None, info=None): +def prepare_apply(dbs, source, branch, remote=None, target=None, # pylint: disable=too-many-arguments + info=None): """Prepare for applying commits from a source branch Get the next commits, set up the branch name and prints info about @@ -1679,8 +1721,9 @@ def prepare_apply(dbs, source, branch, args=None, info=None): dbs (Database): Database instance source (str): Source branch name branch (str): Branch name to use, or None to auto-generate - args (Namespace): Parsed arguments with 'push', 'remote', 'target' - (needed for subtree updates) + remote (str): Git remote name (e.g. 'ci'), or None to skip + subtree updates + target (str): Target branch name (e.g. 'master') info (NextCommitsInfo): Pre-fetched commit info from _prepare_get_commits(), or None to fetch it here @@ -1690,7 +1733,7 @@ def prepare_apply(dbs, source, branch, args=None, info=None): (0 for no commits, 1 for error) """ if info is None: - info, ret = _prepare_get_commits(dbs, source, args) + info, ret = _prepare_get_commits(dbs, source, remote, target) if ret is not None: return None, ret @@ -1910,7 +1953,8 @@ def do_apply(args, dbs, info=None): int: 0 on success, 1 on failure """ source = args.source - info, ret = prepare_apply(dbs, source, args.branch, args, info=info) + info, ret = prepare_apply(dbs, source, args.branch, args.remote, + args.target, info=info) if not info: return ret @@ -2863,7 +2907,7 @@ def do_step(args, dbs): # 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) + info, ret = _prepare_get_commits(dbs, source, remote, args.target) if ret is not None: if ret: return ret diff --git a/tools/pickman/ftest.py b/tools/pickman/ftest.py index 766cc714ed7..590677d6b5b 100644 --- a/tools/pickman/ftest.py +++ b/tools/pickman/ftest.py @@ -1442,7 +1442,8 @@ class TestApply(unittest.TestCase): database.Database.instances.clear() - args = argparse.Namespace(cmd='apply', source='unknown', branch=None) + args = argparse.Namespace(cmd='apply', source='unknown', branch=None, + remote='ci', target='master') with terminal.capture() as (_, stderr): ret = control.do_pickman(args) self.assertEqual(ret, 1) @@ -1460,7 +1461,8 @@ class TestApply(unittest.TestCase): database.Database.instances.clear() command.TEST_RESULT = command.CommandResult(stdout='') - args = argparse.Namespace(cmd='apply', source='us/next', branch=None) + args = argparse.Namespace(cmd='apply', source='us/next', branch=None, + remote='ci', target='master') with terminal.capture() as (stdout, _): ret = control.do_pickman(args) self.assertEqual(ret, 0) @@ -4026,7 +4028,8 @@ class TestApplySubtreeUpdate(unittest.TestCase): return_value=mock_result): ret = control.apply_subtree_update( dbs, 'us/next', 'dts', 'v6.15-dts', - 'merge_hash', args) + 'merge_hash', 'ci', 'master', + push=args.push) self.assertEqual(ret, 0) @@ -4077,7 +4080,48 @@ class TestApplySubtreeUpdate(unittest.TestCase): side_effect=mock_push): ret = control.apply_subtree_update( dbs, 'us/next', 'dts', 'v6.15-dts', - 'merge_hash', args) + 'merge_hash', 'ci', 'master', + push=args.push) + + self.assertEqual(ret, 0) + self.assertTrue(pushed[0]) + dbs.close() + + def test_apply_push_defaults_to_true(self): + """Test apply_subtree_update pushes when push is not specified.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + dbs.source_set('us/next', 'base') + dbs.commit() + + def run_git_handler(git_args): + if 'rev-parse' in git_args: + return 'first_parent\nsquash_hash' + if 'checkout' in git_args: + return '' + if '--format=%s|%an' in git_args: + return 'Commit subject|Author' + return '' + + pushed = [False] + + def mock_push(remote, branch, skip_ci=False): + pushed[0] = True + return True + + mock_result = command.CommandResult('ok', '', '', 0) + with mock.patch.object(control, 'run_git', + side_effect=run_git_handler): + with mock.patch.object( + control.command, 'run_one', + return_value=mock_result): + with mock.patch.object( + control.gitlab_api, 'push_branch', + side_effect=mock_push): + ret = control.apply_subtree_update( + dbs, 'us/next', 'dts', 'v6.15-dts', + 'merge_hash', 'ci', 'master') self.assertEqual(ret, 0) self.assertTrue(pushed[0]) @@ -4105,7 +4149,8 @@ class TestApplySubtreeUpdate(unittest.TestCase): side_effect=run_git_handler): ret = control.apply_subtree_update( dbs, 'us/next', 'dts', 'v6.15-dts', - 'merge_hash', args) + 'merge_hash', 'ci', 'master', + push=args.push) self.assertEqual(ret, 1) # Source should not be advanced @@ -4144,7 +4189,8 @@ class TestApplySubtreeUpdate(unittest.TestCase): return_value=0): ret = control.apply_subtree_update( dbs, 'us/next', 'dts', 'v6.15-dts', - 'merge_hash', args) + 'merge_hash', 'ci', 'master', + push=args.push) self.assertEqual(ret, 0) # Should have tried bare checkout, then fallback @@ -4170,7 +4216,8 @@ class TestApplySubtreeUpdate(unittest.TestCase): return_value='single_parent'): ret = control.apply_subtree_update( dbs, 'us/next', 'dts', 'v6.15-dts', - 'merge_hash', args) + 'merge_hash', 'ci', 'master', + push=args.push) self.assertEqual(ret, 1) dbs.close() @@ -4200,7 +4247,8 @@ class TestApplySubtreeUpdate(unittest.TestCase): side_effect=Exception('script failed')): ret = control.apply_subtree_update( dbs, 'us/next', 'dts', 'v6.15-dts', - 'merge_hash', args) + 'merge_hash', 'ci', 'master', + push=args.push) self.assertEqual(ret, 1) # Source should not be advanced @@ -4208,7 +4256,7 @@ class TestApplySubtreeUpdate(unittest.TestCase): dbs.close() def test_apply_merge_conflict(self): - """Test apply_subtree_update aborts merge on non-zero exit.""" + """Test apply_subtree_update aborts merge on non-conflict failure.""" with terminal.capture(): dbs = database.Database(self.db_path) dbs.start() @@ -4222,6 +4270,8 @@ class TestApplySubtreeUpdate(unittest.TestCase): def run_git_handler(git_args): if 'rev-parse' in git_args: + if '--verify' in git_args: + raise Exception('no MERGE_HEAD') return 'first_parent\nsquash_hash' if 'checkout' in git_args: return '' @@ -4239,7 +4289,96 @@ class TestApplySubtreeUpdate(unittest.TestCase): return_value=mock_result): ret = control.apply_subtree_update( dbs, 'us/next', 'dts', 'v6.15-dts', - 'merge_hash', args) + 'merge_hash', 'ci', 'master', + push=args.push) + + self.assertEqual(ret, 1) + self.assertTrue(merge_aborted[0]) + # Source should not be advanced + self.assertEqual(dbs.source_get('us/next'), 'base') + dbs.close() + + def test_apply_merge_conflict_agent_resolves(self): + """Test apply_subtree_update invokes agent on conflict and succeeds.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + dbs.source_set('us/next', 'base') + dbs.commit() + + args = argparse.Namespace(push=False, remote='ci', + target='master') + + def run_git_handler(git_args): + if 'rev-parse' in git_args: + if '--verify' in git_args: + return 'abc123' + return 'first_parent\nsquash_hash' + if 'checkout' in git_args: + return '' + if '--format=%s|%an' in git_args: + return 'subject|author' + return '' + + mock_result = command.CommandResult( + '', 'CONFLICT (content): Merge conflict', '', 1) + with mock.patch.object(control, 'run_git', + side_effect=run_git_handler): + with mock.patch.object( + control.command, 'run_one', + return_value=mock_result): + with mock.patch.object( + agent, 'resolve_subtree_conflicts', + return_value=(True, 'resolved ok')): + ret = control.apply_subtree_update( + dbs, 'us/next', 'dts', 'v6.15-dts', + 'merge_hash', 'ci', 'master', + push=args.push) + + self.assertEqual(ret, 0) + # Source should be advanced + self.assertEqual(dbs.source_get('us/next'), 'merge_hash') + dbs.close() + + def test_apply_merge_conflict_agent_fails(self): + """Test apply_subtree_update aborts when agent fails to resolve.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + dbs.source_set('us/next', 'base') + dbs.commit() + + args = argparse.Namespace(push=False, remote='ci', + target='master') + + merge_aborted = [False] + + def run_git_handler(git_args): + if 'rev-parse' in git_args: + if '--verify' in git_args: + return 'abc123' + return 'first_parent\nsquash_hash' + if 'checkout' in git_args: + return '' + if 'merge' in git_args and '--abort' in git_args: + merge_aborted[0] = True + return '' + return '' + + mock_result = command.CommandResult( + '', 'CONFLICT (content): Merge conflict', '', 1) + with mock.patch.object(control, 'run_git', + side_effect=run_git_handler): + with mock.patch.object( + control.command, 'run_one', + return_value=mock_result): + with mock.patch.object( + agent, 'resolve_subtree_conflicts', + return_value=(False, 'failed')): + ret = control.apply_subtree_update( + dbs, 'us/next', 'dts', 'v6.15-dts', + 'merge_hash', 'ci', 'master', + push=args.push) self.assertEqual(ret, 1) self.assertTrue(merge_aborted[0]) @@ -4276,8 +4415,6 @@ class TestPrepareApplySubtreeUpdate(unittest.TestCase): dbs.source_set('us/next', 'base') dbs.commit() - args = argparse.Namespace(push=False, remote='ci', - target='master') subtree_info = control.NextCommitsInfo( [], True, 'merge1', ('dts', 'v6.15-dts')) normal_info = control.NextCommitsInfo([], False, None) @@ -4296,11 +4433,12 @@ class TestPrepareApplySubtreeUpdate(unittest.TestCase): control, 'apply_subtree_update', return_value=0) as mock_apply: info, ret = control.prepare_apply( - dbs, 'us/next', None, args) + dbs, 'us/next', None, 'ci', 'master') # Should have called apply_subtree_update mock_apply.assert_called_once_with( - dbs, 'us/next', 'dts', 'v6.15-dts', 'merge1', args) + dbs, 'us/next', 'dts', 'v6.15-dts', 'merge1', + 'ci', 'master') # No commits after retry, so returns None/0 self.assertIsNone(info) self.assertEqual(ret, 0) @@ -4314,8 +4452,6 @@ class TestPrepareApplySubtreeUpdate(unittest.TestCase): dbs.source_set('us/next', 'base') dbs.commit() - args = argparse.Namespace(push=False, remote='ci', - target='master') subtree_info = control.NextCommitsInfo( [], True, 'merge1', ('dts', 'v6.15-dts')) @@ -4325,14 +4461,14 @@ class TestPrepareApplySubtreeUpdate(unittest.TestCase): control, 'apply_subtree_update', return_value=1): info, ret = control.prepare_apply( - dbs, 'us/next', None, args) + dbs, 'us/next', None, 'ci', 'master') self.assertIsNone(info) self.assertEqual(ret, 1) dbs.close() - def test_prepare_apply_subtree_without_args(self): - """Test prepare_apply returns error when subtree needs args=None.""" + def test_prepare_apply_subtree_without_remote(self): + """Test prepare_apply returns error when subtree needs remote=None.""" with terminal.capture(): dbs = database.Database(self.db_path) dbs.start() @@ -6265,5 +6401,129 @@ class TestStepFixRetries(unittest.TestCase): self.assertEqual(args.fix_retries, 1) +class TestIsMergeInProgress(unittest.TestCase): + """Tests for _is_merge_in_progress helper.""" + + def test_merge_in_progress(self): + """Test returns True when MERGE_HEAD exists.""" + with mock.patch.object(control, 'run_git', return_value='abc123'): + self.assertTrue(control._is_merge_in_progress()) + + def test_no_merge_in_progress(self): + """Test returns False when MERGE_HEAD does not exist.""" + with mock.patch.object(control, 'run_git', + side_effect=Exception('not found')): + self.assertFalse(control._is_merge_in_progress()) + + +class TestSubtreeRunUpdateReturnValues(unittest.TestCase): + """Tests for _subtree_run_update return values.""" + + def test_returns_ok_on_success(self): + """Test returns SUBTREE_OK on successful update.""" + mock_result = command.CommandResult('done', '', '', 0) + with terminal.capture(): + with mock.patch.object(control.command, 'run_one', + return_value=mock_result): + ret = control._subtree_run_update('dts', 'v6.15-dts') + self.assertEqual(ret, control.SUBTREE_OK) + + def test_returns_conflict_when_merge_in_progress(self): + """Test returns SUBTREE_CONFLICT when merge state exists.""" + mock_result = command.CommandResult( + '', 'CONFLICT', '', 1) + with terminal.capture(): + with mock.patch.object(control.command, 'run_one', + return_value=mock_result): + with mock.patch.object(control, '_is_merge_in_progress', + return_value=True): + ret = control._subtree_run_update('dts', 'v6.15-dts') + self.assertEqual(ret, control.SUBTREE_CONFLICT) + + def test_returns_fail_when_no_merge(self): + """Test returns SUBTREE_FAIL when script fails without merge.""" + mock_result = command.CommandResult( + '', 'error', '', 1) + with terminal.capture(): + with mock.patch.object(control.command, 'run_one', + return_value=mock_result): + with mock.patch.object(control, '_is_merge_in_progress', + return_value=False): + with mock.patch.object(control, 'run_git'): + ret = control._subtree_run_update( + 'dts', 'v6.15-dts') + self.assertEqual(ret, control.SUBTREE_FAIL) + + def test_returns_fail_on_exception(self): + """Test returns SUBTREE_FAIL when script raises exception.""" + with terminal.capture(): + with mock.patch.object(control.command, 'run_one', + side_effect=Exception('boom')): + ret = control._subtree_run_update('dts', 'v6.15-dts') + self.assertEqual(ret, control.SUBTREE_FAIL) + + +class TestSubtreeConflictPrompt(unittest.TestCase): + """Tests for build_subtree_conflict_prompt.""" + + def test_dts_prompt_content(self): + """Test prompt contains correct details for dts subtree.""" + prompt = agent.build_subtree_conflict_prompt( + 'dts', 'v6.15-dts', 'dts/upstream') + self.assertIn('dts/upstream', prompt) + self.assertIn('v6.15-dts', prompt) + self.assertIn('MERGE_HEAD', prompt) + self.assertIn('git commit --no-edit', prompt) + self.assertIn('git merge --abort', prompt) + + def test_mbedtls_prompt_content(self): + """Test prompt contains correct details for mbedtls subtree.""" + prompt = agent.build_subtree_conflict_prompt( + 'mbedtls', 'v3.6.0', 'lib/mbedtls/external/mbedtls') + self.assertIn('lib/mbedtls/external/mbedtls', prompt) + self.assertIn('v3.6.0', prompt) + self.assertIn("'mbedtls'", prompt) + + +class TestResolveSubtreeConflicts(unittest.TestCase): + """Tests for resolve_subtree_conflicts.""" + + def test_success(self): + """Test successful conflict resolution.""" + mock_collect = mock.AsyncMock(return_value=(True, 'resolved')) + with terminal.capture(): + with mock.patch.object(agent, 'AGENT_AVAILABLE', True): + with mock.patch.object(agent, 'run_agent_collect', + mock_collect): + with mock.patch.object(agent, 'ClaudeAgentOptions'): + success, log = agent.resolve_subtree_conflicts( + 'dts', 'v6.15-dts', 'dts/upstream', + '/tmp/test') + self.assertTrue(success) + self.assertEqual(log, 'resolved') + + def test_failure(self): + """Test failed conflict resolution.""" + mock_collect = mock.AsyncMock(return_value=(False, 'failed')) + with terminal.capture(): + with mock.patch.object(agent, 'AGENT_AVAILABLE', True): + with mock.patch.object(agent, 'run_agent_collect', + mock_collect): + with mock.patch.object(agent, 'ClaudeAgentOptions'): + success, log = agent.resolve_subtree_conflicts( + 'dts', 'v6.15-dts', 'dts/upstream', + '/tmp/test') + self.assertFalse(success) + + def test_sdk_unavailable(self): + """Test returns failure when SDK is not available.""" + with terminal.capture(): + with mock.patch.object(agent, 'AGENT_AVAILABLE', False): + success, log = agent.resolve_subtree_conflicts( + 'dts', 'v6.15-dts', 'dts/upstream', '/tmp/test') + self.assertFalse(success) + self.assertEqual(log, '') + + if __name__ == '__main__': unittest.main() -- 2.43.0

From: Simon Glass <sjg@chromium.org> When the GitLab server drops the connection, do_step() crashes with an unhandled requests.exceptions.ConnectionError. Catch this specific exception in do_step(), report it, and return failure. This means 'poll' continues to the next iteration after the sleep, while 'step' reports the error and aborts cleanly. Signed-off-by: Simon Glass <sjg@chromium.org> --- tools/pickman/control.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tools/pickman/control.py b/tools/pickman/control.py index 6d787d02163..817ae02d900 100644 --- a/tools/pickman/control.py +++ b/tools/pickman/control.py @@ -15,6 +15,8 @@ import tempfile import time import unittest +import requests + # Allow 'from pickman import xxx' to work via symlink our_path = os.path.dirname(os.path.realpath(__file__)) sys.path.insert(0, os.path.join(our_path, '..')) @@ -2872,6 +2874,15 @@ def do_step(args, dbs): Returns: int: 0 on success, 1 on failure """ + try: + return _do_step(args, dbs) + except requests.exceptions.ConnectionError as exc: + tout.error(f'step failed with connection error: {exc}') + return 1 + + +def _do_step(args, dbs): + """Internal implementation of do_step""" remote = args.remote source = args.source -- 2.43.0