From bcb86d0778ac46d05b0ce43fd84838ab648c6cbd Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Wed, 20 May 2026 22:10:51 -0700 Subject: [PATCH] Fix pull status for legacy origin/main worktrees (#2487) - Resolve an effective upstream so legacy branches tracking origin/main use origin/ when that remote branch exists. - Pull, sync, and ahead/behind status now operate on the same branch the UI reports, including after non-fast-forward push rejections. --- src/main/git/remote.test.ts | 71 +++++--- src/main/git/remote.ts | 13 ++ src/main/git/status.test.ts | 37 +++- src/main/git/status.ts | 58 ++++++- src/main/git/upstream.test.ts | 62 ++++++- src/main/git/upstream.ts | 51 +----- src/relay/git-handler-status-ops.ts | 49 ++++-- src/relay/git-handler.test.ts | 27 +++ src/relay/git-handler.ts | 57 +++---- src/renderer/src/store/slices/editor.test.ts | 51 +++++- src/renderer/src/store/slices/editor.ts | 18 ++ src/shared/git-effective-upstream.ts | 169 +++++++++++++++++++ 12 files changed, 529 insertions(+), 134 deletions(-) create mode 100644 src/shared/git-effective-upstream.ts diff --git a/src/main/git/remote.test.ts b/src/main/git/remote.test.ts index 3b45cff98..1de8054f5 100644 --- a/src/main/git/remote.test.ts +++ b/src/main/git/remote.test.ts @@ -155,15 +155,42 @@ describe('git remote operations', () => { }) it("runs pull with the user's configured strategy", async () => { - gitExecFileAsyncMock.mockResolvedValue({ stdout: '', stderr: '' }) + gitExecFileAsyncMock + .mockResolvedValueOnce({ stdout: 'feature\n', stderr: '' }) + .mockResolvedValueOnce({ stdout: 'origin/feature\n', stderr: '' }) + .mockResolvedValueOnce({ stdout: '', stderr: '' }) await gitPull('/repo') - expect(gitExecFileAsyncMock).toHaveBeenCalledWith(['pull'], { cwd: '/repo' }) + expect(gitExecFileAsyncMock.mock.calls).toEqual([ + [['symbolic-ref', '--quiet', '--short', 'HEAD'], { cwd: '/repo' }], + [['rev-parse', '--abbrev-ref', 'HEAD@{u}'], { cwd: '/repo' }], + [['pull'], { cwd: '/repo' }] + ]) + }) + + it('pulls the same-name origin branch for legacy base-tracking worktrees', async () => { + gitExecFileAsyncMock + .mockResolvedValueOnce({ stdout: 'feature\n', stderr: '' }) + .mockResolvedValueOnce({ stdout: 'origin/main\n', stderr: '' }) + .mockResolvedValueOnce({ stdout: 'abc123\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' }], + [['rev-parse', '--verify', '--quiet', 'refs/remotes/origin/feature'], { cwd: '/repo' }], + [['pull', 'origin', 'feature'], { cwd: '/repo' }] + ]) }) it('normalizes pull authentication errors to a friendly message', async () => { - gitExecFileAsyncMock.mockRejectedValueOnce(new Error('Authentication failed')) + gitExecFileAsyncMock + .mockResolvedValueOnce({ stdout: 'feature\n', stderr: '' }) + .mockResolvedValueOnce({ stdout: 'origin/feature\n', stderr: '' }) + .mockRejectedValueOnce(new Error('Authentication failed')) await expect(gitPull('/repo')).rejects.toThrow( 'Authentication failed. Check your remote credentials.' @@ -171,15 +198,18 @@ describe('git remote operations', () => { }) it('normalizes pull dirty-worktree aborts to a friendly message', async () => { - gitExecFileAsyncMock.mockRejectedValueOnce( - new Error( - 'Command failed: git pull\n' + - 'error: Your local changes to the following files would be overwritten by merge:\n' + - '\tsrc/app.ts\n' + - 'Please commit your changes or stash them before you merge.\n' + - 'Aborting' + gitExecFileAsyncMock + .mockResolvedValueOnce({ stdout: 'feature\n', stderr: '' }) + .mockResolvedValueOnce({ stdout: 'origin/feature\n', stderr: '' }) + .mockRejectedValueOnce( + new Error( + 'Command failed: git pull\n' + + 'error: Your local changes to the following files would be overwritten by merge:\n' + + '\tsrc/app.ts\n' + + 'Please commit your changes or stash them before you merge.\n' + + 'Aborting' + ) ) - ) await expect(gitPull('/repo')).rejects.toThrow( 'Pull would overwrite local changes. Commit, stash, or discard them before pulling.' @@ -187,15 +217,18 @@ describe('git remote operations', () => { }) it('normalizes pull untracked-file aborts to a friendly message', async () => { - gitExecFileAsyncMock.mockRejectedValueOnce( - new Error( - 'Command failed: git pull\n' + - 'error: The following untracked working tree files would be overwritten by merge:\n' + - '\tsrc/new.ts\n' + - 'Please move or remove them before you merge.\n' + - 'Aborting' + gitExecFileAsyncMock + .mockResolvedValueOnce({ stdout: 'feature\n', stderr: '' }) + .mockResolvedValueOnce({ stdout: 'origin/feature\n', stderr: '' }) + .mockRejectedValueOnce( + new Error( + 'Command failed: git pull\n' + + 'error: The following untracked working tree files would be overwritten by merge:\n' + + '\tsrc/new.ts\n' + + 'Please move or remove them before you merge.\n' + + 'Aborting' + ) ) - ) await expect(gitPull('/repo')).rejects.toThrow( 'Pull would overwrite untracked files. Move, remove, or add them before pulling.' diff --git a/src/main/git/remote.ts b/src/main/git/remote.ts index 8735a8f41..24f16e2d0 100644 --- a/src/main/git/remote.ts +++ b/src/main/git/remote.ts @@ -1,4 +1,5 @@ import { normalizeGitErrorMessage } from '../../shared/git-remote-error' +import { resolveEffectiveGitUpstream } from '../../shared/git-effective-upstream' import type { GitPushTarget } from '../../shared/types' import { validateGitPushTarget } from './push-target-validation' import { gitExecFileAsync } from './runner' @@ -79,6 +80,18 @@ export async function gitPull(worktreePath: string): Promise { // default) so diverged branches reconcile instead of erroring out. Conflicts // surface through the existing conflict-resolution flow. try { + const upstream = await resolveEffectiveGitUpstream((args) => + gitExecFileAsync(args, { cwd: 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 gitExecFileAsync(['pull', upstream.remoteName, upstream.branchName], { + cwd: worktreePath + }) + return + } + await gitExecFileAsync(['pull'], { cwd: worktreePath }) } catch (error) { throw new Error(normalizeGitErrorMessage(error, 'pull')) diff --git a/src/main/git/status.test.ts b/src/main/git/status.test.ts index 10b58838c..cb53a1436 100644 --- a/src/main/git/status.test.ts +++ b/src/main/git/status.test.ts @@ -433,19 +433,46 @@ describe('getStatus', () => { }) }) - it('reports no upstream from porcelain v2 status without an extra git call', async () => { + it('reports no upstream from porcelain v2 status when no same-name origin branch exists', async () => { readFileMock.mockResolvedValue('gitdir: /repo/.git/worktrees/feature\n') existsSyncMock.mockReturnValue(false) - gitExecFileAsyncMock.mockResolvedValueOnce({ - stdout: '# branch.oid abcdef1234567890\n# branch.head feature/prompts\n' - }) + gitExecFileAsyncMock + .mockResolvedValueOnce({ + stdout: '# branch.oid abcdef1234567890\n# branch.head feature/prompts\n' + }) + .mockResolvedValueOnce({ stdout: 'feature/prompts\n' }) + .mockRejectedValueOnce(new Error('fatal: no upstream configured')) + .mockRejectedValueOnce(new Error('missing remote branch')) const result = await getStatus('/repo') - expect(gitExecFileAsyncMock).toHaveBeenCalledTimes(1) + expect(gitExecFileAsyncMock).toHaveBeenCalledTimes(4) expect(result.upstreamStatus).toEqual({ hasUpstream: false, ahead: 0, behind: 0 }) }) + it('uses same-name origin branch status for legacy base-tracking worktrees', async () => { + readFileMock.mockResolvedValue('gitdir: /repo/.git/worktrees/feature\n') + existsSyncMock.mockReturnValue(false) + gitExecFileAsyncMock + .mockResolvedValueOnce({ + stdout: + '# branch.oid abcdef1234567890\n# branch.head feature/prompts\n# branch.upstream origin/main\n# branch.ab +1 -0\n' + }) + .mockResolvedValueOnce({ stdout: 'feature/prompts\n' }) + .mockResolvedValueOnce({ stdout: 'origin/main\n' }) + .mockResolvedValueOnce({ stdout: 'abc123\n' }) + .mockResolvedValueOnce({ stdout: '3\t1\n' }) + + const result = await getStatus('/repo') + + expect(result.upstreamStatus).toEqual({ + hasUpstream: true, + upstreamName: 'origin/feature/prompts', + ahead: 3, + behind: 1 + }) + }) + it('omits --ignored and ignoredPaths when includeIgnored is not requested', async () => { readFileMock.mockResolvedValue('gitdir: /repo/.git/worktrees/feature\n') existsSyncMock.mockReturnValue(false) diff --git a/src/main/git/status.ts b/src/main/git/status.ts index fe933856b..a72dd3832 100644 --- a/src/main/git/status.ts +++ b/src/main/git/status.ts @@ -13,9 +13,14 @@ import type { GitDiffResult, GitFileStatus, GitStatusEntry, - GitStatusResult + GitStatusResult, + GitUpstreamStatus } from '../../shared/types' import type { CommitMessageDraftContext } from '../../shared/commit-message-generation' +import { + getEffectiveGitUpstreamStatus, + splitRemoteBranchName +} from '../../shared/git-effective-upstream' import { gitExecFileAsync, gitExecFileAsyncBuffer, gitOptionalLocksDisabledEnv } from './runner' const MAX_GIT_SHOW_BYTES = 10 * 1024 * 1024 @@ -39,6 +44,7 @@ export async function getStatus( let branch: string | undefined let upstreamName: string | undefined let upstreamAheadBehind: { ahead: number; behind: number } | null = null + let effectiveUpstreamStatus: GitUpstreamStatus | undefined let statusSucceeded = false // Why: detectConflictOperation (4 existsSync + readFile) and git status are @@ -148,6 +154,18 @@ export async function getStatus( } } statusSucceeded = true + + if (shouldProbeEffectiveUpstreamStatus(branch, upstreamName)) { + try { + effectiveUpstreamStatus = await getEffectiveGitUpstreamStatus((args) => + gitExecFileAsync(args, { cwd: worktreePath }) + ) + } catch { + // Why: git status polling should not fail just because the richer + // upstream probe hit a transient ref/read error; the explicit + // upstream-status path will surface those failures when invoked. + } + } } catch { // Not a git repo or git not available } @@ -160,19 +178,41 @@ export async function getStatus( ...(options.includeIgnored ? { ignoredPaths } : {}), ...(statusSucceeded ? { - upstreamStatus: upstreamName - ? { - hasUpstream: true, - upstreamName, - ahead: upstreamAheadBehind?.ahead ?? 0, - behind: upstreamAheadBehind?.behind ?? 0 - } - : { hasUpstream: false, ahead: 0, behind: 0 } + upstreamStatus: + effectiveUpstreamStatus ?? + (upstreamName + ? { + hasUpstream: true, + upstreamName, + ahead: upstreamAheadBehind?.ahead ?? 0, + behind: upstreamAheadBehind?.behind ?? 0 + } + : { hasUpstream: false, ahead: 0, behind: 0 }) } : {}) } } +function getShortBranchName(branch: string | undefined): string | null { + const prefix = 'refs/heads/' + return branch?.startsWith(prefix) ? branch.slice(prefix.length) : null +} + +function shouldProbeEffectiveUpstreamStatus( + branch: string | undefined, + upstreamName: string | undefined +): boolean { + const branchName = getShortBranchName(branch) + if (!branchName) { + return false + } + if (!upstreamName) { + return true + } + const parsed = splitRemoteBranchName(upstreamName) + return parsed?.remoteName === 'origin' && parsed.branchName !== branchName +} + function parseBranchAheadBehind(line: string): { ahead: number; behind: number } | null { const match = line.match(/^# branch\.ab \+(\d+) -(\d+)$/) if (!match) { diff --git a/src/main/git/upstream.test.ts b/src/main/git/upstream.test.ts index 1ec56a434..08d72af8d 100644 --- a/src/main/git/upstream.test.ts +++ b/src/main/git/upstream.test.ts @@ -23,6 +23,7 @@ describe('getUpstreamStatus', () => { it('returns upstream and ahead/behind counts when tracking is configured', async () => { gitExecFileAsyncMock + .mockResolvedValueOnce({ stdout: 'main\n' }) .mockResolvedValueOnce({ stdout: 'origin/main\n' }) .mockResolvedValueOnce({ stdout: '2\t3\n' }) .mockResolvedValueOnce({ stdout: '+ abc123 remote work\n' }) @@ -40,6 +41,7 @@ describe('getUpstreamStatus', () => { it('marks diverged upstream commits as patch-equivalent after a rebase', async () => { gitExecFileAsyncMock + .mockResolvedValueOnce({ stdout: 'feature\n' }) .mockResolvedValueOnce({ stdout: 'origin/feature\n' }) .mockResolvedValueOnce({ stdout: '14\t3\n' }) .mockResolvedValueOnce({ @@ -59,8 +61,42 @@ describe('getUpstreamStatus', () => { }) }) + it('keeps configured local-branch upstreams', async () => { + gitExecFileAsyncMock + .mockResolvedValueOnce({ stdout: 'feature\n' }) + .mockResolvedValueOnce({ stdout: 'main\n' }) + .mockResolvedValueOnce({ stdout: '1\t0\n' }) + + const result = await getUpstreamStatus('/repo') + + expect(result).toEqual({ + hasUpstream: true, + upstreamName: 'main', + ahead: 1, + behind: 0 + }) + }) + + it('returns hasUpstream=false when upstream output is empty', async () => { + gitExecFileAsyncMock + .mockResolvedValueOnce({ stdout: 'feature\n' }) + .mockResolvedValueOnce({ stdout: '\n' }) + .mockRejectedValueOnce(new Error('missing remote branch')) + + const result = await getUpstreamStatus('/repo') + + expect(result).toEqual({ + hasUpstream: false, + ahead: 0, + behind: 0 + }) + }) + it('returns hasUpstream=false when upstream is missing', async () => { - gitExecFileAsyncMock.mockRejectedValueOnce(new Error('fatal: no upstream configured')) + gitExecFileAsyncMock + .mockResolvedValueOnce({ stdout: 'feature\n' }) + .mockRejectedValueOnce(new Error('fatal: no upstream configured')) + .mockRejectedValueOnce(new Error('missing remote branch')) const result = await getUpstreamStatus('/repo') @@ -72,7 +108,10 @@ describe('getUpstreamStatus', () => { }) it('returns hasUpstream=false when the configured tracking ref is missing', async () => { - gitExecFileAsyncMock.mockRejectedValueOnce(missingTrackingRefError) + gitExecFileAsyncMock + .mockResolvedValueOnce({ stdout: 'feature\n' }) + .mockRejectedValueOnce(missingTrackingRefError) + .mockRejectedValueOnce(new Error('missing remote branch')) const result = await getUpstreamStatus('/repo') @@ -82,4 +121,23 @@ describe('getUpstreamStatus', () => { behind: 0 }) }) + + it('uses the same-name origin branch when a legacy worktree tracks origin/main', async () => { + gitExecFileAsyncMock + .mockResolvedValueOnce({ stdout: 'feature\n' }) + .mockResolvedValueOnce({ stdout: 'origin/main\n' }) + .mockResolvedValueOnce({ stdout: 'abc123\n' }) + .mockResolvedValueOnce({ stdout: '3\t1\n' }) + .mockResolvedValueOnce({ stdout: '+ def456 remote work\n' }) + + const result = await getUpstreamStatus('/repo') + + expect(result).toEqual({ + hasUpstream: true, + upstreamName: 'origin/feature', + ahead: 3, + behind: 1, + behindCommitsArePatchEquivalent: false + }) + }) }) diff --git a/src/main/git/upstream.ts b/src/main/git/upstream.ts index 5c144b4f3..92753116f 100644 --- a/src/main/git/upstream.ts +++ b/src/main/git/upstream.ts @@ -1,12 +1,16 @@ import type { GitUpstreamStatus } from '../../shared/types' import { upstreamOnlyCommitsArePatchEquivalent } from '../../shared/git-upstream-status' import { isNoUpstreamError, normalizeGitErrorMessage } from '../../shared/git-remote-error' +import { getEffectiveGitUpstreamStatus } from '../../shared/git-effective-upstream' import { gitExecFileAsync } from './runner' -async function getBehindCommitsArePatchEquivalent(worktreePath: string): Promise { +async function getBehindCommitsArePatchEquivalent( + worktreePath: string, + upstreamName: string +): Promise { try { const { stdout } = await gitExecFileAsync( - ['log', '--oneline', '--cherry-mark', '--right-only', 'HEAD...@{u}', '--'], + ['log', '--oneline', '--cherry-mark', '--right-only', `HEAD...${upstreamName}`, '--'], { cwd: worktreePath } ) return upstreamOnlyCommitsArePatchEquivalent(stdout) @@ -19,47 +23,10 @@ async function getBehindCommitsArePatchEquivalent(worktreePath: string): Promise export async function getUpstreamStatus(worktreePath: string): Promise { try { - const { stdout: upstreamStdout } = await gitExecFileAsync( - ['rev-parse', '--abbrev-ref', 'HEAD@{u}'], - { - cwd: worktreePath - } + return await getEffectiveGitUpstreamStatus( + (args) => gitExecFileAsync(args, { cwd: worktreePath }), + (upstreamName) => getBehindCommitsArePatchEquivalent(worktreePath, upstreamName) ) - const upstreamName = upstreamStdout.trim() - if (!upstreamName) { - return { hasUpstream: false, ahead: 0, behind: 0 } - } - - const { stdout: countsStdout } = await gitExecFileAsync( - ['rev-list', '--left-right', '--count', 'HEAD...@{u}'], - { - cwd: worktreePath - } - ) - - const tokens = countsStdout.trim().split(/\s+/) - if (tokens.length !== 2) { - // Why: 'rev-list --left-right --count HEAD...@{u}' must emit exactly two - // tokens; anything else (empty stdout, truncation, unexpected locale) is a - // real failure and must not be silently reported as "in sync" 0/0. - throw new Error(`Unexpected git rev-list output: ${JSON.stringify(countsStdout)}`) - } - const ahead = Number.parseInt(tokens[0]!, 10) - const behind = Number.parseInt(tokens[1]!, 10) - if (!Number.isFinite(ahead) || !Number.isFinite(behind) || ahead < 0 || behind < 0) { - throw new Error(`Unparseable git rev-list counts: ${JSON.stringify(countsStdout)}`) - } - - const behindCommitsArePatchEquivalent = - ahead > 0 && behind > 0 ? await getBehindCommitsArePatchEquivalent(worktreePath) : undefined - - return { - hasUpstream: true, - upstreamName, - ahead, - behind, - ...(behindCommitsArePatchEquivalent !== undefined ? { behindCommitsArePatchEquivalent } : {}) - } } catch (error) { // Why: we only swallow clearly-no-upstream signals — that's an expected // state, not a failure. Other errors (auth, corruption, "not a git diff --git a/src/relay/git-handler-status-ops.ts b/src/relay/git-handler-status-ops.ts index 6aa721c54..df4815da4 100644 --- a/src/relay/git-handler-status-ops.ts +++ b/src/relay/git-handler-status-ops.ts @@ -10,6 +10,11 @@ import { readFile } from 'fs/promises' import { parseUnmergedEntry } from './git-handler-utils' import { parseStatusOutput } from './git-status-output-parser' import type { GitExec } from './git-handler-ops' +import type { GitUpstreamStatus } from '../shared/types' +import { + getEffectiveGitUpstreamStatus, + splitRemoteBranchName +} from '../shared/git-effective-upstream' export async function resolveGitDir(worktreePath: string): Promise { const dotGitPath = path.join(worktreePath, '.git') @@ -54,12 +59,7 @@ export async function getStatusOp( conflictOperation: string head?: string branch?: string - upstreamStatus?: { - hasUpstream: boolean - upstreamName?: string - ahead: number - behind: number - } + upstreamStatus?: GitUpstreamStatus ignoredPaths?: string[] }> { const worktreePath = params.worktreePath as string @@ -68,14 +68,7 @@ export async function getStatusOp( const entries: Record[] = [] let head: string | undefined let branch: string | undefined - let upstreamStatus: - | { - hasUpstream: boolean - upstreamName?: string - ahead: number - behind: number - } - | undefined + let upstreamStatus: GitUpstreamStatus | undefined let ignoredPaths: string[] = [] try { @@ -105,6 +98,14 @@ export async function getStatusOp( branch = parsed.branch upstreamStatus = parsed.upstreamStatus ignoredPaths = parsed.ignoredPaths + if (shouldProbeEffectiveUpstreamStatus(branch, upstreamStatus?.upstreamName)) { + try { + upstreamStatus = await getEffectiveGitUpstreamStatus((args) => git(args, worktreePath)) + } catch { + // Why: status polling should keep returning working-tree entries even + // if the richer upstream probe hits a transient SSH/git ref error. + } + } for (const uLine of parsed.unmergedLines) { const entry = parseUnmergedEntry(worktreePath, uLine) @@ -126,6 +127,26 @@ export async function getStatusOp( } } +function getShortBranchName(branch: string | undefined): string | null { + const prefix = 'refs/heads/' + return branch?.startsWith(prefix) ? branch.slice(prefix.length) : null +} + +function shouldProbeEffectiveUpstreamStatus( + branch: string | undefined, + upstreamName: string | undefined +): boolean { + const branchName = getShortBranchName(branch) + if (!branchName) { + return false + } + if (!upstreamName) { + return true + } + const parsed = splitRemoteBranchName(upstreamName) + return parsed?.remoteName === 'origin' && parsed.branchName !== branchName +} + function parseCheckIgnoreOutput(stdout: string): string[] { return stdout.split(/\r?\n/).filter(Boolean) } diff --git a/src/relay/git-handler.test.ts b/src/relay/git-handler.test.ts index f1150eb24..1e504a524 100644 --- a/src/relay/git-handler.test.ts +++ b/src/relay/git-handler.test.ts @@ -570,6 +570,33 @@ describe('GitHandler', () => { } }) + it('reports ahead/behind counts against a configured local-branch upstream', async () => { + gitInit(tmpDir) + writeFileSync(path.join(tmpDir, 'base.txt'), 'base') + gitCommit(tmpDir, 'initial') + const baseRef = execFileSync('git', ['rev-parse', '--abbrev-ref', 'HEAD'], { + cwd: tmpDir, + encoding: 'utf-8' + }).trim() + + execFileSync('git', ['checkout', '-b', 'feature'], { cwd: tmpDir, stdio: 'pipe' }) + execFileSync('git', ['branch', '--set-upstream-to', baseRef], { + cwd: tmpDir, + stdio: 'pipe' + }) + writeFileSync(path.join(tmpDir, 'feature.txt'), 'feature') + gitCommit(tmpDir, 'feature commit') + + const result = (await dispatcher.callRequest('git.upstreamStatus', { + worktreePath: tmpDir + })) as { hasUpstream: boolean; upstreamName?: string; ahead: number; behind: number } + + expect(result.hasUpstream).toBe(true) + expect(result.upstreamName).toBe(baseRef) + expect(result.ahead).toBe(1) + expect(result.behind).toBe(0) + }) + it('fetches from a configured remote without throwing', async () => { const bareDir = mkdtempSync(path.join(tmpdir(), 'relay-git-bare-')) try { diff --git a/src/relay/git-handler.ts b/src/relay/git-handler.ts index 839cfd18a..5aa806553 100644 --- a/src/relay/git-handler.ts +++ b/src/relay/git-handler.ts @@ -20,6 +20,10 @@ import { checkIgnoredPathsOp, detectConflictOperation, getStatusOp } from './git import { resolveRelayPushTarget } from './git-handler-push-target' import { normalizeGitErrorMessage, isNoUpstreamError } from '../shared/git-remote-error' import { upstreamOnlyCommitsArePatchEquivalent } from '../shared/git-upstream-status' +import { + getEffectiveGitUpstreamStatus, + resolveEffectiveGitUpstream +} from '../shared/git-effective-upstream' import { loadGitHistoryFromExecutor } from '../shared/git-history' import { buildRelayCommandEnv } from './relay-command-env' @@ -289,43 +293,10 @@ export class GitHandler { const worktreePath = params.worktreePath as string try { - const { stdout: upstreamStdout } = await this.git( - ['rev-parse', '--abbrev-ref', 'HEAD@{u}'], - worktreePath + return await getEffectiveGitUpstreamStatus( + (args) => this.git(args, worktreePath), + (upstreamName) => this.getBehindCommitsArePatchEquivalent(worktreePath, upstreamName) ) - const upstreamName = upstreamStdout.trim() - if (!upstreamName) { - return { hasUpstream: false, ahead: 0, behind: 0 } - } - const { stdout: countsStdout } = await this.git( - ['rev-list', '--left-right', '--count', 'HEAD...@{u}'], - worktreePath - ) - const tokens = countsStdout.trim().split(/\s+/) - if (tokens.length !== 2) { - // Why: 'rev-list --left-right --count HEAD...@{u}' must emit exactly two - // tokens; anything else (empty stdout, SSH transport truncation, unexpected - // locale) is a real failure and must not be silently reported as "in sync" 0/0. - throw new Error(`Unexpected git rev-list output: ${JSON.stringify(countsStdout)}`) - } - const ahead = Number.parseInt(tokens[0]!, 10) - const behind = Number.parseInt(tokens[1]!, 10) - if (!Number.isFinite(ahead) || !Number.isFinite(behind) || ahead < 0 || behind < 0) { - throw new Error(`Unparseable git rev-list counts: ${JSON.stringify(countsStdout)}`) - } - const behindCommitsArePatchEquivalent = - ahead > 0 && behind > 0 - ? await this.getBehindCommitsArePatchEquivalent(worktreePath) - : undefined - return { - hasUpstream: true, - upstreamName, - ahead, - behind, - ...(behindCommitsArePatchEquivalent !== undefined - ? { behindCommitsArePatchEquivalent } - : {}) - } } catch (error) { // Why: we only swallow the 'no upstream configured' error — that's an // expected state, not a failure. Other errors (auth, corruption, network) @@ -339,10 +310,13 @@ export class GitHandler { } } - private async getBehindCommitsArePatchEquivalent(worktreePath: string): Promise { + private async getBehindCommitsArePatchEquivalent( + worktreePath: string, + upstreamName: string + ): Promise { try { const { stdout } = await this.git( - ['log', '--oneline', '--cherry-mark', '--right-only', 'HEAD...@{u}', '--'], + ['log', '--oneline', '--cherry-mark', '--right-only', `HEAD...${upstreamName}`, '--'], worktreePath ) return upstreamOnlyCommitsArePatchEquivalent(stdout) @@ -395,6 +369,13 @@ export class GitHandler { // Why: plain `git pull` uses the user's configured pull strategy (merge by // default) so diverged branches reconcile instead of erroring out. try { + 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', upstream.remoteName, upstream.branchName], worktreePath) + return + } await this.git(['pull'], worktreePath) } catch (error) { // Why: mirror the local gitPull normalization so SSH users see the same diff --git a/src/renderer/src/store/slices/editor.test.ts b/src/renderer/src/store/slices/editor.test.ts index c205e61ed..8e29012a2 100644 --- a/src/renderer/src/store/slices/editor.test.ts +++ b/src/renderer/src/store/slices/editor.test.ts @@ -51,6 +51,11 @@ function createEditorTabsStore(): StoreApi { })) as unknown as StoreApi } +async function flushAsyncRemoteRefresh(): Promise { + await Promise.resolve() + await Promise.resolve() +} + function ownedEditorFileId( filePath: string, worktreeId: string, @@ -1446,7 +1451,7 @@ describe('createEditorSlice remote branch actions', () => { expect(store.getState().isRemoteOperationActive).toBe(false) }) - it('preserves actionable publish errors and avoids refresh on failure', async () => { + it('preserves actionable publish errors and refreshes upstream after rejection', async () => { const store = createEditorStore() const publishError = new Error( 'Push rejected: remote has newer commits (non-fast-forward). Please pull or sync first.' @@ -1460,8 +1465,17 @@ describe('createEditorSlice remote branch actions', () => { expect(toastErrorMock).toHaveBeenCalledWith( 'Push rejected — remote has changes. Pull first, then try again.' ) + await flushAsyncRemoteRefresh() + expect(gitStatusMock).not.toHaveBeenCalled() - expect(gitUpstreamStatusMock).not.toHaveBeenCalled() + expect(gitFetchMock).toHaveBeenCalledWith({ + worktreePath: '/repo', + connectionId: undefined + }) + expect(gitUpstreamStatusMock).toHaveBeenCalledWith({ + worktreePath: '/repo', + connectionId: undefined + }) expect(store.getState().isRemoteOperationActive).toBe(false) }) @@ -1479,8 +1493,17 @@ describe('createEditorSlice remote branch actions', () => { expect(toastErrorMock).toHaveBeenCalledWith( 'Push rejected — remote has changes. Pull first, then try again.' ) + await flushAsyncRemoteRefresh() + expect(gitStatusMock).not.toHaveBeenCalled() - expect(gitUpstreamStatusMock).not.toHaveBeenCalled() + expect(gitFetchMock).toHaveBeenCalledWith({ + worktreePath: '/repo', + connectionId: undefined + }) + expect(gitUpstreamStatusMock).toHaveBeenCalledWith({ + worktreePath: '/repo', + connectionId: undefined + }) expect(store.getState().isRemoteOperationActive).toBe(false) }) @@ -1531,8 +1554,17 @@ describe('createEditorSlice remote branch actions', () => { expect(toastErrorMock).toHaveBeenCalledWith( 'Push rejected — remote has changes. Pull first, then try again.' ) + await flushAsyncRemoteRefresh() + expect(gitStatusMock).not.toHaveBeenCalled() - expect(gitUpstreamStatusMock).not.toHaveBeenCalled() + expect(gitFetchMock).toHaveBeenCalledWith({ + worktreePath: '/repo', + connectionId: undefined + }) + expect(gitUpstreamStatusMock).toHaveBeenCalledWith({ + worktreePath: '/repo', + connectionId: undefined + }) expect(store.getState().isRemoteOperationActive).toBe(false) }) @@ -1548,8 +1580,17 @@ describe('createEditorSlice remote branch actions', () => { expect(toastErrorMock).toHaveBeenCalledWith( 'Push rejected — remote has changes. Pull first, then try again.' ) + await flushAsyncRemoteRefresh() + expect(gitStatusMock).not.toHaveBeenCalled() - expect(gitUpstreamStatusMock).not.toHaveBeenCalled() + expect(gitFetchMock).toHaveBeenCalledWith({ + worktreePath: '/repo', + connectionId: undefined + }) + expect(gitUpstreamStatusMock).toHaveBeenCalledWith({ + worktreePath: '/repo', + connectionId: undefined + }) expect(store.getState().isRemoteOperationActive).toBe(false) }) diff --git a/src/renderer/src/store/slices/editor.ts b/src/renderer/src/store/slices/editor.ts index 92bc27d89..122d65409 100644 --- a/src/renderer/src/store/slices/editor.ts +++ b/src/renderer/src/store/slices/editor.ts @@ -825,6 +825,13 @@ function extractPublishFailureDetail(message: string): string | null { return null } +function isNonFastForwardRemoteError(error: unknown): boolean { + return ( + error instanceof Error && + /non-fast-forward|fetch first|updates were rejected/i.test(error.message) + ) +} + export function resolveRemoteOperationErrorMessage( error: unknown, options?: { publish?: boolean; isPush?: boolean; isSync?: boolean; isFetch?: boolean } @@ -2695,16 +2702,27 @@ export const createEditorSlice: StateCreator = (s // as fire-and-forget so it doesn't block the mutation but updates the // store as soon as the IPC resolves. get().beginRemoteOperation(publish ? 'publish' : 'push') + let shouldRefreshAfterRejectedPush = false try { await pushRuntimeGit( { settings: get().settings, worktreeId, worktreePath, connectionId }, { publish, pushTarget, forceWithLease: options.forceWithLease } ) } catch (error) { + shouldRefreshAfterRejectedPush = isNonFastForwardRemoteError(error) toast.error(resolveRemoteOperationErrorMessage(error, { publish, isPush: true })) throw error } finally { get().endRemoteOperation() + if (shouldRefreshAfterRejectedPush) { + const context = { settings: get().settings, worktreeId, worktreePath, connectionId } + // Why: the rejected push proved the publish branch moved. Fetch first + // so legacy base-tracking worktrees can discover origin/, then + // refresh ahead/behind so Pull/Sync become actionable immediately. + void fetchRuntimeGit(context) + .catch(() => undefined) + .then(() => get().fetchUpstreamStatus(worktreeId, worktreePath, connectionId)) + } } void get().fetchUpstreamStatus(worktreeId, worktreePath, connectionId) const refreshGitHubForWorktree = get().refreshGitHubForWorktree diff --git a/src/shared/git-effective-upstream.ts b/src/shared/git-effective-upstream.ts new file mode 100644 index 000000000..ad3e09325 --- /dev/null +++ b/src/shared/git-effective-upstream.ts @@ -0,0 +1,169 @@ +import { isNoUpstreamError } from './git-remote-error' +import type { GitUpstreamStatus } from './types' + +export type GitCommandRunner = (args: string[]) => Promise<{ stdout: string }> + +export type EffectiveGitUpstream = + | { + upstreamName: string + remoteName: string | null + branchName: string + isConfiguredUpstream: true + } + | { + upstreamName: string + remoteName: string + branchName: string + isConfiguredUpstream: false + } + +export function splitRemoteBranchName(refName: string): { + remoteName: string + branchName: string +} | null { + const slashIndex = refName.indexOf('/') + if (slashIndex <= 0 || slashIndex === refName.length - 1) { + return null + } + return { + remoteName: refName.slice(0, slashIndex), + branchName: refName.slice(slashIndex + 1) + } +} + +async function getCurrentBranchName(runGit: GitCommandRunner): Promise { + try { + const { stdout } = await runGit(['symbolic-ref', '--quiet', '--short', 'HEAD']) + const branchName = stdout.trim() + return branchName || null + } catch { + return null + } +} + +async function getConfiguredUpstream( + runGit: GitCommandRunner +): Promise { + try { + const { stdout } = await runGit(['rev-parse', '--abbrev-ref', 'HEAD@{u}']) + const upstreamName = stdout.trim() + if (!upstreamName) { + return null + } + const parsed = splitRemoteBranchName(upstreamName) + if (!parsed) { + return { + upstreamName, + remoteName: null, + branchName: upstreamName, + isConfiguredUpstream: true + } + } + return { + upstreamName, + remoteName: parsed.remoteName, + branchName: parsed.branchName, + isConfiguredUpstream: true + } + } catch (error) { + if (isNoUpstreamError(error)) { + return null + } + throw error + } +} + +async function remoteTrackingRefExists( + runGit: GitCommandRunner, + remoteName: string, + branchName: string +): Promise { + try { + await runGit(['rev-parse', '--verify', '--quiet', `refs/remotes/${remoteName}/${branchName}`]) + return true + } catch { + return false + } +} + +export async function resolveEffectiveGitUpstream( + runGit: GitCommandRunner +): Promise { + const currentBranchName = await getCurrentBranchName(runGit) + const configured = await getConfiguredUpstream(runGit) + + if (configured) { + if (!currentBranchName || configured.branchName === currentBranchName) { + return configured + } + + // Why: older Orca worktrees inherited origin/main as their upstream even + // though pushes target origin/. If that same-name remote + // exists, source-control pull/sync must follow the publish branch rather + // than the base branch. + if ( + configured.remoteName === 'origin' && + (await remoteTrackingRefExists(runGit, configured.remoteName, currentBranchName)) + ) { + return { + upstreamName: `${configured.remoteName}/${currentBranchName}`, + remoteName: configured.remoteName, + branchName: currentBranchName, + isConfiguredUpstream: false + } + } + + return configured + } + + if (currentBranchName && (await remoteTrackingRefExists(runGit, 'origin', currentBranchName))) { + return { + upstreamName: `origin/${currentBranchName}`, + remoteName: 'origin', + branchName: currentBranchName, + isConfiguredUpstream: false + } + } + + return null +} + +export async function getEffectiveGitUpstreamStatus( + runGit: GitCommandRunner, + getBehindCommitsArePatchEquivalent?: (upstreamName: string) => Promise +): Promise { + const upstream = await resolveEffectiveGitUpstream(runGit) + if (!upstream) { + return { hasUpstream: false, ahead: 0, behind: 0 } + } + + const { stdout } = await runGit([ + 'rev-list', + '--left-right', + '--count', + `HEAD...${upstream.upstreamName}` + ]) + const tokens = stdout.trim().split(/\s+/) + if (tokens.length !== 2) { + throw new Error(`Unexpected git rev-list output: ${JSON.stringify(stdout)}`) + } + + const ahead = Number.parseInt(tokens[0]!, 10) + const behind = Number.parseInt(tokens[1]!, 10) + if (!Number.isFinite(ahead) || !Number.isFinite(behind) || ahead < 0 || behind < 0) { + throw new Error(`Unparseable git rev-list counts: ${JSON.stringify(stdout)}`) + } + + const behindCommitsArePatchEquivalent = + ahead > 0 && behind > 0 && getBehindCommitsArePatchEquivalent + ? await getBehindCommitsArePatchEquivalent(upstream.upstreamName) + : undefined + + return { + hasUpstream: true, + upstreamName: upstream.upstreamName, + ahead, + behind, + ...(behindCommitsArePatchEquivalent !== undefined ? { behindCommitsArePatchEquivalent } : {}) + } +}