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

[Patrick Steinhardt <ps@pks.im>]

On Wed, Sep 23, 2026 at 10:48:14AM -0700, Elijah Newren wrote:
> Hi Patrick,
> 
> On Wed, Sep 23, 2026 at 6:16 AM Patrick Steinhardt <ps@pks.im> wrote:
> >
> > 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.
> 
> Are they supposed to commit it directly?  The conflict advice tells
> them to stage the resolution and run "git rebase --continue".  In
> fact, there appear to be a number of problems with using a plain "git
> commit"; more on that below.

I dunno. All I can say is that I've always been committing directly
myself. So it's certainly a workflow that used to work alright. And...

> > 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
> 
> After reading ahead, should this be "...has already committed the
> resolved conflict via a plain 'git commit'"?  Resolving it via "git
> rebase --continue" doesn't have this problem.

... honestly I don't think I even had it in my mind that you can just
continue the rebase and that does everything for you. Thing is, I also
like to verify the result of the merge, and committing myself allows me
to do that immediately.

[snip]
> > I'm not particularly happy with the proposed fix -- it feels quite fishy
> > to use the existence of MERGE_MSG as a proxy for whether or not the user
> > has already committed the resolved conflict. I couldn't come up with a
> > better proxy though, so if you have one please let me know.
> 
> Yeah, I'm also a bit worried about using MERGE_MSG here.  In particular,
> 
>     git reset
> 
> removes MERGE_MSG without moving HEAD.  With this patch, a subsequent
> 
>     git commit --amend -a
> 
> is therefore allowed while the conflict resolution is still
> uncommitted, bringing back the foot-gun that 6257588252 was trying to
> prevent.
> 
> For the short-term 2.56, we could either revert that series (it's a
> long-standing bug after all) and try again after the release.
> Alternatively, we could record HEAD when the sequencer stops, perhaps
> in rebase-merge/stopped-head, and then reject the amend while HEAD
> still equals stopped-head and allow it once a plain commit has
> advanced HEAD.  stopped-sha would remain until rebase --continue,
> since it is needed for the rewritten-commit mapping and fixup/squash
> bookkeeping.
> 
> Longer term, I wonder whether plain "git commit" should be rejected
> while resolving conflicts for rebase, am, cherry-pick, and revert,
> with users directed to the corresponding "--continue" command.  Plain
> commit has a surprising collection of behaviors:
> 
>   * During am or an apply-backend rebase, it ignores final-commit and
> author-script, losing the original message, author, and author date.
> The corresponding --continue will report "No changes - did you forget
> to use 'git add'?" even though the user already added and committed
> the resolution.  Amid the generic recovery advice, the user must infer
> that the corresponding "--skip" is now needed to bypass the patch that
> their manual commit already handled.

True, that's an issue I've been hitting a bunch of times.

>   * During a merge-backend rebase, it reads MERGE_MSG, so the message
> survives, but the original author and author date do not.
> 
>   * It may bypass sequencer options such as explicit signing and
> date-handling options.
> 
>   * --abort behavior then varies by operation: rebase returns to the
> original commit (orig-head), `am` leaves you at the manual commit, and
> cherry-pick and revert refuse to rewind because HEAD moved.
> 
> Having the operation own both the commit and its state transition
> seems much easier to reason about.  I would leave "git merge" as an
> exception, given the very long-standing "resolve, add, commit"
> workflow, but I think plain "git commit" should eventually be
> disallowed as a way to resolve conflicts for other commands.

It certainly is much easier to reason about, true. But it's definitely
a breaking change for something that mostly works alright and that does
have some benefits over the "sanctioned" way of doing this via
git-rebase(1).

> That's post-2.56 work.  For now I think either reverting (and trying
> again after the release), or recording HEAD in stopped-head seems
> preferable to relying on MERGE_MSG.
> 
> Thoughts?

I think reverting is probably the safest change for now, and we can then
discuss how to properly handle this. I'm not a fan myself of refusing
the commit outright as that would break my own workflow. And I'd assume
that I'm probably not the only person using that workflow, also because
it does let you inspect the result before you move on.

It makes me wonder whether we can instead fix git-commit(1) itself to
maybe not reset authorship information. But that's probably a much
harder change to do, and probably it would make the mess that we have
with the ".git/rebase-merge" state directory even bigger.

Thanks!

Patrick