From 3d557fc9393c8f5e1c1d534eeeabf0286ccaff2d Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Mon, 13 Jul 2026 01:39:31 -0700 Subject: [PATCH] Fall back to a merge when a divergent pull has no reconciliation strateg - Git 2.27+ refuses `git pull` on divergent branches unless pull.rebase or pull.ff is configured. Retry with `--no-rebase` (Git's historical default) so pulls succeed out of the box on fresh hosts. - Skip the fallback whenever the caller already specified a reconciliation strategy (e.g. --ff-only, --rebase) so explicit policies still fail as expected on divergence. - Applied identically in the local git pull path and the relay/SSH git handler so both surfaces behave the same way. --- src/main/git/remote.test.ts | 39 +++++++++++ src/main/git/remote.ts | 31 +++++++-- .../git-handler-pull-reconciliation.test.ts | 28 ++++++-- src/relay/git-handler.ts | 67 +++++++++++++------ src/shared/git-remote-error.test.ts | 43 +++++++++++- src/shared/git-remote-error.ts | 25 +++++++ 6 files changed, 202 insertions(+), 31 deletions(-) diff --git a/src/main/git/remote.test.ts b/src/main/git/remote.test.ts index 166db56079d..32231d7cc36 100644 --- a/src/main/git/remote.test.ts +++ b/src/main/git/remote.test.ts @@ -363,6 +363,45 @@ describe('git remote operations', () => { ]) }) + it('retries a divergent 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: plain pull rejects with git's reconciliation error. + .mockResolvedValueOnce({ stdout: 'feature\n', stderr: '' }) + .mockResolvedValueOnce({ stdout: 'origin/feature\n', stderr: '' }) + .mockRejectedValueOnce(divergentError) + // Fallback attempt: pull --no-rebase (merge) succeeds. + .mockResolvedValueOnce({ stdout: 'feature\n', stderr: '' }) + .mockResolvedValueOnce({ stdout: 'origin/feature\n', stderr: '' }) + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + + await gitPull('/repo') + + expect(gitExecFileAsyncMock.mock.calls).toEqual([ + [['symbolic-ref', '--quiet', '--short', 'HEAD'], { cwd: '/repo' }], + [['rev-parse', '--abbrev-ref', 'HEAD@{u}'], { cwd: '/repo' }], + [['pull'], { cwd: '/repo' }], + [['symbolic-ref', '--quiet', '--short', 'HEAD'], { cwd: '/repo' }], + [['rev-parse', '--abbrev-ref', 'HEAD@{u}'], { cwd: '/repo' }], + [['pull', '--no-rebase'], { cwd: '/repo' }] + ]) + }) + + it('does not retry a fast-forward-only pull that fails on divergence', async () => { + gitExecFileAsyncMock + .mockResolvedValueOnce({ stdout: 'feature\n', stderr: '' }) + .mockResolvedValueOnce({ stdout: 'origin/feature\n', stderr: '' }) + .mockRejectedValueOnce( + new Error('Command failed: git pull\nfatal: Not possible to fast-forward, aborting.') + ) + + await expect(gitFastForward('/repo')).rejects.toThrow('Not possible to fast-forward') + // No fallback attempt: only the three probe/pull calls ran. + expect(gitExecFileAsyncMock.mock.calls).toHaveLength(3) + }) + 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 ab7fba775f9..bbb5d479505 100644 --- a/src/main/git/remote.ts +++ b/src/main/git/remote.ts @@ -1,4 +1,9 @@ -import { normalizeGitErrorMessage } from '../../shared/git-remote-error' +import { + MERGE_RECONCILIATION_PULL_ARGS, + isDivergentPullReconciliationError, + normalizeGitErrorMessage, + pullArgsSpecifyReconciliation +} 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' @@ -214,11 +219,11 @@ async function gitPullWithArgs( pushTarget?: GitPushTarget, options: GitRuntimeOptions = {} ): Promise { - try { + const runPull = async (effectiveArgs: string[]): Promise => { if (pushTarget) { const target = await validateGitPushTarget(worktreePath, pushTarget, options) await gitExecFileAsync( - ['pull', ...pullArgs, target.remoteName, target.branchName], + ['pull', ...effectiveArgs, target.remoteName, target.branchName], gitOptionsForWorktree(worktreePath, options) ) return @@ -230,13 +235,29 @@ async function gitPullWithArgs( // Why: legacy Orca branches may still track origin/main while pushes // target origin/. Pull the same effective branch the UI reports. await gitExecFileAsync( - ['pull', ...pullArgs, upstream.remoteName, upstream.branchName], + ['pull', ...effectiveArgs, upstream.remoteName, upstream.branchName], gitOptionsForWorktree(worktreePath, options) ) return } - await gitExecFileAsync(['pull', ...pullArgs], gitOptionsForWorktree(worktreePath, options)) + await gitExecFileAsync(['pull', ...effectiveArgs], gitOptionsForWorktree(worktreePath, options)) + } + + 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 + } } catch (error) { throw new Error(normalizeGitErrorMessage(error, 'pull')) } diff --git a/src/relay/git-handler-pull-reconciliation.test.ts b/src/relay/git-handler-pull-reconciliation.test.ts index 1820faa0fec..5db00d16202 100644 --- a/src/relay/git-handler-pull-reconciliation.test.ts +++ b/src/relay/git-handler-pull-reconciliation.test.ts @@ -86,13 +86,33 @@ describe('GitHandler pull reconciliation', () => { return consumerDir } - it('explains how to configure a pull policy when divergent branches have no strategy', async () => { + it('falls back to a merge when divergent branches have no configured strategy', async () => { const consumerDir = createDivergentFixture() - await expect(dispatcher.callRequest('git.pull', { worktreePath: consumerDir })).rejects.toThrow( - 'Pull needs a Git pull policy for divergent branches. Configure one for this repository' - ) + await expect( + dispatcher.callRequest('git.pull', { worktreePath: consumerDir }) + ).resolves.not.toThrow() + // Merge reconciliation yields a two-parent commit and both sides' files. + const parentRefs = execGit(consumerDir, ['log', '-1', '--pretty=%P']).trim().split(/\s+/) + expect(parentRefs).toHaveLength(2) + expect(existsSync(path.join(consumerDir, 'remote.txt'))).toBe(true) + expect(existsSync(path.join(consumerDir, 'local.txt'))).toBe(true) + expect(execGit(consumerDir, ['status', '--short'])).toBe('') + }, 15_000) + + it('preserves configured fast-forward-only pull semantics on divergent branches', async () => { + const consumerDir = createDivergentFixture() + execGit(consumerDir, ['config', 'pull.ff', 'only']) + + // Why: an explicit ff-only policy must still fail on divergence rather than + // getting silently reconciled by the merge fallback. + await expect( + dispatcher.callRequest('git.pull', { worktreePath: consumerDir }) + ).rejects.toThrow() + + const parentRefs = execGit(consumerDir, ['log', '-1', '--pretty=%P']).trim().split(/\s+/) + expect(parentRefs).toHaveLength(1) expect(execGit(consumerDir, ['status', '--short'])).toBe('') }, 15_000) diff --git a/src/relay/git-handler.ts b/src/relay/git-handler.ts index c6709cd3f2a..bb353258af6 100644 --- a/src/relay/git-handler.ts +++ b/src/relay/git-handler.ts @@ -44,7 +44,13 @@ import { gitExecMutatesRepository } from '../shared/git-exec-mutation' import { detectConflictOperation, getStatusOp } from './git-handler-status-ops' import { checkIgnoredPathsOp } from './git-handler-check-ignore' import { resolveRelayPushTarget } from './git-handler-push-target' -import { isNoUpstreamError, normalizeGitErrorMessage } from '../shared/git-remote-error' +import { + MERGE_RECONCILIATION_PULL_ARGS, + isDivergentPullReconciliationError, + isNoUpstreamError, + normalizeGitErrorMessage, + pullArgsSpecifyReconciliation +} from '../shared/git-remote-error' 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' @@ -1021,29 +1027,48 @@ export class GitHandler { private async pullWithArgs(params: Record, pullArgs: string[]) { this.clearGitMutationReadCaches() const worktreePath = params.worktreePath as string + const runPull = async (effectiveArgs: string[]): Promise => { + if (params.pushTarget !== undefined) { + assertGitPushTargetShape(params.pushTarget) + const pushTarget = params.pushTarget as GitPushTarget + await this.git(['check-ref-format', '--branch', pushTarget.branchName], worktreePath) + await this.git( + ['pull', ...effectiveArgs, pushTarget.remoteName, pushTarget.branchName], + worktreePath + ) + return + } + const upstream = await resolveEffectiveGitUpstream((args) => this.git(args, worktreePath)) + if (upstream && !upstream.isConfiguredUpstream) { + // Why: legacy Orca branches may still track origin/main while pushes + // target origin/. Pull the same effective branch the UI reports. + await this.git( + ['pull', ...effectiveArgs, upstream.remoteName, upstream.branchName], + worktreePath + ) + return + } + await this.git(['pull', ...effectiveArgs], worktreePath) + } + try { try { - if (params.pushTarget !== undefined) { - assertGitPushTargetShape(params.pushTarget) - const pushTarget = params.pushTarget as GitPushTarget - await this.git(['check-ref-format', '--branch', pushTarget.branchName], worktreePath) - await this.git( - ['pull', ...pullArgs, pushTarget.remoteName, pushTarget.branchName], - worktreePath - ) - return + 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 } - const upstream = await resolveEffectiveGitUpstream((args) => this.git(args, worktreePath)) - if (upstream && !upstream.isConfiguredUpstream) { - // Why: legacy Orca branches may still track origin/main while pushes - // target origin/. Pull the same effective branch the UI reports. - await this.git( - ['pull', ...pullArgs, upstream.remoteName, upstream.branchName], - worktreePath - ) - return - } - await this.git(['pull', ...pullArgs], worktreePath) } 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 d76d386db11..7dc83f941ce 100644 --- a/src/shared/git-remote-error.test.ts +++ b/src/shared/git-remote-error.test.ts @@ -1,8 +1,10 @@ import { afterEach, describe, expect, it, vi } from 'vitest' import { formatSubmodulePushFailureDetail, + isDivergentPullReconciliationError, isNoUpstreamError, - normalizeGitErrorMessage + normalizeGitErrorMessage, + pullArgsSpecifyReconciliation } from './git-remote-error' afterEach(() => { @@ -147,3 +149,42 @@ describe('isNoUpstreamError', () => { expect(isNoUpstreamError(error)).toBe(false) }) }) + +describe('isDivergentPullReconciliationError', () => { + it('detects git 2.27+ divergent-branch reconciliation failures', () => { + const error = 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.' + ) + + expect(isDivergentPullReconciliationError(error)).toBe(true) + }) + + it('does not match a fast-forward-only abort on divergent branches', () => { + const error = new Error( + 'Command failed: git pull\nfatal: Not possible to fast-forward, aborting.' + ) + + expect(isDivergentPullReconciliationError(error)).toBe(false) + }) + + it('returns false for non-Error values', () => { + expect(isDivergentPullReconciliationError('divergent branches')).toBe(false) + }) +}) + +describe('pullArgsSpecifyReconciliation', () => { + it('is false when no strategy flag is present', () => { + expect(pullArgsSpecifyReconciliation([])).toBe(false) + expect(pullArgsSpecifyReconciliation(['origin', 'main'])).toBe(false) + }) + + it('is true for any explicit reconciliation flag', () => { + expect(pullArgsSpecifyReconciliation(['--ff-only'])).toBe(true) + expect(pullArgsSpecifyReconciliation(['--rebase'])).toBe(true) + expect(pullArgsSpecifyReconciliation(['--no-rebase'])).toBe(true) + expect(pullArgsSpecifyReconciliation(['--rebase=interactive'])).toBe(true) + expect(pullArgsSpecifyReconciliation(['-r'])).toBe(true) + }) +}) diff --git a/src/shared/git-remote-error.ts b/src/shared/git-remote-error.ts index 42cf210292f..c48ba34c8af 100644 --- a/src/shared/git-remote-error.ts +++ b/src/shared/git-remote-error.ts @@ -20,6 +20,15 @@ const NORMALIZED_SUBMODULE_PUSH_FAILURE_PATTERN = /(?:^|:\s)((?:Submodule '[^'\n]+'|A submodule) (?:has remote changes\. Pull inside the submodule, then try again\.|could not be pushed\. Resolve the submodule push error, then try again\.))(?:$|\s)/i const DIVERGENT_PULL_RECONCILIATION_PATTERN = /Need to specify how to reconcile divergent branches|divergent branches and need to specify how to reconcile them/i +// Why: any of these already pin a reconciliation strategy, so the merge +// fallback must not override an explicit caller/user choice (e.g. --ff-only). +const RECONCILIATION_PULL_ARG_PATTERN = + /^(--rebase|--no-rebase|--ff-only|--ff|--no-ff|--merge|-r)(=|$)/ +// Why: merge is Git's historical pre-2.27 default. Falling back to it lets +// divergent pulls reconcile on fresh hosts that never configured pull.rebase +// or pull.ff, instead of failing outright. `--no-rebase` predates the 2.25 +// baseline, so it is safe across all supported Git binaries. +export const MERGE_RECONCILIATION_PULL_ARGS = ['--no-rebase'] export function stripCredentialsFromMessage(message: string): string { return message.replace(USERPASS_URL_PATTERN, '$1').replace(HTTPS_TOKEN_URL_PATTERN, '$1') @@ -81,6 +90,22 @@ function* iterateLinesFromEnd(value: string): Generator { yield value.slice(0, lineEnd) } +// Why: a fresh host may lack any pull.rebase/pull.ff policy, so Git 2.27+ +// refuses to reconcile divergent branches. Detect that specific failure so the +// caller can retry with an explicit merge instead of surfacing a config error. +export function isDivergentPullReconciliationError(error: unknown): boolean { + if (!(error instanceof Error)) { + return false + } + return DIVERGENT_PULL_RECONCILIATION_PATTERN.test(stripCredentialsFromMessage(error.message)) +} + +// Whether the pull already specifies how to reconcile (rebase/merge/ff-only), +// in which case the caller's choice must win over the merge fallback. +export function pullArgsSpecifyReconciliation(pullArgs: string[]): boolean { + return pullArgs.some((arg) => RECONCILIATION_PULL_ARG_PATTERN.test(arg)) +} + export type GitRemoteOperation = 'push' | 'pull' | 'fetch' | 'upstream' export function normalizeGitErrorMessage(error: unknown, operation?: GitRemoteOperation): string {