mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 16:02:32 +00:00
Fix rebase race by fetching to private ref with timeout
Concurrent fetches can interfere with remote-tracking refs between fetch and rebase. Use a unique private ref and 60-second timeout to isolate each rebase operation and prevent hangs on stalled remotes. Extract gitPullRebaseFromBase to a dedicated module.
This commit is contained in:
@@ -0,0 +1,59 @@
|
||||
import { randomUUID } from 'node:crypto'
|
||||
import { normalizeGitErrorMessage } from '../../shared/git-remote-error'
|
||||
import {
|
||||
REBASE_SOURCE_FETCH_TIMEOUT_MS,
|
||||
resolveGitRemoteRebaseSource
|
||||
} from '../../shared/git-rebase-source'
|
||||
import type { GitRuntimeOptions } from './git-runtime-options'
|
||||
import { gitOptionsForWorktree } from './git-runtime-options'
|
||||
import { gitExecFileAsync } from './runner'
|
||||
import { runWithGitReadCacheInvalidation } from './status'
|
||||
|
||||
export async function gitPullRebaseFromBase(
|
||||
worktreePath: string,
|
||||
baseRef: string,
|
||||
options: GitRuntimeOptions = {}
|
||||
): Promise<void> {
|
||||
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(
|
||||
['fetch', source.remoteName, `+refs/heads/${source.branchName}:${rebaseRef}`],
|
||||
{ ...gitOptionsForWorktree(worktreePath, options), timeout: REBASE_SOURCE_FETCH_TIMEOUT_MS }
|
||||
)
|
||||
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.
|
||||
}
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
@@ -8,6 +8,7 @@ vi.mock('./runner', () => ({
|
||||
gitExecFileAsync: gitExecFileAsyncMock
|
||||
}))
|
||||
|
||||
import { REBASE_SOURCE_FETCH_TIMEOUT_MS } from '../../shared/git-rebase-source'
|
||||
import { gitFastForward, gitFetch, gitPull, gitPullRebaseFromBase, gitPush } from './remote'
|
||||
|
||||
describe('git remote operations', () => {
|
||||
@@ -527,12 +528,8 @@ describe('git remote operations', () => {
|
||||
[['check-ref-format', '--branch', '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' }
|
||||
['fetch', 'upstream', expect.stringMatching(/^\+refs\/heads\/main:refs\/orca\/rebase\//)],
|
||||
{ cwd: '/repo', timeout: REBASE_SOURCE_FETCH_TIMEOUT_MS }
|
||||
],
|
||||
[
|
||||
['rebase', '--onto', expect.stringMatching(/^refs\/orca\/rebase\//), 'fork-point'],
|
||||
@@ -579,12 +576,8 @@ describe('git remote operations', () => {
|
||||
|
||||
expect(gitExecFileAsyncMock).toHaveBeenNthCalledWith(
|
||||
4,
|
||||
[
|
||||
'fetch',
|
||||
'upstream',
|
||||
expect.stringMatching(/^\+refs\/heads\/main:refs\/orca\/rebase\//)
|
||||
],
|
||||
{ cwd: '/repo' }
|
||||
['fetch', 'upstream', expect.stringMatching(/^\+refs\/heads\/main:refs\/orca\/rebase\//)],
|
||||
{ cwd: '/repo', timeout: REBASE_SOURCE_FETCH_TIMEOUT_MS }
|
||||
)
|
||||
})
|
||||
|
||||
@@ -602,11 +595,9 @@ describe('git remote operations', () => {
|
||||
)
|
||||
|
||||
const rebasedRef = gitExecFileAsyncMock.mock.calls[4][0][2]
|
||||
expect(gitExecFileAsyncMock).toHaveBeenNthCalledWith(
|
||||
6,
|
||||
['update-ref', '-d', rebasedRef],
|
||||
{ cwd: '/repo' }
|
||||
)
|
||||
expect(gitExecFileAsyncMock).toHaveBeenNthCalledWith(6, ['update-ref', '-d', rebasedRef], {
|
||||
cwd: '/repo'
|
||||
})
|
||||
})
|
||||
|
||||
it('normalizes pull authentication errors to a friendly message', async () => {
|
||||
|
||||
+2
-55
@@ -1,11 +1,9 @@
|
||||
import { randomUUID } from 'node:crypto'
|
||||
import {
|
||||
normalizeGitErrorMessage,
|
||||
runPullWithDivergenceFallback
|
||||
} from '../../shared/git-remote-error'
|
||||
import { resolveEffectiveGitUpstream } from '../../shared/git-effective-upstream'
|
||||
import { gitRefTargetsBranchOnRemote } from '../../shared/git-remote-branch-name'
|
||||
import { resolveGitRemoteRebaseSource } from '../../shared/git-rebase-source'
|
||||
import type { GitPushTarget } from '../../shared/worktree/types'
|
||||
import type { GitRuntimeOptions } from './git-runtime-options'
|
||||
import { gitOptionsForWorktree } from './git-runtime-options'
|
||||
@@ -13,6 +11,8 @@ import { validateGitPushTarget } from './push-target-validation'
|
||||
import { gitExecFileAsync } from './runner'
|
||||
import { runWithGitReadCacheInvalidation } from './status'
|
||||
|
||||
export { gitPullRebaseFromBase } from './remote-rebase'
|
||||
|
||||
async function getConfiguredPushTarget(
|
||||
worktreePath: string,
|
||||
options: GitRuntimeOptions = {}
|
||||
@@ -273,59 +273,6 @@ export async function gitFastForward(
|
||||
)
|
||||
}
|
||||
|
||||
export async function gitPullRebaseFromBase(
|
||||
worktreePath: string,
|
||||
baseRef: string,
|
||||
options: GitRuntimeOptions = {}
|
||||
): Promise<void> {
|
||||
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(
|
||||
[
|
||||
'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.
|
||||
}
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
export async function gitFetch(
|
||||
worktreePath: string,
|
||||
pushTarget?: GitPushTarget,
|
||||
|
||||
@@ -61,7 +61,10 @@ import {
|
||||
import { upstreamOnlyCommitsArePatchEquivalent } from '../shared/git-upstream-status'
|
||||
import { assertGitPushTargetShape } from '../shared/git-push-target-validation'
|
||||
import { getPublishTargetStatus, type GitCommandRunner } from '../shared/git-publish-target-status'
|
||||
import { resolveGitRemoteRebaseSource } from '../shared/git-rebase-source'
|
||||
import {
|
||||
REBASE_SOURCE_FETCH_TIMEOUT_MS,
|
||||
resolveGitRemoteRebaseSource
|
||||
} from '../shared/git-rebase-source'
|
||||
import type { GitPushTarget } from '../shared/worktree/types'
|
||||
import {
|
||||
getEffectiveGitUpstreamStatus,
|
||||
@@ -1153,12 +1156,9 @@ export class GitHandler {
|
||||
// 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
|
||||
['fetch', source.remoteName, `+refs/heads/${source.branchName}:${rebaseRef}`],
|
||||
worktreePath,
|
||||
{ timeout: REBASE_SOURCE_FETCH_TIMEOUT_MS }
|
||||
)
|
||||
await this.git(
|
||||
forkPoint ? ['rebase', '--onto', rebaseRef, forkPoint] : ['rebase', rebaseRef],
|
||||
|
||||
@@ -1,3 +1,6 @@
|
||||
// Why: a stalled remote must fail the rebase fetch, not hang the rebase; client and relay share one bound.
|
||||
export const REBASE_SOURCE_FETCH_TIMEOUT_MS = 60_000
|
||||
|
||||
export type GitCommandRunner = (args: string[]) => Promise<{ stdout: string }>
|
||||
|
||||
export type GitRemoteRebaseSource = {
|
||||
|
||||
Reference in New Issue
Block a user