diff --git a/src/main/git/remote.test.ts b/src/main/git/remote.test.ts index 61b95d0e50d..df43f3e5205 100644 --- a/src/main/git/remote.test.ts +++ b/src/main/git/remote.test.ts @@ -511,10 +511,13 @@ describe('git remote operations', () => { ]) }) - it('rebases from the selected remote base ref', async () => { + it('fetches to a private ref then rebases from the selected remote base ref', async () => { gitExecFileAsyncMock .mockResolvedValueOnce({ stdout: 'origin\nupstream\n', stderr: '' }) .mockResolvedValueOnce({ stdout: '', stderr: '' }) + .mockResolvedValueOnce({ stdout: 'fork-point\n', stderr: '' }) + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + .mockResolvedValueOnce({ stdout: '', stderr: '' }) .mockResolvedValueOnce({ stdout: '', stderr: '' }) await gitPullRebaseFromBase('/repo', 'upstream/main') @@ -522,20 +525,86 @@ describe('git remote operations', () => { expect(gitExecFileAsyncMock.mock.calls).toEqual([ [['remote'], { cwd: '/repo' }], [['check-ref-format', '--branch', 'main'], { cwd: '/repo' }], - [['pull', '--rebase', 'upstream', 'main'], { cwd: '/repo' }] + [['merge-base', '--fork-point', 'refs/remotes/upstream/main', 'HEAD'], { cwd: '/repo' }], + [ + [ + 'fetch', + 'upstream', + expect.stringMatching(/^\+refs\/heads\/main:refs\/orca\/rebase\//) + ], + { cwd: '/repo' } + ], + [ + ['rebase', '--onto', expect.stringMatching(/^refs\/orca\/rebase\//), 'fork-point'], + { cwd: '/repo' } + ], + [['update-ref', '-d', expect.stringMatching(/^refs\/orca\/rebase\//)], { cwd: '/repo' }] ]) + + const fetchRefspec = gitExecFileAsyncMock.mock.calls[3][0][2] + const rebasedRef = gitExecFileAsyncMock.mock.calls[4][0][2] + const deletedRef = gitExecFileAsyncMock.mock.calls[5][0][2] + expect(fetchRefspec).toBe(`+refs/heads/main:${rebasedRef}`) + expect(deletedRef).toBe(rebasedRef) }) it('uses the longest configured remote name when rebasing from a base ref', async () => { gitExecFileAsyncMock .mockResolvedValueOnce({ stdout: 'fork\nfork/team\n', stderr: '' }) .mockResolvedValueOnce({ stdout: '', stderr: '' }) + .mockResolvedValueOnce({ stdout: 'fork-point\n', stderr: '' }) + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + .mockResolvedValueOnce({ stdout: '', stderr: '' }) .mockResolvedValueOnce({ stdout: '', stderr: '' }) await gitPullRebaseFromBase('/repo', 'fork/team/feature/base') - expect(gitExecFileAsyncMock).toHaveBeenLastCalledWith( - ['pull', '--rebase', 'fork/team', 'feature/base'], + expect(gitExecFileAsyncMock).toHaveBeenNthCalledWith( + 5, + ['rebase', '--onto', expect.stringMatching(/^refs\/orca\/rebase\//), 'fork-point'], + { cwd: '/repo' } + ) + }) + + it('rebases when the selected remote has not been fetched before', async () => { + gitExecFileAsyncMock + .mockResolvedValueOnce({ stdout: 'upstream\n', stderr: '' }) + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + .mockRejectedValueOnce(new Error('missing remote-tracking ref')) + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + + await expect(gitPullRebaseFromBase('/repo', 'upstream/main')).resolves.toBeUndefined() + + expect(gitExecFileAsyncMock).toHaveBeenNthCalledWith( + 4, + [ + 'fetch', + 'upstream', + expect.stringMatching(/^\+refs\/heads\/main:refs\/orca\/rebase\//) + ], + { cwd: '/repo' } + ) + }) + + it('removes the private ref when rebase fails', async () => { + gitExecFileAsyncMock + .mockResolvedValueOnce({ stdout: 'upstream\n', stderr: '' }) + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + .mockResolvedValueOnce({ stdout: 'fork-point\n', stderr: '' }) + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + .mockRejectedValueOnce(new Error('fatal: rebase conflict')) + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + + await expect(gitPullRebaseFromBase('/repo', 'upstream/main')).rejects.toThrow( + 'fatal: rebase conflict' + ) + + const rebasedRef = gitExecFileAsyncMock.mock.calls[4][0][2] + expect(gitExecFileAsyncMock).toHaveBeenNthCalledWith( + 6, + ['update-ref', '-d', rebasedRef], { cwd: '/repo' } ) }) diff --git a/src/main/git/remote.ts b/src/main/git/remote.ts index 687102b1184..d0061a2156b 100644 --- a/src/main/git/remote.ts +++ b/src/main/git/remote.ts @@ -1,3 +1,4 @@ +import { randomUUID } from 'node:crypto' import { normalizeGitErrorMessage, runPullWithDivergenceFallback @@ -278,17 +279,49 @@ export async function gitPullRebaseFromBase( options: GitRuntimeOptions = {} ): Promise { await runWithGitReadCacheInvalidation(async () => { + let rebaseRef: string | null = null try { const source = await resolveGitRemoteRebaseSource( (args) => gitExecFileAsync(args, gitOptionsForWorktree(worktreePath, options)), baseRef ) + let forkPoint: string | null = null + try { + const { stdout } = await gitExecFileAsync( + ['merge-base', '--fork-point', `refs/remotes/${source.displayName}`, 'HEAD'], + gitOptionsForWorktree(worktreePath, options) + ) + forkPoint = stdout.trim() || null + } catch { + // A first fetch or an unhelpful reflog falls back to Git's merge-base behavior. + } + // Why: concurrent fetches can replace FETCH_HEAD and remote-tracking refs between fetch and rebase. + rebaseRef = `refs/orca/rebase/${randomUUID()}` await gitExecFileAsync( - ['pull', '--rebase', source.remoteName, source.branchName], + [ + 'fetch', + source.remoteName, + `+refs/heads/${source.branchName}:${rebaseRef}` + ], + gitOptionsForWorktree(worktreePath, options) + ) + await gitExecFileAsync( + forkPoint ? ['rebase', '--onto', rebaseRef, forkPoint] : ['rebase', rebaseRef], gitOptionsForWorktree(worktreePath, options) ) } catch (error) { throw new Error(normalizeGitErrorMessage(error, 'pull')) + } finally { + if (rebaseRef) { + try { + await gitExecFileAsync( + ['update-ref', '-d', rebaseRef], + gitOptionsForWorktree(worktreePath, options) + ) + } catch { + // Cleanup must not hide the fetch or rebase result. + } + } } }) } diff --git a/src/main/git/status-submodule-path-cache.test.ts b/src/main/git/status-submodule-path-cache.test.ts index 60adccf142c..af74553bdb0 100644 --- a/src/main/git/status-submodule-path-cache.test.ts +++ b/src/main/git/status-submodule-path-cache.test.ts @@ -148,7 +148,7 @@ describe('submodule path cache', () => { if (args[0] === 'remote') { return Promise.resolve({ stdout: 'origin\n' }) } - if (args[0] === 'pull') { + if (args[0] === 'pull' || args[0] === 'fetch') { modulePath = 'fresh-lib' return Promise.resolve({ stdout: '' }) } diff --git a/src/relay/git-handler-remote-sync.test.ts b/src/relay/git-handler-remote-sync.test.ts index 59054fad9e9..6cb120e7d75 100644 --- a/src/relay/git-handler-remote-sync.test.ts +++ b/src/relay/git-handler-remote-sync.test.ts @@ -154,6 +154,75 @@ describe('GitHandler', () => { } }) + it('rebases from the original fork point after a remote force-push', async () => { + const bareDir = mkdtempSync(path.join(tmpdir(), 'relay-git-rebase-bare-')) + const producerParent = mkdtempSync(path.join(tmpdir(), 'relay-git-rebase-producer-')) + const producerDir = path.join(producerParent, 'repo') + try { + execFileSync('git', ['init', '--bare'], { cwd: bareDir, stdio: 'pipe' }) + gitInit(tmpDir) + writeFileSync(path.join(tmpDir, 'base.txt'), 'base') + gitCommit(tmpDir, 'base') + const branch = execFileSync('git', ['branch', '--show-current'], { + cwd: tmpDir, + encoding: 'utf-8' + }).trim() + const forkPoint = execFileSync('git', ['rev-parse', 'HEAD'], { + cwd: tmpDir, + encoding: 'utf-8' + }).trim() + execFileSync('git', ['remote', 'add', 'origin', bareDir], { cwd: tmpDir, stdio: 'pipe' }) + execFileSync('git', ['push', '--set-upstream', 'origin', branch], { + cwd: tmpDir, + stdio: 'pipe' + }) + + execFileSync('git', ['clone', bareDir, producerDir], { stdio: 'pipe' }) + execFileSync('git', ['checkout', branch], { cwd: producerDir, stdio: 'pipe' }) + execFileSync('git', ['config', 'user.email', 'test@test.com'], { + cwd: producerDir, + stdio: 'pipe' + }) + execFileSync('git', ['config', 'user.name', 'Test'], { cwd: producerDir, stdio: 'pipe' }) + writeFileSync(path.join(producerDir, 'discarded.txt'), 'discarded') + gitCommit(producerDir, 'discarded remote commit') + execFileSync('git', ['push'], { cwd: producerDir, stdio: 'pipe' }) + execFileSync('git', ['fetch', 'origin'], { cwd: tmpDir, stdio: 'pipe' }) + + execFileSync('git', ['checkout', '-b', 'feature', `origin/${branch}`], { + cwd: tmpDir, + stdio: 'pipe' + }) + writeFileSync(path.join(tmpDir, 'topic.txt'), 'topic') + gitCommit(tmpDir, 'topic commit') + + execFileSync('git', ['reset', '--hard', forkPoint], { cwd: producerDir, stdio: 'pipe' }) + writeFileSync(path.join(producerDir, 'replacement.txt'), 'replacement') + gitCommit(producerDir, 'replacement remote commit') + execFileSync('git', ['push', '--force', 'origin', branch], { cwd: producerDir, stdio: 'pipe' }) + + await dispatcher.callRequest('git.rebaseFromBase', { + worktreePath: tmpDir, + baseRef: `origin/${branch}` + }) + + await expect(fs.access(path.join(tmpDir, 'replacement.txt'))).resolves.toBeUndefined() + await expect(fs.access(path.join(tmpDir, 'topic.txt'))).resolves.toBeUndefined() + await expect(fs.access(path.join(tmpDir, 'discarded.txt'))).rejects.toThrow() + expect( + execFileSync('git', ['for-each-ref', '--format=%(refname)', 'refs/orca/rebase'], { + cwd: tmpDir, + encoding: 'utf-8' + }).trim() + ).toBe('') + } finally { + await Promise.all([ + fs.rm(bareDir, { recursive: true, force: true }), + fs.rm(producerParent, { recursive: true, force: true }) + ]) + } + }, 15_000) + it('fetches the explicit publish target remote', async () => { const bareDir = mkdtempSync(path.join(tmpdir(), 'relay-git-fork-bare-')) try { diff --git a/src/relay/git-handler.ts b/src/relay/git-handler.ts index 0558f710970..effea967a8e 100644 --- a/src/relay/git-handler.ts +++ b/src/relay/git-handler.ts @@ -1,4 +1,5 @@ /* eslint-disable max-lines -- Why: centralizes the git RPC protocol surface so local and SSH git behavior stay in one dispatch table. */ +import { randomUUID } from 'node:crypto' import { execFile, spawn, type ExecFileOptions } from 'node:child_process' import { promisify } from 'node:util' import * as path from 'node:path' @@ -1132,17 +1133,48 @@ export class GitHandler { this.clearGitMutationReadCaches() const worktreePath = params.worktreePath as string const baseRef = params.baseRef as string + let rebaseRef: string | null = null try { try { const source = await resolveGitRemoteRebaseSource( ((args) => this.git(args, worktreePath)) as GitCommandRunner, baseRef ) - await this.git(['pull', '--rebase', source.remoteName, source.branchName], worktreePath) + let forkPoint: string | null = null + try { + const { stdout } = await this.git( + ['merge-base', '--fork-point', `refs/remotes/${source.displayName}`, 'HEAD'], + worktreePath + ) + forkPoint = stdout.trim() || null + } catch { + // A first fetch or an unhelpful reflog falls back to Git's merge-base behavior. + } + // Why: concurrent fetches can replace FETCH_HEAD and remote-tracking refs between fetch and rebase. + rebaseRef = `refs/orca/rebase/${randomUUID()}` + await this.git( + [ + 'fetch', + source.remoteName, + `+refs/heads/${source.branchName}:${rebaseRef}` + ], + worktreePath + ) + await this.git( + forkPoint ? ['rebase', '--onto', rebaseRef, forkPoint] : ['rebase', rebaseRef], + worktreePath + ) } catch (error) { throw new Error(normalizeGitErrorMessage(error, 'pull')) } } finally { + if (rebaseRef) { + try { + await this.git(['update-ref', '-d', rebaseRef], worktreePath) + } catch { + // Cleanup must not hide the fetch or rebase result. + } + } this.clearGitMutationReadCaches() } }