diff --git a/src/main/ipc/filesystem-list-files-git-directory-expansion.test.ts b/src/main/ipc/filesystem-list-files-git-directory-expansion.test.ts new file mode 100644 index 00000000000..5d5cf163746 --- /dev/null +++ b/src/main/ipc/filesystem-list-files-git-directory-expansion.test.ts @@ -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).stdout = new EventEmitter() + ;( + (process as unknown as Record).stdout as EventEmitter & { + setEncoding: () => void + } + ).setEncoding = vi.fn() + ;(process as unknown as Record).stderr = new EventEmitter() + ;(process as unknown as Record).kill = vi.fn() + ;(process as unknown as Record).exitCode = null + ;(process as unknown as Record).signalCode = null + return process +} + +async function writeRel(root: string, relPath: string): Promise { + 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() + }) +}) diff --git a/src/main/ipc/filesystem-list-files-git-fallback.ts b/src/main/ipc/filesystem-list-files-git-fallback.ts index 3b77ac28131..8b5bb526f8e 100644 --- a/src/main/ipc/filesystem-list-files-git-fallback.ts +++ b/src/main/ipc/filesystem-list-files-git-fallback.ts @@ -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 { - 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 + 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 { - 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() + const directoryPaths = new Set() 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() } diff --git a/src/main/ipc/filesystem-list-files.test.ts b/src/main/ipc/filesystem-list-files.test.ts index 638775a4811..a3a556b2340 100644 --- a/src/main/ipc/filesystem-list-files.test.ts +++ b/src/main/ipc/filesystem-list-files.test.ts @@ -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') diff --git a/src/main/ipc/filesystem-list-files.ts b/src/main/ipc/filesystem-list-files.ts index 36376227305..9dcd2c45038 100644 --- a/src/main/ipc/filesystem-list-files.ts +++ b/src/main/ipc/filesystem-list-files.ts @@ -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 { 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() diff --git a/src/main/ipc/filesystem.ts b/src/main/ipc/filesystem.ts index 7e7006a375e..100245dca8b 100644 --- a/src/main/ipc/filesystem.ts +++ b/src/main/ipc/filesystem.ts @@ -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() }) diff --git a/src/relay/fs-handler-git-fallback.ts b/src/relay/fs-handler-git-fallback.ts index 9e69339bfcd..6cceb5de7a7 100644 --- a/src/relay/fs-handler-git-fallback.ts +++ b/src/relay/fs-handler-git-fallback.ts @@ -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() + const directoryPaths = new Set() const { primary, ignoredPass } = buildGitLsFilesArgsForQuickOpen(excludePathPrefixes) const children: { child: ReturnType @@ -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) { diff --git a/src/relay/fs-handler-list-files-ignored.test.ts b/src/relay/fs-handler-list-files-ignored.test.ts index 753ce02fddc..5a1804437bc 100644 --- a/src/relay/fs-handler-list-files-ignored.test.ts +++ b/src/relay/fs-handler-list-files-ignored.test.ts @@ -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 () => { diff --git a/src/shared/quick-open-directory-validation.ts b/src/shared/quick-open-directory-validation.ts new file mode 100644 index 00000000000..501791bb050 --- /dev/null +++ b/src/shared/quick-open-directory-validation.ts @@ -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()) +} diff --git a/src/shared/quick-open-expansion-paths.test.ts b/src/shared/quick-open-expansion-paths.test.ts new file mode 100644 index 00000000000..e5e91e9b40c --- /dev/null +++ b/src/shared/quick-open-expansion-paths.test.ts @@ -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([ + ['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) + }) +}) diff --git a/src/shared/quick-open-expansion-paths.ts b/src/shared/quick-open-expansion-paths.ts new file mode 100644 index 00000000000..43f094edd3f --- /dev/null +++ b/src/shared/quick-open-expansion-paths.ts @@ -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][] { + const sortedPaths = Array.from(expansionPaths).sort(([left], [right]) => + left < right ? -1 : left > right ? 1 : 0 + ) + const collapsedPaths = new Map() + + 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) +} diff --git a/src/shared/quick-open-filter.test.ts b/src/shared/quick-open-filter.test.ts index 2b079f6b7d4..1d96bc1744f 100644 --- a/src/shared/quick-open-filter.test.ts +++ b/src/shared/quick-open-filter.test.ts @@ -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('--') diff --git a/src/shared/quick-open-filter.ts b/src/shared/quick-open-filter.ts index 980c43351a2..899d13aeaa3 100644 --- a/src/shared/quick-open-filter.ts +++ b/src/shared/quick-open-filter.ts @@ -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 } diff --git a/src/shared/quick-open-git-directory-collapse.test.ts b/src/shared/quick-open-git-directory-collapse.test.ts new file mode 100644 index 00000000000..8e197a6d607 --- /dev/null +++ b/src/shared/quick-open-git-directory-collapse.test.ts @@ -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 { + 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) + }) +}) diff --git a/src/shared/quick-open-readdir-walk.test.ts b/src/shared/quick-open-readdir-walk.test.ts index cb73572bc34..2331bc3b1c3 100644 --- a/src/shared/quick-open-readdir-walk.test.ts +++ b/src/shared/quick-open-readdir-walk.test.ts @@ -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('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('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) => { + 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('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) => { + 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('node:fs/promises') // eslint-disable-line @typescript-eslint/consistent-type-imports -- vi.importActual requires inline import() + readdirMock.mockImplementationOnce(async (...args: Parameters) => { + const entries = await actual.readdir(...args) + controller.abort() + return entries + }) + + await expect( + listQuickOpenFilesWithReaddir(root, { signal: controller.signal }) + ).rejects.toSatisfy(isFileListingCancellation) + }) }) diff --git a/src/shared/quick-open-readdir-walk.ts b/src/shared/quick-open-readdir-walk.ts index 90ce3ea9fdf..ada9442cd23 100644 --- a/src/shared/quick-open-readdir-walk.ts +++ b/src/shared/quick-open-readdir-walk.ts @@ -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 { + 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 { 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 { - // 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 + directoryPaths?: Iterable excludePathPrefixes?: readonly string[] budget?: QuickOpenReaddirBudget signal?: AbortSignal @@ -217,6 +293,7 @@ export async function expandQuickOpenGitFilesWithNestedRepos(opts: { const files = new Set() const excludePathPrefixes = opts.excludePathPrefixes ?? [] const budget = opts.budget ?? createQuickOpenReaddirBudget() + const expansionPaths = new Map() 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)