From 91991d61864de3075cb02e08db951cea301b6c1f Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 26 May 2026 18:28:41 -0700 Subject: [PATCH] Improve refresh local base ref UX (#2570) Co-authored-by: Orca --- config/scripts/rebuild-native-deps.mjs | 37 ++++ src/main/git/worktree.test.ts | 102 ++++++++--- src/main/git/worktree.ts | 161 +++++++++++------- src/main/ipc/worktree-remote.ts | 140 ++++++++++----- src/main/ipc/worktrees.test.ts | 98 +++++++++++ src/main/runtime/orca-runtime.ts | 81 ++++----- .../src/components/settings/GitPane.tsx | 34 +++- .../src/components/settings/git-search.ts | 13 +- .../hooks/useVirtualizedScrollAnchor.test.ts | 2 +- .../src/hooks/useVirtualizedScrollAnchor.ts | 8 +- .../hooks/virtualizedScrollOffsetRestore.ts | 6 + .../src/store/slices/worktrees.test.ts | 72 ++++++++ src/renderer/src/store/slices/worktrees.ts | 29 ++++ src/shared/types.ts | 8 + 14 files changed, 610 insertions(+), 181 deletions(-) create mode 100644 src/renderer/src/hooks/virtualizedScrollOffsetRestore.ts diff --git a/config/scripts/rebuild-native-deps.mjs b/config/scripts/rebuild-native-deps.mjs index 3e73aafdd..8bc79ce0f 100644 --- a/config/scripts/rebuild-native-deps.mjs +++ b/config/scripts/rebuild-native-deps.mjs @@ -45,6 +45,8 @@ const NATIVE_MODULES = ['better-sqlite3', 'node-pty', 'cpu-features'] const onlyModules = NATIVE_MODULES.filter((m) => !ignoreModules.includes(m)) const forceRebuild = process.env.ORCA_FORCE_NATIVE_REBUILD === '1' +ensureElectronPackageInstalled() + if (!forceRebuild) { // Why: Windows cannot unlink a loaded .node DLL, so avoid @electron/rebuild // when the current install already works with Electron's ABI. @@ -123,6 +125,37 @@ try { process.exit(1) } +function ensureElectronPackageInstalled() { + try { + require('electron') + return + } catch (/** @type {any} */ err) { + if (!isElectronPackageInstallError(err)) { + throw err + } + } + + // Why: CI has observed Electron's postinstall exiting cleanly without + // writing path.txt; native rebuild and tests both need the binary path. + console.log('[rebuild] Electron package binary is missing; rerunning Electron install.') + try { + execFileSync(process.execPath, [require.resolve('electron/install.js')], { + cwd: projectDir, + stdio: 'inherit' + }) + } catch (/** @type {any} */ err) { + console.error('[rebuild] Electron install retry failed:', err?.message ?? err) + process.exit(1) + } + + try { + require('electron') + } catch (/** @type {any} */ err) { + console.error('[rebuild] Electron package is still unavailable after retry:', err?.message ?? err) + process.exit(1) + } +} + function probeElectronNativeModules(moduleNames) { let electronExecutable try { @@ -217,6 +250,10 @@ function isPostinstall() { return process.env.npm_lifecycle_event === 'postinstall' } +function isElectronPackageInstallError(error) { + return /Electron failed to install correctly/i.test(formatError(error)) +} + function formatError(error) { return error instanceof Error ? error.message : String(error) } diff --git a/src/main/git/worktree.test.ts b/src/main/git/worktree.test.ts index 1653ed3b6..3c9d885a8 100644 --- a/src/main/git/worktree.test.ts +++ b/src/main/git/worktree.test.ts @@ -289,7 +289,7 @@ describe('addWorktree', () => { await expect( addWorktree('/repo', '/repo-feature', 'feature/test', 'origin/main') - ).resolves.toBeUndefined() + ).resolves.toEqual({}) expect(warnSpy).toHaveBeenCalledWith( 'addWorktree: failed to set branch.feature/test.base for /repo-feature', @@ -314,7 +314,7 @@ describe('addWorktree', () => { await expect( addWorktree('/repo', '/repo-feature', 'feature/test', 'origin/main') - ).resolves.toBeUndefined() + ).resolves.toEqual({}) expect(warnSpy).toHaveBeenCalledWith( 'addWorktree: failed to set push.autoSetupRemote for /repo-feature', @@ -336,7 +336,7 @@ describe('addWorktree', () => { await expect( addWorktree('/repo', '/repo-feature', 'feature/test', 'origin/main') - ).resolves.toBeUndefined() + ).resolves.toEqual({}) expect(warnSpy).toHaveBeenCalledWith( 'addWorktree: failed to set push.autoSetupRemote for /repo-feature', @@ -481,11 +481,11 @@ describe('addWorktree', () => { const worktreeListOutput = 'worktree /repo\nHEAD abc123\nbranch refs/heads/main\n\nworktree /repo-other\nHEAD def456\nbranch refs/heads/feature\n' gitExecFileAsyncMock + .mockResolvedValueOnce({ stdout: 'abc123\n' }) // rev-parse refs/remotes/origin/main^{commit} .mockResolvedValueOnce({ stdout: '' }) // merge-base --is-ancestor .mockResolvedValueOnce({ stdout: worktreeListOutput }) // worktree list --porcelain .mockResolvedValueOnce({ stdout: '' }) // status --porcelain (in /repo) .mockResolvedValueOnce({ stdout: '' }) // reset --hard (in /repo) - .mockResolvedValueOnce({ stdout: 'abc123\n' }) // rev-parse refs/remotes/origin/main^{commit} .mockResolvedValueOnce({ stdout: '' }) // worktree add .mockResolvedValueOnce({ stdout: '' }) // config --local --replace-all branch..base .mockRejectedValueOnce(Object.assign(new Error('key unset'), { code: 1 })) // config --get push.autoSetupRemote (unset) @@ -494,11 +494,11 @@ describe('addWorktree', () => { await addWorktree('/repo', '/repo-feature', 'feature/test', 'origin/main', true) expect(gitExecFileAsyncMock.mock.calls).toEqual([ - [['merge-base', '--is-ancestor', 'main', 'origin/main'], { cwd: '/repo' }], + [['rev-parse', '--verify', '--quiet', 'refs/remotes/origin/main^{commit}'], { cwd: '/repo' }], + [['merge-base', '--is-ancestor', 'main', 'refs/remotes/origin/main'], { cwd: '/repo' }], [['worktree', 'list', '--porcelain'], { cwd: '/repo' }], [['status', '--porcelain', '--untracked-files=no'], { cwd: '/repo' }], - [['reset', '--hard', 'origin/main'], { cwd: '/repo' }], - [['rev-parse', '--verify', '--quiet', 'refs/remotes/origin/main^{commit}'], { cwd: '/repo' }], + [['reset', '--hard', 'refs/remotes/origin/main'], { cwd: '/repo' }], [ [ 'worktree', @@ -530,11 +530,11 @@ describe('addWorktree', () => { const worktreeListOutput = 'worktree /repo\nHEAD abc123\nbranch refs/heads/develop\n\nworktree /repo-main-wt\nHEAD def456\nbranch refs/heads/main\n' gitExecFileAsyncMock + .mockResolvedValueOnce({ stdout: 'abc123\n' }) // rev-parse refs/remotes/origin/main^{commit} .mockResolvedValueOnce({ stdout: '' }) // merge-base --is-ancestor .mockResolvedValueOnce({ stdout: worktreeListOutput }) // worktree list --porcelain .mockResolvedValueOnce({ stdout: '' }) // status --porcelain (in /repo-main-wt) .mockResolvedValueOnce({ stdout: '' }) // reset --hard (in /repo-main-wt) - .mockResolvedValueOnce({ stdout: 'abc123\n' }) // rev-parse refs/remotes/origin/main^{commit} .mockResolvedValueOnce({ stdout: '' }) // worktree add .mockResolvedValueOnce({ stdout: '' }) // config --local --replace-all branch..base .mockRejectedValueOnce(Object.assign(new Error('key unset'), { code: 1 })) // config --get push.autoSetupRemote (unset) @@ -542,12 +542,12 @@ describe('addWorktree', () => { await addWorktree('/repo', '/repo-feature', 'feature/test', 'origin/main', true) - expect(gitExecFileAsyncMock.mock.calls[2]).toEqual([ + expect(gitExecFileAsyncMock.mock.calls[3]).toEqual([ ['status', '--porcelain', '--untracked-files=no'], expect.objectContaining({ cwd: '/repo-main-wt' }) ]) - expect(gitExecFileAsyncMock.mock.calls[3]).toEqual([ - ['reset', '--hard', 'origin/main'], + expect(gitExecFileAsyncMock.mock.calls[4]).toEqual([ + ['reset', '--hard', 'refs/remotes/origin/main'], expect.objectContaining({ cwd: '/repo-main-wt' }) ]) }) @@ -555,10 +555,10 @@ describe('addWorktree', () => { it('uses update-ref when localBranch is not checked out in any worktree', async () => { const worktreeListOutput = 'worktree /repo\nHEAD abc123\nbranch refs/heads/develop\n' gitExecFileAsyncMock + .mockResolvedValueOnce({ stdout: 'abc123\n' }) // rev-parse refs/remotes/origin/main^{commit} .mockResolvedValueOnce({ stdout: '' }) // merge-base --is-ancestor .mockResolvedValueOnce({ stdout: worktreeListOutput }) // worktree list --porcelain .mockResolvedValueOnce({ stdout: '' }) // update-ref - .mockResolvedValueOnce({ stdout: 'abc123\n' }) // rev-parse refs/remotes/origin/main^{commit} .mockResolvedValueOnce({ stdout: '' }) // worktree add .mockResolvedValueOnce({ stdout: '' }) // config --local --replace-all branch..base .mockRejectedValueOnce(Object.assign(new Error('key unset'), { code: 1 })) // config --get push.autoSetupRemote (unset) @@ -566,8 +566,8 @@ describe('addWorktree', () => { await addWorktree('/repo', '/repo-feature', 'feature/test', 'origin/main', true) - expect(gitExecFileAsyncMock.mock.calls[2]).toEqual([ - ['update-ref', 'refs/heads/main', 'origin/main'], + expect(gitExecFileAsyncMock.mock.calls[3]).toEqual([ + ['update-ref', 'refs/heads/main', 'refs/remotes/origin/main'], expect.objectContaining({ cwd: '/repo' }) ]) }) @@ -575,20 +575,27 @@ describe('addWorktree', () => { it('skips update when the owning worktree is dirty', async () => { const worktreeListOutput = 'worktree /repo\nHEAD abc123\nbranch refs/heads/main\n' gitExecFileAsyncMock + .mockResolvedValueOnce({ stdout: 'abc123\n' }) // rev-parse refs/remotes/origin/main^{commit} .mockResolvedValueOnce({ stdout: '' }) // merge-base --is-ancestor .mockResolvedValueOnce({ stdout: worktreeListOutput }) // worktree list --porcelain .mockResolvedValueOnce({ stdout: ' M package.json\n' }) // status --porcelain (dirty) - .mockResolvedValueOnce({ stdout: 'abc123\n' }) // rev-parse refs/remotes/origin/main^{commit} .mockResolvedValueOnce({ stdout: '' }) // worktree add .mockResolvedValueOnce({ stdout: '' }) // config --local --replace-all branch..base .mockRejectedValueOnce(Object.assign(new Error('key unset'), { code: 1 })) // config --get push.autoSetupRemote (unset) .mockResolvedValueOnce({ stdout: '' }) // config --local set push.autoSetupRemote - await addWorktree('/repo', '/repo-feature', 'feature/test', 'origin/main', true) + const result = await addWorktree('/repo', '/repo-feature', 'feature/test', 'origin/main', true) - // No reset --hard or update-ref — just merge-base, worktree list, status, worktree add, base config, config --get, config --local set + expect(result.localBaseRefRefresh).toEqual({ + status: 'skipped_dirty_worktree', + baseRef: 'origin/main', + localBranch: 'main', + ownerWorktreePath: '/repo' + }) + + // No reset --hard or update-ref — just base resolution, merge-base, worktree list, status, worktree add, base config, config --get, config --local set expect(gitExecFileAsyncMock.mock.calls).toHaveLength(8) - expect(gitExecFileAsyncMock.mock.calls[3]?.[0]).toEqual([ + expect(gitExecFileAsyncMock.mock.calls[0]?.[0]).toEqual([ 'rev-parse', '--verify', '--quiet', @@ -624,8 +631,8 @@ describe('addWorktree', () => { }) it('skips updating the local branch when it has diverged', async () => { - gitExecFileAsyncMock.mockRejectedValueOnce(new Error('not a fast-forward')) gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: 'abc123\n' }) // rev-parse refs/remotes/origin/main^{commit} + gitExecFileAsyncMock.mockRejectedValueOnce(new Error('not a fast-forward')) gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: '' }) // worktree add resolveCreationBaseConfigWrite() gitExecFileAsyncMock.mockRejectedValueOnce(Object.assign(new Error('key unset'), { code: 1 })) // config --get push.autoSetupRemote (unset) @@ -635,11 +642,11 @@ describe('addWorktree', () => { expect(gitExecFileAsyncMock.mock.calls).toEqual([ [ - ['merge-base', '--is-ancestor', 'main', 'origin/main'], + ['rev-parse', '--verify', '--quiet', 'refs/remotes/origin/main^{commit}'], expect.objectContaining({ cwd: '/repo' }) ], [ - ['rev-parse', '--verify', '--quiet', 'refs/remotes/origin/main^{commit}'], + ['merge-base', '--is-ancestor', 'main', 'refs/remotes/origin/main'], expect.objectContaining({ cwd: '/repo' }) ], [ @@ -739,14 +746,55 @@ describe('addWorktree', () => { ]) }) + it('does not report a local base refresh for slash-containing local branch names', async () => { + gitExecFileAsyncMock.mockRejectedValueOnce(new Error('no remote ref')) // rev-parse refs/remotes/release/main^{commit} + gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: 'abc123\n' }) // rev-parse refs/heads/release/main^{commit} + gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: '' }) // worktree add + resolveCreationBaseConfigWrite() + gitExecFileAsyncMock.mockRejectedValueOnce(Object.assign(new Error('key unset'), { code: 1 })) // config --get push.autoSetupRemote (unset) + gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: '' }) // config --local set push.autoSetupRemote + + const result = await addWorktree( + '/repo', + '/repo-feature', + 'feature/release', + 'release/main', + true + ) + + expect(result).toEqual({}) + expect(gitExecFileAsyncMock.mock.calls.map((call) => call[0])).toEqual([ + ['rev-parse', '--verify', '--quiet', 'refs/remotes/release/main^{commit}'], + ['rev-parse', '--verify', '--quiet', 'refs/heads/release/main^{commit}'], + [ + 'worktree', + 'add', + '--no-track', + '-b', + 'feature/release', + '/repo-feature', + 'refs/heads/release/main' + ], + [ + 'config', + '--local', + '--replace-all', + 'branch.feature/release.base', + 'refs/heads/release/main' + ], + ['config', '--get', 'push.autoSetupRemote'], + ['config', '--local', 'push.autoSetupRemote', 'true'] + ]) + }) + it('uses the remote name from the base ref instead of hardcoding origin', async () => { const worktreeListOutput = 'worktree /repo\nHEAD abc123\nbranch refs/heads/main\n' gitExecFileAsyncMock + .mockResolvedValueOnce({ stdout: 'abc123\n' }) // rev-parse refs/remotes/upstream/main^{commit} .mockResolvedValueOnce({ stdout: '' }) // merge-base --is-ancestor .mockResolvedValueOnce({ stdout: worktreeListOutput }) // worktree list --porcelain .mockResolvedValueOnce({ stdout: '' }) // status --porcelain .mockResolvedValueOnce({ stdout: '' }) // reset --hard - .mockResolvedValueOnce({ stdout: 'abc123\n' }) // rev-parse refs/remotes/upstream/main^{commit} .mockResolvedValueOnce({ stdout: '' }) // worktree add .mockResolvedValueOnce({ stdout: '' }) // config --local --replace-all branch..base .mockRejectedValueOnce(Object.assign(new Error('key unset'), { code: 1 })) // config --get push.autoSetupRemote (unset) @@ -754,13 +802,17 @@ describe('addWorktree', () => { await addWorktree('/repo', '/repo-feature', 'feature/test', 'upstream/main', true) - expect(gitExecFileAsyncMock.mock.calls[0]?.[0]).toEqual([ + expect(gitExecFileAsyncMock.mock.calls[1]?.[0]).toEqual([ 'merge-base', '--is-ancestor', 'main', - 'upstream/main' + 'refs/remotes/upstream/main' + ]) + expect(gitExecFileAsyncMock.mock.calls[4]?.[0]).toEqual([ + 'reset', + '--hard', + 'refs/remotes/upstream/main' ]) - expect(gitExecFileAsyncMock.mock.calls[3]?.[0]).toEqual(['reset', '--hard', 'upstream/main']) }) it('unsets branch base config during sparse setup cleanup after creation succeeds', async () => { diff --git a/src/main/git/worktree.ts b/src/main/git/worktree.ts index 714b10347..318584e97 100644 --- a/src/main/git/worktree.ts +++ b/src/main/git/worktree.ts @@ -2,11 +2,15 @@ import { stat } from 'fs/promises' import { join, posix, win32 } from 'path' import { resolveWorktreeAddBaseRef } from '../../shared/worktree-base-ref' -import type { GitWorktreeInfo } from '../../shared/types' +import type { GitWorktreeInfo, LocalBaseRefRefreshResult } from '../../shared/types' import { gitExecFileAsync, translateWslOutputPaths } from './runner' import { resolveGitDir } from './status' import { hasWorktreeBaseCommitRef } from './worktree-base-ref-probe' +export type AddWorktreeResult = { + localBaseRefRefresh?: LocalBaseRefRefreshResult +} + type SparseWorktreeCreateError = Error & { cleanupFailed?: boolean } @@ -192,6 +196,84 @@ export async function listWorktrees(repoPath: string): Promise { + const remoteRefPrefix = 'refs/remotes/' + if (!remoteTrackingRef.startsWith(remoteRefPrefix)) { + return undefined + } + + // Why: Only refs proven to be remote-tracking refs get refresh status. + // Local branches can contain slashes (e.g. release/2026) and must not + // produce a fake "Local 2026 was not refreshed" warning. + const shortRemoteRef = remoteTrackingRef.slice(remoteRefPrefix.length) + const slashIndex = shortRemoteRef.indexOf('/') + if (slashIndex <= 0) { + return undefined + } + + const localBranch = shortRemoteRef.slice(slashIndex + 1) + const fullRef = `refs/heads/${localBranch}` + const resultBase = { baseRef: baseBranch, localBranch } + + try { + // Why: We only fast-forward the local branch pointer. A force-move (`branch -f`) + // would silently destroy unpushed local commits if the branch has diverged from + // remote. `merge-base --is-ancestor` returns exit 0 when localBranch is an + // ancestor of baseBranch — i.e. the update is a safe fast-forward. + await gitExecFileAsync(['merge-base', '--is-ancestor', localBranch, remoteTrackingRef], { + cwd: repoPath + }) + } catch { + // merge-base fails if the local branch doesn't exist or has diverged. + return { ...resultBase, status: 'skipped_not_fast_forward' } + } + + try { + // Why: We need to find which worktree (if any) has localBranch checked + // out, because moving the ref without updating that worktree's files would + // leave it looking massively dirty. A sibling worktree we don't control is + // just as vulnerable as the primary one. + const { stdout: worktreeListOutput } = await gitExecFileAsync( + ['worktree', 'list', '--porcelain'], + { cwd: repoPath } + ) + const worktrees = parseWorktreeList(translateWslOutputPaths(worktreeListOutput, repoPath)) + const ownerWorktree = worktrees.find((wt) => wt.branch === fullRef) + + if (ownerWorktree) { + // Why: localBranch is checked out in a worktree. We can only safely + // update if that worktree is clean, and we must use `reset --hard` + // (run inside that worktree) so the files move with the ref. + const { stdout: status } = await gitExecFileAsync( + ['status', '--porcelain', '--untracked-files=no'], + { cwd: ownerWorktree.path } + ) + if (status.trim()) { + return { + ...resultBase, + status: 'skipped_dirty_worktree', + ownerWorktreePath: ownerWorktree.path + } + } + await gitExecFileAsync(['reset', '--hard', remoteTrackingRef], { cwd: ownerWorktree.path }) + return { ...resultBase, status: 'updated', ownerWorktreePath: ownerWorktree.path } + } + + // Why: localBranch is not checked out anywhere, so there is no working + // tree to desync. `update-ref` is safe here. + await gitExecFileAsync(['update-ref', fullRef, remoteTrackingRef], { cwd: repoPath }) + return { ...resultBase, status: 'updated' } + } catch { + // update-ref/reset can fail on locked refs, filesystem errors, or unusual + // worktree states. Worktree creation should still proceed. + return { ...resultBase, status: 'skipped_error' } + } +} + /** * Create a new worktree. * @param repoPath - Path to the main repo (or bare repo) @@ -211,61 +293,8 @@ export async function addWorktree( refreshLocalBaseRef = false, noCheckout = false, options: { checkoutExistingBranch?: boolean } = {} -): Promise { - // Why: Some users want Orca-created worktrees to make plain commands like - // `git diff main...HEAD` work out of the box, while others do not want - // worktree creation to mutate their local main/master ref at all. Keep this - // behavior behind an explicit setting so the default stays conservative. - if (baseBranch && refreshLocalBaseRef && !options.checkoutExistingBranch) { - // Why: We split on '/' instead of matching a hardcoded 'origin/' prefix because - // callers may pass arbitrary remotes (e.g. 'upstream/main'), not just 'origin'. - const slashIndex = baseBranch.indexOf('/') - if (slashIndex > 0) { - const localBranch = baseBranch.slice(slashIndex + 1) - try { - // Why: We only fast-forward the local branch pointer. A force-move (`branch -f`) - // would silently destroy unpushed local commits if the branch has diverged from - // remote. `merge-base --is-ancestor` returns exit 0 when localBranch is an - // ancestor of baseBranch — i.e. the update is a safe fast-forward. - await gitExecFileAsync(['merge-base', '--is-ancestor', localBranch, baseBranch], { - cwd: repoPath - }) - // Why: We need to find which worktree (if any) has localBranch checked - // out, because moving the ref without updating that worktree's files would - // leave it looking massively dirty. A sibling worktree we don't control is - // just as vulnerable as the primary one. - const { stdout: worktreeListOutput } = await gitExecFileAsync( - ['worktree', 'list', '--porcelain'], - { cwd: repoPath } - ) - const worktrees = parseWorktreeList(translateWslOutputPaths(worktreeListOutput, repoPath)) - const fullRef = `refs/heads/${localBranch}` - const ownerWorktree = worktrees.find((wt) => wt.branch === fullRef) - - if (ownerWorktree) { - // Why: localBranch is checked out in a worktree. We can only safely - // update if that worktree is clean, and we must use `reset --hard` - // (run inside that worktree) so the files move with the ref. - const { stdout: status } = await gitExecFileAsync( - ['status', '--porcelain', '--untracked-files=no'], - { cwd: ownerWorktree.path } - ) - if (!status.trim()) { - await gitExecFileAsync(['reset', '--hard', baseBranch], { cwd: ownerWorktree.path }) - } - } else { - // Why: localBranch is not checked out anywhere, so there is no working - // tree to desync. `update-ref` is safe here. - await gitExecFileAsync(['update-ref', fullRef, baseBranch], { cwd: repoPath }) - } - } catch { - // merge-base fails if the local branch doesn't exist or has diverged; - // update-ref fails on locked/corrupted refs or filesystem errors. - // Both cases are non-fatal — skip the update silently. - } - } - } - +): Promise { + let localBaseRefRefresh: LocalBaseRefRefreshResult | undefined const args = ['worktree', 'add'] let effectiveBase: string | undefined if (noCheckout) { @@ -285,13 +314,24 @@ export async function addWorktree( effectiveBase = await resolveWorktreeAddBaseRef(baseBranch, (qualifiedRef) => hasWorktreeBaseCommitRef(repoPath, qualifiedRef) ) + // Why: resolving the creation base first distinguishes real + // remote-tracking refs from slash-containing local branch names. + // The mutation stays behind the explicit setting so the default + // remains conservative. + if (refreshLocalBaseRef) { + localBaseRefRefresh = await refreshLocalBaseRefForWorktreeCreate( + repoPath, + baseBranch, + effectiveBase + ) + } args.push(effectiveBase) } } await gitExecFileAsync(args, { cwd: repoPath }) if (options.checkoutExistingBranch) { - return + return localBaseRefRefresh ? { localBaseRefRefresh } : {} } if (effectiveBase) { @@ -354,6 +394,7 @@ export async function addWorktree( } catch (error) { console.warn(`addWorktree: failed to set push.autoSetupRemote for ${worktreePath}`, error) } + return localBaseRefRefresh ? { localBaseRefRefresh } : {} } export async function addSparseWorktree( @@ -364,10 +405,11 @@ export async function addSparseWorktree( baseBranch?: string, refreshLocalBaseRef = false, options: { checkoutExistingBranch?: boolean } = {} -): Promise { +): Promise { let created = false + let addResult: AddWorktreeResult = {} try { - await addWorktree( + addResult = await addWorktree( repoPath, worktreePath, branch, @@ -380,6 +422,7 @@ export async function addSparseWorktree( await gitExecFileAsync(['sparse-checkout', 'init', '--cone'], { cwd: worktreePath }) await gitExecFileAsync(['sparse-checkout', 'set', '--', ...directories], { cwd: worktreePath }) await gitExecFileAsync(['checkout', branch], { cwd: worktreePath }) + return addResult } catch (error) { const wrapped: SparseWorktreeCreateError = error instanceof Error ? (error as SparseWorktreeCreateError) : new Error(String(error)) diff --git a/src/main/ipc/worktree-remote.ts b/src/main/ipc/worktree-remote.ts index 5e736feb5..08166162e 100644 --- a/src/main/ipc/worktree-remote.ts +++ b/src/main/ipc/worktree-remote.ts @@ -16,11 +16,13 @@ import type { CreateWorktreeArgs, CreateWorktreeResult, GitPushTarget, + LocalBaseRefRefreshResult, Repo, WorktreeMeta } from '../../shared/types' import { getPRForBranch } from '../github/client' -import { listWorktrees, addWorktree, addSparseWorktree } from '../git/worktree' +import { listWorktrees, addWorktree, addSparseWorktree, parseWorktreeList } from '../git/worktree' +import type { AddWorktreeResult } from '../git/worktree' import { getGitUsername, getDefaultBaseRef, getBranchConflictKind } from '../git/repo' import { validateGitPushTarget } from '../git/push-target-validation' import { assertGitPushTargetShape } from '../../shared/git-push-target-validation' @@ -635,6 +637,57 @@ async function resolveRemoteTrackingBaseSsh( } } +async function refreshLocalBaseRefForRemoteWorktreeCreate( + provider: SshGitProvider, + repoPath: string, + remoteTrackingBase: RemoteTrackingBase +): Promise { + const resultBase = { + baseRef: remoteTrackingBase.base, + localBranch: remoteTrackingBase.branch + } + const fullRef = `refs/heads/${remoteTrackingBase.branch}` + + try { + await provider.exec( + ['merge-base', '--is-ancestor', remoteTrackingBase.branch, remoteTrackingBase.ref], + repoPath + ) + } catch { + return { ...resultBase, status: 'skipped_not_fast_forward' } + } + + try { + const { stdout: worktreeListOutput } = await provider.exec( + ['worktree', 'list', '--porcelain'], + repoPath + ) + const worktrees = parseWorktreeList(worktreeListOutput) + const ownerWorktree = worktrees.find((wt) => wt.branch === fullRef) + + if (ownerWorktree) { + const { stdout: status } = await provider.exec( + ['status', '--porcelain', '--untracked-files=no'], + ownerWorktree.path + ) + if (status.trim()) { + return { + ...resultBase, + status: 'skipped_dirty_worktree', + ownerWorktreePath: ownerWorktree.path + } + } + await provider.exec(['reset', '--hard', remoteTrackingBase.ref], ownerWorktree.path) + return { ...resultBase, status: 'updated', ownerWorktreePath: ownerWorktree.path } + } + + await provider.exec(['update-ref', fullRef, remoteTrackingBase.ref], repoPath) + return { ...resultBase, status: 'updated' } + } catch { + return { ...resultBase, status: 'skipped_error' } + } +} + export function notifyWorktreesChanged(mainWindow: BrowserWindow, repoId: string): void { if (!mainWindow.isDestroyed()) { mainWindow.webContents.send('worktrees:changed', { repoId }) @@ -779,6 +832,10 @@ export async function createRemoteWorktree( /* best-effort */ } } + const localBaseRefRefresh = + settings.refreshLocalBaseRefOnWorktreeCreate && !checkoutExistingBranch && remoteTrackingBase + ? await refreshLocalBaseRefForRemoteWorktreeCreate(provider, repo.path, remoteTrackingBase) + : undefined const fsProvider = getSshFilesystemProvider(repo.connectionId!) if (fsProvider) { @@ -975,7 +1032,8 @@ export async function createRemoteWorktree( notifyWorktreesChanged(mainWindow, repo.id) return { worktree, - ...(setup ? { setup } : {}) + ...(setup ? { setup } : {}), + ...(localBaseRefRefresh ? { localBaseRefRefresh } : {}) } } @@ -1219,44 +1277,43 @@ export async function createLocalWorktree( } const existingBranchOption = { checkoutExistingBranch } - if (sparseDirectories.length > 0) { - await (checkoutExistingBranch - ? addSparseWorktree( - repo.path, - worktreePath, - branchName, - sparseDirectories, - baseBranch, - settings.refreshLocalBaseRefOnWorktreeCreate, - existingBranchOption - ) - : addSparseWorktree( - repo.path, - worktreePath, - branchName, - sparseDirectories, - baseBranch, - settings.refreshLocalBaseRefOnWorktreeCreate - )) - } else { - await (checkoutExistingBranch - ? addWorktree( - repo.path, - worktreePath, - branchName, - baseBranch, - settings.refreshLocalBaseRefOnWorktreeCreate, - false, - existingBranchOption - ) - : addWorktree( - repo.path, - worktreePath, - branchName, - baseBranch, - settings.refreshLocalBaseRefOnWorktreeCreate - )) - } + const addResult: AddWorktreeResult = + (await (sparseDirectories.length > 0 + ? checkoutExistingBranch + ? addSparseWorktree( + repo.path, + worktreePath, + branchName, + sparseDirectories, + baseBranch, + settings.refreshLocalBaseRefOnWorktreeCreate, + existingBranchOption + ) + : addSparseWorktree( + repo.path, + worktreePath, + branchName, + sparseDirectories, + baseBranch, + settings.refreshLocalBaseRefOnWorktreeCreate + ) + : checkoutExistingBranch + ? addWorktree( + repo.path, + worktreePath, + branchName, + baseBranch, + settings.refreshLocalBaseRefOnWorktreeCreate, + false, + existingBranchOption + ) + : addWorktree( + repo.path, + worktreePath, + branchName, + baseBranch, + settings.refreshLocalBaseRefOnWorktreeCreate + ))) ?? {} let configuredPushTarget: GitPushTarget | undefined if (preparedPushTarget) { @@ -1383,6 +1440,7 @@ export async function createLocalWorktree( notifyWorktreesChanged(mainWindow, repo.id) return { worktree, - ...(setup ? { setup } : {}) + ...(setup ? { setup } : {}), + ...(addResult.localBaseRefRefresh ? { localBaseRefRefresh: addResult.localBaseRefRefresh } : {}) } } diff --git a/src/main/ipc/worktrees.test.ts b/src/main/ipc/worktrees.test.ts index e386d1c64..c806bfe78 100644 --- a/src/main/ipc/worktrees.test.ts +++ b/src/main/ipc/worktrees.test.ts @@ -5,6 +5,7 @@ const { handleMock, removeHandlerMock, listWorktreesMock, + parseWorktreeListMock, assertWorktreeCleanForRemovalMock, addWorktreeMock, addSparseWorktreeMock, @@ -38,6 +39,18 @@ const { handleMock: vi.fn(), removeHandlerMock: vi.fn(), listWorktreesMock: vi.fn(), + parseWorktreeListMock: vi.fn((output: string) => + output + .trim() + .split(/\n\s*\n/) + .filter(Boolean) + .map((block, index) => { + const lines = block.split(/\r?\n/) + const path = lines.find((line) => line.startsWith('worktree '))?.slice(9) ?? '' + const branch = lines.find((line) => line.startsWith('branch '))?.slice(7) ?? '' + return { path, branch, head: String(index), isBare: false, isMainWorktree: index === 0 } + }) + ), assertWorktreeCleanForRemovalMock: vi.fn(), addWorktreeMock: vi.fn(), addSparseWorktreeMock: vi.fn(), @@ -78,6 +91,7 @@ vi.mock('electron', () => ({ vi.mock('../git/worktree', () => ({ listWorktrees: listWorktreesMock, + parseWorktreeList: parseWorktreeListMock, assertWorktreeCleanForRemoval: assertWorktreeCleanForRemovalMock, addWorktree: addWorktreeMock, addSparseWorktree: addSparseWorktreeMock, @@ -1110,6 +1124,90 @@ describe('registerWorktreeHandlers', () => { }) }) + it('returns SSH local base refresh skip status when the owning worktree is dirty', async () => { + const repo = { + id: 'repo-ssh', + path: '/remote/repo', + displayName: 'ssh', + badgeColor: '#000', + addedAt: 0, + connectionId: 'conn-1', + worktreeBaseRef: 'origin/main' + } + const provider = { + exec: vi.fn().mockImplementation(async (args: string[]) => { + if (args[0] === 'remote') { + return { stdout: 'origin\n', stderr: '' } + } + if (args[0] === 'worktree' && args[1] === 'list') { + return { + stdout: 'worktree /remote/repo\nHEAD abc123\nbranch refs/heads/main\n', + stderr: '' + } + } + if (args[0] === 'status') { + return { stdout: ' M package.json\n', stderr: '' } + } + return { stdout: '', stderr: '' } + }), + fetchRemoteTrackingRef: vi.fn().mockResolvedValue(undefined), + addWorktree: vi.fn().mockResolvedValue(undefined), + listWorktrees: vi.fn().mockResolvedValue([ + { + path: '/remote/improve-dashboard', + head: 'abc123', + branch: 'refs/heads/improve-dashboard', + isBare: false, + isMainWorktree: false + } + ]) + } + const mux = { + request: vi.fn().mockResolvedValue(undefined), + notify: vi.fn() + } + store.getSettings.mockReturnValue({ + branchPrefix: 'none', + nestWorkspaces: false, + refreshLocalBaseRefOnWorktreeCreate: true, + workspaceDir: '/workspace' + }) + store.getRepos.mockReturnValue([repo]) + store.getRepo.mockReturnValue(repo) + getSshGitProviderMock.mockReturnValue(provider) + getActiveMultiplexerMock.mockReturnValue(mux) + store.setWorktreeMeta.mockImplementation((_worktreeId, meta) => meta) + + const result = await handlers['worktrees:create'](null, { + repoId: 'repo-ssh', + name: 'improve-dashboard' + }) + + expect(provider.exec).toHaveBeenCalledWith( + ['merge-base', '--is-ancestor', 'main', 'refs/remotes/origin/main'], + '/remote/repo' + ) + expect(provider.exec).toHaveBeenCalledWith(['worktree', 'list', '--porcelain'], '/remote/repo') + expect(provider.exec).toHaveBeenCalledWith( + ['status', '--porcelain', '--untracked-files=no'], + '/remote/repo' + ) + expect(provider.exec).not.toHaveBeenCalledWith( + ['reset', '--hard', 'refs/remotes/origin/main'], + expect.any(String) + ) + expect(result).toEqual( + expect.objectContaining({ + localBaseRefRefresh: { + status: 'skipped_dirty_worktree', + baseRef: 'origin/main', + localBranch: 'main', + ownerWorktreePath: '/remote/repo' + } + }) + ) + }) + it('reads remote orca.yaml and returns a setup launch payload during SSH create', async () => { const repo = { id: 'repo-ssh', diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 75967944f..13b610489 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -304,6 +304,7 @@ import { assertWorktreeCleanForRemoval, removeWorktree } from '../git/worktree' +import type { AddWorktreeResult } from '../git/worktree' import { isENOENT } from '../ipc/filesystem-auth' import { createSetupRunnerScript, @@ -7292,44 +7293,43 @@ export class OrcaRuntimeService { } const existingBranchOption = { checkoutExistingBranch } - if (sparseDirectories.length > 0) { - await (checkoutExistingBranch - ? addSparseWorktree( - repo.path, - worktreePath, - branchName, - sparseDirectories, - baseBranch, - settings.refreshLocalBaseRefOnWorktreeCreate, - existingBranchOption - ) - : addSparseWorktree( - repo.path, - worktreePath, - branchName, - sparseDirectories, - baseBranch, - settings.refreshLocalBaseRefOnWorktreeCreate - )) - } else { - await (checkoutExistingBranch - ? addWorktree( - repo.path, - worktreePath, - branchName, - baseBranch, - settings.refreshLocalBaseRefOnWorktreeCreate, - false, - existingBranchOption - ) - : addWorktree( - repo.path, - worktreePath, - branchName, - baseBranch, - settings.refreshLocalBaseRefOnWorktreeCreate - )) - } + const addResult: AddWorktreeResult = + (await (sparseDirectories.length > 0 + ? checkoutExistingBranch + ? addSparseWorktree( + repo.path, + worktreePath, + branchName, + sparseDirectories, + baseBranch, + settings.refreshLocalBaseRefOnWorktreeCreate, + existingBranchOption + ) + : addSparseWorktree( + repo.path, + worktreePath, + branchName, + sparseDirectories, + baseBranch, + settings.refreshLocalBaseRefOnWorktreeCreate + ) + : checkoutExistingBranch + ? addWorktree( + repo.path, + worktreePath, + branchName, + baseBranch, + settings.refreshLocalBaseRefOnWorktreeCreate, + false, + existingBranchOption + ) + : addWorktree( + repo.path, + worktreePath, + branchName, + baseBranch, + settings.refreshLocalBaseRefOnWorktreeCreate + ))) ?? {} let configuredPushTarget: GitPushTarget | undefined if (preparedPushTarget) { @@ -7602,7 +7602,10 @@ export class OrcaRuntimeService { }, ...(lineageInput ? { lineage, warnings: lineageWarnings } : {}), ...(setup ? { setup } : {}), - ...(warning ? { warning } : {}) + ...(warning ? { warning } : {}), + ...(addResult.localBaseRefRefresh + ? { localBaseRefRefresh: addResult.localBaseRefRefresh } + : {}) } } diff --git a/src/renderer/src/components/settings/GitPane.tsx b/src/renderer/src/components/settings/GitPane.tsx index 73df65a24..5b8a03acf 100644 --- a/src/renderer/src/components/settings/GitPane.tsx +++ b/src/renderer/src/components/settings/GitPane.tsx @@ -71,22 +71,42 @@ export function GitPane({ ) : null, matchesSettingsSearch(searchQuery, { title: 'Refresh Local Base Ref', - description: 'Optionally fast-forward local main or master when creating worktrees.', - keywords: ['main', 'master', 'origin/main', 'git diff', 'base ref', 'worktree'] + description: + 'Safely fast-forward local main or master so AI tools and diffs use a fresh base.', + keywords: [ + 'main', + 'master', + 'origin/main', + 'git diff', + 'base ref', + 'fresh base', + 'safely', + 'worktree' + ] }) ? (

- When enabled, Orca updates your local main or master before - creating a worktree. This helps AI tools and diffs compare your branch against the - latest base branch. Orca only does this when it is safe. + Turn this on if you or AI tools use commands like git diff main...HEAD. + Orca first refreshes the remote base, then safely fast-forwards the matching local{' '} + main or master so those commands do not compare against stale + history. Orca skips the update if the local branch is dirty or diverged.