[PATCH b4 v3 8/9] review-tui: don't leave a take's target worktree mid-conflict

Christian Brauner <[email protected]> Thu, 25 Jun 2026 14:09:28 +0200
Newsgroups org.kernel.linux.tools
Message-ID <[email protected]>
A take applies the series into whichever worktree holds the target branch
-- often the maintainer's real checkout. When git-am or git-merge hits a
conflict there, the take drops into a shell to resolve it in place. If that
shell is left without finishing (no git am/merge --continue or --abort),
_resolve_worktree_take_conflict only printed a "run git ... --abort to clean
up" hint and returned, leaving the half-applied am/merge in the worktree.

That poisons it. A later take's git-merge then refuses to even start
("Merging is not possible because you have unmerged files"), and the
"Aborting merge..." fallback runs git merge --abort, which cannot clear a
bare unmerged index with no MERGE_HEAD -- so every subsequent take fails the
same way until the worktree is cleaned by hand.

Fix it from both ends:

  - A real checkout is never left mid-op. When conflict resolution is
    abandoned, abort what we started so the worktree is restored. Throwaway
    worktrees are still kept for hand-finishing -- they are isolated and
    poison nothing, and the next take force-removes them regardless.

  - A take refuses up front if the target worktree is already mid-op or
    carries a conflicted index -- an earlier abandoned take, a crash, or the
    user's own unfinished am/merge -- naming the op and the fix to run,
    rather than failing cryptically on top of it. Pre-existing state may be
    the user's own, so it is reported, never discarded.

Detection and abort live in b4 core, alongside the worktree am-conflict
plumbing this series already lifted there: _worktree_inprogress_op resolves
the in-progress operation from git's own state files (am/rebase/merge/
cherry-pick/revert), _worktree_has_unmerged spots a bare conflicted index,
and _abort_worktree_op runs the matching --abort, falling back to git reset
--merge for an unmergeable index.

Fixes: 089665dbb6ba ("review-tui: resolve take->merge conflicts in place")
Signed-off-by: Christian Brauner (Amutable) <[email protected]>
---
 src/b4/__init__.py                 | 66 ++++++++++++++++++++++++++++++++++++++
 src/b4/review_tui/_tracking_app.py | 42 +++++++++++++++++++++++-
 2 files changed, 107 insertions(+), 1 deletion(-)

diff --git a/src/b4/__init__.py b/src/b4/__init__.py
index d9e3bff..d7b41ee 100644
--- a/src/b4/__init__.py
+++ b/src/b4/__init__.py
@@ -5727,6 +5727,72 @@ def _worktree_merge_in_progress(worktree: str) -> bool:
     return os.path.exists(os.path.join(gitdir.strip(), 'MERGE_HEAD'))
 
 
+def _worktree_inprogress_op(worktree: str) -> Optional[str]:
+    """Return the git operation mid-flight in *worktree*, or ``None``.
+
+    One of ``'am'``, ``'rebase'``, ``'merge'``, ``'cherry-pick'`` or
+    ``'revert'`` -- mirroring how git-status decides which "in the middle of"
+    banner to show. State lives under the per-worktree git dir (resolved via
+    ``--absolute-git-dir`` so linked and throwaway worktrees work too):
+    ``rebase-apply/`` is shared by ``git am`` and ``git rebase --apply``, told
+    apart by the ``applying`` marker; ``rebase-merge/`` is the rebase merge
+    backend; the rest are recorded as pseudo-ref files.
+    """
+    ecode, gitdir = git_run_command(worktree, ['rev-parse', '--absolute-git-dir'])
+    if ecode != 0:
+        return None
+    gd = gitdir.strip()
+    if os.path.isdir(os.path.join(gd, 'rebase-apply')):
+        applying = os.path.exists(os.path.join(gd, 'rebase-apply', 'applying'))
+        return 'am' if applying else 'rebase'
+    if os.path.isdir(os.path.join(gd, 'rebase-merge')):
+        return 'rebase'
+    for marker, op in (
+        ('MERGE_HEAD', 'merge'),
+        ('CHERRY_PICK_HEAD', 'cherry-pick'),
+        ('REVERT_HEAD', 'revert'),
+    ):
+        if os.path.exists(os.path.join(gd, marker)):
+            return op
+    return None
+
+
+def _worktree_has_unmerged(worktree: str) -> bool:
+    """Return whether *worktree*'s index carries unmerged (conflict) entries.
+
+    This is the state git leaves when it refuses to *start* an operation on top
+    of a conflicted index (e.g. ``git merge`` reporting "you have unmerged
+    files"): there is no in-progress op to ``--abort``, only stage>0 entries.
+    """
+    ecode, out = git_run_command(worktree, ['ls-files', '--unmerged'])
+    return ecode == 0 and bool(out.strip())
+
+
+def _abort_worktree_op(worktree: str) -> Optional[str]:
+    """Abort whatever git operation is mid-flight in *worktree*, restoring it.
+
+    Detects the in-progress op (see :func:`_worktree_inprogress_op`) and runs
+    the matching ``--abort``. When nothing is in progress but the index still
+    carries unmerged entries -- which no ``--abort`` can clear -- falls back to
+    ``git reset --merge`` to drop them. Returns what it did: the op aborted
+    ('am'/'rebase'/'merge'/'cherry-pick'/'revert'), ``'reset'`` for the
+    unmerged-index fallback, or ``None`` if there was nothing to clean up.
+
+    WARNING: this discards the in-progress operation and any half-done conflict
+    resolution in it. Only call it on a worktree whose state is b4's to throw
+    away -- its own incomplete take, or a throwaway worktree -- never on
+    pre-existing state a user may own.
+    """
+    op = _worktree_inprogress_op(worktree)
+    if op is not None:
+        git_run_command(worktree, [op, '--abort'], logstderr=True, rundir=worktree)
+        return op
+    if _worktree_has_unmerged(worktree):
+        git_run_command(worktree, ['reset', '--merge'], logstderr=True, rundir=worktree)
+        return 'reset'
+    return None
+
+
 def _fetch_and_drop_am_worktree(
     dest: str, gwt: str, origin: Optional[str] = None
 ) -> bool:
