From 4572b0a4462b0dd9936cba3da1f734318e91a5da Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sat, 11 Jul 2026 00:41:28 -0700 Subject: [PATCH] perf(worktrees): remove redundant creation probes (#7161) Co-authored-by: OrcaWin --- src/main/git/worktree.test.ts | 37 +++++++++++++++++++ src/main/git/worktree.ts | 11 +++++- src/main/ipc/worktree-remote.ts | 33 ++++++++++++++--- src/main/ipc/worktrees-windows.test.ts | 34 +++++++++++++++++ src/main/ipc/worktrees.test.ts | 51 ++++++++++++++++++-------- src/main/runtime/orca-runtime.test.ts | 5 ++- src/main/runtime/orca-runtime.ts | 7 +++- 7 files changed, 153 insertions(+), 25 deletions(-) diff --git a/src/main/git/worktree.test.ts b/src/main/git/worktree.test.ts index 858b7c2bf..54216ab9a 100644 --- a/src/main/git/worktree.test.ts +++ b/src/main/git/worktree.test.ts @@ -1254,6 +1254,43 @@ describe('addWorktree', () => { ]) }) + it('skips advisory owner probes when the local base is already current', async () => { + gitExecFileAsyncMock + .mockResolvedValueOnce({ stdout: 'abc123\n' }) // resolve creation base + .mockResolvedValueOnce({ stdout: '0\t0\n' }) // local base is current + .mockResolvedValueOnce({ stdout: '' }) // worktree add + .mockResolvedValueOnce({ stdout: '' }) // persist branch base + .mockResolvedValueOnce({ stdout: 'true\n' }) // push.autoSetupRemote already set + + await expect( + addWorktree('/repo', '/repo-feature', 'feature/test', 'origin/main', false, false, { + suggestLocalBaseRefUpdate: true + }) + ).resolves.toEqual({}) + + expect(gitExecFileAsyncMock.mock.calls.map(([args]) => args)).toEqual([ + ['rev-parse', '--verify', '--quiet', 'refs/remotes/origin/main^{commit}'], + ['rev-list', '--left-right', '--count', 'refs/heads/main...refs/remotes/origin/main'], + [ + 'worktree', + 'add', + '--no-track', + '-b', + 'feature/test', + '/repo-feature', + 'refs/remotes/origin/main' + ], + [ + 'config', + '--local', + '--replace-all', + 'branch.feature/test.base', + 'refs/remotes/origin/main' + ], + ['config', '--get', 'push.autoSetupRemote'] + ]) + }) + it('uses normalized branch metadata for slash-containing remotes', async () => { const worktreeListOutput = 'worktree /repo\nHEAD abc123\nbranch refs/heads/main\n' gitExecFileAsyncMock diff --git a/src/main/git/worktree.ts b/src/main/git/worktree.ts index 19b84a7b3..6923858f3 100644 --- a/src/main/git/worktree.ts +++ b/src/main/git/worktree.ts @@ -172,7 +172,8 @@ async function evaluateLocalBaseRefRefreshability( baseBranch: string, remoteTrackingRef: string, remoteTrackingBase?: AddWorktreeOptions['remoteTrackingBase'], - options: GitWorktreeExecOptions = {} + options: GitWorktreeExecOptions = {}, + shouldInspectOwner: (behind: number) => boolean = () => true ): Promise { const parsed = parseRemoteTrackingLocalBaseRef(baseBranch, remoteTrackingRef, remoteTrackingBase) if (!parsed) { @@ -196,6 +197,11 @@ async function evaluateLocalBaseRefRefreshability( if (!parsedDrift || parsedDrift.ahead !== 0) { return { refreshable: false, result: { ...resultBase, status: 'skipped_not_fast_forward' } } } + if (!shouldInspectOwner(parsedDrift.behind)) { + // Why: a current local ref cannot produce an update suggestion, so the + // advisory path need not resolve OIDs or inspect its owner worktree. + return undefined + } const { stdout: localOidOutput } = await gitExecFileAsync( ['rev-parse', '--verify', `${parsed.fullRef}^{commit}`], gitExecOptions(repoPath, options) @@ -289,7 +295,8 @@ async function getLocalBaseRefUpdateSuggestionForWorktreeCreate( baseBranch, remoteTrackingRef, remoteTrackingBase, - options + options, + (behind) => behind > 0 ) if (!evaluation?.refreshable || evaluation.behind <= 0) { return undefined diff --git a/src/main/ipc/worktree-remote.ts b/src/main/ipc/worktree-remote.ts index 43ef357ee..3f50a5ce0 100644 --- a/src/main/ipc/worktree-remote.ts +++ b/src/main/ipc/worktree-remote.ts @@ -1395,7 +1395,8 @@ async function refreshLocalBaseRefForRemoteWorktreeCreate( async function evaluateRemoteLocalBaseRefRefreshability( provider: SshGitProvider, repoPath: string, - remoteTrackingBase: RemoteTrackingBase + remoteTrackingBase: RemoteTrackingBase, + shouldInspectOwner: (behind: number) => boolean = () => true ): Promise { const resultBase = { baseRef: remoteTrackingBase.base, @@ -1413,6 +1414,17 @@ async function evaluateRemoteLocalBaseRefRefreshability( repoPath ) behind = countNonEmptyGitOutputLines(stdout) + if (!shouldInspectOwner(behind)) { + // Why: no behind commits means the advisory cannot offer an update; + // avoid remote worktree/status round trips that cannot change that. + return { + refreshable: true, + ...resultBase, + fullRef, + remoteTrackingRef: remoteTrackingBase.ref, + behind + } + } } catch { return { refreshable: false, result: { ...resultBase, status: 'skipped_not_fast_forward' } } } @@ -1467,7 +1479,8 @@ async function getRemoteLocalBaseRefUpdateSuggestionForWorktreeCreate( const evaluation = await evaluateRemoteLocalBaseRefRefreshability( provider, repoPath, - remoteTrackingBase + remoteTrackingBase, + (behind) => behind > 0 ) if (!evaluation.refreshable || evaluation.behind <= 0) { return undefined @@ -1536,9 +1549,12 @@ export async function createRemoteWorktree( // Register the repo root first so relays do not report a valid base as stale. await registerRequiredSshWorktreeCreateRoots(repo.connectionId!, [repo.path]) - // Why: SSH targets cannot use the local `gh` account, and git email/name are - // commit author identity rather than hosted-account usernames. - const username = await getSshGitUsername(provider, repo.path) + // Why: explicit branches and non-username prefix modes never consume this + // value; skipping the remote config probes preserves the exact branch name. + const username = + !args.branchNameOverride && settings.branchPrefix === 'git-username' + ? await getSshGitUsername(provider, repo.path) + : '' const branchConflictSubject = args.branchNameOverride ? 'branch name' : 'worktree name' // Determine base branch @@ -1968,12 +1984,17 @@ export async function createLocalWorktree( return { ...options, ...localWorktreeGitOptions } } - const username = await resolveLocalGitUsername(repo.path) const requestedName = args.name const sanitizedName = sanitizeWorktreeName(args.name) const requestedDisplayName = args.displayName ? sanitizeWorktreeDisplayName(args.displayName) : undefined + // Why: explicit branches and non-username prefix modes never consume this + // value; skipping the probes preserves the exact generated branch name. + const username = + !args.branchNameOverride && settings.branchPrefix === 'git-username' + ? await resolveLocalGitUsername(repo.path) + : '' const baseBranch = await resolveWorktreeCreateBase({ requestedBaseBranch: args.baseBranch, diff --git a/src/main/ipc/worktrees-windows.test.ts b/src/main/ipc/worktrees-windows.test.ts index 3ea68e4ba..7c6d0572d 100644 --- a/src/main/ipc/worktrees-windows.test.ts +++ b/src/main/ipc/worktrees-windows.test.ts @@ -287,6 +287,7 @@ describe('registerWorktreeHandlers – Windows path handling', () => { 'origin/main', false ) + expect(resolveLocalGitUsernameMock).not.toHaveBeenCalled() expect(store.setWorktreeMeta).toHaveBeenCalledWith( 'repo-1::C:/workspaces/improve-dashboard', expect.objectContaining({ @@ -302,6 +303,39 @@ describe('registerWorktreeHandlers – Windows path handling', () => { }) }) + it('resolves the Git username when the configured prefix consumes it', async () => { + store.getSettings.mockReturnValue({ + branchPrefix: 'git-username', + nestWorkspaces: false, + refreshLocalBaseRefOnWorktreeCreate: false, + workspaceDir: 'C:\\workspaces' + }) + resolveLocalGitUsernameMock.mockResolvedValue('octocat') + listWorktreesMock.mockResolvedValue([ + { + path: 'C:/workspaces/improve-dashboard', + head: 'abc123', + branch: 'refs/heads/octocat/improve-dashboard', + isBare: false, + isMainWorktree: false + } + ]) + + await handlers['worktrees:create'](null, { + repoId: 'repo-1', + name: 'improve-dashboard' + }) + + expect(resolveLocalGitUsernameMock).toHaveBeenCalledWith('C:\\repo') + expect(addWorktreeMock).toHaveBeenCalledWith( + 'C:\\repo', + 'C:\\workspaces\\improve-dashboard', + 'octocat/improve-dashboard', + 'origin/main', + false + ) + }) + it('preserves create-time metadata on the next list when Windows path formatting differs', async () => { const worktreeEntry = { path: 'C:/workspaces/improve-dashboard', diff --git a/src/main/ipc/worktrees.test.ts b/src/main/ipc/worktrees.test.ts index a278656ee..ade839b50 100644 --- a/src/main/ipc/worktrees.test.ts +++ b/src/main/ipc/worktrees.test.ts @@ -908,6 +908,13 @@ describe('registerWorktreeHandlers', () => { }) it('uses branchNameOverride for the git branch while keeping the sanitized worktree path', async () => { + store.getSettings.mockReturnValue({ + branchPrefix: 'git-username', + nestWorkspaces: false, + refreshLocalBaseRefOnWorktreeCreate: false, + workspaceDir: '/workspace' + }) + resolveLocalGitUsernameMock.mockResolvedValue('unused-user') listWorktreesMock.mockResolvedValue([ { path: '/workspace/feature-something', @@ -935,6 +942,7 @@ describe('registerWorktreeHandlers', () => { 'origin/main', false ) + expect(resolveLocalGitUsernameMock).not.toHaveBeenCalled() expect(result).toMatchObject({ worktree: expect.objectContaining({ path: '/workspace/feature-something', @@ -2809,6 +2817,13 @@ describe('registerWorktreeHandlers', () => { fetchRemoteTrackingRef: vi.fn().mockResolvedValue(undefined), addWorktree: vi.fn().mockResolvedValue(undefined), listWorktrees: vi.fn().mockResolvedValue([ + { + path: '/remote/repo', + head: 'base123', + branch: 'refs/heads/main', + isBare: false, + isMainWorktree: true + }, { path: '/remote/improve-dashboard', head: 'abc123', @@ -2816,7 +2831,8 @@ describe('registerWorktreeHandlers', () => { isBare: false, isMainWorktree: false } - ]) + ]), + worktreeIsClean: vi.fn().mockResolvedValue({ clean: true }) } const mux = { request: vi.fn().mockResolvedValue(undefined), @@ -2838,6 +2854,16 @@ describe('registerWorktreeHandlers', () => { manualOrder: 123_456 }) + expect(provider.exec).not.toHaveBeenCalledWith( + ['config', '--get', 'github.user'], + '/remote/repo' + ) + expect(provider.exec).not.toHaveBeenCalledWith( + ['config', '--get', 'user.username'], + '/remote/repo' + ) + expect(provider.listWorktrees).toHaveBeenCalledTimes(1) + expect(provider.worktreeIsClean).not.toHaveBeenCalled() expect(store.setWorktreeMeta).toHaveBeenCalledWith( 'repo-ssh::/remote/improve-dashboard', expect.objectContaining({ @@ -3798,18 +3824,15 @@ describe('registerWorktreeHandlers', () => { }), fetchRemoteTrackingRef: vi.fn().mockResolvedValue(undefined), addWorktree: vi.fn().mockResolvedValue(undefined), - listWorktrees: vi - .fn() - .mockResolvedValueOnce([]) - .mockResolvedValueOnce([ - { - path: '/remote/improve-dashboard', - head: 'abc123', - branch: 'refs/heads/improve-dashboard', - isBare: false, - isMainWorktree: false - } - ]) + listWorktrees: vi.fn().mockResolvedValueOnce([ + { + path: '/remote/improve-dashboard', + head: 'abc123', + branch: 'refs/heads/improve-dashboard', + isBare: false, + isMainWorktree: false + } + ]) } const mux = { request: vi.fn().mockResolvedValue(undefined), @@ -4019,7 +4042,6 @@ describe('registerWorktreeHandlers', () => { addWorktree: vi.fn().mockResolvedValue(undefined), listWorktrees: vi .fn() - .mockResolvedValueOnce([]) .mockResolvedValueOnce([ { path: '/remote/first-worktree', @@ -4029,7 +4051,6 @@ describe('registerWorktreeHandlers', () => { isMainWorktree: false } ]) - .mockResolvedValueOnce([]) .mockResolvedValueOnce([ { path: '/remote/second-worktree', diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index 7a9e927cb..681eedc9a 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -91,6 +91,7 @@ import { TERMINAL_METHODS } from './rpc/methods/terminal' const ORIGINAL_PLATFORM = process.platform const ORIGINAL_PLATFORM_DESCRIPTOR = Object.getOwnPropertyDescriptor(process, 'platform') const removeWorktreeLinkedPathsMock = vi.hoisted(() => vi.fn()) +const resolveLocalGitUsernameMock = vi.hoisted(() => vi.fn(async () => '')) vi.mock('../ipc/worktree-symlinks', () => ({ createWorktreeLinkedPaths: vi.fn(), @@ -532,7 +533,7 @@ vi.mock('../git/repo', async (importOriginal) => { vi.mock('../git/git-username', async () => { const actual = await vi.importActual('../git/git-username') - return { ...actual, resolveLocalGitUsername: vi.fn(async () => '') } + return { ...actual, resolveLocalGitUsername: resolveLocalGitUsernameMock } }) function resetRuntimeTestMocks(): void { @@ -552,6 +553,7 @@ function resetRuntimeTestMocks(): void { vi.mocked(assertWorktreeCleanForRemoval).mockResolvedValue(undefined) vi.mocked(removeWorktree).mockReset() removeWorktreeLinkedPathsMock.mockReset() + resolveLocalGitUsernameMock.mockReset().mockResolvedValue('') vi.mocked(forceDeleteLocalBranchMock).mockReset() vi.mocked(forceDeleteLocalBranchMock).mockResolvedValue(undefined) sshGitProviders.clear() @@ -3048,6 +3050,7 @@ describe('OrcaRuntimeService', () => { 'origin/feature/something', false ) + expect(resolveLocalGitUsernameMock).not.toHaveBeenCalled() expect(result.worktree).toMatchObject({ path: '/tmp/workspaces/feature-something', branch: 'feature/something' diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 75beac994..63eae9a06 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -14249,7 +14249,12 @@ export class OrcaRuntimeService { const requestedDisplayName = args.displayName?.trim() || undefined const sanitizedName = sanitizeWorktreeName(args.name) let effectiveSanitizedName = sanitizedName - const username = await resolveLocalGitUsername(repo.path) + // Why: explicit branches and non-username prefix modes never consume this + // value; skipping the probes preserves the exact generated branch name. + const username = + !args.branchNameOverride && settings.branchPrefix === 'git-username' + ? await resolveLocalGitUsername(repo.path) + : '' const baseBranch = await resolveWorktreeCreateBase({ requestedBaseBranch: args.baseBranch,