From: Elijah Newren Date: Wed, 23 Sep 2026 17:48:14 GMT Subject: Re: [PATCH REGRESSION] builtin/rebase: allow user to amend committed conflicts again Message-ID: In-Reply-To: <20260923-pks-rebase-conflict-bug-v1-1-3d3ccf5022bc@pks.im> Hi Patrick, On Wed, Sep 23, 2026 at 6:16 AM Patrick Steinhardt 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. > 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. > 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. Oof. Thanks for finding and reporting this. > 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. > > 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. But the whole way > that we track rebase state is somewhat iffy in the first place. > > Signed-off-by: Patrick Steinhardt > --- > Hi, > > this is a regression caused by 6257588252 (commit: refuse to amend > during conflict resolution, 2026-09-01). Ideally, we should probably fix > it before we release Git 2.56. > > 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. * 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. 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?