Re: [PATCH 1/2] worktree remove: refuse a worktree in the middle of a rebase or bisect

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Jasper

On 30/10/2026 14:21, Jasper Mayone wrote:
> "git worktree remove" refuses a worktree with modified or untracked

"refuses to move" would be clearer

> files unless --force is given, but it removes a worktree that is in
> the middle of a rebase, a "git am" session, a bisect or a cherry-pick
> or revert sequence when its files happen to be clean. The state of
> that operation lives in the worktree's own $GIT_DIR, so it is lost
> with the worktree. A rebase stopped at an "edit" or "break" line, an
> "exec" that failed, or a cherry-pick sequence after a conflict was
> resolved and committed is clean in this sense and still has work in
> progress.

Cherry picking or reverting a single commit that contains no new changes 
also looks clean, but there is still a cherry-pick operation in progress 
as CHERRY_PICK_HEAD exists. One could argue that, because the user 
doesn't really lose any work, we don't lose much by deleting such a 
worktree but we're still deleting a worktree that contains an 
in-progress operation. Fixing that case simply requires us to see if 
CHERRY_PICK_HEAD, or REVERT_HEAD exist.

> Treat such a worktree as unclean: refuse to remove it unless --force
> is given, and name the operation in the message.
> 
> A worktree whose directory is already gone is still removed without
> --force, as before: the check is part of the check for a clean
> worktree, which is only done when the directory exists.

Sensible

> Reported-by: D. Ben Knoble <ben.knoble@gmail.com>
> Helped-by: Phillip Wood <phillip.wood123@gmail.com>

I'm not sure what I've done anything to help with this patch

> diff --git a/Documentation/git-worktree.adoc b/Documentation/git-worktree.adoc
> index e6e77252ab..da4c38f909 100644
> --- a/Documentation/git-worktree.adoc
> +++ b/Documentation/git-worktree.adoc
> @@ -142,9 +142,10 @@ the new working tree location.
>   `remove`::
>   
>   Remove a worktree. Only clean worktrees (no untracked files and no
> -modification in tracked files) can be removed. Unclean worktrees or ones
> -with submodules can be removed with `--force`. The main worktree cannot be
> -removed.
> +modification in tracked files) can be removed. A worktree in the middle
> +of a rebase, a `git am` session, a bisect or a cherry-pick or revert

"bisect, cherry-pick or revert"

> +sequence is not clean either, 

"either" doesn't make sense, because you've not described the unclean 
case yet. This line needs folding as well.

> because removing it loses that state. Unclean worktrees or ones with submodules
> +can be removed with `--force`. The main worktree cannot be removed.

A list of the conditions that require --force might make this section 
clearer.
>   `repair [<path>...]`::
>   
> @@ -189,7 +190,9 @@ twice. If the destination is already assigned to some other worktree but is
>   missing (for instance, if _<new-path>_ was deleted manually), then `--force`
>   allows the move to proceed; use `--force` twice if the destination is locked.
>   +
> -`remove` refuses to remove an unclean worktree unless `--force` is used.
> +`remove` refuses to remove an unclean worktree, or one in the middle of a
> +rebase, `git am` session, bisect, or cherry-pick or revert sequence,

too many "or"s as above

> +unless `--force` is used.
>   To remove a locked worktree, specify `--force` twice.
>   
>   `-b <new-branch>`::
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> index 77ecd0f71f..ed08de1f43 100644
> --- a/builtin/worktree.c
> +++ b/builtin/worktree.c
> @@ -26,6 +26,7 @@
>   #include "submodule.h"
>   #include "utf8.h"
>   #include "worktree.h"
> +#include "wt-status.h"
>   #include "quote.h"
>   
>   #define BUILTIN_WORKTREE_ADD_USAGE \
> @@ -1408,6 +1409,32 @@ static void check_clean_worktree(struct worktree *wt,
>   			  original_path, ret);
>   }
>   
> +/*
> + * A rebase, "git am", bisect or cherry-pick/revert sequence keeps its
> + * state in the worktree's own $GIT_DIR, and removing the worktree
> + * throws that away even when "git status" says the files are clean.
> + */
> +static void check_no_operation_in_progress(struct worktree *wt,
> +					   const char *original_path)
> +{
> +	struct wt_status_state state = { 0 };
> +
> +	if (wt_status_check_rebase(wt, &state)) {
> +		if (state.am_in_progress)
> +			die(_("'%s' is in the middle of an am session, use --force to delete it"),
> +			    original_path);
> +		die(_("'%s' is in the middle of a rebase, use --force to delete it"),
> +		    original_path);
> +	}
> +	if (wt_status_check_bisect(wt, &state))
> +		die(_("'%s' is in the middle of a bisect, use --force to delete it"),
> +		    original_path);
> +	wt_status_state_free_buffers(&state);
> +	if (is_directory(worktree_git_path(wt, "sequencer")))
> +		die(_("'%s' is in the middle of a cherry-pick or revert, use --force to delete it"),
> +		    original_path);
> +}

