fix(worktrees): bound shared worktree list scans (#9786)

This commit is contained in:
Brennan Benson
2026-07-21 16:10:01 -07:00
committed by GitHub
parent 21e26de456
commit 71bbfa022d
3 changed files with 90 additions and 19 deletions
+7 -3
View File
@@ -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()
+68 -7
View File
@@ -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', () => {
+15 -9
View File
@@ -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