perf(worktrees): avoid redundant fetch during deletion (#11918)

src/main/git/remove-worktree.test.ts

@@ -574,6 +574,9 @@ branch refs/heads/main
       'git rev-parse --verify --quiet refs/remotes/origin/main^{commit}': {
         stdout: 'base123\n'
       },
+      'git rev-parse --verify --quiet HEAD^{commit}': {
+        stdout: 'base123\n'
+      },
       'git merge-tree --write-tree base123 refs/heads/feature/test': {
         stdout: 'tree123\n'
       },
@@ -589,6 +592,7 @@ branch refs/heads/main
     expect(calls).toContain('git merge-tree --write-tree base123 refs/heads/feature/test')
     expect(calls).toContain('git update-ref -d refs/heads/feature/test def456')
     expect(calls).toContain('git config --remove-section branch.feature/test')
+    expect(calls).not.toContain('git remote')
   })
 
   it('deletes a squash-merged branch with branch-only merge commits via expected head', async () => {
@@ -732,13 +736,15 @@ branch refs/heads/main
     await expect(removeWorktree('/repo', '/repo-feature')).resolves.toEqual({})
 
     const calls = getGitCalls()
+    const mergeTreeCall = 'git merge-tree --write-tree base123 refs/heads/feature/test'
+    const mergeTreeIndexes = calls.flatMap((call, index) => (call === mergeTreeCall ? [index] : []))
+    const fetchIndex = calls.indexOf('git fetch --prune origin')
+    const updateRefIndex = calls.indexOf('git update-ref -d refs/heads/feature/test def456')
     expect(calls).toContain('git fetch --prune origin')
     expect(calls).toContain('git update-ref -d refs/heads/feature/test def456')
-    expectGitCallOrder(
-      calls,
-      'git fetch --prune origin',
-      'git merge-tree --write-tree base123 refs/heads/feature/test'
-    )
+    expect(mergeTreeIndexes).toHaveLength(1)
+    expect(fetchIndex).toBeLessThan(mergeTreeIndexes[0])
+    expect(mergeTreeIndexes[0]).toBeLessThan(updateRefIndex)
     expectGitCallOrder(
       calls,
       'git fetch --prune origin',

src/main/git/worktree.ts

@@ -2,9 +2,8 @@
 import { readFile, stat } from 'node:fs/promises'
 import { isAbsolute, join, posix, resolve, win32 } from 'node:path'
 import {
-  branchHasNoUnmergedChangesOnAnyTarget,
-  getBranchCleanupTargetRefs,
-  refreshBranchCleanupTargetRefs
+  branchHasNoUnmergedChangesWithLazyTargetRefresh,
+  getBranchCleanupTargetRefs
 } from '../../shared/git-branch-cleanup'
 import { resolveWorktreeAddBaseRef } from '../../shared/worktree-base-ref'
 import { withSpan } from '../observability/tracer'
@@ -1267,10 +1266,9 @@ async function deleteAlreadyMergedBranchAfterSafeDeleteFailure(
       ...(execOptions?.stdin !== undefined ? { stdin: execOptions.stdin } : {})
     })
   const targetRefs = await getBranchCleanupTargetRefs(runGit, branchName)
-  await refreshBranchCleanupTargetRefs(runGit, targetRefs)
   // Why: squash merges rewrite commit IDs, so `branch -d` rejects already-merged branches; delete only when Git proves no unmerged tree changes.
   if (
-    !(await branchHasNoUnmergedChangesOnAnyTarget(
+    !(await branchHasNoUnmergedChangesWithLazyTargetRefresh(
       runGit,
       branchName,
       targetRefs,

src/relay/git-handler-branch-cleanup.test.ts

@@ -191,6 +191,9 @@ describe('removeWorktreeOp branch cleanup', () => {
       if (args[0] === 'rev-parse' && args.includes('refs/remotes/origin/main^{commit}')) {
         return { stdout: 'base123\n', stderr: '' }
       }
+      if (args[0] === 'rev-parse' && args.includes('HEAD^{commit}')) {
+        return { stdout: 'base123\n', stderr: '' }
+      }
       if (args[0] === 'merge-tree') {
         return { stdout: 'tree123\n', stderr: '' }
       }
@@ -217,6 +220,7 @@ describe('removeWorktreeOp branch cleanup', () => {
       ['config', '--remove-section', 'branch.feature/test'],
       expect.any(String)
     )
+    expect(git).not.toHaveBeenCalledWith(['remote'], expect.any(String))
   })
 
   it('deletes a squash-merged SSH branch with branch-only merge commits via expected head', async () => {
@@ -360,17 +364,17 @@ describe('removeWorktreeOp branch cleanup', () => {
     const commandIndex = (expectedArgs: string[]) =>
       calls.findIndex(({ args }) => JSON.stringify(args) === JSON.stringify(expectedArgs))
     const fetchIndex = commandIndex(['fetch', '--prune', 'origin'])
-    const mergeTreeIndex = commandIndex([
-      'merge-tree',
-      '--write-tree',
-      'base123',
-      'refs/heads/feature/test'
-    ])
+    const mergeTreeArgs = ['merge-tree', '--write-tree', 'base123', 'refs/heads/feature/test']
+    const mergeTreeIndexes = calls.flatMap(({ args }, index) =>
+      JSON.stringify(args) === JSON.stringify(mergeTreeArgs) ? [index] : []
+    )
     const updateRefIndex = commandIndex(['update-ref', '-d', 'refs/heads/feature/test', '1'])
 
     expect(fetchIndex).toBeGreaterThanOrEqual(0)
     expect(calls[fetchIndex]?.cwd).toBe(resolvedRepoPath())
-    expect(fetchIndex).toBeLessThan(mergeTreeIndex)
+    expect(mergeTreeIndexes).toHaveLength(1)
+    expect(fetchIndex).toBeLessThan(mergeTreeIndexes[0])
+    expect(mergeTreeIndexes[0]).toBeLessThan(updateRefIndex)
     expect(fetchIndex).toBeLessThan(updateRefIndex)
   })
 

src/relay/git-handler-branch-cleanup.ts

@@ -1,7 +1,6 @@
 import {
-  branchHasNoUnmergedChangesOnAnyTarget,
-  getBranchCleanupTargetRefs,
-  refreshBranchCleanupTargetRefs
+  branchHasNoUnmergedChangesWithLazyTargetRefresh,
+  getBranchCleanupTargetRefs
 } from '../shared/git-branch-cleanup'
 import type { GitCapabilityCache } from '../shared/git-capability-cache'
 import type { GitExec } from './git-handler-ops'
@@ -17,12 +16,16 @@ export async function deleteAlreadyMergedRelayBranchAfterSafeDeleteFailure(
   const runGit = (args: string[], options?: { stdin?: string }) =>
     options ? git(args, repoPath, options) : git(args, repoPath)
   const targetRefs = await getBranchCleanupTargetRefs(runGit, branchName)
-  await refreshBranchCleanupTargetRefs(runGit, targetRefs)
   // Why: SSH worktrees hit the same squash-merge shape as local worktrees.
   // Git's no-op merge proof lets us clean up only branches whose changes
   // already exist on the saved base ref.
   if (
-    !(await branchHasNoUnmergedChangesOnAnyTarget(runGit, branchName, targetRefs, capabilities))
+    !(await branchHasNoUnmergedChangesWithLazyTargetRefresh(
+      runGit,
+      branchName,
+      targetRefs,
+      capabilities
+    ))
   ) {
     return false
   }

src/shared/git-branch-cleanup.test.ts

@@ -1,6 +1,7 @@
 import { describe, expect, it, vi } from 'vitest'
 import {
   branchHasNoUnmergedChangesOnAnyTarget,
+  branchHasNoUnmergedChangesWithLazyTargetRefresh,
   refreshBranchCleanupTargetRefs,
   type GitBranchCleanupExec
 } from './git-branch-cleanup'
@@ -195,3 +196,76 @@ describe('branchHasNoUnmergedChangesOnAnyTarget', () => {
     expect(mergeTreeCalls).toHaveLength(1)
   })
 })
+
+describe('branchHasNoUnmergedChangesWithLazyTargetRefresh', () => {
+  it('skips refresh when local HEAD proves the branch changes are retained', async () => {
+    const runGit = vi.fn<GitBranchCleanupExec>(async (args) => {
+      const command = args.join(' ')
+      const stdout =
+        {
+          'rev-parse --verify --quiet HEAD^{commit}': 'local-target\n',
+          'merge-tree --write-tree local-target refs/heads/feature/test': 'local-tree\n',
+          'rev-parse --verify --quiet local-target^{tree}': 'local-tree\n'
+        }[command] ?? ''
+      return { stdout }
+    })
+
+    await expect(
+      branchHasNoUnmergedChangesWithLazyTargetRefresh(
+        runGit,
+        'feature/test',
+        ['refs/remotes/origin/main', 'HEAD'],
+        new GitCapabilityCache()
+      )
+    ).resolves.toBe(true)
+
+    expect(runGit.mock.calls.map(([args]) => args)).not.toContainEqual(['remote'])
+  })
+
+  it('refreshes before trusting a stale remote-tracking proof', async () => {
+    let refreshed = false
+    const runGit = vi.fn<GitBranchCleanupExec>(async (args) => {
+      const command = args.join(' ')
+      if (command === 'remote') {
+        return { stdout: 'origin\n' }
+      }
+      if (command === 'fetch --prune origin') {
+        refreshed = true
+        return { stdout: '' }
+      }
+      if (command === 'rev-parse --verify --quiet refs/remotes/origin/main^{commit}') {
+        return { stdout: 'remote-target\n' }
+      }
+      if (command === 'rev-parse --verify --quiet origin/main^{commit}') {
+        return { stdout: 'short-remote-target\n' }
+      }
+      if (command === 'rev-parse --verify --quiet HEAD^{commit}') {
+        return { stdout: 'local-target\n' }
+      }
+      if (command === 'merge-tree --write-tree remote-target refs/heads/feature/test') {
+        return { stdout: refreshed ? 'changed-tree\n' : 'remote-tree\n' }
+      }
+      if (command === 'merge-tree --write-tree short-remote-target refs/heads/feature/test') {
+        return { stdout: refreshed ? 'changed-tree\n' : 'short-remote-tree\n' }
+      }
+      if (command === 'rev-parse --verify --quiet remote-target^{tree}') {
+        return { stdout: 'remote-tree\n' }
+      }
+      if (command === 'rev-parse --verify --quiet short-remote-target^{tree}') {
+        return { stdout: 'short-remote-tree\n' }
+      }
+      return { stdout: '' }
+    })
+
+    await expect(
+      branchHasNoUnmergedChangesWithLazyTargetRefresh(
+        runGit,
+        'feature/test',
+        ['refs/remotes/origin/main', 'origin/main', 'HEAD'],
+        new GitCapabilityCache()
+      )
+    ).resolves.toBe(false)
+
+    expect(runGit.mock.calls.map(([args]) => args)).toContainEqual(['fetch', '--prune', 'origin'])
+  })
+})

src/shared/git-branch-cleanup.ts

@@ -8,6 +8,10 @@ export type GitBranchCleanupExec = (
 
 const SQUASH_PATCH_SCAN_LIMIT = 200
 
+function isLocalTargetRef(ref: string): boolean {
+  return ref === 'HEAD' || ref.startsWith('refs/heads/') || ref.startsWith('refs/tags/')
+}
+
 async function readOptionalGitStdout(
   runGit: GitBranchCleanupExec,
   argv: string[],
@@ -253,3 +257,26 @@ export async function branchHasNoUnmergedChangesOnAnyTarget(
 
   return false
 }
+
+export async function branchHasNoUnmergedChangesWithLazyTargetRefresh(
+  runGit: GitBranchCleanupExec,
+  branchName: string,
+  targetRefs: string[],
+  capabilities: GitCapabilityCache
+): Promise<boolean> {
+  // Why: an unrefreshed remote-tracking ref may no longer represent the remote's branch contents.
+  const localTargetRefs = targetRefs.filter(isLocalTargetRef)
+  const refreshDependentTargetRefs = targetRefs.filter((targetRef) => !isLocalTargetRef(targetRef))
+  if (
+    await branchHasNoUnmergedChangesOnAnyTarget(runGit, branchName, localTargetRefs, capabilities)
+  ) {
+    return true
+  }
+  await refreshBranchCleanupTargetRefs(runGit, targetRefs)
+  return branchHasNoUnmergedChangesOnAnyTarget(
+    runGit,
+    branchName,
+    refreshDependentTargetRefs,
+    capabilities
+  )
+}

tests/tools/benchmarks/worktree-deletion-branch-fixture.mjs

@@ -0,0 +1,69 @@
+import { spawnSync } from 'node:child_process'
+import net from 'node:net'
+import path from 'node:path'
+
+function runGit(args, cwd) {
+  const result = spawnSync('git', args, { cwd, encoding: 'utf8' })
+  if (result.status !== 0) {
+    throw new Error(`git ${args.join(' ')} failed (${result.status})\n${result.stderr}`)
+  }
+  return result.stdout.trim()
+}
+
+export function initializeBranchCleanupRemote(fixtureRoot, repoPath) {
+  const remotePath = path.join(fixtureRoot, 'remote.git')
+  runGit(['init', '--bare', remotePath], fixtureRoot)
+  runGit(['symbolic-ref', 'HEAD', 'refs/heads/main'], remotePath)
+  runGit(['remote', 'add', 'origin', remotePath], repoPath)
+  runGit(['push', 'origin', 'main'], repoPath)
+}
+
+export function seedBranchCleanupRepro(repoPath, worktreePath) {
+  runGit(
+    ['commit', '--allow-empty', '-m', 'Trigger safe branch cleanup', '--no-gpg-sign'],
+    worktreePath
+  )
+  const branch = runGit(['symbolic-ref', '--short', 'HEAD'], worktreePath)
+  runGit(['config', `branch.${branch}.base`, 'refs/remotes/origin/main'], repoPath)
+}
+
+export async function startDelayedFetchServer(delayMs) {
+  const sockets = new Set()
+  const timers = new Set()
+  const server = net.createServer((socket) => {
+    sockets.add(socket)
+    const timer = setTimeout(() => {
+      timers.delete(timer)
+      socket.end('HTTP/1.1 503 Service Unavailable\r\nContent-Length: 0\r\n\r\n')
+    }, delayMs)
+    timers.add(timer)
+    socket.on('close', () => {
+      clearTimeout(timer)
+      timers.delete(timer)
+      sockets.delete(socket)
+    })
+    socket.on('error', () => undefined)
+  })
+  await new Promise((resolve, reject) => {
+    server.once('error', reject)
+    server.listen(0, '127.0.0.1', resolve)
+  })
+  const address = server.address()
+  if (!address || typeof address === 'string') {
+    throw new Error('Delayed fetch server did not expose a TCP port')
+  }
+  return {
+    url: `http://127.0.0.1:${address.port}/remote.git`,
+    close: () =>
+      new Promise((resolve) => {
+        for (const timer of timers) {
+          clearTimeout(timer)
+        }
+        timers.clear()
+        for (const socket of sockets) {
+          socket.destroy()
+        }
+        server.close(resolve)
+      })
+  }
+}

tests/tools/benchmarks/worktree-deletion-dev-bench.mjs

@@ -16,9 +16,11 @@ import path from 'node:path'
 // Why @playwright/test, not playwright: only the former is a declared devDependency; it re-exports
 // the same browser types, so the benchmark resolves without relying on a hoisted transitive install.
 import { chromium } from '@playwright/test'
+import * as branchFixture from './worktree-deletion-branch-fixture.mjs'
 
 const DEFAULT_ITERATIONS = 3
 const DEFAULT_HISTORY_FILES = 10_000
+const DEFAULT_FETCH_DELAY_MS = 1_500
 const CDP_START_PORT = 9_700
 const START_TIMEOUT_MS = 180_000
 const IPC_TIMEOUT_MS = 90_000
@@ -29,6 +31,7 @@ function parseArgs(argv) {
     instances: [],
     iterations: DEFAULT_ITERATIONS,
     historyFiles: DEFAULT_HISTORY_FILES,
+    fetchDelayMs: DEFAULT_FETCH_DELAY_MS,
     keepFixture: false
   }
   for (let index = 2; index < argv.length; index += 1) {
@@ -65,6 +68,8 @@ function parseArgs(argv) {
       options.iterations = readPositiveInteger(value, next())
     } else if (value === '--history-files') {
       options.historyFiles = readPositiveInteger(value, next())
+    } else if (value === '--fetch-delay-ms') {
+      options.fetchDelayMs = readPositiveInteger(value, next())
     } else {
       throw new Error(`Unknown argument: ${value}`)
     }
@@ -91,6 +96,7 @@ Options:
   --instance <label=path>  Dev checkout to launch; repeat for A/B comparison
   --iterations <count>     Deletions per instance (default: ${DEFAULT_ITERATIONS})
   --history-files <count>  Files seeded in each worktree history (default: ${DEFAULT_HISTORY_FILES})
+  --fetch-delay-ms <ms>    Slow-remote delay for branch cleanup (default: ${DEFAULT_FETCH_DELAY_MS})
   --keep-fixture           Keep disposable profiles and repos for inspection`)
 }
 
@@ -117,6 +123,7 @@ function createFixture(instanceLabel) {
   run('git', ['add', 'README.md'], repoPath)
   run('git', ['commit', '-m', 'Initialize benchmark fixture', '--no-gpg-sign'], repoPath)
   run('git', ['branch', '-m', 'main'], repoPath)
+  branchFixture.initializeBranchCleanupRemote(root, repoPath)
   return { root, repoPath, userDataPath }
 }
 
@@ -406,6 +413,7 @@ async function verifyDeletion(page, worktree, historyPath, measurement) {
 async function runIteration(page, fixture, repoState, iteration, historyFiles) {
   const worktree = await createMeasuredWorktree(page, repoState.repoId, iteration)
   await assertWorktreeRowVisible(page, worktree.id)
+  branchFixture.seedBranchCleanupRepro(fixture.repoPath, worktree.path)
   const historyPath = seedTerminalHistory(fixture.userDataPath, worktree.id, historyFiles)
   const measurement = await measureDeletion(page, worktree.id, repoState.rootWorktreeId)
   await verifyDeletion(page, worktree, historyPath, measurement)
@@ -467,14 +475,14 @@ async function benchmarkInstance(instanceConfig, index, options) {
     instanceConfig.repoRoot
   )
   const fixture = createFixture(instanceConfig.label)
-  const port = await findAvailablePort(CDP_START_PORT + index)
-  const instance = launchDevInstance(instanceConfig, fixture, port)
-  let browser = null
-  console.log(`[${instanceConfig.label}] launching ${instanceConfig.repoRoot}`)
+  let delayedFetchServer, instance, browser
   try {
-    const connection = await connectToOrca(instance)
-    browser = connection.browser
-    const { page } = connection
+    delayedFetchServer = await branchFixture.startDelayedFetchServer(options.fetchDelayMs)
+    run('git', ['remote', 'set-url', 'origin', delayedFetchServer.url], fixture.repoPath)
+    const port = await findAvailablePort(CDP_START_PORT + index)
+    instance = launchDevInstance(instanceConfig, fixture, port)
+    const { browser: connectedBrowser, page } = await connectToOrca(instance)
+    browser = connectedBrowser
     const repoState = await addFixtureRepo(page, fixture.repoPath)
     const iterations = []
     for (let iteration = 1; iteration <= options.iterations; iteration += 1) {
@@ -500,6 +508,7 @@ async function benchmarkInstance(instanceConfig, index, options) {
       label: instanceConfig.label,
       repoRoot: instanceConfig.repoRoot,
       historyFiles: options.historyFiles,
+      fetchDelayMs: options.fetchDelayMs,
       iterations,
       summary: summarize(iterations.map((entry) => entry.totalMs)),
       ipcSummary: summarize(iterations.flatMap((entry) => entry.ipcLatencyMs.samples)),
@@ -508,7 +517,10 @@ async function benchmarkInstance(instanceConfig, index, options) {
     }
   } finally {
     await browser?.close().catch(() => undefined)
-    await stopDevInstance(instance)
+    if (instance) {
+      await stopDevInstance(instance)
+    }
+    await delayedFetchServer?.close()
     if (!options.keepFixture) {
       await rm(fixture.root, {
         recursive: true,