This looks correct, though I wish we had a standard function that 
returned an enum describing which operation was in progress, rather than 
having to open code this sort of thing.

>   static int delete_git_work_tree(struct worktree *wt)
>   {
>   	struct strbuf sb = STRBUF_INIT;
> @@ -1461,8 +1488,10 @@ static int remove_worktree(int ac, const char **av, const char *prefix,
>   	strbuf_release(&errmsg);
>   
>   	if (file_exists(wt->path)) {
> -		if (!force)
> +		if (!force) {
> +			check_no_operation_in_progress(wt, av[0]);

This triggers the new check when the worktree directory exists - good.

>   			check_clean_worktree(wt, av[0]);
> +		}
>   
>   		ret |= delete_git_work_tree(wt);
>   	}
> diff --git a/t/t2403-worktree-move.sh b/t/t2403-worktree-move.sh
> index 69768c1207..4eb56a8911 100755
> --- a/t/t2403-worktree-move.sh
> +++ b/t/t2403-worktree-move.sh
> @@ -271,4 +271,63 @@ test_expect_success 'move worktree with relative path to absolute path' '
>   	test_cmp expect .git/worktrees/absolute/gitdir
>   '
>   
> +test_expect_success 'remove worktree in the middle of a rebase' '
> +	test_when_finished "rm -rf mid-rebase; git worktree prune" &&
> +	git worktree add --detach mid-rebase init &&
> +	test_commit -C mid-rebase rebase-me &&
> +	test_must_fail git -C mid-rebase rebase -f -x false init &&
> +	test_must_fail git worktree remove mid-rebase 2>err &&
> +	test_grep "in the middle of a rebase" err &&
> +	test_path_is_dir mid-rebase &&
> +	git worktree remove --force mid-rebase &&
> +	test_path_is_missing mid-rebase
> +'
> +
> +test_expect_success 'remove worktree in the middle of an am session' '
> +	test_when_finished "rm -rf mid-am patch; git worktree prune" &&
> +	git worktree add --detach mid-am init &&
> +	test_commit -C mid-am am-me &&
> +	git -C mid-am format-patch -1 --stdout >patch &&
> +	git -C mid-am reset --hard init &&
> +	echo conflict >mid-am/am-me.t &&
> +	git -C mid-am add am-me.t &&
> +	git -C mid-am commit -m conflict &&
> +	test_must_fail git -C mid-am am ../patch &&
> +	git -C mid-am reset --hard &&

If we're running a lot of consecutive commands in a worktree, it is 
clearer to use a subshell

	(
		cd mid-am &&
		...
	) &&

I wonder how hard it would be to extend the "setup" test to create a 
branch that has conflicts when it is applied with am, cherry-pick, or 
rebase, so that we don't have to set up the conflicts in each test.

Thanks

Phillip

> +	test_must_fail git worktree remove mid-am 2>err &&
> +	test_grep "in the middle of an am session" err &&
> +	git worktree remove --force mid-am
> +'
> +
> +test_expect_success 'remove worktree in the middle of a bisect' '
> +	test_when_finished "rm -rf mid-bisect; git worktree prune" &&
> +	git worktree add --detach mid-bisect init &&
> +	test_commit -C mid-bisect bisect-one &&
> +	test_commit -C mid-bisect bisect-two &&
> +	git -C mid-bisect bisect start HEAD init &&
> +	test_must_fail git worktree remove mid-bisect 2>err &&
> +	test_grep "in the middle of a bisect" err &&
> +	git worktree remove --force mid-bisect
> +'
> +
> +test_expect_success 'remove worktree in the middle of a cherry-pick sequence' '
> +	test_when_finished "rm -rf mid-pick; git worktree prune" &&
> +	git worktree add --detach mid-pick init &&
> +	git -C mid-pick switch -c pick-src &&
> +	test_commit -C mid-pick pick-one &&
> +	test_commit -C mid-pick pick-two &&
> +	git -C mid-pick switch --detach init &&
> +	echo conflict >mid-pick/pick-one.t &&
> +	git -C mid-pick add pick-one.t &&
> +	git -C mid-pick commit -m conflict &&
> +	test_must_fail git -C mid-pick cherry-pick pick-src~1 pick-src &&
> +	git -C mid-pick checkout --theirs pick-one.t &&
> +	git -C mid-pick add pick-one.t &&
> +	git -C mid-pick commit --no-edit &&
> +	test_path_is_dir .git/worktrees/mid-pick/sequencer &&
> +	test_must_fail git worktree remove mid-pick 2>err &&
> +	test_grep "in the middle of a cherry-pick or revert" err &&
> +	git worktree remove --force mid-pick
> +'
> +
>   test_done