Treat sequencer progress as success when HEAD moves

When `git rebase --continue`, `git merge --continue`, or `git
cherry-pick --continue` commit a resolution and encounter the next
conflict, Git exits with an error despite having made progress. Read
HEAD before and after to detect progress — if it moved, treat the
nonzero exit as success.

Also shorten bare commit oids in rebase banner to 8 chars.
This commit is contained in:
Jinjing
2026-08-31 19:06:00 -07:00
parent f33b831556
commit bed840a7dd
4 changed files with 103 additions and 11 deletions
+54 -3
View File
@@ -27,10 +27,25 @@ const CASES: readonly [string, SequencerAction, string[]][] = [
['continueCherryPick', continueCherryPick, ['cherry-pick', '--continue']]
]
// The HEAD probe runs before the sequencer step, so calls are matched by argv, not index.
function optionsFor(args: readonly string[]): { env?: NodeJS.ProcessEnv } | undefined {
const call = gitExecFileAsyncMock.mock.calls.find(
(called: unknown[]) => (called[0] as string[]).join(' ') === args.join(' ')
)
return call?.[1] as { env?: NodeJS.ProcessEnv } | undefined
}
function headProbe(oid: string) {
return (args: readonly string[]) =>
args[0] === 'rev-parse'
? Promise.resolve({ stdout: `${oid}\n`, stderr: '' })
: Promise.resolve({ stdout: '', stderr: '' })
}
describe('git sequencer actions', () => {
beforeEach(() => {
gitExecFileAsyncMock.mockReset()
gitExecFileAsyncMock.mockResolvedValue({ stdout: '', stderr: '' })
gitExecFileAsyncMock.mockImplementation(headProbe('abc123'))
})
it.each(CASES)('%s runs the matching git command in the worktree', async (_name, run, args) => {
@@ -43,10 +58,10 @@ describe('git sequencer actions', () => {
})
// Regression guard: without GIT_EDITOR the `--continue` child waits forever on the commit editor.
it.each(CASES)('%s suppresses the commit-message editor', async (_name, run) => {
it.each(CASES)('%s suppresses the commit-message editor', async (_name, run, args) => {
await run('/repo')
expect(gitExecFileAsyncMock.mock.calls[0][1].env.GIT_EDITOR).toBe('true')
expect(optionsFor(args)?.env?.GIT_EDITOR).toBe('true')
})
it('forwards runtime options such as the WSL distro', async () => {
@@ -57,4 +72,40 @@ describe('git sequencer actions', () => {
expect.objectContaining({ cwd: '/repo', wslDistro: 'Ubuntu' })
)
})
// `git rebase --continue` exits nonzero when it lands the resolution and then stops on the
// NEXT commit's conflict. HEAD moved, so that is the sequencer advancing, not a failed step.
it('treats a stop on the next commit as progress once HEAD has moved', async () => {
let head = 'aaa111'
gitExecFileAsyncMock.mockImplementation((args: string[]) => {
if (args[0] === 'rev-parse') {
return Promise.resolve({ stdout: `${head}\n`, stderr: '' })
}
head = 'bbb222'
return Promise.reject(new Error('error: could not apply ec9b3362... feat: add thing'))
})
await expect(continueRebase('/repo')).resolves.toBeUndefined()
})
it('still fails a step that refused to run, leaving HEAD where it was', async () => {
gitExecFileAsyncMock.mockImplementation((args: string[]) =>
args[0] === 'rev-parse'
? Promise.resolve({ stdout: 'aaa111\n', stderr: '' })
: Promise.reject(new Error('f.txt: needs merge'))
)
await expect(continueRebase('/repo')).rejects.toThrow('needs merge')
})
// An unborn HEAD (or an unreadable one) proves nothing, so the original failure stands.
it('rethrows when HEAD cannot be read', async () => {
gitExecFileAsyncMock.mockImplementation((args: string[]) =>
args[0] === 'rev-parse'
? Promise.reject(new Error('fatal: bad revision'))
: Promise.reject(new Error('cherry-pick failed'))
)
await expect(continueCherryPick('/repo')).rejects.toThrow('cherry-pick failed')
})
})
+33 -7
View File
@@ -4,6 +4,21 @@ import { gitOptionsForWorktree } from './git-runtime-options'
import { gitExecFileAsync } from './runner'
import { runWithGitReadCacheInvalidation } from './status'
async function readHeadOid(
worktreePath: string,
options: GitRuntimeOptions
): Promise<string | null> {
try {
const { stdout } = await gitExecFileAsync(
['rev-parse', '--verify', 'HEAD'],
gitOptionsForWorktree(worktreePath, options)
)
return stdout.trim() || null
} catch {
return null
}
}
// Why: every subcommand here predates the Git 2.25 baseline (`merge --continue` 2.12,
// the rest older), so no capability probe or fallback is needed.
async function runSequencerAction(
@@ -11,13 +26,24 @@ async function runSequencerAction(
worktreePath: string,
options: GitRuntimeOptions
): Promise<void> {
await runWithGitReadCacheInvalidation(() =>
gitExecFileAsync([...args], {
...gitOptionsForWorktree(worktreePath, options),
// Why: `--continue` opens the commit-message editor and would hang with no terminal to close it.
env: editorSuppressedGitEnv()
})
)
const headBefore = await readHeadOid(worktreePath, options)
try {
await runWithGitReadCacheInvalidation(() =>
gitExecFileAsync([...args], {
...gitOptionsForWorktree(worktreePath, options),
// Why: `--continue` opens the commit-message editor and would hang with no terminal to close it.
env: editorSuppressedGitEnv()
})
)
} catch (error) {
// Why: `--continue` also exits nonzero when it DID commit the resolution and the
// sequencer then stopped on the next commit. A moved HEAD is the proof it advanced;
// only an unmoved one means the step refused and nothing happened.
const headAfter = await readHeadOid(worktreePath, options)
if (!headBefore || !headAfter || headAfter === headBefore) {
throw error
}
}
}
export async function continueMerge(
@@ -48,6 +48,11 @@ function conflictsHeading(conflictOperation: GitConflictOperation): string {
)
}
// Full 40-char oids only get truncated by CSS, which reads as a broken value.
function shortenOnto(onto: string | undefined): string | undefined {
return onto && /^[0-9a-f]{40}$/i.test(onto) ? onto.slice(0, 8) : onto
}
function inProgressHeading(conflictOperation: GitConflictOperation): string {
if (conflictOperation === 'merge') {
return translate(
@@ -155,7 +160,7 @@ export function OperationBannerBody({
onResolveWithAI?: () => void
}): React.JSX.Element {
const Icon = conflictOperation === 'rebase' ? GitPullRequestArrow : GitMerge
const onto = operationProgress?.onto?.trim()
const onto = shortenOnto(operationProgress?.onto?.trim())
const heading =
conflictOperation === 'rebase' && onto
? translate(
@@ -42,6 +42,16 @@ describe('OperationBanner heading', () => {
expect(render()).toContain('Rebasing onto origin/main')
})
// The banner is narrow; a full oid only gets CSS-truncated, which reads as broken.
it('shortens a bare oid it is replaying onto', () => {
const markup = render({
operationProgress: { ...progress, onto: 'bc98655a3965fe350f77acb14bf4a53f1e2d3c4b' }
})
expect(markup).toContain('Rebasing onto bc98655a')
expect(markup).not.toContain('bc98655a3965fe350f77acb14bf4a53f1e2d3c4b')
})
// Wire compatibility: a host that predates operationProgress omits it entirely.
it('degrades to the plain in-progress banner when the host reported no progress', () => {
const markup = render({ operationProgress: null })