diff --git a/src/main/git/remote.test.ts b/src/main/git/remote.test.ts index 32231d7cc36..61b95d0e50d 100644 --- a/src/main/git/remote.test.ts +++ b/src/main/git/remote.test.ts @@ -402,6 +402,51 @@ describe('git remote operations', () => { expect(gitExecFileAsyncMock.mock.calls).toHaveLength(3) }) + it('retries a divergent pushTarget pull as a merge when no strategy is configured', async () => { + const divergentError = new Error( + 'Command failed: git pull\n' + 'fatal: Need to specify how to reconcile divergent branches.' + ) + gitExecFileAsyncMock + // First attempt: validate the target, then the plain pull rejects. + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + .mockRejectedValueOnce(divergentError) + // Fallback attempt: re-validate, then pull --no-rebase (merge) succeeds. + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + + await gitPull('/repo', { remoteName: 'fork', branchName: 'feature/fix' }) + + // The merge flag is spliced ahead of the positional remote/branch args. + expect(gitExecFileAsyncMock.mock.calls).toEqual([ + [['check-ref-format', '--branch', 'feature/fix'], { cwd: '/repo' }], + [['pull', 'fork', 'feature/fix'], { cwd: '/repo' }], + [['check-ref-format', '--branch', 'feature/fix'], { cwd: '/repo' }], + [['pull', '--no-rebase', 'fork', 'feature/fix'], { cwd: '/repo' }] + ]) + }) + + it('surfaces a normalized error and does not loop when the merge fallback itself fails', async () => { + const divergentError = new Error( + 'Command failed: git pull\n' + 'fatal: Need to specify how to reconcile divergent branches.' + ) + const mergeConflictError = new Error( + 'Command failed: git pull --no-rebase\nCONFLICT (content): Merge conflict in file.txt' + ) + gitExecFileAsyncMock + // First attempt fails with the reconciliation error. + .mockResolvedValueOnce({ stdout: 'feature\n', stderr: '' }) + .mockResolvedValueOnce({ stdout: 'origin/feature\n', stderr: '' }) + .mockRejectedValueOnce(divergentError) + // The single merge fallback then fails on a real conflict. + .mockResolvedValueOnce({ stdout: 'feature\n', stderr: '' }) + .mockResolvedValueOnce({ stdout: 'origin/feature\n', stderr: '' }) + .mockRejectedValueOnce(mergeConflictError) + + await expect(gitPull('/repo')).rejects.toThrow() + // At-most-once retry: probe+pull, then probe+fallback-pull — no further attempts. + expect(gitExecFileAsyncMock.mock.calls).toHaveLength(6) + }) + it('pulls the same-name origin branch for legacy base-tracking worktrees', async () => { gitExecFileAsyncMock .mockResolvedValueOnce({ stdout: 'feature\n', stderr: '' }) diff --git a/src/main/git/remote.ts b/src/main/git/remote.ts index bbb5d479505..5c3f40cafe0 100644 --- a/src/main/git/remote.ts +++ b/src/main/git/remote.ts @@ -1,8 +1,6 @@ import { - MERGE_RECONCILIATION_PULL_ARGS, - isDivergentPullReconciliationError, normalizeGitErrorMessage, - pullArgsSpecifyReconciliation + runPullWithDivergenceFallback } from '../../shared/git-remote-error' import { resolveEffectiveGitUpstream } from '../../shared/git-effective-upstream' import { gitRefTargetsBranchOnRemote } from '../../shared/git-remote-branch-name' @@ -245,19 +243,7 @@ async function gitPullWithArgs( } try { - try { - await runPull(pullArgs) - } catch (error) { - // Why: on hosts with no pull.rebase/pull.ff policy, Git 2.27+ refuses to - // reconcile divergent branches. Retry as a merge (Git's historical - // default) so pulls succeed out of the box; callers that already forced a - // strategy — or users who configured rebase — never reach this fallback. - if (!pullArgsSpecifyReconciliation(pullArgs) && isDivergentPullReconciliationError(error)) { - await runPull([...MERGE_RECONCILIATION_PULL_ARGS, ...pullArgs]) - return - } - throw error - } + await runPullWithDivergenceFallback(pullArgs, runPull) } catch (error) { throw new Error(normalizeGitErrorMessage(error, 'pull')) } diff --git a/src/relay/git-handler.ts b/src/relay/git-handler.ts index bb353258af6..510dd60f84b 100644 --- a/src/relay/git-handler.ts +++ b/src/relay/git-handler.ts @@ -45,11 +45,9 @@ import { detectConflictOperation, getStatusOp } from './git-handler-status-ops' import { checkIgnoredPathsOp } from './git-handler-check-ignore' import { resolveRelayPushTarget } from './git-handler-push-target' import { - MERGE_RECONCILIATION_PULL_ARGS, - isDivergentPullReconciliationError, isNoUpstreamError, normalizeGitErrorMessage, - pullArgsSpecifyReconciliation + runPullWithDivergenceFallback } from '../shared/git-remote-error' import { upstreamOnlyCommitsArePatchEquivalent } from '../shared/git-upstream-status' import { assertGitPushTargetShape } from '../shared/git-push-target-validation' @@ -1053,22 +1051,7 @@ export class GitHandler { try { try { - try { - await runPull(pullArgs) - } catch (error) { - // Why: on hosts with no pull.rebase/pull.ff policy, Git 2.27+ refuses - // to reconcile divergent branches. Retry as a merge (Git's historical - // default) so SSH pulls succeed out of the box; forced strategies and - // rebase-configured users never reach this fallback. - if ( - !pullArgsSpecifyReconciliation(pullArgs) && - isDivergentPullReconciliationError(error) - ) { - await runPull([...MERGE_RECONCILIATION_PULL_ARGS, ...pullArgs]) - return - } - throw error - } + await runPullWithDivergenceFallback(pullArgs, runPull) } catch (error) { // Why: mirror the local gitPull normalization so SSH users see the same // actionable messages instead of raw git stderr. diff --git a/src/shared/git-remote-error.test.ts b/src/shared/git-remote-error.test.ts index 7dc83f941ce..0a1e6be56e6 100644 --- a/src/shared/git-remote-error.test.ts +++ b/src/shared/git-remote-error.test.ts @@ -3,8 +3,10 @@ import { formatSubmodulePushFailureDetail, isDivergentPullReconciliationError, isNoUpstreamError, + MERGE_RECONCILIATION_PULL_ARGS, normalizeGitErrorMessage, - pullArgsSpecifyReconciliation + pullArgsSpecifyReconciliation, + runPullWithDivergenceFallback } from './git-remote-error' afterEach(() => { @@ -188,3 +190,46 @@ describe('pullArgsSpecifyReconciliation', () => { expect(pullArgsSpecifyReconciliation(['-r'])).toBe(true) }) }) + +describe('runPullWithDivergenceFallback', () => { + const divergentError = new Error( + 'Command failed: git pull\n' + + 'hint: You have divergent branches and need to specify how to reconcile them.\n' + + 'fatal: Need to specify how to reconcile divergent branches.' + ) + + it('retries with merge reconciliation args on a policy error', async () => { + const calls: string[][] = [] + const runPull = vi.fn(async (effectiveArgs: string[]) => { + calls.push(effectiveArgs) + if (effectiveArgs.length === 0) { + throw divergentError + } + }) + + await runPullWithDivergenceFallback([], runPull) + + expect(calls).toEqual([[], [...MERGE_RECONCILIATION_PULL_ARGS]]) + expect(runPull).toHaveBeenCalledTimes(2) + }) + + it('does not retry when pull args already specify reconciliation', async () => { + const runPull = vi.fn(async () => { + throw divergentError + }) + + await expect(runPullWithDivergenceFallback(['--ff-only'], runPull)).rejects.toBe(divergentError) + expect(runPull).toHaveBeenCalledTimes(1) + expect(runPull).toHaveBeenCalledWith(['--ff-only']) + }) + + it('rethrows non-divergence errors without retrying', async () => { + const otherError = new Error('fatal: Not possible to fast-forward, aborting.') + const runPull = vi.fn(async () => { + throw otherError + }) + + await expect(runPullWithDivergenceFallback([], runPull)).rejects.toBe(otherError) + expect(runPull).toHaveBeenCalledTimes(1) + }) +}) diff --git a/src/shared/git-remote-error.ts b/src/shared/git-remote-error.ts index c48ba34c8af..d2171f8a6a2 100644 --- a/src/shared/git-remote-error.ts +++ b/src/shared/git-remote-error.ts @@ -106,6 +106,28 @@ export function pullArgsSpecifyReconciliation(pullArgs: string[]): boolean { return pullArgs.some((arg) => RECONCILIATION_PULL_ARG_PATTERN.test(arg)) } +// Why: on hosts with no pull.rebase/pull.ff policy, Git 2.27+ refuses to +// reconcile divergent branches. Retry as a merge (Git's historical default) so +// pulls succeed out of the box; callers that already forced a strategy — or +// users who configured rebase — never reach this fallback. +// Not routed through GitCapabilityCache: this is per-repo config/branch state, +// not a stable host capability, and only fires on an actual divergence error, +// so there is nothing host-scoped to cache. +export async function runPullWithDivergenceFallback( + pullArgs: string[], + runPull: (effectiveArgs: string[]) => Promise +): Promise { + try { + await runPull(pullArgs) + } catch (error) { + if (!pullArgsSpecifyReconciliation(pullArgs) && isDivergentPullReconciliationError(error)) { + await runPull([...MERGE_RECONCILIATION_PULL_ARGS, ...pullArgs]) + return + } + throw error + } +} + export type GitRemoteOperation = 'push' | 'pull' | 'fetch' | 'upstream' export function normalizeGitErrorMessage(error: unknown, operation?: GitRemoteOperation): string {