From bf4b12438a83d81f2c8df6e39f6114ddd5002430 Mon Sep 17 00:00:00 2001
From: KrisPointCSGO <KrisPointCSGO@outlook.com>
Date: Tue, 7 Jul 2026 20:10:00 +0800
Subject: [PATCH] merge --abort: don't delete autostash before reset succeeds
To: git@vger.kernel.org
Cc: gitster@pobox.com

In cmd_merge()'s --abort path, MERGE_AUTOSTASH was deleted before cmd_reset() was called. If cmd_reset() failed (e.g. due to a locked index), the autostash was permanently lost.

Instead, read the MERGE_AUTOSTASH OID without deleting the ref, run cmd_reset() (which itself calls remove_branch_state() -> save_autostash_ref() to persist the stash), and only apply the autostash on success.

Reported-by: KrisPoint
Signed-off-by: KrisPoint <KrisPointCSGO@outlook.com>
---
 builtin/merge.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

Hi Kris

On 08/07/2026 02:51, Kris Point wrote:

That's bad

I'm afraid I don't think this is the right solution. We only want to save the stash if there are conflicts when we apply it - that is why MERGE_AUTOSTASH is deleted before we do the reset - we want to prevent remove_branch_state() from saving it. If the stash applies cleanly then we should not save it. If the reset fails then we should keep MERGE_AUTOSTASH along with the other merge state files rather than saving the stash (which is actually what happens after this patch because cmd_reset() dies before it calls remove_branch_state()).

I think the solution is probably to stop calling builtin/reset.c:cmd_reset() and instead extend reset.c:reset_working_tree()[1] to do a "merge" reset by adding a "RESET_WORKING_TREE_MERGE" flag (or possibly we want to remove RESET_WORKTING_TREE_HARD from the flags and add a reset_mode member). Then we can call

	struct reset_working_tree opts = {
		.flags = RESET_WORKING_TREE_MERGE;
	};
	if (reset_working_tree(the_repository, &opts))
		die(_("could not reset index and working tree")); 
apply_autostash_ref(...); /* apply the stash */ 
remove_branch_state(...); /* remove merge state */

So we only delete MERGE_AUTOSTASH after a successful reset and we only save the stash if it applies with conflicts. That's all a bit more involved than the patch here - please do give me a shout if you want some more information.

Thanks

Phillip

[1] Note that in the master branch this function is called reset_head(),
     you should base the fix on top of the "ps/history-drop" branch which
     is in "seen" (currently the tip is d11b348f784 (builtin/history:
     implement "drop" subcommand, 2026-07-01) but that might change when
     Junio rebuilds "seen".

Phillip Wood <phillip.wood123@gmail.com> writes:

Thanks for pointing it out that reset calls remove_branch_state(), which in turn calls remove_merge_branch_state(), which in turn calls save_autostash_ref(). We end up (when cmd_reset() is successful) applying the autostash (which is good) but also saving a new stash.

Great. In general, it is a bad pattern we should find and fix for cmd_A() to call cmd_B() in its implementation as a subroutine. To clean any such instance is a great thing to do.

Thanks.