Re: [PATCH REGRESSION] builtin/rebase: allow user to amend committed conflicts again

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Sep 23, 2026, 14:02 UTC
Message-ID: <c12d2ac3-5263-4301-aa64-a311a343dd40@gmail.com>
In-Reply-To: <20260923-pks-rebase-conflict-bug-v1-1-3d3ccf5022bc@pks.im>

Hi Patrick

On 23/09/2026 14:16, Patrick Steinhardt wrote:

Show 28 quoted lines

> In 6257588252 (commit: refuse to amend during conflict resolution,
> 2026-09-01), we have introduced logic to git-commit(1) that makes it
> refuse creating a commit in some cases. This was done to remove a set of
> common foot guns.
> 
> One of these foot guns is when the user is performing an interactive
> rebase that stops at a conflict. Most of the time when we stop at a
> specific commit we want the user to amend the HEAD commit, so they have
> been trained to use `git commit --amend`. But when there's a conflict,
> they are instead supposed to commit it directly without amending the
> HEAD commit. So to remove that common pit fall, git-commit(1) now
> refuses amending in that situation.
> 
> The logic that detects this scenario checks whether the file
> "rebase-merge/stopped-sha" exists, while "rebase-merge/amend" doesn't.
> And this is exactly the case when git-rebase(1) has stopped at such a
> conflicting commit.
> 
> But there's one problem here: this state persists even after the user
> has already committed the resolved conflict, and consequently they still
> cannot amend after they have done so. This is overly restrictive though,
> as it's quite likely that a user may want to change the resolved commit
> once again.
> 
> Ideally, we'd be able to easily check whether HEAD has already been
> updated to have the resolved conflict. But it seems like we do not have
> sufficient information to determine the original state of HEAD when the
> interactive rebase has stopped, so this is not a workable solution.

Yes, that's unfortunate - I think there is an argument that rebase should be writing ".git/rebase-merge/stopped-head" when it stops. That would make it easy to detect if the user has committed since the rebase stopped. At the moment "git rebase --continue" will happily commit any staged changes with the message from the commit that was being picked when the rebase stopped, even if the user has already committed a conflict resolution. Fixing that is definitely not -rc2 material.

In general we should be discouraging users from committing conflict resolutions themselves as it is a hang-over from the way "git merge" originally worked that is error prone and loses the original authorship when applied to "git rebase"

> Instead, use the existence of "MERGE_MSG" to figure out whether the user
> has already resolved and committed the conflict. It feels somewhat fishy
> to base our decisions on the existence of that particular file, as it
> really is only a proxy for what we are actually after.

I think that's probably the best we can do. If, after committing a conflict resolution from "git rebase", the user runs a merge/cherry-pick/revert that has conflicts, then "MERGE_MSG" will also exist, but we don't want them to amend that case either so it should be fine.

The code changes look good, but I'm not convinced by the test

Show 11 quoted lines

> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index 8c63682b7f..d55afaa113 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -2486,6 +2486,40 @@ test_expect_success 'non-merge commands reject merge commits' '
>   	test_cmp expect actual
>   '
>   
> +test_expect_success 'can amend after committing a conflict' '
> +	test_when_finished rm -rf repo &&
> +	git init repo &&

This test file is one of the slowest already, surely we don't need a whole new repository and commit setup - can't we just add

	git commit -F .git/MERGE_MSG &&
	git commit --amend -m amended

to the end of 'commit --amend is refused at a rebase conflict stop' which was added by 6257588252. That would also check that committing a conflict resolution works as well.

Thanks

Phillip

Show 38 quoted lines

> +	(
> +		cd repo &&
> +
> +		test_commit original file &&
> +		test_commit modified file &&
> +		cat >todo <<-EOF &&
> +		break
> +		edit $(git rev-parse HEAD)
> +		EOF
> +		set_replace_editor todo &&
> +		git rebase -i HEAD~ &&
> +
> +		# Modify "file" to cause a conflict.
> +		echo conflict >file &&
> +		git commit -a --message conflict &&
> +		test_must_fail git rebase --continue 2>err &&
> +		test_grep "Resolve all conflicts manually" err &&
> +
> +		# Resolve the conflict.
> +		echo resolved >file &&
> +		git add file &&
> +		git commit --message resolve &&
> +
> +		# And now try to amend to the conflict. This operation should
> +		# succeed.
> +		echo change >file &&
> +		git commit --amend -a --no-edit &&
> +		git rebase --continue
> +	)
> +'
> +
>   # This must be the last test in this file
>   test_expect_success '$EDITOR and friends are unchanged' '
>   	test_editor_unchanged
> 
> ---
> base-commit: 3bc0341126508f78f5869cbfc0005e987efdf0c7
> change-id: 20260923-pks-rebase-conflict-bug-176e325ad079