diff --git a/src/b4/review_tui/_tracking_app.py b/src/b4/review_tui/_tracking_app.py
index 0c00917..7e7f142 100644
--- a/src/b4/review_tui/_tracking_app.py
+++ b/src/b4/review_tui/_tracking_app.py
@@ -244,6 +244,36 @@ def _take_worktree(
             yield None
             return
         work_dir = temp_wt
+    else:
+        # The target is checked out in a real worktree. If it is mid-op or its
+        # index is conflicted -- an earlier take the user walked away from, a
+        # crash, or their own unfinished am/merge -- a take's git-am/git-merge
+        # would fail cryptically on top of it (and "git merge --abort" cannot
+        # clear a bare unmerged index). Refuse up front with the fix rather than
+        # poison the take; the state may be the user's own, so we never discard
+        # it for them.
+        stuck = b4._worktree_inprogress_op(work_dir)
+        if stuck is None and b4._worktree_has_unmerged(work_dir):
+            logger.critical(
+                'Target worktree %s has a conflicted index (unmerged files).',
+                work_dir,
+            )
+            logger.critical(
+                'Resolve them, or run "git reset --merge" there, then retry.'
+            )
+            _wait_for_enter()
+            yield None
+            return
+        if stuck is not None:
+            logger.critical(
+                'Target worktree %s has an unfinished git-%s.', work_dir, stuck
+            )
+            logger.critical(
+                'Finish it, or run "git %s --abort" there, then retry.', stuck
+            )
+            _wait_for_enter()
+            yield None
+            return
 
     handle = _TakeWorktree(work_dir, is_temp=temp_wt is not None)
     try:
@@ -280,9 +310,19 @@ def _resolve_worktree_take_conflict(
     if in_progress(wt.path):
         logger.warning('Conflict resolution incomplete')
         if wt.is_temp:
+            # A throwaway worktree is isolated, so an unfinished op poisons
+            # nothing -- keep it for the user to finish or abort by hand (the
+            # next take force-removes it regardless).
             wt.keep()
             logger.warning('Finish or abort it in: %s', wt.path)
-        logger.warning('Run "git %s --abort" to clean up', op)
+            logger.warning('Run "git %s --abort" to clean up', op)
+        else:
+            # A real checkout must never be left mid-op: the unmerged index
+            # breaks the user's own git work and makes the next take fail
+            # cryptically on top of it (and "git merge --abort" cannot clear a
+            # bare unmerged index). Abort what we started.
+            if b4._abort_worktree_op(wt.path):
+                logger.warning('Aborted incomplete %s in %s', op, wt.path)
         return False
     ecode, current_head = b4.git_run_command(
         wt.path, ['rev-parse', 'HEAD'], logstderr=True

-- 
2.53.0