diff --git a/src/main/git/remove-worktree.test.ts b/src/main/git/remove-worktree.test.ts index c0789d1529c..5e4bf992979 100644 --- a/src/main/git/remove-worktree.test.ts +++ b/src/main/git/remove-worktree.test.ts @@ -41,6 +41,7 @@ import { forceDeleteLocalBranch, listWorktrees, removeWorktree, + WORKTREE_LIST_TIMEOUT_MS, WORKTREE_REMOVAL_PREFLIGHT_TIMEOUT_MS } from './worktree' @@ -1009,7 +1010,8 @@ describe('listWorktrees', () => { ]) expect(gitExecFileAsyncMock).toHaveBeenCalledWith(['worktree', 'list', '--porcelain', '-z'], { cwd: 'C:\\Users\\me\\repo', - wslDistro: 'Ubuntu' + wslDistro: 'Ubuntu', + timeout: WORKTREE_LIST_TIMEOUT_MS }) expect(translateWslOutputPathsMock).toHaveBeenCalledWith( expect.any(String), @@ -1030,7 +1032,8 @@ describe('listWorktrees', () => { await expect(listWorktrees('/workspace/deleted-repo')).resolves.toEqual([]) expect(gitExecFileAsyncMock).toHaveBeenCalledWith(['worktree', 'list', '--porcelain', '-z'], { - cwd: '/workspace/deleted-repo' + cwd: '/workspace/deleted-repo', + timeout: WORKTREE_LIST_TIMEOUT_MS }) expect(statMock).toHaveBeenCalledWith('/workspace/deleted-repo') expect(warnSpy).toHaveBeenCalledWith( @@ -1052,7 +1055,8 @@ describe('listWorktrees', () => { await expect(listWorktrees('/private/tmp/orca-issue-1582-test/my-repo')).resolves.toEqual([]) expect(gitExecFileAsyncMock).toHaveBeenCalledWith(['worktree', 'list', '--porcelain', '-z'], { - cwd: '/private/tmp/orca-issue-1582-test/my-repo' + cwd: '/private/tmp/orca-issue-1582-test/my-repo', + timeout: WORKTREE_LIST_TIMEOUT_MS }) expect(warnSpy).not.toHaveBeenCalled() warnSpy.mockRestore() diff --git a/src/main/git/worktree.test.ts b/src/main/git/worktree.test.ts index 258ecfa36df..493beb1d91f 100644 --- a/src/main/git/worktree.test.ts +++ b/src/main/git/worktree.test.ts @@ -30,7 +30,8 @@ import { moveWorktree, parseWorktreeList, removeWorktree, - WORKTREE_ADD_TIMEOUT_MS + WORKTREE_ADD_TIMEOUT_MS, + WORKTREE_LIST_TIMEOUT_MS } from './worktree' beforeEach(() => { @@ -73,6 +74,21 @@ describe('listWorktrees in-flight sharing', () => { expect(gitExecFileAsyncMock).toHaveBeenCalledTimes(1) }) + it('does not share scans across different timeout contracts', async () => { + const scanOutput = 'worktree /repo\nHEAD abc123\nbranch refs/heads/main\n' + gitExecFileAsyncMock.mockResolvedValue({ stdout: scanOutput }) + + await Promise.all([listWorktrees('/repo'), listWorktrees('/repo', { timeout: 5_000 })]) + + expect(gitExecFileAsyncMock.mock.calls).toEqual([ + [ + ['worktree', 'list', '--porcelain', '-z'], + { cwd: '/repo', timeout: WORKTREE_LIST_TIMEOUT_MS } + ], + [['worktree', 'list', '--porcelain', '-z'], { cwd: '/repo', timeout: 5_000 }] + ]) + }) + it('runs a fresh scan once the shared one has settled', async () => { const scanOutput = 'worktree /repo\nHEAD abc123\nbranch refs/heads/main\n' gitExecFileAsyncMock.mockResolvedValue({ stdout: scanOutput }) @@ -83,6 +99,27 @@ describe('listWorktrees in-flight sharing', () => { expect(gitExecFileAsyncMock).toHaveBeenCalledTimes(2) }) + it('runs a fresh scan after a timed-out shared scan settles', async () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined) + try { + gitExecFileAsyncMock + .mockRejectedValueOnce(new Error('git timed out.')) + .mockResolvedValueOnce({ + stdout: 'worktree /repo\nHEAD abc123\nbranch refs/heads/main\n' + }) + + await expect(listWorktrees('/repo')).resolves.toEqual([]) + await expect(listWorktrees('/repo')).resolves.toEqual([ + expect.objectContaining({ path: '/repo', head: 'abc123' }) + ]) + + expect(gitExecFileAsyncMock).toHaveBeenCalledTimes(2) + expect(_getWorktreeScanCacheSizesForTests()).toEqual({ inFlight: 0, generations: 0 }) + } finally { + warnSpy.mockRestore() + } + }) + it('does not share scans across different repos', async () => { gitExecFileAsyncMock.mockImplementation((_args: string[], options: { cwd: string }) => Promise.resolve({ @@ -551,7 +588,8 @@ branch refs/heads/feature/test expect(gitExecFileAsyncMock).toHaveBeenCalledTimes(1) expect(gitExecFileAsyncMock).toHaveBeenCalledWith(['worktree', 'list', '--porcelain', '-z'], { - cwd: '/repo' + cwd: '/repo', + timeout: WORKTREE_LIST_TIMEOUT_MS }) }) @@ -595,8 +633,11 @@ branch refs/heads/main-2 ]) expect(gitExecFileAsyncMock.mock.calls).toEqual([ - [['worktree', 'list', '--porcelain', '-z'], { cwd: '/repo' }], - [['worktree', 'list', '--porcelain'], { cwd: '/repo' }] + [ + ['worktree', 'list', '--porcelain', '-z'], + { cwd: '/repo', timeout: WORKTREE_LIST_TIMEOUT_MS } + ], + [['worktree', 'list', '--porcelain'], { cwd: '/repo', timeout: WORKTREE_LIST_TIMEOUT_MS }] ]) }) @@ -628,8 +669,11 @@ branch refs/heads/main ]) expect(gitExecFileAsyncMock.mock.calls).toEqual([ - [['worktree', 'list', '--porcelain', '-z'], { cwd: '/repo' }], - [['worktree', 'list', '--porcelain'], { cwd: '/repo' }] + [ + ['worktree', 'list', '--porcelain', '-z'], + { cwd: '/repo', timeout: WORKTREE_LIST_TIMEOUT_MS } + ], + [['worktree', 'list', '--porcelain'], { cwd: '/repo', timeout: WORKTREE_LIST_TIMEOUT_MS }] ]) }) @@ -646,7 +690,10 @@ branch refs/heads/main await expect(listWorktreeGraph('/repo')).resolves.toEqual([]) expect(gitExecFileAsyncMock.mock.calls).toEqual([ - [['worktree', 'list', '--porcelain', '-z'], { cwd: '/repo' }] + [ + ['worktree', 'list', '--porcelain', '-z'], + { cwd: '/repo', timeout: WORKTREE_LIST_TIMEOUT_MS } + ] ]) }) @@ -655,6 +702,20 @@ branch refs/heads/main await expect(listWorktreeGraph('/not-a-repo')).resolves.toEqual([]) }) + + it('lets callers override the default worktree list timeout', async () => { + gitExecFileAsyncMock.mockResolvedValueOnce({ + stdout: 'worktree /repo\nHEAD abc123\nbranch refs/heads/main\n' + }) + + await listWorktreeGraph('/repo', { timeout: 5_000 }) + + expect(gitExecFileAsyncMock).toHaveBeenCalledWith(['worktree', 'list', '--porcelain', '-z'], { + cwd: '/repo', + timeout: 5_000 + }) + expect(WORKTREE_LIST_TIMEOUT_MS).toBe(30_000) + }) }) describe('addWorktree', () => { diff --git a/src/main/git/worktree.ts b/src/main/git/worktree.ts index 92fb4437d77..08decaefcaf 100644 --- a/src/main/git/worktree.ts +++ b/src/main/git/worktree.ts @@ -87,6 +87,8 @@ const PRUNABLE_EXISTENCE_PROBE_CONCURRENCY = 8 // Why: bound `git worktree add` so a OneDrive cloud-placeholder stall fails fast (STA-1292); generous enough for a legit large checkout (#7225). export const WORKTREE_ADD_TIMEOUT_MS = 180_000 export const WORKTREE_REMOVAL_PREFLIGHT_TIMEOUT_MS = 30_000 +// Why: one wedged shared scan otherwise hangs every later list, including create's post-add re-list. +export const WORKTREE_LIST_TIMEOUT_MS = 30_000 function gitExecOptions( cwd: string, @@ -572,13 +574,18 @@ async function readWorktreeList( cwd: repoPath, wslDistro: options.wslDistro }) + const execOptions = { + cwd: repoPath, + ...options, + timeout: options.timeout ?? WORKTREE_LIST_TIMEOUT_MS + } return capabilities.runWithFallback( 'worktree-list-z', async () => { - const { stdout } = await gitExecFileAsync(['worktree', 'list', '--porcelain', '-z'], { - cwd: repoPath, - ...options - }) + const { stdout } = await gitExecFileAsync( + ['worktree', 'list', '--porcelain', '-z'], + execOptions + ) return normalizeMainWorktreePath( repoPath, parseWorktreeList(stdout, { nulDelimited: true }), @@ -587,10 +594,7 @@ async function readWorktreeList( }, async () => { // Why: `-z` preserves worktree paths with newlines but Git <2.36 rejects it; fall back to the line parser. - const { stdout } = await gitExecFileAsync(['worktree', 'list', '--porcelain'], { - cwd: repoPath, - ...options - }) + const { stdout } = await gitExecFileAsync(['worktree', 'list', '--porcelain'], execOptions) const normalized = await normalizeMainWorktreePath( repoPath, parseWorktreeList(stdout), @@ -737,7 +741,9 @@ export function listWorktrees( return listWorktreesUnshared(repoPath, options) } const generation = worktreeScanGenerations.get(repoPath) ?? 0 - const key = `${repoPath}\0${options.wslDistro ?? ''}\0${generation}` + const timeout = options.timeout ?? WORKTREE_LIST_TIMEOUT_MS + // Why: callers with different deadlines cannot safely share which timeout wins the scan. + const key = `${repoPath}\0${options.wslDistro ?? ''}\0${timeout}\0${generation}` const inFlight = inFlightWorktreeScans.get(key) if (inFlight) { return inFlight