fix(quick-open): prune generated dirs in git fallback (#7842)

* fix(quick-open): prune generated dirs in git fallback

* fix(source-control): narrow manual review provider switch

* fix(quick-open): collapse git fallback directories

Co-authored-by: Orca <help@stably.ai>

* fix(quick-open): harden collapsed git directory expansion

Co-authored-by: Orca <help@stably.ai>

* fix(quick-open): close directory expansion races

Co-authored-by: Orca <help@stably.ai>

* perf(quick-open): collapse placeholders efficiently

Co-authored-by: Orca <help@stably.ai>

* fix(quick-open): preserve root and cancellation semantics

Co-authored-by: Orca <help@stably.ai>

---------

Co-authored-by: Jinwoo-H <jinwoo0825@gmail.com>
Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
BingZ
2026-07-10 03:06:44 -07:00
committed by GitHub
co-authored by Orca Jinwoo-H
parent 506f6ffbb9
commit c5669cceb5
15 changed files with 778 additions and 102 deletions
@@ -0,0 +1,103 @@
import { EventEmitter } from 'node:events'
import type { ChildProcess } from 'node:child_process'
import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises'
import { tmpdir } from 'node:os'
import { dirname, join } from 'node:path'
import { afterEach, describe, expect, it, vi } from 'vitest'
const { gitSpawnMock } = vi.hoisted(() => ({
gitSpawnMock: vi.fn()
}))
vi.mock('../git/runner', () => ({
gitSpawn: gitSpawnMock
}))
import { listFilesWithGit } from './filesystem-list-files-git-fallback'
import { isFileListingCancellation } from '../../shared/file-listing-cancellation'
const tempDirs: string[] = []
const SHA1 = '0123456789abcdef0123456789abcdef01234567'
function createMockProcess(): ChildProcess {
const process = new EventEmitter() as unknown as ChildProcess
;(process as unknown as Record<string, unknown>).stdout = new EventEmitter()
;(
(process as unknown as Record<string, unknown>).stdout as EventEmitter & {
setEncoding: () => void
}
).setEncoding = vi.fn()
;(process as unknown as Record<string, unknown>).stderr = new EventEmitter()
;(process as unknown as Record<string, unknown>).kill = vi.fn()
;(process as unknown as Record<string, unknown>).exitCode = null
;(process as unknown as Record<string, unknown>).signalCode = null
return process
}
async function writeRel(root: string, relPath: string): Promise<void> {
const absPath = join(root, ...relPath.split('/'))
await mkdir(dirname(absPath), { recursive: true })
await writeFile(absPath, 'x')
}
afterEach(async () => {
vi.clearAllMocks()
await Promise.all(tempDirs.splice(0).map((dir) => rm(dir, { recursive: true, force: true })))
})
describe('main Quick Open git directory expansion', () => {
it('expands placeholders emitted by both directory-collapsing passes', async () => {
const root = await mkdtemp(join(tmpdir(), 'orca-main-git-ignored-dir-'))
tempDirs.push(root)
await writeRel(root, 'dist/generated.js')
await writeRel(root, 'scratch/notes.txt')
const revParse = createMockProcess()
const primary = createMockProcess()
const ignored = createMockProcess()
gitSpawnMock
.mockReturnValueOnce(revParse)
.mockReturnValueOnce(primary)
.mockReturnValueOnce(ignored)
const promise = listFilesWithGit(root, [], {})
revParse.emit('close', 0, null)
await vi.waitFor(() => expect(gitSpawnMock).toHaveBeenCalledTimes(3))
;(primary.stdout as unknown as EventEmitter).emit(
'data',
`100644 ${SHA1} 0\tsrc/index.ts\0scratch/\0`
)
primary.emit('close', 0, null)
;(ignored.stdout as unknown as EventEmitter).emit('data', 'dist/\0')
ignored.emit('close', 0, null)
await expect(promise).resolves.toEqual([
'dist/generated.js',
'scratch/notes.txt',
'src/index.ts'
])
expect(gitSpawnMock.mock.calls[2][0]).toContain('--directory')
})
it('cancels both local Git passes when Quick Open abandons the request', async () => {
const root = await mkdtemp(join(tmpdir(), 'orca-main-git-cancel-'))
tempDirs.push(root)
const revParse = createMockProcess()
const primary = createMockProcess()
const ignored = createMockProcess()
gitSpawnMock
.mockReturnValueOnce(revParse)
.mockReturnValueOnce(primary)
.mockReturnValueOnce(ignored)
const controller = new AbortController()
const promise = listFilesWithGit(root, [], {}, controller.signal)
revParse.emit('close', 0, null)
await vi.waitFor(() => expect(gitSpawnMock).toHaveBeenCalledTimes(3))
controller.abort()
await expect(promise).rejects.toSatisfy(isFileListingCancellation)
expect(primary.kill).toHaveBeenCalled()
expect(ignored.kill).toHaveBeenCalled()
})
})
@@ -3,9 +3,10 @@ import { gitSpawn } from '../git/runner'
import { buildGitLsFilesArgsForQuickOpen } from '../../shared/quick-open-filter'
import {
createQuickOpenReaddirBudget,
expandQuickOpenGitFilesWithNestedRepos,
expandQuickOpenGitFileListing,
listQuickOpenFilesWithReaddir
} from '../../shared/quick-open-readdir-walk'
import { fileListingCancellationError } from '../../shared/file-listing-cancellation'
/**
* Fallback file lister using git ls-files. Used when rg is not available.
@@ -16,9 +17,13 @@ import {
*/
async function isInsideGitWorkTree(
rootPath: string,
localGitOptions: { wslDistro?: string }
localGitOptions: { wslDistro?: string },
signal?: AbortSignal
): Promise<boolean> {
return new Promise((resolve) => {
if (signal?.aborted) {
throw fileListingCancellationError(signal)
}
return new Promise((resolve, reject) => {
const child = gitSpawn(['rev-parse', '--is-inside-work-tree'], {
cwd: rootPath,
...(localGitOptions.wslDistro ? { wslDistro: localGitOptions.wslDistro } : {}),
@@ -26,22 +31,37 @@ async function isInsideGitWorkTree(
})
let done = false
let timer: ReturnType<typeof setTimeout>
const cleanup = (): void => {
clearTimeout(timer)
child.off('error', handleError)
child.off('close', handleClose)
signal?.removeEventListener('abort', handleAbort)
}
const finish = (isGitRepo: boolean): void => {
if (done) {
return
}
done = true
clearTimeout(timer)
child.off('error', handleError)
child.off('close', handleClose)
cleanup()
resolve(isGitRepo)
}
const cancel = (): void => {
if (done) {
return
}
done = true
child.kill()
cleanup()
reject(fileListingCancellationError(signal))
}
const handleError = (): void => finish(false)
const handleClose = (code: number | null, signal: NodeJS.Signals | null): void =>
finish(code === 0 && signal === null)
const handleAbort = (): void => cancel()
child.once('error', handleError)
child.once('close', handleClose)
signal?.addEventListener('abort', handleAbort, { once: true })
timer = setTimeout(() => {
child.kill()
finish(false)
@@ -52,16 +72,23 @@ async function isInsideGitWorkTree(
export async function listFilesWithGit(
rootPath: string,
excludePathPrefixes: readonly string[],
localGitOptions: { wslDistro?: string }
localGitOptions: { wslDistro?: string },
signal?: AbortSignal
): Promise<string[]> {
if (!(await isInsideGitWorkTree(rootPath, localGitOptions))) {
const isGitWorkTree = await isInsideGitWorkTree(rootPath, localGitOptions, signal)
if (signal?.aborted) {
throw fileListingCancellationError(signal)
}
if (!isGitWorkTree) {
return listQuickOpenFilesWithReaddir(rootPath, {
excludePathPrefixes,
budget: createQuickOpenReaddirBudget()
budget: createQuickOpenReaddirBudget(),
signal
})
}
const gitPaths = new Set<string>()
const directoryPaths = new Set<string>()
const { primary, ignoredPass } = buildGitLsFilesArgsForQuickOpen(excludePathPrefixes)
const children: {
child: ChildProcess
@@ -78,7 +105,11 @@ export async function listFilesWithGit(
if (!path) {
return
}
gitPaths.add(path)
if (path.endsWith('/')) {
directoryPaths.add(path)
} else {
gitPaths.add(path)
}
}
// Why: git ls-files outputs paths relative to cwd, so we set cwd to
@@ -168,7 +199,7 @@ export async function listFilesWithGit(
})
}
const killSurvivors = (): void => {
const killSurvivors = (reason = 'git ls-files canceled after sibling failure'): void => {
// Why: Promise.all rejects on the first failed pass; cancel the sibling so
// a stuck git process cannot keep scanning after Quick Open has failed.
for (const entry of children) {
@@ -178,20 +209,32 @@ export async function listFilesWithGit(
if (entry.child.exitCode === null && entry.child.signalCode === null) {
entry.child.kill()
}
entry.reject(new Error('git ls-files canceled after sibling failure'))
entry.reject(new Error(reason))
}
}
const onAbort = (): void => killSurvivors('git ls-files cancelled')
signal?.addEventListener('abort', onAbort, { once: true })
try {
await Promise.all([runGitLsFiles(primary), runGitLsFiles(ignoredPass)])
} catch (err) {
killSurvivors()
if (signal?.aborted) {
throw fileListingCancellationError(signal)
}
throw err
} finally {
signal?.removeEventListener('abort', onAbort)
}
return expandQuickOpenGitFilesWithNestedRepos({
const files = await expandQuickOpenGitFileListing({
rootPath,
gitPaths,
excludePathPrefixes
directoryPaths,
excludePathPrefixes,
signal
})
// Why: directory placeholders are expanded after Git exits; restore Git's
// path order so empty queries and fuzzy-score ties remain stable.
return files.sort()
}
@@ -365,6 +365,9 @@ describe('filesystem-list-files', () => {
expect(gitCalls.length).toBe(2)
expect(gitCalls[0][1]).toContain('ls-files')
expect(gitCalls[0][1]).toContain('-s')
expect(gitCalls[0][1]).toContain('--directory')
expect(gitCalls[1][1]).toContain('--directory')
expect(gitCalls[1][1]).toContain('--no-empty-directory')
// Should include valid files and filter node_modules
expect(result).toContain('src/index.ts')
+3 -2
View File
@@ -19,7 +19,8 @@ import { listFilesWithGit } from './filesystem-list-files-git-fallback'
export async function listQuickOpenFiles(
rootPath: string,
store: Store,
excludePaths?: string[]
excludePaths?: string[],
signal?: AbortSignal
): Promise<string[]> {
const authorizedRootPath = await resolveAuthorizedPath(rootPath, store)
const localGitOptions = getLocalGitOptionsForRegisteredWorktree(
@@ -40,7 +41,7 @@ export async function listQuickOpenFiles(
// can run.
const rgAvailable = await checkRgAvailable(authorizedRootPath, localGitOptions.wslDistro)
if (!rgAvailable) {
return listFilesWithGit(authorizedRootPath, excludePathPrefixes, localGitOptions)
return listFilesWithGit(authorizedRootPath, excludePathPrefixes, localGitOptions, signal)
}
const files = new Set<string>()
+2 -3
View File
@@ -1063,7 +1063,7 @@ export function registerFilesystemHandlers(
signal: controller?.signal
})
}
return await listQuickOpenFiles(args.rootPath, store, args.excludePaths)
return await listQuickOpenFiles(args.rootPath, store, args.excludePaths, controller?.signal)
} finally {
if (args.requestToken) {
listFilesCancellations.delete(args.requestToken)
@@ -1073,8 +1073,7 @@ export function registerFilesystemHandlers(
)
ipcMain.handle('fs:cancelListFiles', (_event, args: { requestToken: string }): void => {
// Why: best-effort — the entry is gone once the listing settles, and
// local scans are fast enough to simply let them finish.
// Why: best-effort — the entry is gone once the listing settles.
listFilesCancellations.get(args.requestToken)?.abort()
})
+14 -5
View File
@@ -10,7 +10,7 @@ import { spawn } from 'node:child_process'
import { fileListingCancellationError } from '../shared/file-listing-cancellation'
import type { SearchOptions, SearchResult } from './fs-handler-utils'
import { buildGitLsFilesArgsForQuickOpen } from '../shared/quick-open-filter'
import { expandQuickOpenGitFilesWithNestedRepos } from '../shared/quick-open-readdir-walk'
import { expandQuickOpenGitFileListing } from '../shared/quick-open-readdir-walk'
import {
buildGitGrepArgs,
buildSubmatchRegex,
@@ -40,6 +40,7 @@ export function listFilesWithGit(
return Promise.reject(fileListingCancellationError(signal))
}
const gitPaths = new Set<string>()
const directoryPaths = new Set<string>()
const { primary, ignoredPass } = buildGitLsFilesArgsForQuickOpen(excludePathPrefixes)
const children: {
child: ReturnType<typeof spawn>
@@ -56,7 +57,11 @@ export function listFilesWithGit(
if (!path) {
return
}
gitPaths.add(path)
if (path.endsWith('/')) {
directoryPaths.add(path)
} else {
gitPaths.add(path)
}
}
const child = spawn('git', ['ls-files', ...args], {
@@ -172,14 +177,18 @@ export function listFilesWithGit(
signal?.addEventListener('abort', onAbort, { once: true })
return Promise.all([runGitLsFiles(primary), runGitLsFiles(ignoredPass)])
.then(() =>
expandQuickOpenGitFilesWithNestedRepos({
.then(async () => {
const files = await expandQuickOpenGitFileListing({
rootPath,
gitPaths,
directoryPaths,
excludePathPrefixes,
signal
})
)
// Why: directory placeholders are expanded after Git exits; restore
// Git's path order for empty queries and fuzzy-score ties over SSH.
return files.sort()
})
.catch((err) => {
killSurvivors('git ls-files canceled after sibling failure')
if (signal?.aborted) {
@@ -98,6 +98,8 @@ describe('relay quick open ignored file listing', () => {
})
it('git fallback ignored pass includes ignored non-env files', async () => {
const root = await makeTempRoot()
await writeRel(root, 'dist/generated.js')
const primaryProc = createMockProcess()
const ignoredProc = createMockProcess()
let callIndex = 0
@@ -107,7 +109,7 @@ describe('relay quick open ignored file listing', () => {
return callIndex === 1 ? primaryProc : ignoredProc
})
const promise = listFilesWithGit('/remote/root', ['packages/other'])
const promise = listFilesWithGit(root, ['packages/other'])
setTimeout(() => {
;(primaryProc.stdout as unknown as EventEmitter).emit(
@@ -120,26 +122,28 @@ describe('relay quick open ignored file listing', () => {
)
primaryProc.emit('close', 0, null)
;(ignoredProc.stdout as unknown as EventEmitter).emit('data', 'dist/generated.js\0')
;(ignoredProc.stdout as unknown as EventEmitter).emit('data', 'dist/\0')
;(ignoredProc.stdout as unknown as EventEmitter).emit('data', 'packages/other/src/x.ts\0')
ignoredProc.emit('close', 0, null)
}, 10)
await expect(promise).resolves.toEqual(['src/index.ts', 'tab\tfile.txt', 'dist/generated.js'])
await expect(promise).resolves.toEqual(['dist/generated.js', 'src/index.ts', 'tab\tfile.txt'])
const ignoredArgs = spawnMock.mock.calls[1][1] as string[]
expect(ignoredArgs).toEqual([
expect(ignoredArgs.slice(0, 6)).toEqual([
'ls-files',
'-z',
'-s',
'--others',
'--ignored',
'--exclude-standard',
'--',
'.',
':(exclude,glob)packages/other',
':(exclude,glob)packages/other/**'
'--exclude-standard'
])
expect(ignoredArgs).toContain('--')
expect(ignoredArgs).toContain('.')
expect(ignoredArgs).toContain('--directory')
expect(ignoredArgs).toContain('--no-empty-directory')
expect(ignoredArgs).toContain(':(exclude,glob)packages/other')
expect(ignoredArgs).toContain(':(exclude,glob)packages/other/**')
})
it('git fallback fills nested git repos returned as root-relative placeholders', async () => {
@@ -0,0 +1,7 @@
import type { Stats } from 'node:fs'
export function isQuickOpenReadableDirectory(stat: Stats, allowSymlinkedRoot = false): boolean {
// Why: an explicitly selected workspace root may be a symlink, while nested
// traversal must never follow a symlink outside that authorized root.
return stat.isDirectory() || Boolean(allowSymlinkedRoot && stat.isSymbolicLink())
}
@@ -0,0 +1,26 @@
import { describe, expect, it } from 'vitest'
import { collapseQuickOpenExpansionPaths } from './quick-open-expansion-paths'
describe('collapseQuickOpenExpansionPaths', () => {
it('collapses descendants without confusing shared-prefix siblings', () => {
const paths = new Map<string, boolean>([
['foo/bar/deep', false],
['foo-sibling', false],
['foo', false],
['foo/bar', true]
])
expect(collapseQuickOpenExpansionPaths(paths)).toEqual([
['foo', true],
['foo-sibling', false]
])
})
it('keeps many sibling placeholders distinct', () => {
const paths = new Map(
Array.from({ length: 10_001 }, (_, index) => [`generated-${index}`, true] as const)
)
expect(collapseQuickOpenExpansionPaths(paths)).toHaveLength(paths.size)
})
})
+37
View File
@@ -0,0 +1,37 @@
/**
* Remove descendant placeholders already covered by an ancestor. Sorting puts
* ancestors first; prefix lookups avoid quadratic scans across sibling paths.
*/
export function collapseQuickOpenExpansionPaths(
expansionPaths: ReadonlyMap<string, boolean>
): [string, boolean][] {
const sortedPaths = Array.from(expansionPaths).sort(([left], [right]) =>
left < right ? -1 : left > right ? 1 : 0
)
const collapsedPaths = new Map<string, boolean>()
for (const [relPath, includeSymlinks] of sortedPaths) {
let ancestorPath: string | undefined
let slashIndex = relPath.indexOf('/')
while (slashIndex !== -1) {
const candidate = relPath.substring(0, slashIndex)
if (collapsedPaths.has(candidate)) {
ancestorPath = candidate
break
}
slashIndex = relPath.indexOf('/', slashIndex + 1)
}
if (ancestorPath) {
// Why: primary and ignored passes can overlap; the ancestor walk covers
// the descendant, but must preserve either pass's symlink-leaf contract.
if (includeSymlinks) {
collapsedPaths.set(ancestorPath, true)
}
continue
}
collapsedPaths.set(relPath, includeSymlinks)
}
return Array.from(collapsedPaths)
}
+29 -2
View File
@@ -85,6 +85,9 @@ describe('buildExcludePathPrefixes', () => {
expect(buildExcludePathPrefixes('C:\\repo', ['C:\\repo\\packages\\app'])).toEqual([
'packages/app'
])
expect(
buildExcludePathPrefixes('//Server/Share/Repo', ['//server/share/repo/packages/app'])
).toEqual(['packages/app'])
})
it('strips trailing slashes', () => {
@@ -239,16 +242,40 @@ describe('normalizeQuickOpenRgLine', () => {
describe('buildGitLsFilesArgsForQuickOpen', () => {
it('primary pass is --cached --others --exclude-standard', () => {
const { primary } = buildGitLsFilesArgsForQuickOpen()
expect(primary).toEqual(['-z', '-s', '--cached', '--others', '--exclude-standard'])
expect(primary).toEqual([
'-z',
'-s',
'--cached',
'--others',
'--exclude-standard',
'--directory',
'--no-empty-directory'
])
})
it('ignored pass surfaces ignored files without .env* pathspec whitelist', () => {
const { ignoredPass } = buildGitLsFilesArgsForQuickOpen()
expect(ignoredPass).toEqual(['-z', '-s', '--others', '--ignored', '--exclude-standard'])
expect(ignoredPass).toEqual([
'-z',
'-s',
'--others',
'--ignored',
'--exclude-standard',
'--directory',
'--no-empty-directory'
])
expect(ignoredPass).not.toContain('.env*')
expect(ignoredPass).not.toContain(':(glob)**/.env*')
})
it('collapses untracked directories in both passes without generated pathspec churn', () => {
const { primary, ignoredPass } = buildGitLsFilesArgsForQuickOpen()
expect(primary).toContain('--directory')
expect(ignoredPass).toContain('--directory')
expect(ignoredPass).toContain('--no-empty-directory')
expect([...primary, ...ignoredPass]).not.toContain(':(exclude,glob)**/node_modules/**')
})
it('exclude prefixes prepend positive "." pathspec', () => {
const { primary, ignoredPass } = buildGitLsFilesArgsForQuickOpen(['packages/app'])
const dashDashIdx = primary.indexOf('--')
+15 -3
View File
@@ -104,8 +104,8 @@ function pathFlavor(rootPath: string): typeof posix | typeof win32 {
if (/^[a-zA-Z]:[\\/]/.test(rootPath)) {
return win32
}
// UNC \\server\share
if (rootPath.startsWith('\\\\')) {
// UNC \\server\share or //server/share
if (rootPath.startsWith('\\\\') || rootPath.startsWith('//')) {
return win32
}
return posix
@@ -376,16 +376,28 @@ export function buildGitLsFilesArgsForQuickOpen(
excludeSpecs.push(`:(exclude,glob)${escapeGlobPath(prefix)}/**`)
}
const trailingPathspecs = excludeSpecs.length > 0 ? ['--', '.', ...excludeSpecs] : []
// Why: collapse untracked trees before Git traverses them; callers expand
// only allowed directory placeholders with the shared bounded walker.
const directoryCollapseArgs = ['--directory', '--no-empty-directory']
// Why: NUL preserves real Git paths; stage mode identifies gitlinks without
// lstat probes for ordinary tracked files.
const primary = ['-z', '-s', '--cached', '--others', '--exclude-standard', ...trailingPathspecs]
const primary = [
'-z',
'-s',
'--cached',
'--others',
'--exclude-standard',
...directoryCollapseArgs,
...trailingPathspecs
]
const ignoredPass = [
'-z',
'-s',
'--others',
'--ignored',
'--exclude-standard',
...directoryCollapseArgs,
...trailingPathspecs
]
return { primary, ignoredPass }
@@ -0,0 +1,58 @@
import { execFile } from 'node:child_process'
import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises'
import { tmpdir } from 'node:os'
import { dirname, join } from 'node:path'
import { promisify } from 'node:util'
import { afterEach, describe, expect, it } from 'vitest'
import { buildGitLsFilesArgsForQuickOpen } from './quick-open-filter'
const execFileAsync = promisify(execFile)
const tempDirs: string[] = []
async function writeRel(root: string, relPath: string): Promise<void> {
const absPath = join(root, ...relPath.split('/'))
await mkdir(dirname(absPath), { recursive: true })
await writeFile(absPath, 'x')
}
afterEach(async () => {
await Promise.all(tempDirs.splice(0).map((dir) => rm(dir, { recursive: true, force: true })))
})
describe('Quick Open git directory collapse', () => {
it('collapses unignored and ignored trees while preserving individual ignored files', async () => {
const root = await mkdtemp(join(tmpdir(), 'orca-quick-open-git-collapse-'))
tempDirs.push(root)
await execFileAsync('git', ['init', '--quiet'], { cwd: root })
await writeFile(
join(root, '.gitignore'),
['.cache/', '.local/share/', 'dist/', '*.log'].join('\n')
)
await Promise.all([
...Array.from({ length: 200 }, (_, index) =>
writeRel(root, `node_modules/pkg/file-${index}.js`)
),
writeRel(root, '.cache/state.json'),
writeRel(root, '.local/share/state.json'),
writeRel(root, 'dist/generated.js'),
writeRel(root, 'debug.log')
])
const { primary, ignoredPass } = buildGitLsFilesArgsForQuickOpen()
const [primaryResult, ignoredResult] = await Promise.all(
[primary, ignoredPass].map((args) =>
execFileAsync('git', ['-c', 'core.excludesFile=', 'ls-files', ...args], {
cwd: root,
encoding: 'buffer'
})
)
)
const primaryPaths = primaryResult.stdout.toString().split('\0').filter(Boolean)
const ignoredPaths = ignoredResult.stdout.toString().split('\0').filter(Boolean)
expect(primaryPaths).toEqual(['.gitignore', 'node_modules/'])
expect(primaryPaths.some((path) => path.startsWith('node_modules/pkg/'))).toBe(false)
expect(ignoredPaths).toEqual(['.cache/', '.local/', 'debug.log', 'dist/'])
expect(ignoredPaths.some((path) => path.startsWith('.local/share/'))).toBe(false)
})
})
+255 -14
View File
@@ -1,28 +1,32 @@
import { afterEach, describe, expect, it, vi } from 'vitest'
const { lstatMock } = vi.hoisted(() => ({
lstatMock: vi.fn()
const { lstatMock, readdirMock } = vi.hoisted(() => ({
lstatMock: vi.fn(),
readdirMock: vi.fn()
}))
vi.mock('fs/promises', async () => {
const actual = await vi.importActual<typeof import('fs/promises')>('fs/promises') // eslint-disable-line @typescript-eslint/consistent-type-imports -- vi.importActual requires inline import()
lstatMock.mockImplementation(actual.lstat)
readdirMock.mockImplementation(actual.readdir)
return {
...actual,
lstat: lstatMock
lstat: lstatMock,
readdir: readdirMock
}
})
import { mkdtemp, mkdir, rm, symlink, writeFile } from 'node:fs/promises'
import { mkdtemp, mkdir, rename, rm, symlink, writeFile } from 'node:fs/promises'
import { tmpdir } from 'node:os'
import { dirname, join } from 'node:path'
import {
classifyQuickOpenGitEntry,
createQuickOpenReaddirBudget,
expandQuickOpenGitFilesWithNestedRepos,
expandQuickOpenGitFileListing,
isQuickOpenReaddirBudgetError,
listQuickOpenFilesWithReaddir,
parseQuickOpenGitLsFilesEntry
parseQuickOpenGitLsFilesEntry,
QUICK_OPEN_READDIR_MAX_FILES
} from './quick-open-readdir-walk'
import { isFileListingCancellation } from './file-listing-cancellation'
@@ -94,7 +98,7 @@ describe('quick-open readdir walk', () => {
it('keeps ordinary git entries without lstat calls', async () => {
await expect(
expandQuickOpenGitFilesWithNestedRepos({
expandQuickOpenGitFileListing({
rootPath: '/unused/root',
gitPaths: [
staged('100644', 'README.md'),
@@ -152,7 +156,7 @@ describe('quick-open readdir walk', () => {
await writeRel(root, 'packages/lib/src/lib.ts')
await expect(
expandQuickOpenGitFilesWithNestedRepos({
expandQuickOpenGitFileListing({
rootPath: root,
gitPaths: [
staged('100644', 'README.md'),
@@ -180,7 +184,7 @@ describe('quick-open readdir walk', () => {
await writeRel(root, 'packages/lib/c.ts')
await expect(
expandQuickOpenGitFilesWithNestedRepos({
expandQuickOpenGitFileListing({
rootPath: root,
gitPaths: [staged('160000', 'packages/app'), staged('160000', 'packages/lib')],
budget: createQuickOpenReaddirBudget({ maxFiles: 2 })
@@ -199,7 +203,7 @@ describe('quick-open readdir walk', () => {
}
await expect(
expandQuickOpenGitFilesWithNestedRepos({
expandQuickOpenGitFileListing({
rootPath: root,
gitPaths: [staged('160000', 'packages/app')],
excludePathPrefixes: ['packages/app/excluded'],
@@ -208,6 +212,209 @@ describe('quick-open readdir walk', () => {
).resolves.toEqual(['packages/app/keep.ts'])
})
it('expands allowed ignored directories without walking blocked or excluded directories', async () => {
const root = await makeTempRoot()
await writeRel(root, 'dist/generated.js')
await writeRel(root, 'node_modules/pkg/index.js')
await writeRel(root, '.cache/state.json')
await writeRel(root, '.local/share/state.json')
await writeRel(root, '.local/config.toml')
await writeRel(root, 'excluded/other.js')
await expect(
expandQuickOpenGitFileListing({
rootPath: root,
gitPaths: [],
directoryPaths: [
'dist/',
'.local/',
'node_modules/',
'.cache/',
'.local/share/',
'excluded/'
],
excludePathPrefixes: ['excluded'],
budget: createQuickOpenReaddirBudget({ maxFiles: 2 })
})
).resolves.toEqual(['.local/config.toml', 'dist/generated.js'])
const walkedPaths = readdirMock.mock.calls.map(([path]) => path)
expect(walkedPaths).toContain(join(root, 'dist'))
expect(walkedPaths).toContain(join(root, '.local'))
expect(walkedPaths).not.toContain(join(root, '.local', 'share'))
})
it('batches many allowed directory placeholders with bounded concurrency', async () => {
const root = await makeTempRoot()
const directoryPaths = Array.from({ length: 40 }, (_, index) => `generated-${index}/`)
await Promise.all(
directoryPaths.map((directoryPath, index) =>
writeRel(root, `${directoryPath}file-${index}.ts`)
)
)
const actual = await vi.importActual<typeof import('node:fs/promises')>('node:fs/promises') // eslint-disable-line @typescript-eslint/consistent-type-imports -- vi.importActual requires inline import()
let activeReads = 0
let maxActiveReads = 0
readdirMock.mockImplementation(async (...args: Parameters<typeof actual.readdir>) => {
activeReads++
maxActiveReads = Math.max(maxActiveReads, activeReads)
await new Promise((resolve) => setTimeout(resolve, 5))
try {
return await actual.readdir(...args)
} finally {
activeReads--
}
})
try {
const files = await expandQuickOpenGitFileListing({
rootPath: root,
gitPaths: [],
directoryPaths
})
expect(files).toHaveLength(directoryPaths.length)
expect(maxActiveReads).toBeGreaterThan(1)
expect(maxActiveReads).toBeLessThanOrEqual(32)
} finally {
readdirMock.mockImplementation(actual.readdir)
}
})
it('preserves symlink leaves from collapsed Git directories without following them', async () => {
const root = await makeTempRoot()
await mkdirRel(root, 'scratch')
await writeRel(root, 'target/file.ts')
try {
await symlink(join(root, 'target', 'file.ts'), join(root, 'scratch', 'link.ts'))
await symlink(join(root, 'target'), join(root, 'scratch', 'linked-dir'), 'dir')
} catch (err) {
if ((err as NodeJS.ErrnoException).code === 'EPERM') {
return
}
throw err
}
await expect(
expandQuickOpenGitFileListing({
rootPath: root,
gitPaths: [],
directoryPaths: ['scratch/']
})
).resolves.toEqual(['scratch/link.ts', 'scratch/linked-dir'])
})
it('does not follow a collapsed directory replaced by a symlink', async () => {
const root = await makeTempRoot()
const outsideRoot = await makeTempRoot()
await writeRel(outsideRoot, 'secret.ts')
try {
await symlink(outsideRoot, join(root, 'dist'), 'dir')
} catch (err) {
if ((err as NodeJS.ErrnoException).code === 'EPERM') {
return
}
throw err
}
await expect(
expandQuickOpenGitFileListing({
rootPath: root,
gitPaths: [],
directoryPaths: ['dist/']
})
).resolves.toEqual([])
})
it('discards entries when a collapsed directory changes during readdir', async () => {
const root = await makeTempRoot()
const outsideRoot = await makeTempRoot()
await mkdirRel(root, 'dist')
await writeRel(outsideRoot, 'secret.ts')
const actual = await vi.importActual<typeof import('node:fs/promises')>('node:fs/promises') // eslint-disable-line @typescript-eslint/consistent-type-imports -- vi.importActual requires inline import()
const distPath = join(root, 'dist')
let swapped = false
readdirMock.mockImplementation(async (...args: Parameters<typeof actual.readdir>) => {
if (!swapped && args[0] === distPath) {
swapped = true
await rename(distPath, join(root, 'old-dist'))
await symlink(outsideRoot, distPath, 'dir')
}
return actual.readdir(...args)
})
try {
await expect(
expandQuickOpenGitFileListing({
rootPath: root,
gitPaths: [],
directoryPaths: ['dist/']
})
).resolves.toEqual([])
} finally {
readdirMock.mockImplementation(actual.readdir)
}
})
it('walks overlapping primary and ignored placeholders only once', async () => {
const root = await makeTempRoot()
await writeRel(root, 'foo/a.ts')
await writeRel(root, 'foo/bar/b.ts')
await expect(
expandQuickOpenGitFileListing({
rootPath: root,
gitPaths: [],
directoryPaths: ['foo/', 'foo/bar/'],
budget: createQuickOpenReaddirBudget({ maxFiles: 2 })
})
).resolves.toEqual(['foo/a.ts', 'foo/bar/b.ts'])
expect(
readdirMock.mock.calls.filter(([path]) => path === join(root, 'foo', 'bar'))
).toHaveLength(1)
})
it('rejects instead of returning a partial ignored-directory expansion', async () => {
const root = await makeTempRoot()
await writeRel(root, 'dist/a.js')
await writeRel(root, 'dist/b.js')
await expect(
expandQuickOpenGitFileListing({
rootPath: root,
gitPaths: [],
directoryPaths: ['dist/'],
budget: createQuickOpenReaddirBudget({ maxFiles: 1 })
})
).rejects.toThrow('File listing exceeded')
})
it('keeps the default safety cap for a very large collapsed directory', async () => {
const root = await makeTempRoot()
await mkdirRel(root, 'dist')
readdirMock.mockResolvedValueOnce(
Array.from({ length: QUICK_OPEN_READDIR_MAX_FILES + 1 }, (_, index) => ({
name: `file-${index}.ts`,
isDirectory: () => false,
isFile: () => true,
isSymbolicLink: () => false
}))
)
// Why: directory collapse prevents generated trees from flooding the relay;
// the Git fallback must reject rather than silently return a partial list.
await expect(
expandQuickOpenGitFileListing({
rootPath: root,
gitPaths: [],
directoryPaths: ['dist/']
})
).rejects.toThrow('File listing exceeded 10000 files')
})
it('identifies budget errors so callers can translate only those to install-rg guidance', () => {
expect(isQuickOpenReaddirBudgetError(new Error('File listing timed out'))).toBe(true)
expect(isQuickOpenReaddirBudgetError(new Error('File listing exceeded 10000 files'))).toBe(true)
@@ -248,13 +455,31 @@ describe('quick-open readdir walk', () => {
expect(files).not.toContain('linked-dir/file.ts')
})
it('walks an explicitly selected symlinked workspace root', async () => {
const targetRoot = await makeTempRoot()
const linkContainer = await makeTempRoot()
await writeRel(targetRoot, 'src/index.ts')
const linkedRoot = join(linkContainer, 'linked-workspace')
try {
await symlink(targetRoot, linkedRoot, 'dir')
} catch (err) {
if ((err as NodeJS.ErrnoException).code === 'EPERM') {
return
}
throw err
}
await expect(listQuickOpenFilesWithReaddir(linkedRoot)).resolves.toEqual(['src/index.ts'])
})
it('fills nested repo paths containing spaces and glob metacharacters', async () => {
const root = await makeTempRoot()
await makeNestedRepo(root, 'packages/app [one] space')
await writeRel(root, 'packages/app [one] space/src/main.ts')
await expect(
expandQuickOpenGitFilesWithNestedRepos({
expandQuickOpenGitFileListing({
rootPath: root,
gitPaths: ['packages/app [one] space/']
})
@@ -276,7 +501,7 @@ describe('quick-open readdir walk', () => {
await rejection.catch((err) => expect(isQuickOpenReaddirBudgetError(err)).toBe(false))
})
it('stops nested-repo expansion when the signal aborts (#7721)', async () => {
it('stops ignored-directory expansion when the signal aborts (#7721)', async () => {
const root = await makeTempRoot()
await writeRel(root, 'src/kept.ts')
@@ -284,11 +509,27 @@ describe('quick-open readdir walk', () => {
controller.abort()
await expect(
expandQuickOpenGitFilesWithNestedRepos({
expandQuickOpenGitFileListing({
rootPath: root,
gitPaths: ['src/kept.ts'],
gitPaths: [],
directoryPaths: ['src/'],
signal: controller.signal
})
).rejects.toSatisfy(isFileListingCancellation)
})
it('rejects when cancellation lands during an empty readdir batch', async () => {
const root = await makeTempRoot()
const controller = new AbortController()
const actual = await vi.importActual<typeof import('node:fs/promises')>('node:fs/promises') // eslint-disable-line @typescript-eslint/consistent-type-imports -- vi.importActual requires inline import()
readdirMock.mockImplementationOnce(async (...args: Parameters<typeof actual.readdir>) => {
const entries = await actual.readdir(...args)
controller.abort()
return entries
})
await expect(
listQuickOpenFilesWithReaddir(root, { signal: controller.signal })
).rejects.toSatisfy(isFileListingCancellation)
})
})
+156 -50
View File
@@ -1,6 +1,8 @@
import { lstat, readdir } from 'node:fs/promises'
import { join, relative } from 'node:path'
import { throwIfFileListingCancelled } from './file-listing-cancellation'
import { isQuickOpenReadableDirectory } from './quick-open-directory-validation'
import { collapseQuickOpenExpansionPaths } from './quick-open-expansion-paths'
import {
HIDDEN_DIR_BLOCKLIST,
shouldExcludeQuickOpenRelPath,
@@ -9,6 +11,7 @@ import {
export const QUICK_OPEN_READDIR_MAX_FILES = 10_000
export const QUICK_OPEN_READDIR_TIMEOUT_MS = 10_000
const QUICK_OPEN_READDIR_CONCURRENCY = 32
export type QuickOpenReaddirBudget = {
remainingFiles: number
@@ -96,13 +99,13 @@ function normalizeGitEntry(entry: string): string {
}
// Translate workspace-root-relative exclude prefixes into prefixes relative to
// a nested repo at `nestedRelPath`, so the nested walk can prune them during
// traversal. Prefixes outside the nested repo are dropped (they cannot match).
function rebaseExcludePrefixesForNestedRepo(
// one expanded subtree, so its walk prunes them during traversal. Prefixes
// outside that subtree are dropped because they cannot match.
function rebaseExcludePrefixesForSubtree(
excludePathPrefixes: readonly string[],
nestedRelPath: string
subtreeRelPath: string
): string[] {
const base = `${nestedRelPath}/`
const base = `${subtreeRelPath}/`
const rebased: string[] = []
for (const prefix of excludePathPrefixes) {
if (prefix.startsWith(base)) {
@@ -157,59 +160,132 @@ export async function listQuickOpenFilesWithReaddir(
rootPath: string,
opts: {
excludePathPrefixes?: readonly string[]
workspaceRelPathPrefix?: string
budget?: QuickOpenReaddirBudget
signal?: AbortSignal
} = {}
): Promise<string[]> {
return listQuickOpenFilesFromRoots(
[
{
rootPath,
excludePathPrefixes: opts.excludePathPrefixes ?? [],
workspaceRelPathPrefix: opts.workspaceRelPathPrefix,
allowRootSymlink: true
}
],
opts.budget ?? createQuickOpenReaddirBudget(),
opts.signal
)
}
type QuickOpenReaddirRoot = {
rootPath: string
excludePathPrefixes: readonly string[]
workspaceRelPathPrefix?: string
outputPathPrefix?: string
includeSymlinks?: boolean
allowRootSymlink?: boolean
}
async function listQuickOpenFilesFromRoots(
roots: readonly QuickOpenReaddirRoot[],
budget: QuickOpenReaddirBudget,
signal?: AbortSignal
): Promise<string[]> {
const files: string[] = []
const budget = opts.budget ?? createQuickOpenReaddirBudget()
const excludePathPrefixes = opts.excludePathPrefixes ?? []
let pendingDirectories = roots.map((root) => ({
root,
absPath: root.rootPath,
isRoot: true
}))
async function walk(dir: string): Promise<void> {
// Why: an abandoned scan (workspace switch) must stop consuming IO and
// event-loop time on the single-threaded relay, not just run to budget.
throwIfFileListingCancelled(opts.signal)
assertWithinDeadline(budget)
let entries
try {
entries = await readdir(dir, { withFileTypes: true })
} catch {
// Why: permission denied on an individual subtree is common for broad
// roots. Skipping that subtree preserves the existing relay fallback.
return
}
for (const entry of entries) {
throwIfFileListingCancelled(opts.signal)
while (pendingDirectories.length > 0) {
const nextDirectories: typeof pendingDirectories = []
for (
let offset = 0;
offset < pendingDirectories.length;
offset += QUICK_OPEN_READDIR_CONCURRENCY
) {
// Why: batch only the filesystem calls. Result processing stays serial,
// so the shared cap remains exact while shallow placeholder-heavy repos
// do not pay one relay event-loop turn per directory.
throwIfFileListingCancelled(signal)
assertWithinDeadline(budget)
const batch = pendingDirectories.slice(offset, offset + QUICK_OPEN_READDIR_CONCURRENCY)
const entryGroups = await Promise.all(
batch.map(async (pending) => {
try {
// Why: Git's placeholder may have been replaced with a symlink
// before expansion. Never let readdir follow it outside the root.
const stat = await lstat(pending.absPath)
const allowSymlinkedRoot = pending.isRoot && pending.root.allowRootSymlink
if (!isQuickOpenReadableDirectory(stat, allowSymlinkedRoot)) {
return { pending, entries: [] }
}
const entries = await readdir(pending.absPath, { withFileTypes: true })
// Why: close the ordinary check/use race. If the directory became
// a symlink while readdir was pending, discard everything read.
const statAfterRead = await lstat(pending.absPath)
if (!isQuickOpenReadableDirectory(statAfterRead, allowSymlinkedRoot)) {
return { pending, entries: [] }
}
return { pending, entries }
} catch {
// Why: permission denied on one subtree is common for broad roots.
return { pending, entries: [] }
}
})
)
// Why: an empty directory has no per-entry checkpoint below. Cancellation
// or timeout that lands during readdir must still reject, never resolve [].
throwIfFileListingCancelled(signal)
assertWithinDeadline(budget)
const name = entry.name
const absPath = join(dir, name)
const relPath = toRelPath(rootPath, absPath)
if (shouldExcludeQuickOpenRelPath(relPath, excludePathPrefixes)) {
continue
}
if (entry.isDirectory()) {
if (shouldDescend(name)) {
await walk(absPath)
for (const { pending, entries } of entryGroups) {
for (const entry of entries) {
throwIfFileListingCancelled(signal)
assertWithinDeadline(budget)
const name = entry.name
const absPath = join(pending.absPath, name)
const relPath = toRelPath(pending.root.rootPath, absPath)
const workspaceRelPath = pending.root.workspaceRelPathPrefix
? `${pending.root.workspaceRelPathPrefix}/${relPath}`
: relPath
if (shouldExcludeQuickOpenRelPath(relPath, pending.root.excludePathPrefixes)) {
continue
}
if (entry.isDirectory()) {
if (shouldDescend(name) && shouldIncludeQuickOpenPath(workspaceRelPath)) {
nextDirectories.push({ root: pending.root, absPath, isRoot: false })
}
continue
}
if (
(entry.isFile() || (pending.root.includeSymlinks && entry.isSymbolicLink())) &&
shouldIncludeQuickOpenPath(workspaceRelPath)
) {
consumeFileBudget(budget)
files.push(
pending.root.outputPathPrefix
? `${pending.root.outputPathPrefix}/${relPath}`
: relPath
)
}
}
continue
}
if (entry.isFile()) {
consumeFileBudget(budget)
files.push(relPath)
}
}
pendingDirectories = nextDirectories
}
await walk(rootPath)
return files
}
export async function expandQuickOpenGitFilesWithNestedRepos(opts: {
export async function expandQuickOpenGitFileListing(opts: {
rootPath: string
gitPaths: Iterable<string>
directoryPaths?: Iterable<string>
excludePathPrefixes?: readonly string[]
budget?: QuickOpenReaddirBudget
signal?: AbortSignal
@@ -217,6 +293,7 @@ export async function expandQuickOpenGitFilesWithNestedRepos(opts: {
const files = new Set<string>()
const excludePathPrefixes = opts.excludePathPrefixes ?? []
const budget = opts.budget ?? createQuickOpenReaddirBudget()
const expansionPaths = new Map<string, boolean>()
const addFinalPath = (relPath: string): void => {
if (!relPath) {
@@ -243,17 +320,46 @@ export async function expandQuickOpenGitFilesWithNestedRepos(opts: {
continue
}
const nestedFiles = await listQuickOpenFilesWithReaddir(joinRootRel(opts.rootPath, relPath), {
// Why: exclude prefixes are workspace-root-relative; rebase them onto the
// nested repo so the walk prunes excluded subtrees during traversal
// instead of burning the shared budget and filtering them at the end.
excludePathPrefixes: rebaseExcludePrefixesForNestedRepo(excludePathPrefixes, relPath),
budget,
signal: opts.signal
})
for (const nestedFile of nestedFiles) {
addFinalPath(`${relPath}/${nestedFile}`)
expansionPaths.set(relPath, expansionPaths.get(relPath) ?? false)
}
for (const rawPath of opts.directoryPaths ?? []) {
throwIfFileListingCancelled(opts.signal)
assertWithinDeadline(budget)
const relPath = normalizeGitEntry(rawPath)
// Why: Git intentionally leaves collapsed directories unexpanded; reject
// blocked and nested-worktree placeholders before any filesystem IO.
if (
!relPath ||
shouldExcludeQuickOpenRelPath(relPath, excludePathPrefixes) ||
!shouldIncludeQuickOpenPath(relPath)
) {
continue
}
// Why: before directory collapse, Git returned untracked symlink entries
// without following them. Preserve those paths when expanding placeholders.
expansionPaths.set(relPath, true)
}
const expandedFiles = await listQuickOpenFilesFromRoots(
collapseQuickOpenExpansionPaths(expansionPaths).map(([relPath, includeSymlinks]) => ({
rootPath: joinRootRel(opts.rootPath, relPath),
// Why: exclude prefixes are workspace-root-relative; rebase them onto
// each expanded subtree so blocked work is pruned before consuming cap.
excludePathPrefixes: rebaseExcludePrefixesForSubtree(excludePathPrefixes, relPath),
// Why: Git can collapse `.local/share/` to `.local/`; keep workspace
// context so the walker still prunes the multi-segment blocklist.
workspaceRelPathPrefix: relPath,
outputPathPrefix: relPath,
includeSymlinks
})),
budget,
opts.signal
)
for (const expandedFile of expandedFiles) {
addFinalPath(expandedFile)
}
return Array.from(files)