diff --git a/src/main/ipc/worktree-remote.ts b/src/main/ipc/worktree-remote.ts index 26f583a5..27bb922e 100644 --- a/src/main/ipc/worktree-remote.ts +++ b/src/main/ipc/worktree-remote.ts @@ -15,7 +15,7 @@ import type { import { getPRForBranch } from '../github/client' import { listWorktrees, addWorktree } from '../git/worktree' import { getGitUsername, getDefaultBaseRef, getBranchConflictKind } from '../git/repo' -import { gitExecFileSync } from '../git/runner' +import { gitExecFileAsync } from '../git/runner' import { isWslPath, parseWslPath, getWslHome } from '../wsl' import { createSetupRunnerScript, getEffectiveHooks, shouldRunSetupForCreate } from '../hooks' import { getSshGitProvider } from '../providers/ssh-git-dispatch' @@ -29,7 +29,7 @@ import { mergeWorktree, areWorktreePathsEqual } from './worktree-logic' -import { rebuildAuthorizedRootsCache } from './filesystem-auth' +import { invalidateAuthorizedRootsCache } from './filesystem-auth' export function notifyWorktreesChanged(mainWindow: BrowserWindow, repoId: string): void { if (!mainWindow.isDestroyed()) { @@ -177,17 +177,22 @@ export async function createLocalWorktree( continue } - // Why: the UI resolves PR status by branch name alone. Reusing a historical - // PR head name would make a fresh worktree inherit that old merged/closed PR - // immediately, so auto-suffix until we land on a fresh branch identity. - lastExistingPR = null - try { - lastExistingPR = await getPRForBranch(repo.path, branchName) - } catch { - // GitHub API may be unreachable, rate-limited, or token missing - } - if (lastExistingPR) { - continue + // Why: `gh pr list` is a network round-trip that previously ran on every + // create, adding ~1–3s to the happy path even when no conflict exists. We + // only probe PR conflicts once a local/remote branch collision has already + // forced us past the first suffix — at that point uniqueness matters + // enough to justify the GitHub call. The common case (brand-new branch + // name, no collisions) skips the network entirely. + if (suffix > 1) { + lastExistingPR = null + try { + lastExistingPR = await getPRForBranch(repo.path, branchName) + } catch { + // GitHub API may be unreachable, rate-limited, or token missing + } + if (lastExistingPR) { + continue + } } worktreePath = ensurePathWithinWorkspace( @@ -229,13 +234,16 @@ export async function createLocalWorktree( // a real worktree on disk while the renderer reports "create failed". const shouldLaunchSetup = setupScript ? shouldRunSetupForCreate(repo, args.setupDecision) : false - // Fetch latest from remote so the worktree starts with up-to-date content + // Why: `git fetch` previously blocked worktree creation for 1–5s on every + // click, even though the fetch result isn't actually required — the + // subsequent `git worktree add` uses whatever local ref `baseBranch` points + // at. Kicking fetch off in parallel lets the worktree be created off the + // last-known tip while the fetch completes in the background; the next + // user action (pull, diff, PR create) will see the refreshed remote state. const remote = baseBranch.includes('/') ? baseBranch.split('/')[0] : 'origin' - try { - gitExecFileSync(['fetch', remote], { cwd: repo.path }) - } catch { + void gitExecFileAsync(['fetch', remote], { cwd: repo.path }).catch(() => { // Fetch is best-effort — don't block worktree creation if offline - } + }) await addWorktree( repo.path, @@ -264,7 +272,12 @@ export async function createLocalWorktree( } const meta = store.setWorktreeMeta(worktreeId, metaUpdates) const worktree = mergeWorktree(repo.id, created, meta) - await rebuildAuthorizedRootsCache(store) + // Why: the authorized-roots cache is consulted lazily on the next filesystem + // access (`ensureAuthorizedRootsCache` rebuilds on demand when dirty). We + // just invalidate the cache marker instead of blocking worktree creation on + // an immediate rebuild, which can spawn `git worktree list` per repo and + // adds 100ms+ to every create. + invalidateAuthorizedRootsCache() let setup: CreateWorktreeResult['setup'] if (setupScript && shouldLaunchSetup) { diff --git a/src/main/ipc/worktrees-windows.test.ts b/src/main/ipc/worktrees-windows.test.ts index 5fa3f528..3c8f6a79 100644 --- a/src/main/ipc/worktrees-windows.test.ts +++ b/src/main/ipc/worktrees-windows.test.ts @@ -53,6 +53,14 @@ vi.mock('../git/worktree', () => ({ removeWorktree: removeWorktreeMock })) +vi.mock('../git/runner', () => ({ + // Why: createLocalWorktree now fires `git fetch` via gitExecFileAsync in the + // background. Return a resolved promise so the fire-and-forget `.catch()` + // chain has a valid Promise to attach to. + gitExecFileAsync: vi.fn().mockResolvedValue({ stdout: '', stderr: '' }), + gitExecFileSync: vi.fn() +})) + vi.mock('../git/repo', () => ({ getGitUsername: getGitUsernameMock, getDefaultBaseRef: getDefaultBaseRefMock, diff --git a/src/main/ipc/worktrees.test.ts b/src/main/ipc/worktrees.test.ts index 2bd3e1c8..80432f01 100644 --- a/src/main/ipc/worktrees.test.ts +++ b/src/main/ipc/worktrees.test.ts @@ -180,6 +180,10 @@ describe('registerWorktreeHandlers', () => { getDefaultBaseRefMock.mockReturnValue('origin/main') getBranchConflictKindMock.mockResolvedValue(null) getPRForBranchMock.mockResolvedValue(null) + // Why: createLocalWorktree now fires `git fetch` in the background via + // gitExecFileAsync. The default mock must return a resolved promise so + // the fire-and-forget `.catch()` chain doesn't trip on undefined. + gitExecFileAsyncMock.mockResolvedValue({ stdout: '', stderr: '' }) getEffectiveHooksMock.mockReturnValue(null) shouldRunSetupForCreateMock.mockReturnValue(false) createSetupRunnerScriptMock.mockReturnValue({ @@ -312,12 +316,19 @@ describe('registerWorktreeHandlers', () => { expect(listWorktreesMock).not.toHaveBeenCalled() }) - it('auto-suffixes the branch name when the first choice already belongs to a PR', async () => { - // Why: reusing a historical PR head would make a fresh worktree inherit - // that old PR, so the loop suffixes past the name until it finds one that - // is not associated with any PR. + it('skips past a suffix that already belongs to a PR after an initial branch conflict', async () => { + // Why: `gh pr list` is network-bound and previously fired on every single + // create, adding 1–3s to the happy path. We now only probe PR conflicts + // from suffix=2 onward — once a local/remote branch collision has already + // forced us past the first candidate and uniqueness matters enough to + // justify the GitHub round-trip. This test covers that delayed path: + // suffix=1 is a branch conflict, suffix=2 is owned by an old PR, so the + // loop lands on suffix=3. + getBranchConflictKindMock.mockImplementation(async (_repoPath: string, branch: string) => + branch === 'improve-dashboard' ? 'remote' : null + ) getPRForBranchMock.mockImplementation(async (_repoPath: string, branch: string) => - branch === 'improve-dashboard' + branch === 'improve-dashboard-2' ? { number: 3127, title: 'Existing PR', @@ -331,9 +342,9 @@ describe('registerWorktreeHandlers', () => { ) listWorktreesMock.mockResolvedValue([ { - path: '/workspace/improve-dashboard-2', + path: '/workspace/improve-dashboard-3', head: 'abc123', - branch: 'improve-dashboard-2', + branch: 'improve-dashboard-3', isBare: false, isMainWorktree: false } @@ -346,19 +357,42 @@ describe('registerWorktreeHandlers', () => { expect(addWorktreeMock).toHaveBeenCalledWith( '/workspace/repo', - '/workspace/improve-dashboard-2', - 'improve-dashboard-2', + '/workspace/improve-dashboard-3', + 'improve-dashboard-3', 'origin/main', false ) expect(result).toEqual({ worktree: expect.objectContaining({ - path: '/workspace/improve-dashboard-2', - branch: 'improve-dashboard-2' + path: '/workspace/improve-dashboard-3', + branch: 'improve-dashboard-3' }) }) }) + it('does not call `gh pr list` on the happy path (no branch conflict)', async () => { + // Why: guards the speed optimization. If a future refactor accidentally + // reintroduces the PR probe on the first iteration, the happy path will + // silently regain a 1–3s GitHub round-trip per click; this test fails + // loudly instead. + listWorktreesMock.mockResolvedValue([ + { + path: '/workspace/improve-dashboard', + head: 'abc123', + branch: 'improve-dashboard', + isBare: false, + isMainWorktree: false + } + ]) + + await handlers['worktrees:create'](null, { + repoId: 'repo-1', + name: 'improve-dashboard' + }) + + expect(getPRForBranchMock).not.toHaveBeenCalled() + }) + const createdWorktreeList = [ { path: '/workspace/improve-dashboard',