diff --git a/src/main/ipc/filesystem-list-files.test.ts b/src/main/ipc/filesystem-list-files.test.ts index b6fe3453c22..08a52536b7a 100644 --- a/src/main/ipc/filesystem-list-files.test.ts +++ b/src/main/ipc/filesystem-list-files.test.ts @@ -104,6 +104,43 @@ describe('filesystem-list-files', () => { } }) + it('retains a late 25,002nd file in a complete inventory', async () => { + const child = createMockProcess() + spawnMock.mockReturnValue(child) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: The mocked authorization and runtime options do not read the store. + const result = listQuickOpenFiles('/mock/root', {} as unknown as Store) + await vi.waitFor(() => expect(spawnMock).toHaveBeenCalledTimes(1)) + child.stdout?.emit( + 'data', + Array.from({ length: 25002 }, (_, i) => `src/file-${i}.ts\0`).join('') + ) + child.emit('close', 0, null) + const paths = await result + expect(paths).toHaveLength(25002) + expect(paths.at(-1)).toBe('src/file-25001.ts') + }) + + it('stops a full-inventory producer at its aggregate retained-byte ceiling', async () => { + const child = createMockProcess() + spawnMock.mockReturnValue(child) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: The mocked authorization and runtime options do not read the store. + const result = listQuickOpenFiles('/mock/root', {} as unknown as Store) + await vi.waitFor(() => expect(spawnMock).toHaveBeenCalledTimes(1)) + const rejected = expect(result).rejects.toThrow('inventory is too large') + let produced = 0 + while (child.stdout?.listenerCount('data') && produced < 100000) { + child.stdout.emit( + 'data', + Array.from({ length: 100 }, () => `src/${'x'.repeat(1000)}-${produced++}.ts\0`).join('') + ) + } + await rejected + expect(produced).toBeLessThan(40000) + expect(child.kill).toHaveBeenCalled() + expect(child.stdout?.listenerCount('data')).toBe(0) + expect(child.listenerCount('close')).toBe(0) + }) + it('counts NUL-delimited filenames containing newlines as one result each', async () => { const child = createMockProcess() spawnMock.mockReturnValue(child) diff --git a/src/main/ipc/filesystem-list-files.ts b/src/main/ipc/filesystem-list-files.ts index 9d9042584d9..01143e2c5b3 100644 --- a/src/main/ipc/filesystem-list-files.ts +++ b/src/main/ipc/filesystem-list-files.ts @@ -1,3 +1,4 @@ +import { FileInventoryBudget, FileInventoryCapacityError } from '../../shared/file-inventory-budget' import { getQuickOpenRgOutputMode } from '../../shared/quick-open-ripgrep-output-mode' import { RipgrepFilenameDecoder, RipgrepFilenameError } from '../../shared/ripgrep-filename-decoder' import { sep } from 'node:path' @@ -56,12 +57,14 @@ export async function listQuickOpenFiles( const excludePathPrefixes = buildExcludePathPrefixes(authorizedRootPath, excludePaths) const wslDistroForOutput = parseWslPath(authorizedRootPath)?.distro ?? localGitOptions.wslDistro + const inventoryBudget = + maxResults === undefined && maxSerializedBytes === undefined ? new FileInventoryBudget() : null const files = new Set() let serializedBytes = 2 // [] const children: { child: ChildProcess isDone: () => boolean - finish: () => void + finish: (error?: Error) => void }[] = [] // Why: WSL-routed rg can emit Linux-native absolute paths. UNC repos carry // their distro in the path; Windows-path repos carry it in project runtime. @@ -125,6 +128,14 @@ export async function listQuickOpenFiles( } serializedBytes += nextBytes } + try { + inventoryBudget?.record(relPath) + } catch (error) { + buf = '' + files.clear() + killSurvivors(error instanceof Error ? error : new FileInventoryCapacityError()) + return true + } files.add(relPath) return maxResults !== undefined && files.size >= maxResults } @@ -285,13 +296,13 @@ export async function listQuickOpenFiles( }) } - const killSurvivors = (): void => { + const killSurvivors = (error?: Error): void => { // Failed listings must release any process still walking the tree. for (const entry of children) { if (entry.isDone()) { continue } - entry.finish() + entry.finish(error) if (entry.child.exitCode === null && entry.child.signalCode === null) { killSpawnedRipgrepProcess(entry.child) } diff --git a/src/main/ipc/filesystem-markdown-document-listing.test.ts b/src/main/ipc/filesystem-markdown-document-listing.test.ts index 448e0188ab3..7430465ecbd 100644 --- a/src/main/ipc/filesystem-markdown-document-listing.test.ts +++ b/src/main/ipc/filesystem-markdown-document-listing.test.ts @@ -6,6 +6,7 @@ import { store, WORKTREE_FEATURE_PATH, readdirMock, + realpathMock, getSshFilesystemProviderMock, resetFilesystemIpcMocks } from './filesystem-test-harness' @@ -114,6 +115,52 @@ describe('registerFilesystemHandlers', () => { expect(listMarkdownDocumentsMock).not.toHaveBeenCalled() }) + it('exposes registered alias paths that remain readable and rejects child symlink escapes', async () => { + const alias = path.resolve('/alias-folder') + const canonical = path.resolve('/canonical-folder') + const outside = path.resolve('/outside/secret.md') + const folderStore = { + ...store, + getFolderWorkspaces: () => [{ id: 'folder', folderPath: alias, projectGroupId: 'group' }] + } + realpathMock.mockImplementation(async (target: string) => + target === path.join(alias, 'escape.md') + ? outside + : target === alias || target.startsWith(alias + path.sep) + ? canonical + target.slice(alias.length) + : target + ) + listMarkdownDocumentsMock.mockResolvedValue([ + { + filePath: path.join(canonical, 'Target.md'), + relativePath: 'Target.md', + basename: 'Target.md', + name: 'Target' + } + ]) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: This IPC fixture implements the store reads used by filesystem authorization. + registerFilesystemHandlers(folderStore as never) + const documents = await handlers.get('fs:listMarkdownDocuments')!(null, { rootPath: alias }) + expect(documents).toEqual([ + { + filePath: path.join(alias, 'Target.md'), + relativePath: 'Target.md', + basename: 'Target.md', + name: 'Target' + } + ]) + expect(listMarkdownDocumentsMock).toHaveBeenCalledWith(canonical, {}) + await expect( + handlers.get('fs:readFile')!(null, { filePath: path.join(alias, 'Target.md') }) + ).resolves.toEqual({ content: 'a'.repeat(10), isBinary: false }) + await expect( + handlers.get('fs:stat')!(null, { filePath: path.join(alias, 'Target.md') }) + ).resolves.toHaveProperty('isDirectory', false) + await expect( + handlers.get('fs:readFile')!(null, { filePath: path.join(alias, 'escape.md') }) + ).rejects.toThrow('Access denied') + }) + it('lists remote markdown documents through the SSH filesystem provider', async () => { const provider = { listFiles: vi @@ -146,4 +193,41 @@ describe('registerFilesystemHandlers', () => { expect(listMarkdownDocumentsMock).not.toHaveBeenCalled() expect(localOptionsMock).not.toHaveBeenCalled() }) + + it('keeps late Markdown documents from legacy providers with large source inventories', async () => { + const paths = Array.from({ length: 25_002 }, (_, index) => `src/file-${index}.ts`) + paths.push('docs/late.md') + const provider = { listFiles: vi.fn().mockResolvedValue(paths) } + getSshFilesystemProviderMock.mockReturnValue(provider) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: This fixture supplies the store reads used by filesystem handler registration. + registerFilesystemHandlers(store as never) + await expect( + handlers.get('fs:listMarkdownDocuments')!(null, { + rootPath: '/repo', + connectionId: 'legacy' + }) + ).resolves.toEqual([ + { + filePath: '/repo/docs/late.md', + relativePath: 'docs/late.md', + basename: 'late.md', + name: 'late' + } + ]) + expect(provider.listFiles).toHaveBeenCalledWith('/repo') + }) + + it('still bounds legacy source inventories before constructing Markdown metadata', async () => { + getSshFilesystemProviderMock.mockReturnValue({ + listFiles: vi.fn().mockResolvedValue([`${'x'.repeat(65_537)}.ts`, 'README.md']) + }) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: This fixture supplies the store reads used by filesystem handler registration. + registerFilesystemHandlers(store as never) + await expect( + handlers.get('fs:listMarkdownDocuments')!(null, { + rootPath: '/repo', + connectionId: 'legacy' + }) + ).rejects.toThrow('File inventory is too large') + }) }) diff --git a/src/main/ipc/filesystem/filesystem-read-handlers.ts b/src/main/ipc/filesystem/filesystem-read-handlers.ts index f414f1d0c16..389148fb3df 100644 --- a/src/main/ipc/filesystem/filesystem-read-handlers.ts +++ b/src/main/ipc/filesystem/filesystem-read-handlers.ts @@ -1,3 +1,5 @@ +import { listFilesystemMarkdownDocuments } from '../../providers/filesystem-markdown-listing' +import { markdownDocumentsFromRelativePaths } from '../../../shared/markdown-document-paths' import { capturePathExistence, validatePathExistenceBatch, @@ -15,7 +17,7 @@ import { resolveLocalFileRequestPath } from '../local-file-access-resolution' import { isENOENT } from '../filesystem-path-containment' -import { listMarkdownDocuments, markdownDocumentsFromRelativePaths } from '../markdown-documents' +import { listMarkdownDocuments } from '../markdown-documents' import { getLocalGitOptionsForRegisteredWorktree } from '../local-worktree-runtime-options' import { recordCrashBreadcrumb } from '../../crash-reporting/crash-breadcrumb-store' import { buildReadDirErrorBreadcrumb, type ReadDirThrowSite } from '../readdir-error-diagnostics' @@ -98,14 +100,24 @@ export function registerFilesystemReadHandlers(context: FilesystemHandlerContext ): Promise => { if (args.connectionId) { const provider = requireSshFilesystemProvider(args.connectionId) - const relativePaths = await provider.listFiles(args.rootPath) - return markdownDocumentsFromRelativePaths(args.rootPath, relativePaths) + return listFilesystemMarkdownDocuments(provider, args.rootPath) } - const rootPath = await resolveRegisteredWorktreePath(args.rootPath, store) - return listMarkdownDocuments( + const isFolderRoot = store + .getFolderWorkspaces?.() + .some((workspace) => workspace.folderPath === args.rootPath) + const rootPath = isFolderRoot + ? await resolveDesktopAuthorizedPath(args.rootPath, store) + : await resolveRegisteredWorktreePath(args.rootPath, store) + const documents = await listMarkdownDocuments( rootPath, getLocalGitOptionsForRegisteredWorktree(store, args.rootPath, rootPath) ) + return rootPath === args.rootPath + ? documents + : markdownDocumentsFromRelativePaths( + args.rootPath, + documents.map((document) => document.relativePath) + ) } ) diff --git a/src/main/ipc/markdown-documents-ripgrep.test.ts b/src/main/ipc/markdown-documents-ripgrep.test.ts index 13f48b78e17..18b4b149180 100644 --- a/src/main/ipc/markdown-documents-ripgrep.test.ts +++ b/src/main/ipc/markdown-documents-ripgrep.test.ts @@ -1,10 +1,11 @@ import { EventEmitter } from 'node:events' import { PassThrough } from 'node:stream' -import { resolve } from 'node:path' +import { resolve, win32 } from 'node:path' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -const { spawnMock } = vi.hoisted(() => ({ spawnMock: vi.fn() })) +const { spawnMock, stopMock } = vi.hoisted(() => ({ spawnMock: vi.fn(), stopMock: vi.fn() })) vi.mock('../ripgrep/bundled-ripgrep-spawn', () => ({ spawnBundledRipgrep: spawnMock })) +vi.mock('../ripgrep/bundled-ripgrep-stop', () => ({ stopBundledRipgrep: stopMock })) import { listMarkdownDocuments } from './markdown-documents' @@ -12,7 +13,7 @@ class ListingProcess extends EventEmitter { stdout = new PassThrough() stderr = new PassThrough() pid: number | undefined = 123 - kill = vi.fn(() => true) + kill = vi.fn<(signal?: NodeJS.Signals) => boolean>(() => true) } const root = resolve('/workspace/docs') @@ -22,6 +23,7 @@ let child: ListingProcess beforeEach(() => { child = new ListingProcess() spawnMock.mockReset().mockReturnValue(child) + stopMock.mockReset().mockImplementation((process: ListingProcess) => process.kill('SIGKILL')) }) afterEach(() => { vi.useRealTimers() @@ -71,6 +73,23 @@ describe('Markdown document ripgrep lifecycle', () => { await expect(result).resolves.toEqual([]) }) + it.each(['C:\\repo', '\\\\server\\share\\repo'])( + 'preserves native Windows editor path identity under %s', + async (windowsRoot) => { + const result = listMarkdownDocuments(windowsRoot) + child.stdout.write('./docs/README.md\0') + child.emit('close', 0, null) + expect(await result).toEqual([ + { + filePath: win32.join(windowsRoot, 'docs', 'README.md'), + relativePath: 'docs/README.md', + basename: 'README.md', + name: 'README' + } + ]) + } + ) + it('rejects an unreadable subtree even after receiving valid documents', async () => { const result = listMarkdownDocuments(root) child.stdout.write('./README.md\0') @@ -99,7 +118,7 @@ describe('Markdown document ripgrep lifecycle', () => { it('rejects an oversized unfinished record without retaining the process', async () => { const result = listMarkdownDocuments(root) child.stdout.write(`./${'a'.repeat(1024 * 1024)}`) - await expect(result).rejects.toThrow('path exceeds') + await expect(result).rejects.toThrow('Workspace is too large') expect(child.kill).toHaveBeenCalledWith('SIGKILL') }) @@ -162,3 +181,54 @@ describe('Markdown document ripgrep lifecycle', () => { await result }) }) + +it('returns the complete 20,000-document boundary', async () => { + const result = listMarkdownDocuments(root) + child.stdout.write(Array.from({ length: 20_000 }, (_, index) => `./doc-${index}.md\0`).join('')) + child.emit('close', 0, null) + expect(await result).toHaveLength(20_000) + expect(child.kill).not.toHaveBeenCalled() +}) + +it('rejects the 20,001st document without retaining the child', async () => { + const result = listMarkdownDocuments(root) + const paths = Array.from({ length: 20_000 }, (_, index) => `./doc-${index}.md\0`).join('') + child.stdout.write(paths) + child.stdout.write('./overflow.md\0') + await expect(result).rejects.toThrow('Workspace is too large') + expect(child.kill).toHaveBeenCalledWith('SIGKILL') + expect(child.stdout.listenerCount('data')).toBe(0) +}) + +it('cancels the filtered producer and permits a fresh request', async () => { + const controller = new AbortController() + const result = listMarkdownDocuments(root, { signal: controller.signal }) + child.stdout.write('./partial') + controller.abort(new Error('editor closed')) + await expect(result).rejects.toThrow('editor closed') + expect(child.kill).toHaveBeenCalledWith('SIGKILL') + expect(child.stdout.listenerCount('data')).toBe(0) +}) + +it.each(['abort', 'timeout', 'capacity'] as const)( + 'uses the bundled WSL process-tree stop for Markdown %s', + async (reason) => { + vi.useFakeTimers() + const controller = new AbortController() + const result = listMarkdownDocuments(root, { + wslDistro: 'Ubuntu', + signal: controller.signal + }) + const rejected = expect(result).rejects.toThrow() + if (reason === 'abort') { + controller.abort(new Error('editor closed')) + } else if (reason === 'timeout') { + await vi.advanceTimersByTimeAsync(15_000) + } else { + child.stdout.write(`./${'a'.repeat(65_537)}`) + } + await rejected + expect(stopMock).toHaveBeenCalledExactlyOnceWith(child, true) + expect(child.stdout.listenerCount('data')).toBe(0) + } +) diff --git a/src/main/ipc/markdown-documents.ts b/src/main/ipc/markdown-documents.ts index 1f457dba97e..61f7521ef28 100644 --- a/src/main/ipc/markdown-documents.ts +++ b/src/main/ipc/markdown-documents.ts @@ -1,247 +1,26 @@ -import { RipgrepFilenameDecoder } from '../../shared/ripgrep-filename-decoder' -import { isWindowsAbsolutePathLike } from '../../shared/cross-platform-path' -import { normalizeRelativePath } from '../../shared/text-search-paths' -import { - basename as pathBasename, - extname, - isAbsolute, - join, - posix, - relative, - resolve -} from 'node:path' -import type { FileDocument, MarkdownDocument } from '../../shared/filesystem-entry-types' import { spawnBundledRipgrep } from '../ripgrep/bundled-ripgrep-spawn' +import { stopBundledRipgrep } from '../ripgrep/bundled-ripgrep-stop' import { parseWslPath } from '../wsl' import { - isRipgrepMissingCwdExit, - ripgrepMissingCwdError -} from '../../shared/ripgrep-process-availability' - -export function isMarkdownDocumentName(name: string): boolean { - return isMarkdownExtension(extname(name)) -} - -function isMarkdownExtension(extension: string): boolean { - const normalized = extension.toLowerCase() - return normalized === '.md' || normalized === '.mdx' || normalized === '.markdown' -} - -function basenameFromRelativePath(relativePath: string): string { - return relativePath.slice(relativePath.lastIndexOf('/') + 1) -} - -function isSafeRelativePath(relativePath: string): boolean { - return !relativePath.split('/').includes('..') -} - -function rootRelativePath(rootPath: string, filePath: string): string | null { - const resolvedRoot = resolve(rootPath) - const resolvedFile = resolve(filePath) - const relativePath = relative(resolvedRoot, resolvedFile) - if ( - !isSafeRelativePath(normalizeRelativePath(relativePath, rootPath)) || - isAbsolute(relativePath) - ) { - return null - } - return normalizeRelativePath(relativePath, rootPath) -} - -export function fileDocumentFromFilePath( - rootPath: string, - filePath: string, - options: { outsideRootRelativePath?: 'basename' | 'relative' } = {} -): FileDocument { - const basename = pathBasename(filePath) - const extension = extname(basename) - const relativePath = - rootRelativePath(rootPath, filePath) ?? - (options.outsideRootRelativePath === 'basename' - ? basename - : normalizeRelativePath(relative(rootPath, filePath), rootPath)) - return { - filePath, - relativePath, - basename, - name: extension ? basename.slice(0, -extension.length) : basename - } -} - -export const markdownDocumentFromFilePath = fileDocumentFromFilePath - -export function markdownDocumentFromRelativePath( - rootPath: string, - relativePath: string -): MarkdownDocument | null { - const normalizedRelativePath = normalizeRelativePath(relativePath, rootPath) - // Why: SSH providers should return root-relative paths; reject escape - // segments before building a synthetic absolute path for renderer use. - if (!isSafeRelativePath(normalizedRelativePath)) { - return null - } - const basename = basenameFromRelativePath(normalizedRelativePath) - // Remote separators are already normalized; a POSIX backslash stays part of the name. - const extension = posix.extname(basename) - if (!isMarkdownExtension(extension)) { - return null - } - const normalizedRoot = rootPath.replace( - isWindowsAbsolutePathLike(rootPath) ? /[\\/]+$/ : /\/+$/, - '' - ) - return { - filePath: `${normalizedRoot}/${normalizedRelativePath}`, - relativePath: normalizedRelativePath, - basename, - name: extension ? basename.slice(0, -extension.length) : basename - } -} - -export function markdownDocumentsFromRelativePaths( - rootPath: string, - relativePaths: string[] -): MarkdownDocument[] { - return relativePaths - .map((relativePath) => markdownDocumentFromRelativePath(rootPath, relativePath)) - .filter((document): document is MarkdownDocument => document !== null) - .sort((a, b) => a.relativePath.localeCompare(b.relativePath)) -} - -const MARKDOWN_LISTING_TIMEOUT_MS = 15_000 -const MAX_MARKDOWN_PATH_BYTES = 1024 * 1024 + collectMarkdownDocuments, + MARKDOWN_DOCUMENT_LISTING_ARGS +} from '../../shared/node-markdown-document-listing' +import type { MarkdownDocument } from '../../shared/filesystem-entry-types' +export * from '../../shared/markdown-document-paths' export async function listMarkdownDocuments( rootPath: string, - options: { wslDistro?: string } = {} + options: { wslDistro?: string; signal?: AbortSignal } = {} ): Promise { - const child = spawnBundledRipgrep( - [ - '--files', - '--hidden', - '--no-ignore', - '--no-config', - '--null', - '--path-separator', - '/', - // Keep case variants in --glob: --iglob is applied after exclusions and can reopen hidden folders. - '--glob', - '*.{[mM][dD],[mM][dD][xX],[mM][aA][rR][kK][dD][oO][wW][nN]}', - '--glob', - '!**/.*/', - '--glob', - '**/.github/', - '--glob', - '!**/node_modules/', - '.' - ], - { - cwd: rootPath, - wslDistro: options.wslDistro, - wslDistroForOutput: parseWslPath(rootPath)?.distro ?? options.wslDistro, - stdio: ['ignore', 'pipe', 'pipe'] - } - ) - - return new Promise((resolveListing, reject) => { - const filenameDecoder = new RipgrepFilenameDecoder( - (error) => finish(error), - Boolean(parseWslPath(rootPath)?.distro ?? options.wslDistro) - ) - const documents: MarkdownDocument[] = [] - let carry = '' - let stderr = '' - let settled = false - const finish = (error?: Error): void => { - if (settled) { - return - } - settled = true - clearTimeout(timer) - child.stdout?.off('data', onData) - child.stderr?.off('data', onStderr) - child.stdout?.off('error', onError) - child.stderr?.off('error', onError) - child.off('close', onClose) - child.off('error', onError) - // A spawn or pipe error can arrive after a timeout has already settled the listing. - child.on('error', ignoreLateError) - child.stdout?.on('error', ignoreLateError) - child.stderr?.on('error', ignoreLateError) - carry = '' - if (error) { - if (child.pid !== undefined) { - try { - child.kill('SIGKILL') - } catch { - // The process may have exited before the timeout or stream error arrived. - } - } - documents.length = 0 - child.stdout?.resume() - child.stderr?.resume() - reject(error) - } else { - resolveListing(documents.sort((a, b) => a.relativePath.localeCompare(b.relativePath))) - } - } - const onError = (error: Error): void => finish(error) - const onStderr = (chunk: string): void => { - stderr = (stderr + chunk).slice(0, 4096) - } - const onData = (chunk: Buffer | string): void => { - const decoded = filenameDecoder.decode(chunk) - if (decoded === null) { - return - } - carry += decoded - let start = 0 - let end: number - while ((end = carry.indexOf('\0', start)) !== -1) { - const path = carry.slice(start, end) - if (Buffer.byteLength(path) > MAX_MARKDOWN_PATH_BYTES) { - finish(new Error('Markdown document path exceeds the listing limit')) - return - } - if (!path.startsWith('./') || path.split('/').includes('..')) { - finish(new Error('Invalid path in Markdown document listing')) - return - } - if (isMarkdownDocumentName(path)) { - documents.push(markdownDocumentFromFilePath(rootPath, join(rootPath, path.slice(2)))) - } - start = end + 1 - } - carry = carry.slice(start) - if (Buffer.byteLength(carry) > MAX_MARKDOWN_PATH_BYTES) { - finish(new Error('Markdown document path exceeds the listing limit')) - } - } - const onClose = (code: number | null, signal: NodeJS.Signals | null): void => { - if (isRipgrepMissingCwdExit(code)) { - finish(ripgrepMissingCwdError(rootPath)) - } else if (signal || (code !== 0 && code !== 1)) { - finish(new Error(`Markdown document listing failed (${signal ?? code}): ${stderr.trim()}`)) - } else { - if (!filenameDecoder.finish()) { - return - } - finish(carry ? new Error('Incomplete path in Markdown document listing') : undefined) - } - } - const timer = setTimeout( - () => finish(new Error('Markdown document listing timed out')), - MARKDOWN_LISTING_TIMEOUT_MS - ) - timer.unref?.() - child.stderr?.setEncoding('utf8') - child.stdout?.on('data', onData) - child.stderr?.on('data', onStderr) - child.stdout?.on('error', onError) - child.stderr?.on('error', onError) - child.once('error', onError) - child.once('close', onClose) + options.signal?.throwIfAborted() + const distro = parseWslPath(rootPath)?.distro ?? options.wslDistro + const child = spawnBundledRipgrep(MARKDOWN_DOCUMENT_LISTING_ARGS, { + cwd: rootPath, + wslDistro: options.wslDistro, + wslDistroForOutput: distro, + stdio: ['ignore', 'pipe', 'pipe'] + }) + return collectMarkdownDocuments(child, rootPath, Boolean(distro), options.signal, { + stopProcess: () => stopBundledRipgrep(child, Boolean(distro)) }) } - -function ignoreLateError(): void {} diff --git a/src/main/providers/filesystem-markdown-listing.ts b/src/main/providers/filesystem-markdown-listing.ts new file mode 100644 index 00000000000..e2a45e2c82d --- /dev/null +++ b/src/main/providers/filesystem-markdown-listing.ts @@ -0,0 +1,18 @@ +import type { IFilesystemProvider } from './types' +import { markdownDocumentsFromRelativePaths } from '../../shared/markdown-document-paths' +import { FileInventoryBudget } from '../../shared/file-inventory-budget' + +export async function listFilesystemMarkdownDocuments( + provider: IFilesystemProvider, + rootPath: string +) { + if (provider.listMarkdownDocuments) { + return provider.listMarkdownDocuments(rootPath) + } + const paths = await provider.listFiles(rootPath) + const budget = new FileInventoryBudget() + for (const path of paths) { + budget.record(path) + } + return markdownDocumentsFromRelativePaths(rootPath, paths) +} diff --git a/src/main/providers/filesystem-provider-contract.ts b/src/main/providers/filesystem-provider-contract.ts index ae4a59eb7bb..774a056cb59 100644 --- a/src/main/providers/filesystem-provider-contract.ts +++ b/src/main/providers/filesystem-provider-contract.ts @@ -4,7 +4,7 @@ import type { DocPreviewFileAccessRequest, DocPreviewFileAccessResult } from '../../shared/doc-preview-file-access' -import type { DirEntry, FsChangeEvent } from '../../shared/filesystem-entry-types' +import type { DirEntry, FsChangeEvent, MarkdownDocument } from '../../shared/filesystem-entry-types' import type { WorkspaceSpaceDirectoryScanResult } from '../../shared/workspace-space-types' export type FileStat = { @@ -108,6 +108,10 @@ export type IFilesystemProvider = { searchQuery?: string } ): Promise + listMarkdownDocuments?( + rootPath: string, + options?: { signal?: AbortSignal } + ): Promise supportsQuickOpenSearch?(options?: { signal?: AbortSignal }): Promise scanWorkspaceSpace?( rootPath: string, diff --git a/src/main/providers/sftp-directory-test-fixture.ts b/src/main/providers/sftp-directory-test-fixture.ts new file mode 100644 index 00000000000..9a62f083e4e --- /dev/null +++ b/src/main/providers/sftp-directory-test-fixture.ts @@ -0,0 +1,24 @@ +import type { SFTPWrapper } from 'ssh2' +import { vi } from 'vitest' + +type ListingCallback = (error?: Error | null, result?: unknown) => void + +export function withSftpDirectoryHandles< + T extends { readdir: (path: string, callback: ListingCallback) => void } +>(sftp: T) { + const read = sftp.readdir + const exhausted = new WeakSet() + Object.assign(sftp, { + opendir: vi.fn((path: string, callback: ListingCallback) => callback(null, Buffer.from(path))), + close: vi.fn((_handle: Buffer, callback: ListingCallback) => callback()), + readdir: vi.fn((handle: Buffer, callback: ListingCallback) => { + if (exhausted.has(handle)) { + return callback(null, false) + } + exhausted.add(handle) + read(handle.toString(), callback) + }) + }) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Fixtures provide the file operations under test; the adapter adds opendir, handle readdir, and close. + return sftp as unknown as SFTPWrapper +} diff --git a/src/main/providers/ssh-directory-legacy-compatibility.test.ts b/src/main/providers/ssh-directory-legacy-compatibility.test.ts new file mode 100644 index 00000000000..f6234efaee0 --- /dev/null +++ b/src/main/providers/ssh-directory-legacy-compatibility.test.ts @@ -0,0 +1,96 @@ +import { expect, it, vi } from 'vitest' +import type { SshChannelMultiplexer } from '../ssh/ssh-channel-multiplexer' +import { JsonRpcErrorCode } from '../ssh/relay-protocol' +import { readSshDirectoryBounded } from './ssh-directory-listing' + +function fixture() { + const listeners = new Map) => void>() + const mux = { + request: vi.fn(), + notify: vi.fn(), + isDisposed: () => false, + onDispose: () => () => {}, + onNotificationByMethod: ( + method: string, + callback: (params: Record) => void + ) => { + listeners.set(method, callback) + return () => listeners.delete(method) + } + } + mux.request.mockRejectedValueOnce( + Object.assign(new Error('old host'), { code: JsonRpcErrorCode.MethodNotFound }) + ) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: The fixture implements all mux methods used by the response reader. + return { mux: mux as unknown as SshChannelMultiplexer, mock: mux, listeners } +} + +const entries = [{ name: 'README.md', isDirectory: false, isSymlink: false }] + +it('preserves small old plain replies on system SSH without SFTP', async () => { + const f = fixture() + f.mock.request.mockResolvedValueOnce(entries) + await expect(readSshDirectoryBounded(f.mux, '/remote')).resolves.toEqual(entries) + expect(f.mock.request).toHaveBeenLastCalledWith('fs.readDir', { + dirPath: '/remote', + __streamResponse: true + }) +}) + +it('preserves old streamed replies without SFTP', async () => { + const f = fixture() + const encoded = Buffer.from(JSON.stringify(entries)) + f.mock.request.mockImplementationOnce(async () => { + f.listeners.get('git.responseChunk')?.({ + streamId: 7, + seq: 0, + data: encoded.toString('base64') + }) + f.listeners.get('git.responseEnd')?.({ streamId: 7 }) + return { __orcaGitResponseStream: { streamId: 7, totalBytes: encoded.length, chunkCount: 1 } } + }) + await expect(readSshDirectoryBounded(f.mux, '/remote')).resolves.toEqual(entries) +}) + +it('prefers bounded SFTP to legacy producer allocation', async () => { + const f = fixture() + const fallback = vi.fn().mockResolvedValue(entries) + await expect(readSshDirectoryBounded(f.mux, '/remote', fallback)).resolves.toEqual(entries) + expect(f.mock.request).toHaveBeenCalledTimes(1) + expect(fallback).toHaveBeenCalledOnce() +}) + +it('does not fall back for arbitrary failures', async () => { + const f = fixture() + f.mock.request.mockReset().mockRejectedValue(new Error('permission denied')) + const fallback = vi.fn() + await expect(readSshDirectoryBounded(f.mux, '/remote', fallback)).rejects.toThrow( + 'permission denied' + ) + expect(f.mock.request).toHaveBeenCalledTimes(1) + expect(fallback).not.toHaveBeenCalled() +}) + +it('rejects complete old replies that exceed metadata capacity', async () => { + const f = fixture() + f.mock.request.mockResolvedValueOnce(Array.from({ length: 100001 }, () => entries[0])) + await expect(readSshDirectoryBounded(f.mux, '/remote')).rejects.toThrow('too large') +}) + +it('rejects oversized streamed old replies before accepting chunks', async () => { + const f = fixture() + f.mock.request.mockResolvedValueOnce({ + __orcaGitResponseStream: { streamId: 7, totalBytes: 17 * 1024 * 1024, chunkCount: 1 } + }) + await expect(readSshDirectoryBounded(f.mux, '/remote')).rejects.toThrow('retention budget') + expect(f.mock.notify).toHaveBeenCalledWith('git.cancelResponseStream', { streamId: 7 }) +}) + +it('propagates legacy errors without another fallback', async () => { + const f = fixture() + f.mock.request.mockRejectedValueOnce(new Error('legacy permission denied')) + await expect(readSshDirectoryBounded(f.mux, '/remote')).rejects.toThrow( + 'legacy permission denied' + ) + expect(f.mock.request).toHaveBeenCalledTimes(2) +}) diff --git a/src/main/providers/ssh-directory-listing.ts b/src/main/providers/ssh-directory-listing.ts new file mode 100644 index 00000000000..ad3828a2826 --- /dev/null +++ b/src/main/providers/ssh-directory-listing.ts @@ -0,0 +1,63 @@ +import type { DirEntry } from '../../shared/filesystem-entry-types' +import type { SshChannelMultiplexer } from '../ssh/ssh-channel-multiplexer' +import { requestGitStreamable } from '../ssh/ssh-git-response-stream-reader' +import { isMethodNotFoundError } from '../ssh/ssh-filesystem-stream-reader' +import { validateDirectoryListing } from '../../shared/directory-listing-budget' +import { readSftpDirectory } from './ssh-sftp-filesystem-provider' +import type { SftpFactory } from './ssh-filesystem-download' + +export function readSshDirectoryWithSftpFallback( + mux: SshChannelMultiplexer, + dirPath: string, + createSftp?: SftpFactory +): Promise { + return readSshDirectoryBounded( + mux, + dirPath, + createSftp + ? async () => { + const sftp = await createSftp() + try { + return await readSftpDirectory(sftp, dirPath) + } finally { + sftp.end() + } + } + : undefined + ) +} + +export async function readSshDirectoryBounded( + mux: SshChannelMultiplexer, + dirPath: string, + fallback?: () => Promise +) { + try { + return validateDirectoryListing( + await requestGitStreamable( + mux, + 'fs.readDirBounded', + { dirPath }, + { maxResponseBytes: 16 * 1024 * 1024 } + ) + ) + } catch (error) { + if (isMethodNotFoundError(error)) { + if (fallback) { + return fallback() + } + // Old hosts allocate before replying; bound transport retention and validate the complete result. + return validateDirectoryListing( + await requestGitStreamable( + mux, + 'fs.readDir', + { dirPath }, + { + maxResponseBytes: 16 * 1024 * 1024 + } + ) + ) + } + throw error + } +} diff --git a/src/main/providers/ssh-directory-streaming-real.test.ts b/src/main/providers/ssh-directory-streaming-real.test.ts new file mode 100644 index 00000000000..8d5645af087 --- /dev/null +++ b/src/main/providers/ssh-directory-streaming-real.test.ts @@ -0,0 +1,103 @@ +import { generateKeyPairSync } from 'node:crypto' +import { Client, Server } from 'ssh2' +import type { SFTPWrapper } from 'ssh2' +import { describe, expect, it } from 'vitest' +import { readDirectoryEntriesViaSftp, readDirViaSftp } from './ssh-filesystem-provider-sftp' + +async function createServer() { + const { privateKey } = generateKeyPairSync('rsa', { + modulusLength: 2048, + privateKeyEncoding: { type: 'pkcs1', format: 'pem' }, + publicKeyEncoding: { type: 'pkcs1', format: 'pem' } + }) + let reads = 0 + let closes = 0 + const positions = new Map() + const server = new Server({ hostKeys: [privateKey] }, (connection) => { + connection + .on('authentication', (context) => context.accept()) + .on('ready', () => { + connection.on('session', (accept) => { + accept().on('sftp', (acceptSftp) => { + const stream = acceptSftp() + stream.on('OPENDIR', (id, path) => { + positions.set(path, 0) + stream.handle(id, Buffer.from(path)) + }) + stream.on('READDIR', (id, handle) => { + reads++ + const key = handle.toString() + const start = positions.get(key) ?? 0 + if (start >= 1000) { + stream.status(id, 1) + return + } + positions.set(key, start + 100) + stream.name( + id, + Array.from({ length: 100 }, (_, offset) => ({ + filename: `file-${start + offset}.txt`, + longname: '', + attrs: { mode: 0o100644, size: 0, uid: 0, gid: 0, atime: 0, mtime: 0 } + })) + ) + }) + stream.on('CLOSE', (id, handle) => { + closes++ + positions.delete(handle.toString()) + stream.status(id, 0) + }) + }) + }) + }) + }) + await new Promise((resolve, reject) => { + server.once('error', reject) + server.listen(0, '127.0.0.1', resolve) + }) + const address = server.address() + if (!address || typeof address === 'string') { + throw new Error('No fixture port') + } + const client = new Client() + await new Promise((resolve, reject) => { + client.once('ready', resolve).once('error', reject) + client.connect({ + host: '127.0.0.1', + port: address.port, + username: 'fixture', + password: 'fixture' + }) + }) + const sftp = await new Promise((resolve, reject) => + client.sftp((error, value) => (error ? reject(error) : resolve(value))) + ) + return { + sftp, + counts: () => ({ reads, closes }), + close: async () => { + sftp.end() + client.end() + await new Promise((resolve, reject) => + server.close((error) => (error ? reject(error) : resolve())) + ) + } + } +} + +describe('real SFTP directory packet contract', () => { + it('stops server enumeration after one packet and closes the remote handle', async () => { + const fixture = await createServer() + try { + for await (const entry of readDirectoryEntriesViaSftp(fixture.sftp, '/early')) { + expect(entry.filename).toBe('file-0.txt') + break + } + expect(fixture.counts()).toEqual({ reads: 1, closes: 1 }) + expect(await readDirViaSftp(fixture.sftp, '/complete')).toHaveLength(1000) + expect(fixture.counts()).toEqual({ reads: 12, closes: 2 }) + } finally { + await fixture.close() + } + }) +}) diff --git a/src/main/providers/ssh-directory-streaming.test.ts b/src/main/providers/ssh-directory-streaming.test.ts new file mode 100644 index 00000000000..3f06c41447d --- /dev/null +++ b/src/main/providers/ssh-directory-streaming.test.ts @@ -0,0 +1,216 @@ +import { describe, expect, it, vi } from 'vitest' +import type { SFTPWrapper } from 'ssh2' +import { readDirectoryEntriesViaSftp, readDirViaSftp } from './ssh-filesystem-provider-sftp' + +function fixture(packets: string[][]) { + let next = 0 + const handle = Buffer.from('directory') + const sftp = { + opendir: vi.fn((_path, callback) => callback(null, handle)), + readdir: vi.fn((_handle, callback) => + next < packets.length + ? callback( + null, + packets[next++].map((filename) => ({ filename, attrs: {} })) + ) + : callback(Object.assign(new Error('EOF'), { code: 1 })) + ), + close: vi.fn((_handle, callback) => callback(null)) + } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: The fixture implements all three handle operations used by the reader. + return { mock: sftp, sftp: sftp as unknown as SFTPWrapper } +} + +describe('SFTP directory handle ownership', () => { + it('does not request later packets after a consumer stops', async () => { + const { sftp, mock } = fixture([['first'], ['second']]) + for await (const entry of readDirectoryEntriesViaSftp(sftp, '/folder')) { + expect(entry.filename).toBe('first') + break + } + expect(mock.readdir).toHaveBeenCalledTimes(1) + expect(mock.close).toHaveBeenCalledTimes(1) + }) + + it('continues through empty filtered packets until the protocol EOF error', async () => { + const { sftp, mock } = fixture([[], ['.', '..'], ['visible']]) + expect((await readDirViaSftp(sftp, '/folder')).map((entry) => entry.filename)).toEqual([ + 'visible' + ]) + expect(mock.close).toHaveBeenCalledTimes(1) + }) + + it('rejects capacity before fetching the remaining million-entry directory', async () => { + const name = 'x'.repeat(1000) + const { sftp, mock } = fixture(Array.from({ length: 1000 }, () => Array(100).fill(name))) + await expect(readDirViaSftp(sftp, '/folder')).rejects.toThrow('too large') + expect(mock.readdir.mock.calls.length).toBeLessThan(50) + expect(mock.close).toHaveBeenCalledTimes(1) + }) + + it('closes after cancellation between packets', async () => { + const { sftp, mock } = fixture([['first'], ['second']]) + const controller = new AbortController() + await expect( + (async () => { + for await (const _entry of readDirectoryEntriesViaSftp(sftp, '/folder', { + signal: controller.signal + })) { + controller.abort(new Error('closed')) + } + })() + ).rejects.toThrow('closed') + expect(mock.close).toHaveBeenCalledTimes(1) + }) +}) + +it('bounds a silent CLOSE after early stop and ignores its late callback', async () => { + vi.useFakeTimers() + try { + const { sftp, mock } = fixture([['first']]) + let lateClose: (() => void) | undefined + mock.close.mockImplementation((_handle, callback) => { + lateClose = () => callback(null) + }) + const iterator = readDirectoryEntriesViaSftp(sftp, '/folder') + await iterator.next() + const stopped = iterator.return(undefined) + await vi.advanceTimersByTimeAsync(5000) + await stopped + expect(mock.close).toHaveBeenCalledTimes(1) + expect(vi.getTimerCount()).toBe(0) + lateClose?.() + expect(vi.getTimerCount()).toBe(0) + } finally { + vi.useRealTimers() + } +}) + +it('preserves cancellation when CLOSE never acknowledges', async () => { + vi.useFakeTimers() + try { + const { sftp, mock } = fixture([['first'], ['second']]) + mock.close.mockImplementation(() => {}) + const controller = new AbortController() + const iterator = readDirectoryEntriesViaSftp(sftp, '/folder', { signal: controller.signal }) + await iterator.next() + controller.abort(new Error('original cancellation')) + const rejected = expect(iterator.next()).rejects.toThrow('original cancellation') + await vi.advanceTimersByTimeAsync(5000) + await rejected + expect(mock.close).toHaveBeenCalledTimes(1) + expect(vi.getTimerCount()).toBe(0) + } finally { + vi.useRealTimers() + } +}) + +it('closes a late OPENDIR handle even after cancellation settled', async () => { + vi.useFakeTimers() + try { + const { sftp, mock } = fixture([]) + let lateOpen: (() => void) | undefined + mock.opendir.mockImplementation((_path, callback) => { + lateOpen = () => callback(null, Buffer.from('late-handle')) + }) + mock.close.mockImplementation(() => {}) + const controller = new AbortController() + const pending = readDirectoryEntriesViaSftp(sftp, '/folder', { + signal: controller.signal + }).next() + const rejected = expect(pending).rejects.toThrow('original cancellation') + controller.abort(new Error('original cancellation')) + await vi.advanceTimersByTimeAsync(5000) + await rejected + lateOpen?.() + expect(mock.close).toHaveBeenCalledWith(Buffer.from('late-handle'), expect.any(Function)) + await vi.advanceTimersByTimeAsync(5000) + expect(vi.getTimerCount()).toBe(0) + } finally { + vi.useRealTimers() + } +}) + +it('preserves capacity failure when CLOSE never acknowledges', async () => { + vi.useFakeTimers() + try { + const { sftp, mock } = fixture([['x'.repeat(5 * 1024 * 1024)]]) + mock.close.mockImplementation(() => {}) + const rejected = expect(readDirViaSftp(sftp, '/folder')).rejects.toThrow('too large') + await vi.advanceTimersByTimeAsync(5000) + await rejected + expect(mock.close).toHaveBeenCalledTimes(1) + expect(vi.getTimerCount()).toBe(0) + } finally { + vi.useRealTimers() + } +}) + +it('rejects explicit failed CLOSE and retires the persistent channel', async () => { + const { sftp, mock } = fixture([]) + const end = vi.fn() + Object.assign(sftp, { end }) + const failure = new Error('CLOSE failed') + mock.close.mockImplementation((_handle, callback) => callback(failure)) + await expect(readDirViaSftp(sftp, '/folder')).rejects.toBe(failure) + expect(end).toHaveBeenCalledOnce() +}) + +it('rejects EOF CLOSE timeout without claiming acknowledgement; late callback stays inert', async () => { + vi.useFakeTimers() + try { + const { sftp, mock } = fixture([]) + const end = vi.fn() + Object.assign(sftp, { end }) + let lateClose: (() => void) | undefined + mock.close.mockImplementation((_handle, callback) => { + lateClose = () => callback(null) + }) + const rejected = expect(readDirViaSftp(sftp, '/folder')).rejects.toThrow('CLOSE timed out') + await vi.advanceTimersByTimeAsync(5000) + await rejected + expect(end).toHaveBeenCalledOnce() + lateClose?.() + expect(end).toHaveBeenCalledOnce() + expect(vi.getTimerCount()).toBe(0) + } finally { + vi.useRealTimers() + } +}) + +it.each(['EOF callback', 'CLOSE callback'])( + 'preserves abort reason during final %s', + async (boundary) => { + const { sftp, mock } = fixture([]) + const controller = new AbortController() + const reason = new Error('original final cancellation') + if (boundary === 'EOF callback') { + mock.readdir.mockImplementation((_handle, callback) => { + callback(null, false) + controller.abort(reason) + }) + } else { + mock.close.mockImplementation((_handle, callback) => { + controller.abort(reason) + callback(null) + }) + } + await expect(readDirViaSftp(sftp, '/folder', { signal: controller.signal })).rejects.toBe( + reason + ) + } +) + +it('preserves a consumer capacity failure when CLOSE explicitly fails', async () => { + const { sftp, mock } = fixture([['x'.repeat(5 * 1024 * 1024)]]) + mock.close.mockImplementation((_handle, callback) => callback(new Error('cleanup failed'))) + await expect(readDirViaSftp(sftp, '/folder')).rejects.toThrow('too large') +}) + +it('keeps the persistent channel for acknowledged normal EOF', async () => { + const { sftp } = fixture([['visible']]) + const end = vi.fn() + Object.assign(sftp, { end }) + await expect(readDirViaSftp(sftp, '/folder')).resolves.toHaveLength(1) + expect(end).not.toHaveBeenCalled() +}) diff --git a/src/main/providers/ssh-file-listing.ts b/src/main/providers/ssh-file-listing.ts new file mode 100644 index 00000000000..7e0b6e5e573 --- /dev/null +++ b/src/main/providers/ssh-file-listing.ts @@ -0,0 +1,46 @@ +import { FileInventoryBudget, FILE_INVENTORY_MAX_BYTES } from '../../shared/file-inventory-budget' +import type { SshChannelMultiplexer } from '../ssh/ssh-channel-multiplexer' +import type { IFilesystemProvider } from './types' +import { requestGitStreamable } from '../ssh/ssh-git-response-stream-reader' + +export async function listSshFiles( + mux: SshChannelMultiplexer, + rootPath: string, + options?: Parameters[1] +): Promise { + const params: Record = { rootPath } + if (options?.excludePaths && options.excludePaths.length > 0) { + params.excludePaths = options.excludePaths + } + if (options?.maxResults !== undefined) { + params.maxResults = options.maxResults + } + if (options?.searchQuery !== undefined) { + params.searchQuery = options.searchQuery + } + // Why #7721: the signal lets a workspace switch send rpc.cancel so the + // relay aborts the full-tree scan instead of stacking abandoned scans + // that starve interactive fs.readDir/fs.stat on the shared SSH channel. + // Why streamable: a monorepo listing serializes past the relay's 1 MiB control lane, and the + // lane it demotes to is refused under unrelated producer load. Opting in moves it to the bulk + // lane in chunks; an old relay ignores the flag and answers plainly, which the reader detects + // by the sentinel marker being absent. + const result = await requestGitStreamable(mux, 'fs.listFiles', params, { + signal: options?.signal, + maxResponseBytes: + options?.maxResults !== undefined ? 16 * 1024 * 1024 : FILE_INVENTORY_MAX_BYTES + }) + if (!Array.isArray(result) || result.some((path) => typeof path !== 'string')) { + throw new Error('Invalid remote file listing') + } + if (options?.maxResults !== undefined && result.length > options.maxResults) { + throw new Error('Remote file listing exceeds the requested capacity') + } + if (options?.maxResults === undefined && options?.searchQuery === undefined) { + const budget = new FileInventoryBudget() + for (const path of result) { + budget.record(path) + } + } + return result +} diff --git a/src/main/providers/ssh-filesystem-download.test.ts b/src/main/providers/ssh-filesystem-download.test.ts index d65735cb806..1017f105608 100644 --- a/src/main/providers/ssh-filesystem-download.test.ts +++ b/src/main/providers/ssh-filesystem-download.test.ts @@ -1,3 +1,5 @@ +import { DirectoryTransferBudget } from '../ssh/ssh-directory-transfer-budget' +import { withSftpDirectoryHandles } from './sftp-directory-test-fixture' import { afterEach, describe, expect, it, vi } from 'vitest' import { mkdtemp, rm, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' @@ -64,7 +66,7 @@ describe('downloadFolderViaSftp', () => { } await expect( - downloadFolderViaSftp(async () => sftp as never, '/remote/src', destination) + downloadFolderViaSftp(async () => withSftpDirectoryHandles(sftp), '/remote/src', destination) ).rejects.toThrow("Remote entries map to the same local name 'a.txt'") expect(sftp.fastGet).toHaveBeenCalledTimes(1) }) @@ -83,7 +85,7 @@ describe('downloadFolderViaSftp', () => { } await expect( - downloadFolderViaSftp(async () => sftp as never, '/remote/src', destination) + downloadFolderViaSftp(async () => withSftpDirectoryHandles(sftp), '/remote/src', destination) ).rejects.toThrow("Cannot download unsupported remote entry 'build.pipe'") expect(sftp.fastGet).not.toHaveBeenCalled() }) @@ -103,7 +105,7 @@ describe('downloadFolderViaSftp', () => { } await expect( - downloadFolderViaSftp(async () => sftp as never, '/remote/src', destination) + downloadFolderViaSftp(async () => withSftpDirectoryHandles(sftp), '/remote/src', destination) ).rejects.toThrow("Cannot download symbolic link 'creds'") // The link target could be /etc/passwd; rejecting from directory-entry // metadata means it is never followed with stat or opened by fastGet. @@ -113,6 +115,8 @@ describe('downloadFolderViaSftp', () => { it('sanitizes extended Windows device names in nested entries', async () => { const destination = await createDestination() + const records = vi.spyOn(DirectoryTransferBudget.prototype, 'record') + const releases = vi.spyOn(DirectoryTransferBudget.prototype, 'release') const sftp = { stat: vi.fn((_path: string, callback: (err: Error | undefined, value: unknown) => void) => callback(undefined, sftpStats('directory')) @@ -125,9 +129,16 @@ describe('downloadFolderViaSftp', () => { } await expect( - downloadFolderViaSftp(async () => sftp as never, '/remote/src', destination) + downloadFolderViaSftp(async () => withSftpDirectoryHandles(sftp), '/remote/src', destination) ).rejects.toThrow("Remote entries map to the same local name 'download'") expect(sftp.fastGet).not.toHaveBeenCalled() + expect(records).toHaveBeenCalledTimes(3) + expect(releases).toHaveBeenCalledWith( + records.mock.results.reduce((bytes, record) => bytes + record.value, 0), + 3 + ) + records.mockRestore() + releases.mockRestore() }) it('preserves legal POSIX backslashes in opaque SFTP child names', async () => { @@ -146,9 +157,14 @@ describe('downloadFolderViaSftp', () => { end: vi.fn() } - await downloadFolderViaSftp(async () => sftp as never, sourcePath, destination, { - windowsRemotePaths: false - }) + await downloadFolderViaSftp( + async () => withSftpDirectoryHandles(sftp), + sourcePath, + destination, + { + windowsRemotePaths: false + } + ) expect(sftp.fastGet).toHaveBeenCalledWith( '/remote/parent\\literal/..\\secret.txt', @@ -171,9 +187,14 @@ describe('downloadFolderViaSftp', () => { } await expect( - downloadFolderViaSftp(async () => sftp as never, 'C:/remote/src', destination, { - windowsRemotePaths: true - }) + downloadFolderViaSftp( + async () => withSftpDirectoryHandles(sftp), + 'C:/remote/src', + destination, + { + windowsRemotePaths: true + } + ) ).rejects.toThrow("Invalid remote directory entry '..\\secret.txt'") expect(sftp.fastGet).not.toHaveBeenCalled() }) @@ -195,9 +216,14 @@ describe('downloadFolderViaSftp', () => { } const controller = new AbortController() - const result = downloadFolderViaSftp(async () => sftp as never, '/remote/src', destination, { - signal: controller.signal - }) + const result = downloadFolderViaSftp( + async () => withSftpDirectoryHandles(sftp), + '/remote/src', + destination, + { + signal: controller.signal + } + ) await vi.waitFor(() => expect(sftp.fastGet).toHaveBeenCalledTimes(1)) controller.abort(new Error('renderer closed')) @@ -234,9 +260,14 @@ describe('downloadFolderViaSftp', () => { } const controller = new AbortController() - const result = downloadFolderViaSftp(async () => sftp as never, '/remote/src', destination, { - signal: controller.signal - }) + const result = downloadFolderViaSftp( + async () => withSftpDirectoryHandles(sftp), + '/remote/src', + destination, + { + signal: controller.signal + } + ) await vi.waitFor(() => expect(sftp.readdir).toHaveBeenCalledTimes(1)) controller.abort(new Error('renderer closed')) readDirCallback?.(new Error('channel closed')) @@ -246,3 +277,45 @@ describe('downloadFolderViaSftp', () => { expect(sftp.end).toHaveBeenCalledTimes(1) }) }) + +it.each(['EOF', 'CLOSE'])( + 'does not create an empty destination when cancelled at %s', + async (boundary) => { + const root = await mkdtemp(join(tmpdir(), 'orca-sftp-final-abort-')) + const destination = join(root, 'target') + const controller = new AbortController() + const reason = new Error('cancel at final boundary') + const sftp = withSftpDirectoryHandles({ + stat: (_path: string, callback: (error: undefined, stats: unknown) => void) => + callback(undefined, sftpStats('directory')), + readdir: (_path: string, callback: (error: undefined, entries: unknown) => void) => + callback(undefined, []), + end: vi.fn() + }) + sftp.readdir = (_handle, callback) => { + callback(Object.assign(new Error('EOF'), { code: 1 }), []) + } + if (boundary === 'EOF') { + sftp.readdir = (_handle, callback) => { + callback(Object.assign(new Error('EOF'), { code: 1 }), []) + controller.abort(reason) + } + } else { + sftp.close = (_handle, callback) => { + controller.abort(reason) + callback(null) + } + } + try { + await expect( + downloadFolderViaSftp(async () => sftp, '/empty', destination, { + signal: controller.signal + }) + ).rejects.toBe(reason) + const { access } = await import('node:fs/promises') + await expect(access(destination)).rejects.toMatchObject({ code: 'ENOENT' }) + } finally { + await rm(root, { recursive: true, force: true }) + } + } +) diff --git a/src/main/providers/ssh-filesystem-download.ts b/src/main/providers/ssh-filesystem-download.ts index f73ef4c655f..97ef78e1a91 100644 --- a/src/main/providers/ssh-filesystem-download.ts +++ b/src/main/providers/ssh-filesystem-download.ts @@ -1,3 +1,4 @@ +import { DirectoryTransferBudget } from '../ssh/ssh-directory-transfer-budget' import { mkdir, open } from 'node:fs/promises' import { join } from 'node:path' import type { FileEntryWithStats, SFTPWrapper } from 'ssh2' @@ -7,7 +8,11 @@ import { normalizeRuntimePathSeparators } from '../../shared/cross-platform-path' import { sanitizeLocalDownloadFilename } from '../local-download-filename' -import { fastGetViaSftp, readDirViaSftp, statViaSftp } from './ssh-filesystem-provider-sftp' +import { + fastGetViaSftp, + readDirectoryEntriesViaSftp, + statViaSftp +} from './ssh-filesystem-provider-sftp' export type SftpFactory = (options?: { signal?: AbortSignal }) => Promise @@ -83,45 +88,62 @@ async function downloadDirectoryTree( sourceDir: string, destinationDir: string, signal?: AbortSignal, - windowsRemotePaths?: boolean + windowsRemotePaths?: boolean, + budget = new DirectoryTransferBudget(), + depth = 0 ): Promise { - signal?.throwIfAborted() - const entries = (await readDirViaSftp(sftp, sourceDir, { signal })).filter( - (entry) => entry.filename !== '.' && entry.filename !== '..' - ) signal?.throwIfAborted() const usedLocalNames = new Set() const plannedEntries: { - entry: FileEntryWithStats + remoteName: string kind: 'directory' | 'file' localName: string }[] = [] - for (const entry of entries) { - const localName = sanitizeLocalDownloadFilename(entry.filename) - if (usedLocalNames.has(localName)) { - throw new Error(`Remote entries map to the same local name '${localName}'`) + let retainedBytes = budget.record([sourceDir, destinationDir], depth) + let retainedEntries = 1 + try { + for await (const entry of readDirectoryEntriesViaSftp(sftp, sourceDir, { signal })) { + const localName = sanitizeLocalDownloadFilename(entry.filename) + retainedBytes += budget.record([sourceDir, destinationDir, entry.filename, localName], depth) + retainedEntries++ + if (usedLocalNames.has(localName)) { + throw new Error(`Remote entries map to the same local name '${localName}'`) + } + usedLocalNames.add(localName) + plannedEntries.push({ + remoteName: entry.filename, + kind: classifySftpEntry(entry), + localName + }) } - usedLocalNames.add(localName) - plannedEntries.push({ - entry, - kind: classifySftpEntry(entry), - localName - }) - } - await mkdir(destinationDir, { recursive: false }) - for (const { entry, kind, localName } of plannedEntries) { signal?.throwIfAborted() - const remotePath = joinSftpChildPath(sourceDir, entry.filename, windowsRemotePaths) - const localPath = join(destinationDir, localName) - if (kind === 'directory') { - await downloadDirectoryTree(sftp, remotePath, localPath, signal, windowsRemotePaths) - continue + await mkdir(destinationDir, { recursive: false }) + signal?.throwIfAborted() + for (const { remoteName, kind, localName } of plannedEntries) { + signal?.throwIfAborted() + const remotePath = joinSftpChildPath(sourceDir, remoteName, windowsRemotePaths) + const localPath = join(destinationDir, localName) + if (kind === 'directory') { + await downloadDirectoryTree( + sftp, + remotePath, + localPath, + signal, + windowsRemotePaths, + budget, + depth + 1 + ) + continue + } + // Why: filesystem semantics belong to the selected volume, not the host OS; + // an exclusive placeholder prevents case/Unicode aliases from overwriting. + await reserveLocalFile(localPath, localName) + await fastGetViaSftp(sftp, remotePath, localPath, { signal }) } - // Why: filesystem semantics belong to the selected volume, not the host OS; - // an exclusive placeholder prevents case/Unicode aliases from overwriting. - await reserveLocalFile(localPath, localName) - await fastGetViaSftp(sftp, remotePath, localPath, { signal }) + signal?.throwIfAborted() + } finally { + budget.release(retainedBytes, retainedEntries) } } @@ -177,6 +199,7 @@ export async function downloadFolderViaSftp( signal, options?.windowsRemotePaths ) + signal?.throwIfAborted() } finally { signal?.removeEventListener('abort', endSftp) endSftp() diff --git a/src/main/providers/ssh-filesystem-provider-download-folder.test.ts b/src/main/providers/ssh-filesystem-provider-download-folder.test.ts index c875301883d..9166fde83b4 100644 --- a/src/main/providers/ssh-filesystem-provider-download-folder.test.ts +++ b/src/main/providers/ssh-filesystem-provider-download-folder.test.ts @@ -1,3 +1,4 @@ +import { withSftpDirectoryHandles } from './sftp-directory-test-fixture' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { mkdtemp, rm, stat } from 'node:fs/promises' import { tmpdir } from 'node:os' @@ -83,7 +84,7 @@ describe('SshFilesystemProvider downloadFolder', () => { ), end: vi.fn() } - const createSftp = vi.fn(async () => sftp as never) + const createSftp = vi.fn(async () => withSftpDirectoryHandles(sftp)) provider = new SshFilesystemProvider('conn-1', mux as never, createSftp) const destination = join(root, 'src') @@ -113,7 +114,10 @@ describe('SshFilesystemProvider downloadFolder', () => { fastGet: vi.fn(), end: vi.fn() } - provider = new SshFilesystemProvider('conn-1', mux as never, async () => sftp as never) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: The fixture multiplexer implements the request and notification methods used by this provider. + provider = new SshFilesystemProvider('conn-1', mux as never, async () => + withSftpDirectoryHandles(sftp) + ) await expect(provider.downloadFolder!('/remote/src', join(root, 'src'))).rejects.toThrow( "Cannot download symbolic link 'linked-dir'" @@ -136,7 +140,10 @@ describe('SshFilesystemProvider downloadFolder', () => { fastGet: vi.fn(), end: vi.fn() } - provider = new SshFilesystemProvider('conn-1', mux as never, async () => sftp as never) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: The fixture multiplexer implements the request and notification methods used by this provider. + provider = new SshFilesystemProvider('conn-1', mux as never, async () => + withSftpDirectoryHandles(sftp) + ) await expect(provider.downloadFolder!('/remote/src', join(root, 'src'))).rejects.toThrow( "Remote entries map to the same local name 'a_b.txt'" diff --git a/src/main/providers/ssh-filesystem-provider-sftp.ts b/src/main/providers/ssh-filesystem-provider-sftp.ts index a3b1b840be3..3e78f56a9f8 100644 --- a/src/main/providers/ssh-filesystem-provider-sftp.ts +++ b/src/main/providers/ssh-filesystem-provider-sftp.ts @@ -1,3 +1,5 @@ +import { closeSftpDirectoryHandle } from './ssh-sftp-directory-close' +import { DirectoryListingBudget } from '../../shared/directory-listing-budget' import type { FileEntryWithStats, SFTPWrapper, Stats } from 'ssh2' import type { FileStat } from './types' @@ -95,15 +97,80 @@ export function fastGetViaSftp( ) } -export function readDirViaSftp( +export async function* readDirectoryEntriesViaSftp( + sftp: SFTPWrapper, + dirPath: string, + options?: { signal?: AbortSignal } +): AsyncGenerator { + // Keep the late handle visible to finally when cancellation races opendir. + options?.signal?.throwIfAborted() + const handle = await waitForSftpCallback( + (callback) => + sftp.opendir(dirPath, (error, value) => { + if (!error && options?.signal?.aborted) { + void closeSftpDirectoryHandle(sftp, value) + callback(new Error('Download canceled')) + return + } + callback(error, value) + }), + options + ) + let reachedEof = false + let closeError: Error | undefined + try { + options?.signal?.throwIfAborted() + while (true) { + let chunk: FileEntryWithStats[] | false + try { + chunk = await waitForSftpCallback( + (callback) => sftp.readdir(handle, callback), + options + ) + } catch (error) { + options?.signal?.throwIfAborted() + if (error instanceof Error && 'code' in error && error.code === 1) { + reachedEof = true + break + } + throw error + } + if (chunk === false) { + reachedEof = true + break + } + for (const entry of chunk) { + options?.signal?.throwIfAborted() + if (entry.filename !== '.' && entry.filename !== '..') { + yield entry + } + } + } + } finally { + closeError = await closeSftpDirectoryHandle(sftp, handle) + } + // Consumer failures enter finally via return(); do not replace their reason. + if (reachedEof) { + options?.signal?.throwIfAborted() + if (closeError) { + throw closeError + } + } +} + +export async function readDirViaSftp( sftp: SFTPWrapper, dirPath: string, options?: { signal?: AbortSignal } ): Promise { - return waitForSftpCallback( - (callback) => sftp.readdir(dirPath, callback), - options - ) + const budget = new DirectoryListingBudget() + const entries: FileEntryWithStats[] = [] + for await (const entry of readDirectoryEntriesViaSftp(sftp, dirPath, options)) { + budget.record(entry.filename) + entries.push(entry) + } + options?.signal?.throwIfAborted() + return entries } export function statViaSftp( diff --git a/src/main/providers/ssh-filesystem-provider.test.ts b/src/main/providers/ssh-filesystem-provider.test.ts index 33b063ffa0c..1c842888370 100644 --- a/src/main/providers/ssh-filesystem-provider.test.ts +++ b/src/main/providers/ssh-filesystem-provider.test.ts @@ -68,7 +68,10 @@ describe('SshFilesystemProvider', () => { mux.request.mockResolvedValue(entries) const result = await provider.readDir('/home/user/project') - expect(mux.request).toHaveBeenCalledWith('fs.readDir', { dirPath: '/home/user/project' }) + expect(mux.request).toHaveBeenCalledWith('fs.readDirBounded', { + dirPath: '/home/user/project', + __streamResponse: true + }) expect(result).toEqual(entries) }) }) @@ -488,6 +491,7 @@ describe('SshFilesystemProvider', () => { }) it('listFiles forwards listing and query options', async () => { + mux.request.mockResolvedValue([]) await provider.listFiles('/home/user/project', { excludePaths: ['/home/user/project/worktrees/b'], maxResults: 20_000, diff --git a/src/main/providers/ssh-filesystem-provider.ts b/src/main/providers/ssh-filesystem-provider.ts index f688d2789b1..a44ec27f16f 100644 --- a/src/main/providers/ssh-filesystem-provider.ts +++ b/src/main/providers/ssh-filesystem-provider.ts @@ -1,9 +1,11 @@ +import { readSshDirectoryWithSftpFallback } from './ssh-directory-listing' +import { readSshMarkdownDocuments } from './ssh-markdown-document-listing' import { readSshPathExistenceBatch } from './ssh-filesystem-path-existence' import type { PathExistenceResult } from '../../shared/path-existence-batch' import type { SshChannelMultiplexer } from '../ssh/ssh-channel-multiplexer' import { isMethodNotFoundError, readFileViaStream } from '../ssh/ssh-filesystem-stream-reader' import { uploadBuffer } from '../ssh/sftp-upload' -import { requestGitStreamable } from '../ssh/ssh-git-response-stream-reader' +import { listSshFiles } from './ssh-file-listing' import { lstatViaSftp } from './ssh-filesystem-provider-sftp' import { downloadFileViaSftp, @@ -99,7 +101,7 @@ export class SshFilesystemProvider implements IFilesystemProvider { } async readDir(dirPath: string): Promise { - return (await this.mux.request('fs.readDir', { dirPath })) as DirEntry[] + return readSshDirectoryWithSftpFallback(this.mux, dirPath, this.createSftp) } async readFile(filePath: string, limits?: FileReadLimits): Promise { @@ -308,28 +310,14 @@ export class SshFilesystemProvider implements IFilesystemProvider { rootPath: string, options?: Parameters[1] ): Promise { - const params: Record = { rootPath } - if (options?.excludePaths && options.excludePaths.length > 0) { - params.excludePaths = options.excludePaths - } - if (options?.maxResults !== undefined) { - params.maxResults = options.maxResults - } - if (options?.searchQuery !== undefined) { - params.searchQuery = options.searchQuery - } - // Why #7721: the signal lets a workspace switch send rpc.cancel so the - // relay aborts the full-tree scan instead of stacking abandoned scans - // that starve interactive fs.readDir/fs.stat on the shared SSH channel. - // Why streamable: a monorepo listing serializes past the relay's 1 MiB control lane, and the - // lane it demotes to is refused under unrelated producer load. Opting in moves it to the bulk - // lane in chunks; an old relay ignores the flag and answers plainly, which the reader detects - // by the sentinel marker being absent. - return (await requestGitStreamable(this.mux, 'fs.listFiles', params, { - signal: options?.signal - })) as string[] + return listSshFiles(this.mux, rootPath, options) } + listMarkdownDocuments = (rootPath: string, options?: { signal?: AbortSignal }) => + readSshMarkdownDocuments(this.mux, rootPath, options?.signal, () => + this.listFiles(rootPath, { signal: options?.signal }) + ) + supportsQuickOpenSearch = (options: { signal?: AbortSignal } = {}): Promise => probeSshQuickOpenSearchCapability(this.mux, options.signal) async watch( diff --git a/src/main/providers/ssh-listing-compatibility.test.ts b/src/main/providers/ssh-listing-compatibility.test.ts new file mode 100644 index 00000000000..903834aa89e --- /dev/null +++ b/src/main/providers/ssh-listing-compatibility.test.ts @@ -0,0 +1,98 @@ +import { listSshFiles } from './ssh-file-listing' +import { describe, expect, it, vi } from 'vitest' +import { readSshMarkdownDocuments } from './ssh-markdown-document-listing' +import { readSshDirectoryBounded } from './ssh-directory-listing' +import { SshFilesystemProvider } from './ssh-filesystem-provider' +import type { SshChannelMultiplexer } from '../ssh/ssh-channel-multiplexer' + +function muxFixture(result: unknown, error?: Error) { + const mock = { + request: error ? vi.fn().mockRejectedValue(error) : vi.fn().mockResolvedValue(result), + notify: vi.fn(), + onNotification: vi.fn(() => () => {}), + onNotificationByMethod: vi.fn(() => () => {}), + onDispose: vi.fn(() => () => {}), + isDisposed: () => false + } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: The reader uses only these request, notification, and disposal operations. + return { mock, mux: mock as unknown as SshChannelMultiplexer } +} +const unsupported = () => Object.assign(new Error('Method not found'), { code: -32601 }) + +describe('SSH listing compatibility', () => { + it('accepts late full-inventory files in old plain replies without a count cap', async () => { + const paths = Array.from({ length: 25002 }, (_, i) => `src/file-${i}.ts`) + const { mux, mock } = muxFixture(paths) + expect((await listSshFiles(mux, '/repo')).at(-1)).toBe('src/file-25001.ts') + expect(mock.request.mock.calls[0][1].maxResults).toBeUndefined() + const limited = muxFixture(paths.slice(0, 3)) + expect(await listSshFiles(limited.mux, '/repo', { maxResults: 3 })).toHaveLength(3) + }) + + it('keeps complete small old-peer Markdown inventories useful', async () => { + const { mux } = muxFixture(undefined, unsupported()) + const loadLegacy = vi.fn().mockResolvedValue(['source.ts', 'docs/README.md']) + const result = await readSshMarkdownDocuments(mux, '/repo', undefined, loadLegacy) + expect(result.map((document) => document.relativePath)).toEqual(['docs/README.md']) + expect(loadLegacy).toHaveBeenCalledTimes(1) + }) + + it('keeps late Markdown files in large old-peer source inventories', async () => { + const paths = Array.from({ length: 25_002 }, (_, index) => `src/file-${index}.ts`) + paths.push('docs/late.md') + const { mux, mock } = muxFixture(paths) + mock.request.mockRejectedValueOnce(unsupported()) + const provider = new SshFilesystemProvider('legacy', mux) + await expect(provider.listMarkdownDocuments('/repo')).resolves.toEqual([ + { + filePath: '/repo/docs/late.md', + relativePath: 'docs/late.md', + basename: 'late.md', + name: 'late' + } + ]) + expect(mock.request).toHaveBeenLastCalledWith('fs.listFiles', { + rootPath: '/repo', + __streamResponse: true + }) + provider.dispose() + }) + + it('still rejects an old-peer inventory with too many Markdown documents', async () => { + const { mux } = muxFixture(undefined, unsupported()) + await expect( + readSshMarkdownDocuments(mux, '/repo', undefined, async () => Array(20_001).fill('source.md')) + ).rejects.toThrow('Workspace is too large') + }) + + it('uses bounded SFTP fallback for old directory peers and preserves failures', async () => { + const { mux } = muxFixture(undefined, unsupported()) + const fallback = vi + .fn() + .mockResolvedValue([{ name: 'folder', isDirectory: true, isSymlink: false }]) + expect(await readSshDirectoryBounded(mux, '/repo', fallback)).toHaveLength(1) + const failure = muxFixture(undefined, new Error('Permission denied')) + await expect(readSshDirectoryBounded(failure.mux, '/repo', fallback)).rejects.toThrow( + 'Permission denied' + ) + expect(fallback).toHaveBeenCalledTimes(1) + }) + + it('validates new-peer directory and Markdown metadata before exposing it', async () => { + const directory = muxFixture([ + { name: 'x'.repeat(5 * 1024 * 1024), isDirectory: false, isSymlink: false } + ]) + await expect(readSshDirectoryBounded(directory.mux, '/repo')).rejects.toThrow() + const markdown = muxFixture( + Array.from({ length: 20_001 }, () => ({ + filePath: '/repo/a.md', + relativePath: 'a.md', + basename: 'a.md', + name: 'a' + })) + ) + await expect(readSshMarkdownDocuments(markdown.mux, '/repo')).rejects.toThrow( + 'Workspace is too large' + ) + }) +}) diff --git a/src/main/providers/ssh-markdown-document-listing.ts b/src/main/providers/ssh-markdown-document-listing.ts new file mode 100644 index 00000000000..d3b6be57b8b --- /dev/null +++ b/src/main/providers/ssh-markdown-document-listing.ts @@ -0,0 +1,34 @@ +import { markdownDocumentsFromRelativePaths } from '../../shared/markdown-document-paths' +import type { SshChannelMultiplexer } from '../ssh/ssh-channel-multiplexer' +import { requestGitStreamable } from '../ssh/ssh-git-response-stream-reader' +import { isMethodNotFoundError } from '../ssh/ssh-filesystem-stream-reader' +import type { MarkdownDocument } from '../../shared/filesystem-entry-types' +import { assertMarkdownDocumentsWithinLimit } from '../../shared/markdown-document-listing-limits' + +export async function readSshMarkdownDocuments( + mux: SshChannelMultiplexer, + rootPath: string, + signal?: AbortSignal, + loadLegacy?: () => Promise +): Promise { + let result: unknown + try { + result = await requestGitStreamable( + mux, + 'fs.listMarkdownDocuments', + { rootPath }, + { signal, maxResponseBytes: 16 * 1024 * 1024 } + ) + } catch (error) { + if (isMethodNotFoundError(error)) { + if (loadLegacy) { + return markdownDocumentsFromRelativePaths(rootPath, await loadLegacy()) + } + throw new Error('Markdown discovery requires an updated SSH relay. Reconnect and retry.') + } + throw error + } + assertMarkdownDocumentsWithinLimit(result) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: The shared validator checked every document field and the aggregate budget. + return result as MarkdownDocument[] +} diff --git a/src/main/providers/ssh-sftp-directory-close.ts b/src/main/providers/ssh-sftp-directory-close.ts new file mode 100644 index 00000000000..d610f1cf2b3 --- /dev/null +++ b/src/main/providers/ssh-sftp-directory-close.ts @@ -0,0 +1,42 @@ +import type { SFTPWrapper } from 'ssh2' + +const SFTP_DIRECTORY_CLOSE_TIMEOUT_MS = 5_000 +const retiredChannels = new WeakSet() + +export function isSftpDirectoryChannelRetired(sftp: SFTPWrapper): boolean { + return retiredChannels.has(sftp) +} + +export function closeSftpDirectoryHandle( + sftp: SFTPWrapper, + handle: Buffer +): Promise { + return new Promise((resolve) => { + let settled = false + const finish = (error?: Error): void => { + if (settled) { + return + } + settled = true + clearTimeout(timer) + if (error && !retiredChannels.has(sftp)) { + retiredChannels.add(sftp) + try { + sftp.end() + } catch { + // Preserve the CLOSE failure if channel teardown also fails. + } + } + resolve(error) + } + const timer = setTimeout( + () => finish(new Error('SFTP directory CLOSE timed out')), + SFTP_DIRECTORY_CLOSE_TIMEOUT_MS + ) + try { + sftp.close(handle, (error) => finish(error ?? undefined)) + } catch (error) { + finish(error instanceof Error ? error : new Error(String(error))) + } + }) +} diff --git a/src/main/providers/ssh-sftp-filesystem-channel.ts b/src/main/providers/ssh-sftp-filesystem-channel.ts new file mode 100644 index 00000000000..aaef5c9c2c1 --- /dev/null +++ b/src/main/providers/ssh-sftp-filesystem-channel.ts @@ -0,0 +1,55 @@ +import type { SFTPWrapper } from 'ssh2' +import type { SftpFactory } from './ssh-filesystem-download' +import { isSftpDirectoryChannelRetired } from './ssh-sftp-directory-close' + +export class SftpFilesystemChannel { + private sftpPromise: Promise | null = null + private disposed = false + + constructor(private readonly createSftp: SftpFactory) {} + + dispose(): void { + this.disposed = true + const pending = this.sftpPromise + this.sftpPromise = null + void pending?.then( + (sftp) => sftp.end(), + () => {} + ) + } + + async get(): Promise { + if (this.disposed) { + throw new Error('SSH connection is not active') + } + if (!this.sftpPromise) { + const opening = this.createSftp().then((sftp) => { + if (isSftpDirectoryChannelRetired(sftp)) { + throw new Error('SFTP factory returned a retired directory channel') + } + // Why: a closed channel must not be reused; the next call reopens one. + sftp.once('close', () => { + if (this.sftpPromise === opening) { + this.sftpPromise = null + } + }) + return sftp + }) + opening.catch(() => { + if (this.sftpPromise === opening) { + this.sftpPromise = null + } + }) + this.sftpPromise = opening + } + const opening = this.sftpPromise + const sftp = await opening + if (isSftpDirectoryChannelRetired(sftp)) { + if (this.sftpPromise === opening) { + this.sftpPromise = null + } + return this.get() + } + return sftp + } +} diff --git a/src/main/providers/ssh-sftp-filesystem-provider.test.ts b/src/main/providers/ssh-sftp-filesystem-provider.test.ts index e2597f888a4..f2093ce48de 100644 --- a/src/main/providers/ssh-sftp-filesystem-provider.test.ts +++ b/src/main/providers/ssh-sftp-filesystem-provider.test.ts @@ -42,7 +42,21 @@ class FakeSftp extends EventEmitter { return node.kind === 'file' ? node.content.length : 0 } - readdir(path: string, cb: Callback): void { + private directoryReads = new WeakSet() + + opendir(path: string, cb: Callback): void { + cb(null, Buffer.from(path)) + } + close(_handle: Buffer, cb: Callback): void { + cb(null) + } + + readdir(handle: Buffer, cb: Callback): void { + if (this.directoryReads.has(handle)) { + return cb(null, false) + } + this.directoryReads.add(handle) + const path = handle.toString() const prefix = `${path}/` const entries = [...this.nodes] .filter(([p]) => p.startsWith(prefix) && !p.slice(prefix.length).includes('/')) @@ -263,3 +277,57 @@ describe('SshSftpFilesystemProvider', () => { await expect(provider.stat('/x')).rejects.toThrow('not active') }) }) + +it('retires the persistent directory channel after failed CLOSE and reopens for the next read', async () => { + const first = new FakeSftp() + first.nodes.set('/dir/file.txt', { kind: 'file', content: Buffer.from('content') }) + first.close = (_handle, callback) => callback(new Error('CLOSE rejected')) + const second = new FakeSftp() + second.nodes.set('/dir/file.txt', { kind: 'file', content: Buffer.from('content') }) + const createSftp = vi.fn(async () => { + const next = createSftp.mock.calls.length === 1 ? first : second + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: FakeSftp implements the directory operations and channel close event used by this provider. + return next as unknown as SFTPWrapper + }) + const provider = new SshSftpFilesystemProvider('target-1', createSftp, MODE) + await expect(provider.readDir('/dir')).rejects.toThrow('CLOSE rejected') + expect(first.end).toHaveBeenCalledOnce() + await expect(provider.readDir('/dir')).resolves.toEqual([ + { name: 'file.txt', isDirectory: false, isSymlink: false } + ]) + expect(createSftp).toHaveBeenCalledTimes(2) + expect(second.end).not.toHaveBeenCalled() + first.emit('close') + await expect(provider.readDir('/dir')).resolves.toHaveLength(1) + expect(createSftp).toHaveBeenCalledTimes(2) + provider.dispose() +}) + +it('replaces a timed-out channel immediately without waiting for its close event', async () => { + vi.useFakeTimers() + try { + const first = new FakeSftp() + first.close = () => {} + const second = new FakeSftp() + const createSftp = vi.fn(async () => { + const next = createSftp.mock.calls.length === 1 ? first : second + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: FakeSftp provides handle operations and channel lifecycle events used by the directory provider. + return next as unknown as SFTPWrapper + }) + const provider = new SshSftpFilesystemProvider('target-1', createSftp, MODE) + const rejected = expect(provider.readDir('/dir')).rejects.toThrow('CLOSE timed out') + await vi.advanceTimersByTimeAsync(5000) + await rejected + expect(first.end).toHaveBeenCalledOnce() + const [a, b] = await Promise.all([provider.readDir('/dir'), provider.readDir('/dir')]) + expect(a).toEqual([]) + expect(b).toEqual([]) + expect(createSftp).toHaveBeenCalledTimes(2) + first.emit('close') + await expect(provider.readDir('/dir')).resolves.toEqual([]) + expect(createSftp).toHaveBeenCalledTimes(2) + provider.dispose() + } finally { + vi.useRealTimers() + } +}) diff --git a/src/main/providers/ssh-sftp-filesystem-provider.ts b/src/main/providers/ssh-sftp-filesystem-provider.ts index 8855e55b0ea..41031a09732 100644 --- a/src/main/providers/ssh-sftp-filesystem-provider.ts +++ b/src/main/providers/ssh-sftp-filesystem-provider.ts @@ -1,3 +1,5 @@ +import { DirectoryListingBudget } from '../../shared/directory-listing-budget' +import { SftpFilesystemChannel } from './ssh-sftp-filesystem-channel' /** * Filesystem provider for plain SSH mode (design D6 rung D): read, list, stat and write over * one reused SFTP channel. Anything that needs the Orca remote server (search, file lists, @@ -21,7 +23,7 @@ import { import { fileStatFromSftpStats, lstatViaSftp, - readDirViaSftp, + readDirectoryEntriesViaSftp, statViaSftp } from './ssh-filesystem-provider-sftp' import type { FileReadLimits, FileReadResult, FileStat, IFilesystemProvider } from './types' @@ -63,52 +65,23 @@ function isBinaryBuffer(buffer: Buffer): boolean { } export class SshSftpFilesystemProvider implements IFilesystemProvider { - private sftpPromise: Promise | null = null - private disposed = false + private readonly channel: SftpFilesystemChannel constructor( private readonly connectionId: string, private readonly createSftp: SftpFactory, private readonly mode: SshPlainSshMode, private readonly windowsRemotePaths = false - ) {} + ) { + this.channel = new SftpFilesystemChannel(createSftp) + } getConnectionId(): string { return this.connectionId } dispose(): void { - this.disposed = true - const pending = this.sftpPromise - this.sftpPromise = null - void pending?.then( - (sftp) => sftp.end(), - () => {} - ) - } - - private async sftp(): Promise { - if (this.disposed) { - throw new Error('SSH connection is not active') - } - if (!this.sftpPromise) { - const opening = this.createSftp().then((sftp) => { - // Why: a closed channel must not be reused; the next call reopens one. - sftp.once('close', () => { - if (this.sftpPromise === opening) { - this.sftpPromise = null - } - }) - return sftp - }) - opening.catch(() => { - if (this.sftpPromise === opening) { - this.sftpPromise = null - } - }) - this.sftpPromise = opening - } - return this.sftpPromise + this.channel.dispose() } /** Round-trips one SFTP request; a silent transport times out as not alive. */ @@ -132,7 +105,7 @@ export class SshSftpFilesystemProvider implements IFilesystemProvider { private async run(op: (sftp: SFTPWrapper) => Promise): Promise { try { - return await op(await this.sftp()) + return await op(await this.channel.get()) } catch (error) { throw normalizeSftpError(error) } @@ -159,21 +132,7 @@ export class SshSftpFilesystemProvider implements IFilesystemProvider { async readDir(dirPath: string): Promise { const path = toSftpPath(dirPath) return this.run(async (sftp) => { - const entries = await readDirViaSftp(sftp, path) - const mapped = await Promise.all( - entries.map(async (entry): Promise => { - const isSymlink = entry.attrs.isSymbolicLink() - let isDirectory = entry.attrs.isDirectory() - if (isSymlink) { - // Why: a symlink to a directory must expand in the tree like its target. - isDirectory = await statViaSftp(sftp, `${path.replace(/\/$/, '')}/${entry.filename}`) - .then((stats) => stats.isDirectory()) - .catch(() => false) - } - return { name: entry.filename, isDirectory, isSymlink } - }) - ) - return sortDirEntries(mapped) + return readSftpDirectory(sftp, path) }) } @@ -345,3 +304,20 @@ export class SshSftpFilesystemProvider implements IFilesystemProvider { throw this.unsupported('Watching files for changes') } } + +export async function readSftpDirectory(sftp: SFTPWrapper, path: string): Promise { + const budget = new DirectoryListingBudget() + const mapped: DirEntry[] = [] + for await (const entry of readDirectoryEntriesViaSftp(sftp, path)) { + budget.record(entry.filename) + const isSymlink = entry.attrs.isSymbolicLink() + let isDirectory = entry.attrs.isDirectory() + if (isSymlink) { + isDirectory = await statViaSftp(sftp, `${path.replace(/\/$/, '')}/${entry.filename}`) + .then((stats) => stats.isDirectory()) + .catch(() => false) + } + mapped.push({ name: entry.filename, isDirectory, isSymlink }) + } + return sortDirEntries(mapped) +} diff --git a/src/main/ripgrep/bundled-ripgrep-path.test.ts b/src/main/ripgrep/bundled-ripgrep-path.test.ts index e05c1ba706d..6146d715749 100644 --- a/src/main/ripgrep/bundled-ripgrep-path.test.ts +++ b/src/main/ripgrep/bundled-ripgrep-path.test.ts @@ -73,7 +73,8 @@ describe('bundled ripgrep path', () => { const key = bundledRipgrepContentKey(platform) expect(key).toMatch(/^[0-9a-f]{16}$/) - expect(bundledRipgrepContentKey('win32-x64')).not.toBe(key) + const otherPlatform = platform === 'win32-x64' ? 'linux-x64' : 'win32-x64' + expect(bundledRipgrepContentKey(otherPlatform)).not.toBe(key) }) it('picks the distro-arch Linux build and fails closed when its drive is unavailable', () => { diff --git a/src/main/ripgrep/bundled-ripgrep-stop.test.ts b/src/main/ripgrep/bundled-ripgrep-stop.test.ts new file mode 100644 index 00000000000..cd194632731 --- /dev/null +++ b/src/main/ripgrep/bundled-ripgrep-stop.test.ts @@ -0,0 +1,36 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { spawnProcess } from '../../shared/child-process/run-process' +import { stopBundledRipgrep } from './bundled-ripgrep-stop' + +const { signalTree, killChild } = vi.hoisted(() => ({ signalTree: vi.fn(), killChild: vi.fn() })) +vi.mock('../../shared/child-process/process-tree-termination', () => ({ + signalProcessTree: signalTree +})) +vi.mock('../../shared/ripgrep-process-availability', () => ({ + killSpawnedRipgrepProcess: killChild +})) +afterEach(() => { + vi.restoreAllMocks() + vi.clearAllMocks() +}) + +describe('bundled search process termination', () => { + it('uses the existing bounded tree terminator for Windows WSL and coalesces duplicate stops', async () => { + const child = spawnProcess({ + program: process.execPath, + args: ['-e', 'setInterval(() => {}, 1000)'] + }) + const exited = new Promise((resolve) => child.once('close', resolve)) + try { + vi.spyOn(process, 'platform', 'get').mockReturnValue('win32') + signalTree.mockResolvedValue(true) + stopBundledRipgrep(child, true) + stopBundledRipgrep(child, true) + expect(signalTree).toHaveBeenCalledExactlyOnceWith(child) + expect(killChild).not.toHaveBeenCalled() + } finally { + child.kill() + await exited + } + }) +}) diff --git a/src/main/ripgrep/bundled-ripgrep-stop.ts b/src/main/ripgrep/bundled-ripgrep-stop.ts new file mode 100644 index 00000000000..071691fb820 --- /dev/null +++ b/src/main/ripgrep/bundled-ripgrep-stop.ts @@ -0,0 +1,17 @@ +import { signalProcessTree } from '../../shared/child-process/process-tree-termination' +import { killSpawnedRipgrepProcess } from '../../shared/ripgrep-process-availability' +import type { ChildProcessHandle } from '../../shared/child-process/process-spec' + +const stoppingChildren = new WeakSet() + +export function stopBundledRipgrep(child: ChildProcessHandle, wsl = false): void { + if (stoppingChildren.has(child)) { + return + } + stoppingChildren.add(child) + if (process.platform === 'win32' && wsl && child.pid !== undefined) { + void signalProcessTree(child).catch(() => killSpawnedRipgrepProcess(child)) + } else { + killSpawnedRipgrepProcess(child) + } +} diff --git a/src/main/runtime/runtime-file-commands-constructor.ts b/src/main/runtime/runtime-file-commands-constructor.ts index 93242607f70..427f80b36d8 100644 --- a/src/main/runtime/runtime-file-commands-constructor.ts +++ b/src/main/runtime/runtime-file-commands-constructor.ts @@ -42,8 +42,19 @@ export class RuntimeFileCommandsWithConstructor extends RuntimeFileCommandsWithA const route = runtimeFileRouteForTarget(target) const files = route.kind === 'ssh' - ? await this.listRemoteMobileFiles(worktree.path, route.provider, undefined, options.signal) - : await listQuickOpenFiles(worktree.path, store, undefined, options.signal) + ? await this.listRemoteMobileFiles( + worktree.path, + route.provider, + MOBILE_FILE_LIST_LIMIT + 1, + options.signal + ) + : await listQuickOpenFiles( + worktree.path, + store, + undefined, + options.signal, + MOBILE_FILE_LIST_LIMIT + 1 + ) const entries = files .filter((relativePath) => isSafeMobileRelativePath(relativePath)) .sort((a, b) => a.localeCompare(b)) diff --git a/src/main/runtime/runtime-file-commands-search-runtime-files.ts b/src/main/runtime/runtime-file-commands-search-runtime-files.ts index 5a192c4e50d..ddd85a78dcb 100644 --- a/src/main/runtime/runtime-file-commands-search-runtime-files.ts +++ b/src/main/runtime/runtime-file-commands-search-runtime-files.ts @@ -1,5 +1,6 @@ // @ts-nocheck -- mechanically split class members. import { RuntimeFileCommandsWithCreateFileExplorerDirNoClobber } from './runtime-file-commands-create-file-explorer-dir-no-clobber' +import { listFilesystemMarkdownDocuments } from '../providers/filesystem-markdown-listing' import type { SearchOptions, SearchResult } from '../../shared/code-search-types' import { requireRuntimeFileProvider, @@ -9,10 +10,7 @@ import { QUICK_OPEN_LISTING_MAX_RESULTS } from '../../shared/quick-open-listing- import { limitQuickOpenFilesBySerializedBytes } from '../../shared/quick-open-transport-budget' import { listQuickOpenFiles } from '../ipc/filesystem-list-files' import type { MarkdownDocument } from '../../shared/filesystem-entry-types' -import { - listMarkdownDocuments, - markdownDocumentsFromRelativePaths -} from '../ipc/markdown-documents' +import { listMarkdownDocuments } from '../ipc/markdown-documents' import { getLocalGitOptionsForRegisteredWorktree } from '../ipc/local-worktree-runtime-options' import { validatePathExistenceBatch, @@ -80,8 +78,7 @@ export class RuntimeFileCommandsWithSearchRuntimeFiles extends RuntimeFileComman const target = await this.host.resolveRuntimeFileTarget(worktreeSelector) const provider = requireRuntimeFileProvider(target) if (provider) { - const relativePaths = await provider.listFiles(target.worktree.path) - return markdownDocumentsFromRelativePaths(target.worktree.path, relativePaths) + return listFilesystemMarkdownDocuments(provider, target.worktree.path) } return listMarkdownDocuments( target.worktree.path, diff --git a/src/main/runtime/runtime-file-listing-producer-budgets.test.ts b/src/main/runtime/runtime-file-listing-producer-budgets.test.ts new file mode 100644 index 00000000000..59a895d907e --- /dev/null +++ b/src/main/runtime/runtime-file-listing-producer-budgets.test.ts @@ -0,0 +1,81 @@ +import { describe, expect, it, vi } from 'vitest' +import { getSshFilesystemProviderMock } from './orca-runtime-files-mock-registry' +import { + createRuntimeFileCommands, + useRuntimeFileCommandsLifecycle +} from './orca-runtime-files-test-harness' +const { localList } = vi.hoisted(() => ({ localList: vi.fn() })) +vi.mock('../ipc/filesystem-list-files', () => ({ listQuickOpenFiles: localList })) +vi.mock( + '../providers/ssh-filesystem-dispatch', + async () => (await import('./orca-runtime-files-mock-registry')).sshFilesystemDispatchMock +) + +function inventory(count: number) { + const paths = Array.from({ length: count }, (_, index) => `src/file-${index}.ts`) + return vi.fn(async (_root: string, options?: { maxResults?: number }) => + paths.slice(0, options?.maxResults) + ) +} + +describe('runtime producer listing budgets', () => { + useRuntimeFileCommandsLifecycle() + it.each([5000, 5001, 5002])( + 'passes the mobile sentinel budget on SSH for %i paths', + async (count) => { + const listFiles = inventory(count) + getSshFilesystemProviderMock.mockReturnValue({ listFiles }) + const { commands } = createRuntimeFileCommands({ hostId: 'ssh:host' }) + const result = await commands.listMobileFiles('id:wt-1') + expect(listFiles).toHaveBeenCalledWith('/repo', { maxResults: 5001, signal: undefined }) + expect(result.files).toHaveLength(5000) + expect(result.totalCount).toBe(Math.min(count, 5001)) + expect(result.truncated).toBe(count > 5000) + } + ) + + it('passes the same sentinel budget before local enumeration', async () => { + localList.mockImplementation(async (_root, _store, _excluded, _signal, maxResults) => + Array.from({ length: Math.min(5002, maxResults) }, (_, i) => `file-${i}.txt`) + ) + const { commands } = createRuntimeFileCommands() + const result = await commands.listMobileFiles('id:wt-1') + expect(localList.mock.calls[0][4]).toBe(5001) + expect(result.totalCount).toBe(5001) + expect(result.truncated).toBe(true) + }) + + it.each([25002, 100000])( + 'keeps late files in an unqualified %i-file SSH inventory', + async (count) => { + const listFiles = inventory(count) + getSshFilesystemProviderMock.mockReturnValue({ listFiles }) + const { commands } = createRuntimeFileCommands({ hostId: 'ssh:host' }) + const files = await commands.listRuntimeFiles('id:wt-1') + expect(files).toHaveLength(count) + expect(files.at(-1)).toBe(`src/file-${count - 1}.ts`) + expect(listFiles.mock.calls[0][1]?.maxResults).toBeUndefined() + } + ) + + it('preserves the full local inventory and explicit caller limits', async () => { + localList.mockClear() + localList.mockImplementation(async (_root, _store, _excluded, _signal, maxResults) => + Array.from({ length: 25002 }, (_, i) => `file-${i}.txt`).slice(0, maxResults) + ) + const { commands } = createRuntimeFileCommands() + expect((await commands.listRuntimeFiles('id:wt-1')).at(-1)).toBe('file-25001.txt') + expect(localList.mock.calls[0][4]).toBeUndefined() + expect(await commands.listRuntimeFiles('id:wt-1', { maxResults: 3 })).toHaveLength(3) + }) + + it('requests Markdown from its semantic producer, without retaining unrelated paths', async () => { + const listFiles = vi.fn() + const listMarkdownDocuments = vi.fn().mockResolvedValue([]) + getSshFilesystemProviderMock.mockReturnValue({ listFiles, listMarkdownDocuments }) + const { commands } = createRuntimeFileCommands({ hostId: 'ssh:host' }) + await commands.listRuntimeMarkdownDocuments('id:wt-1') + expect(listMarkdownDocuments).toHaveBeenCalledWith('/repo') + expect(listFiles).not.toHaveBeenCalled() + }) +}) diff --git a/src/main/runtime/runtime-file-target-execution-host.test.ts b/src/main/runtime/runtime-file-target-execution-host.test.ts index 40729da528c..5064fdba011 100644 --- a/src/main/runtime/runtime-file-target-execution-host.test.ts +++ b/src/main/runtime/runtime-file-target-execution-host.test.ts @@ -64,7 +64,7 @@ function makeRuntime(repos: readonly Record[], hostId?: string) } function stubProvider() { - return { listFiles: vi.fn().mockResolvedValue(['README.md']) } + return { listMarkdownDocuments: vi.fn().mockResolvedValue([]) } } describe('runtime file target execution host', () => { @@ -103,8 +103,8 @@ describe('runtime file target execution host', () => { await runtime.listRuntimeMarkdownDocuments(`id:${WORKTREE_ID}`) - expect(m4air.listFiles).toHaveBeenCalledWith(REMOTE_PATH) - expect(openclaw.listFiles).not.toHaveBeenCalled() + expect(m4air.listMarkdownDocuments).toHaveBeenCalledWith(REMOTE_PATH) + expect(openclaw.listMarkdownDocuments).not.toHaveBeenCalled() expect(mocks.listMarkdownDocuments).not.toHaveBeenCalled() }) @@ -118,8 +118,8 @@ describe('runtime file target execution host', () => { await runtime.listRuntimeMarkdownDocuments(`id:${WORKTREE_ID}`) - expect(m4air.listFiles).toHaveBeenCalledWith(REMOTE_PATH) - expect(openclaw.listFiles).not.toHaveBeenCalled() + expect(m4air.listMarkdownDocuments).toHaveBeenCalledWith(REMOTE_PATH) + expect(openclaw.listMarkdownDocuments).not.toHaveBeenCalled() }) // `local` has no SSH namespace to nest in, so a surviving `connectionId` is a row contradicting @@ -140,7 +140,7 @@ describe('runtime file target execution host', () => { await runtime.listRuntimeMarkdownDocuments(`id:${WORKTREE_ID}`) - expect(m4air.listFiles).not.toHaveBeenCalled() + expect(m4air.listMarkdownDocuments).not.toHaveBeenCalled() expect(mocks.listMarkdownDocuments).toHaveBeenCalledWith(REMOTE_PATH, {}) }) @@ -164,7 +164,7 @@ describe('runtime file target execution host', () => { await expect(runtime.listRuntimeMarkdownDocuments(`id:${WORKTREE_ID}`)).rejects.toThrow( ExecutionHostNotDispatchableError ) - expect(impostor.listFiles).not.toHaveBeenCalled() + expect(impostor.listMarkdownDocuments).not.toHaveBeenCalled() expect(mocks.listMarkdownDocuments).not.toHaveBeenCalled() }) @@ -205,7 +205,7 @@ describe('runtime file target execution host', () => { await runtime.listRuntimeMarkdownDocuments(`id:${WORKTREE_ID}`) - expect(m4air.listFiles).toHaveBeenCalledWith(REMOTE_PATH) + expect(m4air.listMarkdownDocuments).toHaveBeenCalledWith(REMOTE_PATH) }) // Losing contact with a remote host is never evidence that its files are here diff --git a/src/main/ssh/ssh-directory-transfer-budget.ts b/src/main/ssh/ssh-directory-transfer-budget.ts new file mode 100644 index 00000000000..047b79bb67c --- /dev/null +++ b/src/main/ssh/ssh-directory-transfer-budget.ts @@ -0,0 +1,43 @@ +export const TRANSFER_PLAN_MAX_RETAINED_BYTES = 32 * 1024 * 1024 +export const TRANSFER_PLAN_MAX_ENTRIES = 100_000 +export const TRANSFER_PLAN_MAX_DEPTH = 256 +export const TRANSFER_PLAN_MAX_PATH_BYTES = 64 * 1024 + +export class DirectoryTransferCapacityError extends Error { + readonly code = 'directory_transfer_capacity' + constructor() { + super( + 'Folder transfer plan is too large to retain safely. Transfer smaller folders separately.' + ) + this.name = 'DirectoryTransferCapacityError' + } +} + +export class DirectoryTransferBudget { + private retainedBytes = 0 + private entries = 0 + + record(paths: readonly string[], depth: number): number { + if (depth > TRANSFER_PLAN_MAX_DEPTH || this.entries >= TRANSFER_PLAN_MAX_ENTRIES) { + throw new DirectoryTransferCapacityError() + } + let bytes = 256 + for (const path of paths) { + if (Buffer.byteLength(path) > TRANSFER_PLAN_MAX_PATH_BYTES) { + throw new DirectoryTransferCapacityError() + } + bytes += path.length * 2 + } + if (this.retainedBytes + bytes > TRANSFER_PLAN_MAX_RETAINED_BYTES) { + throw new DirectoryTransferCapacityError() + } + this.entries += 1 + this.retainedBytes += bytes + return bytes + } + + release(bytes: number, entries: number): void { + this.retainedBytes -= bytes + this.entries -= entries + } +} diff --git a/src/main/ssh/ssh-git-response-pending-retention.test.ts b/src/main/ssh/ssh-git-response-pending-retention.test.ts new file mode 100644 index 00000000000..3659169b9c5 --- /dev/null +++ b/src/main/ssh/ssh-git-response-pending-retention.test.ts @@ -0,0 +1,245 @@ +import { expect, it, vi } from 'vitest' +import type { SshChannelMultiplexer } from './ssh-channel-multiplexer' +import { requestGitStreamable } from './ssh-git-response-stream-reader' + +function fixture() { + const listeners = new Map) => void>>() + const replies: ((value: unknown) => void)[] = [] + const mux = { + request: vi.fn(() => new Promise((resolve) => replies.push(resolve))), + notify: vi.fn(), + isDisposed: () => false, + onDispose: () => () => {}, + onNotificationByMethod: ( + method: string, + callback: (params: Record) => void + ) => { + const callbacks = listeners.get(method) ?? new Set() + callbacks.add(callback) + listeners.set(method, callbacks) + return () => callbacks.delete(callback) + } + } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: The fixture implements the request, disposal and notification surface used by this reader. + const typedMux = mux as unknown as SshChannelMultiplexer + return { + mux: typedMux, + mock: mux, + replies, + emit: (method: string, params: Record) => { + for (const callback of listeners.get(method) ?? []) { + callback(params) + } + }, + listenerCount: () => [...listeners.values()].reduce((sum, callbacks) => sum + callbacks.size, 0) + } +} + +const marker = (streamId: number, totalBytes: number, chunkCount: number) => ({ + __orcaGitResponseStream: { streamId, totalBytes, chunkCount } +}) + +it('drops oversized frames before metadata and fails only their identified owner', async () => { + const f = fixture() + const own = requestGitStreamable(f.mux, 'fs.readDir', {}, { maxResponseBytes: 64 }) + const foreign = requestGitStreamable(f.mux, 'fs.readDir', {}, { maxResponseBytes: 64 }) + const rejected = expect(own).rejects.toThrow('retention budget') + f.emit('git.responseChunk', { streamId: 1, seq: 0, data: 'x'.repeat(1024 * 1024) }) + f.emit('git.responseChunk', { streamId: 2, seq: 0, data: Buffer.from('[]').toString('base64') }) + f.emit('git.responseEnd', { streamId: 2 }) + f.replies[1](marker(2, 2, 1)) + await expect(foreign).resolves.toEqual([]) + f.replies[0](marker(1, 64, 1)) + await rejected + expect(f.mock.notify).toHaveBeenCalledWith('git.cancelResponseStream', { streamId: 1 }) + expect(f.listenerCount()).toBe(0) +}) + +it('refuses chunked pre-metadata overflow even when the discarded data was foreign-shaped', async () => { + const f = fixture() + const result = requestGitStreamable(f.mux, 'fs.readDir', {}, { maxResponseBytes: 64 }) + const rejected = expect(result).rejects.toThrow('retention budget') + for (let seq = 0; seq < 100; seq++) { + f.emit('git.responseChunk', { streamId: 1, seq, data: 'x'.repeat(40) }) + } + f.replies[0](marker(1, 64, 100)) + await rejected + expect(f.listenerCount()).toBe(0) +}) + +it('cleans abandoned queues and cancels a sentinel that arrives after abort', async () => { + const f = fixture() + const controller = new AbortController() + const result = requestGitStreamable( + f.mux, + 'fs.readDir', + {}, + { signal: controller.signal, maxResponseBytes: 64 } + ) + const rejected = expect(result).rejects.toThrow('cancelled') + f.emit('git.responseChunk', { streamId: 1, seq: 0, data: 'e30=' }) + controller.abort() + await rejected + expect(f.listenerCount()).toBe(0) + f.replies[0](marker(1, 2, 1)) + await new Promise((resolve) => setImmediate(resolve)) + expect(f.mock.notify).toHaveBeenCalledWith('git.cancelResponseStream', { streamId: 1 }) +}) + +it('preserves honest ordered pre-metadata chunks and ignores malformed foreign params', async () => { + const f = fixture() + const result = requestGitStreamable(f.mux, 'fs.readDir', {}, { maxResponseBytes: 64 }) + f.emit('git.responseChunk', { streamId: { large: 'x'.repeat(1024) }, data: 'x'.repeat(1024) }) + f.emit('git.responseChunk', { streamId: 1, seq: 0, data: Buffer.from('[1,').toString('base64') }) + f.emit('git.responseChunk', { streamId: 1, seq: 1, data: Buffer.from('2]').toString('base64') }) + f.emit('git.responseEnd', { streamId: 1 }) + f.replies[0](marker(1, 5, 2)) + await expect(result).resolves.toEqual([1, 2]) + expect(f.listenerCount()).toBe(0) +}) + +it('cleans the pending queue when the metadata request fails', async () => { + const f = fixture() + f.mock.request.mockRejectedValueOnce(new Error('metadata unavailable')) + const result = requestGitStreamable(f.mux, 'fs.readDir', {}, { maxResponseBytes: 64 }) + f.emit('git.responseChunk', { streamId: 1, seq: 0, data: 'e30=' }) + await expect(result).rejects.toThrow('metadata unavailable') + expect(f.listenerCount()).toBe(0) +}) + +it('drops assembled parts and subscriptions on a stalled owned stream', async () => { + vi.useFakeTimers() + try { + const f = fixture() + const result = requestGitStreamable( + f.mux, + 'fs.readDir', + {}, + { maxResponseBytes: 64, inactivityTimeoutMs: 10 } + ) + const rejected = expect(result).rejects.toThrow('stalled') + f.replies[0](marker(1, 4, 2)) + await Promise.resolve() + f.emit('git.responseChunk', { streamId: 1, seq: 0, data: Buffer.from('[').toString('base64') }) + await vi.advanceTimersByTimeAsync(10) + await rejected + expect(f.listenerCount()).toBe(0) + expect(vi.getTimerCount()).toBe(0) + expect(f.mock.notify).toHaveBeenCalledWith('git.cancelResponseStream', { streamId: 1 }) + } finally { + vi.useRealTimers() + } +}) + +it('accepts an exact-budget honest response split across padded base64 chunks', async () => { + const f = fixture() + const result = requestGitStreamable(f.mux, 'fs.readDir', {}, { maxResponseBytes: 64 }) + const payload = JSON.stringify('x'.repeat(62)) + expect(Buffer.byteLength(payload)).toBe(64) + for (let seq = 0; seq < 4; seq++) { + f.emit('git.responseChunk', { + streamId: 1, + seq, + data: Buffer.from(payload.slice(seq * 16, (seq + 1) * 16)).toString('base64') + }) + } + f.emit('git.responseEnd', { streamId: 1 }) + f.replies[0](marker(1, 64, 4)) + await expect(result).resolves.toBe('x'.repeat(62)) +}) + +it.each(['', '====', '!'])( + 'rejects zero-progress chunks %j without retaining or ACKing them', + async (data) => { + const f = fixture() + const result = requestGitStreamable(f.mux, 'fs.readDir', {}, { maxResponseBytes: 64 }) + const rejected = expect(result).rejects.toThrow('byte progress') + f.replies[0](marker(1, 2, 1)) + await Promise.resolve() + for (let seq = 0; seq < 100_000; seq++) { + f.emit('git.responseChunk', { streamId: 1, seq, data }) + } + await rejected + expect(f.mock.notify).not.toHaveBeenCalledWith('git.responseAck', expect.anything()) + expect(f.mock.notify).toHaveBeenCalledWith('git.cancelResponseStream', { streamId: 1 }) + expect(f.listenerCount()).toBe(0) + } +) + +it.each([ + ['chunks', 2, 1, '['], + ['bytes', 2, 2, '[]'] +])('rejects excess declared %s before ACKing', async (_kind, total, chunks, text) => { + const f = fixture() + const result = requestGitStreamable(f.mux, 'fs.readDir', {}, { maxResponseBytes: 64 }) + const rejected = expect(result).rejects.toThrow('declared') + f.replies[0](marker(1, Number(total), Number(chunks))) + await Promise.resolve() + f.emit('git.responseChunk', { + streamId: 1, + seq: 0, + data: Buffer.from(String(text)).toString('base64') + }) + f.emit('git.responseChunk', { streamId: 1, seq: 1, data: Buffer.from(']').toString('base64') }) + await rejected + expect(f.mock.notify.mock.calls.filter(([method]) => method === 'git.responseAck')).toHaveLength( + 1 + ) + expect(f.listenerCount()).toBe(0) +}) + +it('rejects an enormous declared count independently of continued empty frames', async () => { + const f = fixture() + const result = requestGitStreamable(f.mux, 'fs.readDir', {}, { maxResponseBytes: 64 }) + const rejected = expect(result).rejects.toThrow('chunk count') + f.replies[0](marker(1, 2, Number.MAX_SAFE_INTEGER)) + await rejected + expect(f.listenerCount()).toBe(0) +}) + +it.each([true, false])( + 'bounds owned error diagnostics with metadata first: %s', + async (metadataFirst) => { + const f = fixture() + const result = requestGitStreamable(f.mux, 'fs.readDir', {}, { maxResponseBytes: 64 }) + const rejected = expect(result).rejects.toThrow('retention budget') + if (metadataFirst) { + f.replies[0](marker(1, 2, 1)) + await Promise.resolve() + } + f.emit('git.responseError', { streamId: 1, message: 'x'.repeat(1024 * 1024) }) + if (!metadataFirst) { + f.replies[0](marker(1, 2, 1)) + } + await rejected + expect(f.listenerCount()).toBe(0) + } +) + +it('preserves short upstream error text', async () => { + const f = fixture() + const result = requestGitStreamable(f.mux, 'fs.readDir', {}, { maxResponseBytes: 64 }) + const rejected = expect(result).rejects.toThrow('permission denied') + f.replies[0](marker(1, 2, 1)) + await Promise.resolve() + f.emit('git.responseError', { streamId: 1, message: 'permission denied' }) + await rejected +}) + +it('reassembles one-byte valid chunks without a per-chunk retained buffer', async () => { + const f = fixture() + const text = JSON.stringify('x'.repeat(100_000)) + const result = requestGitStreamable(f.mux, 'fs.readDir', {}, { maxResponseBytes: text.length }) + f.replies[0](marker(1, text.length, text.length)) + await Promise.resolve() + for (let seq = 0; seq < text.length; seq++) { + f.emit('git.responseChunk', { + streamId: 1, + seq, + data: Buffer.from(text[seq]).toString('base64') + }) + } + f.emit('git.responseEnd', { streamId: 1 }) + await expect(result).resolves.toBe('x'.repeat(100_000)) + expect(f.listenerCount()).toBe(0) +}) diff --git a/src/main/ssh/ssh-git-response-stream-reader.ts b/src/main/ssh/ssh-git-response-stream-reader.ts index 0a8b26aa779..1bf084ab0c5 100644 --- a/src/main/ssh/ssh-git-response-stream-reader.ts +++ b/src/main/ssh/ssh-git-response-stream-reader.ts @@ -1,3 +1,9 @@ +import { stringifyJsonWithinByteLimit } from '../../shared/node-bounded-json-stringify' +import { + SshResponsePendingFrames, + boundedSshResponseDiagnostic +} from './ssh-response-pending-frames' +import { SshResponsePayload } from './ssh-response-payload' import type { SshChannelMultiplexer } from './ssh-channel-multiplexer' import { createSshDisposalError } from './ssh-channel-multiplexer' import { RelayErrorCode, isGitResponseStreamMarker } from './relay-protocol' @@ -10,12 +16,6 @@ const SENTINEL_STREAM_ID = -1 * responseEnd) while the SSH channel stays up would hang the client forever. */ const STREAM_INACTIVITY_TIMEOUT_MS = 30_000 -/** Bound transient buffering of other concurrent streams' chunks while this - * reader awaits its sentinel: every reader sees all git.responseChunk frames - * and can't filter by streamId until its own sentinel resolves. Foreign frames - * are dropped on drain anyway; this just caps the pre-sentinel backlog. */ -const MAX_PENDING_FRAMES = 64 - export class GitResponseStreamError extends Error { readonly code = RelayErrorCode.StreamProtocolError constructor(message: string) { @@ -23,11 +23,6 @@ export class GitResponseStreamError extends Error { } } -type PendingFrame = - | { kind: 'chunk'; params: Record } - | { kind: 'end'; params: Record } - | { kind: 'error'; params: Record } - /** * Request a git method that may return a large payload, opting into response * streaming so a big diff/exec response is chunked onto the relay's bulk lane @@ -49,6 +44,7 @@ export function requestGitStreamable( /** Bounds only the sentinel request (forwarded to mux.request), like today. */ timeoutMs?: number /** Bounds the post-sentinel reassembly stall; resets on each chunk. */ + maxResponseBytes?: number inactivityTimeoutMs?: number } ): Promise { @@ -69,14 +65,13 @@ export function requestGitStreamable( } return new Promise((resolve, reject) => { - const parts: Buffer[] = [] + let payload: SshResponsePayload | undefined let expectedSeq = 0 - let receivedBytes = 0 let totalBytes = 0 let chunkCount = 0 let settled = false let metadataReady = false - const pending: PendingFrame[] = [] + const pending = new SshResponsePendingFrames(options?.maxResponseBytes) const inactivityMs = options?.inactivityTimeoutMs ?? STREAM_INACTIVITY_TIMEOUT_MS let inactivityTimer: ReturnType | null = null @@ -118,6 +113,8 @@ export function requestGitStreamable( return } settled = true + payload?.clear() + pending.clear() clearInactivity() cancel() cleanup() @@ -128,6 +125,8 @@ export function requestGitStreamable( return } settled = true + payload?.clear() + pending.clear() clearInactivity() cleanup() resolve(value) @@ -137,8 +136,8 @@ export function requestGitStreamable( if (settled || p.streamId !== streamIdRef.current) { return } - const seq = p.seq as number - const data = p.data as string + const seq = p.seq + const data = p.data if (typeof seq !== 'number' || typeof data !== 'string') { fail(new GitResponseStreamError(`Malformed chunk for git stream ${streamIdRef.current}`)) return @@ -151,9 +150,12 @@ export function requestGitStreamable( ) return } - const decoded = Buffer.from(data, 'base64') - parts.push(decoded) - receivedBytes += decoded.length + try { + payload?.append(data) + } catch (error) { + fail(new GitResponseStreamError(String(error))) + return + } expectedSeq += 1 armInactivity() // Why: credit-based flow control — the relay caps unacked chunks so a big @@ -171,20 +173,20 @@ export function requestGitStreamable( if (settled || p.streamId !== streamIdRef.current) { return } - if (expectedSeq !== chunkCount || receivedBytes !== totalBytes) { + if (expectedSeq !== chunkCount || payload?.receivedBytes !== totalBytes) { fail( new GitResponseStreamError( - `Git stream ${streamIdRef.current} incomplete: chunks ${expectedSeq}/${chunkCount}, bytes ${receivedBytes}/${totalBytes}` + `Git stream ${streamIdRef.current} incomplete: chunks ${expectedSeq}/${chunkCount}, bytes ${payload?.receivedBytes}/${totalBytes}` ) ) return } try { - succeed(JSON.parse(Buffer.concat(parts).toString('utf-8'))) + succeed(JSON.parse(payload?.takeString() ?? '')) } catch (err) { fail( new GitResponseStreamError( - `Git stream ${streamIdRef.current} JSON parse failed: ${String(err)}` + `Git stream ${streamIdRef.current} JSON parse failed: ${boundedSshResponseDiagnostic(String(err), options?.maxResponseBytes)}` ) ) } @@ -194,12 +196,12 @@ export function requestGitStreamable( if (settled || p.streamId !== streamIdRef.current) { return } - fail(new Error((p.message as string | undefined) ?? 'git response stream error')) + fail(new Error(boundedSshResponseDiagnostic(p.message, options?.maxResponseBytes))) } const drainPending = (): void => { - while (!settled && pending.length > 0) { - const frame = pending.shift()! + let frame = pending.shift() + while (!settled && frame) { if (frame.kind === 'chunk') { handleChunk(frame.params) } else if (frame.kind === 'end') { @@ -207,25 +209,14 @@ export function requestGitStreamable( } else { handleStreamError(frame.params) } - } - } - - // Why: pre-sentinel we cannot filter by streamId (our id is unknown yet), so - // every concurrent reader transiently buffers all readers' chunks. Cap the - // backlog by dropping the oldest; foreign frames are dropped on drain anyway, - // and if our own seq-0 were ever dropped the seq check fails loudly rather - // than corrupting. The sentinel normally resolves long before this cap. - const pushPending = (frame: PendingFrame): void => { - pending.push(frame) - if (pending.length > MAX_PENDING_FRAMES) { - pending.shift() + frame = pending.shift() } } unsubscribers.push( mux.onNotificationByMethod('git.responseChunk', (p) => { if (!metadataReady) { - pushPending({ kind: 'chunk', params: p }) + pending.push('chunk', p) return } handleChunk(p) @@ -234,7 +225,7 @@ export function requestGitStreamable( unsubscribers.push( mux.onNotificationByMethod('git.responseEnd', (p) => { if (!metadataReady) { - pushPending({ kind: 'end', params: p }) + pending.push('end', p) return } handleEnd(p) @@ -243,7 +234,7 @@ export function requestGitStreamable( unsubscribers.push( mux.onNotificationByMethod('git.responseError', (p) => { if (!metadataReady) { - pushPending({ kind: 'error', params: p }) + pending.push('error', p) return } handleStreamError(p) @@ -285,10 +276,18 @@ export function requestGitStreamable( void requestPromise .then((result) => { if (settled) { + if (isGitResponseStreamMarker(result) && !mux.isDisposed()) { + mux.notify('git.cancelResponseStream', { + streamId: result.__orcaGitResponseStream.streamId + }) + } return } // Old relay / small result: plain single-frame value, no stream follows. if (!isGitResponseStreamMarker(result)) { + if (options?.maxResponseBytes !== undefined) { + stringifyJsonWithinByteLimit(result, options.maxResponseBytes) + } succeed(result) return } @@ -296,6 +295,15 @@ export function requestGitStreamable( totalBytes = marker.totalBytes chunkCount = marker.chunkCount streamIdRef.current = marker.streamId + if (pending.lostFrames(marker.streamId)) { + fail( + new GitResponseStreamError( + 'Filesystem response exceeds the retention budget before metadata' + ) + ) + return + } + payload = new SshResponsePayload(totalBytes, chunkCount, options?.maxResponseBytes) metadataReady = true // Why: start the inactivity deadline now — mux.request's timeout only // covered the sentinel; the reassembly phase needs its own guard. diff --git a/src/main/ssh/ssh-listing-response-retention.test.ts b/src/main/ssh/ssh-listing-response-retention.test.ts new file mode 100644 index 00000000000..3846b4185b7 --- /dev/null +++ b/src/main/ssh/ssh-listing-response-retention.test.ts @@ -0,0 +1,60 @@ +import { expect, it, vi } from 'vitest' +import type { SshChannelMultiplexer } from './ssh-channel-multiplexer' +import { requestGitStreamable } from './ssh-git-response-stream-reader' + +function fixture(result: unknown) { + const listeners = new Map) => void>() + const mock = { + request: vi.fn().mockResolvedValue(result), + notify: vi.fn(), + isDisposed: () => false, + onDispose: () => () => {}, + onNotificationByMethod: ( + method: string, + listener: (params: Record) => void + ) => { + listeners.set(method, listener) + return () => { + listeners.delete(method) + } + } + } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: This fixture implements the reader's request, notification, and disposal operations. + return { mux: mock as unknown as SshChannelMultiplexer, mock, listeners } +} + +it('refuses oversized stream metadata before retaining any chunks and cancels the pump', async () => { + const { mux, mock, listeners } = fixture({ + __orcaGitResponseStream: { streamId: 1, totalBytes: 129, chunkCount: 2 } + }) + await expect( + requestGitStreamable(mux, 'fs.readDirBounded', {}, { maxResponseBytes: 128 }) + ).rejects.toThrow('retention budget') + expect(mock.notify).toHaveBeenCalledWith('git.cancelResponseStream', { streamId: 1 }) + expect(listeners.size).toBe(0) +}) + +it('rejects a chunk past the advertised retention limit and detaches listeners', async () => { + const { mux, mock, listeners } = fixture({ + __orcaGitResponseStream: { streamId: 2, totalBytes: 64, chunkCount: 1 } + }) + const result = requestGitStreamable(mux, 'fs.readDirBounded', {}, { maxResponseBytes: 64 }) + const outcome = expect(result).rejects.toThrow('retention budget') + await Promise.resolve() + listeners.get('git.responseChunk')?.({ + streamId: 2, + seq: 0, + data: Buffer.alloc(65).toString('base64') + }) + await outcome + expect(mock.notify).toHaveBeenCalledWith('git.cancelResponseStream', { streamId: 2 }) + expect(listeners.size).toBe(0) +}) + +it('validates old-peer plain replies against the same reader byte budget', async () => { + const { mux, listeners } = fixture(['x'.repeat(129)]) + await expect( + requestGitStreamable(mux, 'fs.listFiles', {}, { maxResponseBytes: 128 }) + ).rejects.toThrow('exceeds') + expect(listeners.size).toBe(0) +}) diff --git a/src/main/ssh/ssh-response-payload.ts b/src/main/ssh/ssh-response-payload.ts new file mode 100644 index 00000000000..e6a9bff2a6d --- /dev/null +++ b/src/main/ssh/ssh-response-payload.ts @@ -0,0 +1,67 @@ +import { GrowingByteBuffer } from '../../shared/growing-byte-buffer' + +export class SshResponsePayload { + private readonly bytes = new GrowingByteBuffer() + private receivedChunks = 0 + + constructor( + readonly totalBytes: number, + readonly chunkCount: number, + private readonly maxResponseBytes?: number + ) { + if ( + !Number.isSafeInteger(totalBytes) || + !Number.isSafeInteger(chunkCount) || + chunkCount > totalBytes || + (totalBytes > 0 && chunkCount === 0) + ) { + throw new Error('Invalid git response chunk count or byte total') + } + if (maxResponseBytes !== undefined && totalBytes > maxResponseBytes) { + throw new Error('Filesystem response exceeds the retention budget') + } + } + + append(data: string): void { + if (this.receivedChunks >= this.chunkCount) { + throw new Error('Git response exceeds its declared chunk count') + } + if ( + this.maxResponseBytes !== undefined && + data.length > Math.ceil(this.maxResponseBytes / 3) * 4 + ) { + throw new Error('Filesystem response exceeds the retention budget') + } + const remaining = this.totalBytes - this.bytes.byteLength + if (data.length > Math.ceil(remaining / 3) * 4) { + throw new Error('Git response exceeds its declared byte total') + } + const decoded = Buffer.from(data, 'base64') + if (decoded.length === 0) { + throw new Error('Git response chunk made no byte progress') + } + if ( + this.maxResponseBytes !== undefined && + this.bytes.byteLength + decoded.length > this.maxResponseBytes + ) { + throw new Error('Filesystem response exceeds the retention budget') + } + if (decoded.length > remaining) { + throw new Error('Git response exceeds its declared byte total') + } + this.bytes.append(decoded) + this.receivedChunks += 1 + } + + get receivedBytes(): number { + return this.bytes.byteLength + } + + takeString(): string { + return this.bytes.takeString() + } + + clear(): void { + this.bytes.clear() + } +} diff --git a/src/main/ssh/ssh-response-pending-frames.test.ts b/src/main/ssh/ssh-response-pending-frames.test.ts new file mode 100644 index 00000000000..9265845645f --- /dev/null +++ b/src/main/ssh/ssh-response-pending-frames.test.ts @@ -0,0 +1,35 @@ +import { expect, it } from 'vitest' +import { SshResponsePendingFrames } from './ssh-response-pending-frames' + +it('bounds encoded bytes before retaining oversized payloads or extra fields', () => { + const pending = new SshResponsePendingFrames(64) + pending.push('chunk', { streamId: 1, seq: 0, data: 'x'.repeat(1024 * 1024) }) + expect(pending.retainedBytes).toBe(0) + expect(pending.lostFrames(1)).toBe(true) + pending.push('chunk', { streamId: 2, seq: 0, data: 'e30=', unrelated: 'x'.repeat(1024 * 1024) }) + expect(pending.shift()?.params).toEqual({ streamId: 2, seq: 0, data: 'e30=' }) + expect(pending.lostFrames(2)).toBe(false) +}) + +it('bounds chunked retention and releases backing references on clear', () => { + const pending = new SshResponsePendingFrames(64) + for (let seq = 0; seq < 1000; seq++) { + pending.push('chunk', { streamId: 1, seq, data: 'x'.repeat(40) }) + expect(pending.retainedBytes).toBeLessThanOrEqual(pending.maxEncodedBytes) + } + expect(pending.lostFrames(1)).toBe(true) + pending.clear() + expect(pending.retainedBytes).toBe(0) + expect(pending.shift()).toBeUndefined() + expect(pending.lostFrames(1)).toBe(false) +}) + +it('bounds loss evidence and refuses ambiguous success after evidence overflow', () => { + const pending = new SshResponsePendingFrames(0) + for (let streamId = 0; streamId < 10000; streamId++) { + pending.push('chunk', { streamId, data: 'x' }) + pending.push('chunk', { streamId: 'x'.repeat(1000), data: 'x' }) + } + expect(pending.retainedBytes).toBe(0) + expect(pending.lostFrames(10001)).toBe(true) +}) diff --git a/src/main/ssh/ssh-response-pending-frames.ts b/src/main/ssh/ssh-response-pending-frames.ts new file mode 100644 index 00000000000..cb80062bc8d --- /dev/null +++ b/src/main/ssh/ssh-response-pending-frames.ts @@ -0,0 +1,101 @@ +const MAX_PENDING_FRAMES = 64 +const DEFAULT_PENDING_RESPONSE_BYTES = 64 * 1024 * 1024 + +export function boundedSshResponseDiagnostic(message: unknown, maxResponseBytes?: number): string { + if (typeof message !== 'string') { + return 'git response stream error' + } + return message.length * 2 <= Math.min(8192, maxResponseBytes ?? 8192) + ? message + : 'Filesystem response error exceeds the retention budget' +} + +export type PendingResponseFrame = { + kind: 'chunk' | 'end' | 'error' + params: Record + encodedBytes: number +} + +/** Holds only known scalar fields until the sentinel identifies the request's stream. */ +export class SshResponsePendingFrames { + private readonly frames: PendingResponseFrame[] = [] + private readonly droppedStreams = new Set() + private encodedBytes = 0 + private evidenceOverflow = false + readonly maxEncodedBytes: number + private readonly maxDiagnosticBytes: number + + constructor(maxResponseBytes = DEFAULT_PENDING_RESPONSE_BYTES) { + this.maxDiagnosticBytes = Math.min(8192, maxResponseBytes) + // Each chunk pads independently; allow bounded padding as well as two-byte code units. + this.maxEncodedBytes = + Math.ceil(maxResponseBytes / 3) * 8 + (maxResponseBytes > 0 ? MAX_PENDING_FRAMES * 8 : 0) + } + + private recordDrop(streamId: number): void { + if (this.droppedStreams.size < MAX_PENDING_FRAMES) { + this.droppedStreams.add(streamId) + } else if (!this.droppedStreams.has(streamId)) { + this.evidenceOverflow = true + } + } + + push(kind: PendingResponseFrame['kind'], source: Record): void { + const streamId = source.streamId + if (typeof streamId !== 'number' || !Number.isSafeInteger(streamId) || streamId < 0) { + return + } + const text = kind === 'chunk' ? source.data : kind === 'error' ? source.message : undefined + const encodedBytes = typeof text === 'string' ? text.length * 2 : 0 + if ( + encodedBytes > this.maxEncodedBytes || + (kind === 'error' && encodedBytes > this.maxDiagnosticBytes) + ) { + this.recordDrop(streamId) + return + } + while ( + this.frames.length > 0 && + (this.frames.length >= MAX_PENDING_FRAMES || + this.encodedBytes + encodedBytes > this.maxEncodedBytes) + ) { + const discarded = this.shift()! + const discardedId = discarded.params.streamId + if (typeof discardedId === 'number') { + this.recordDrop(discardedId) + } + } + const params: Record = { streamId } + if (kind === 'chunk') { + params.seq = typeof source.seq === 'number' ? source.seq : undefined + params.data = typeof text === 'string' ? text : undefined + } else if (kind === 'error') { + params.message = typeof text === 'string' ? text : undefined + } + this.frames.push({ kind, params, encodedBytes }) + this.encodedBytes += encodedBytes + } + + lostFrames(streamId: number): boolean { + return this.evidenceOverflow || this.droppedStreams.has(streamId) + } + + shift(): PendingResponseFrame | undefined { + const frame = this.frames.shift() + if (frame) { + this.encodedBytes -= frame.encodedBytes + } + return frame + } + + clear(): void { + this.frames.length = 0 + this.droppedStreams.clear() + this.encodedBytes = 0 + this.evidenceOverflow = false + } + + get retainedBytes(): number { + return this.encodedBytes + } +} diff --git a/src/main/ssh/system-ssh-file-transfer.ts b/src/main/ssh/system-ssh-file-transfer.ts index ada0f85a013..352386c3fb5 100644 --- a/src/main/ssh/system-ssh-file-transfer.ts +++ b/src/main/ssh/system-ssh-file-transfer.ts @@ -1,5 +1,6 @@ +import { DirectoryTransferBudget } from './ssh-directory-transfer-budget' import { spawn } from 'node:child_process' -import { lstat, readdir } from 'node:fs/promises' +import { lstat, opendir } from 'node:fs/promises' import { join as pathJoin } from 'node:path' import { pipeline } from 'node:stream/promises' import type { SshTarget } from '../../shared/ssh-types' @@ -146,11 +147,14 @@ export async function collectLocalUploadPlan( remoteDir: string, hostPlatform: RemoteHostPlatform, signal: AbortSignal | undefined, - plan: LocalUploadPlan = { directories: [], files: [] } + plan: LocalUploadPlan = { directories: [], files: [] }, + budget = new DirectoryTransferBudget(), + depth = 0 ): Promise { + throwIfAborted(signal) + budget.record([localDir, remoteDir], depth) plan.directories.push(remoteDir) - const dirEntries = await readdir(localDir, { withFileTypes: true }) - for (const entry of dirEntries) { + for await (const entry of await opendir(localDir)) { throwIfAborted(signal) const localPath = pathJoin(localDir, entry.name) const remotePath = joinRemotePath(hostPlatform, remoteDir, entry.name) @@ -159,9 +163,18 @@ export async function collectLocalUploadPlan( continue } if (statResult.isDirectory()) { - await collectLocalUploadPlan(localPath, remotePath, hostPlatform, signal, plan) + await collectLocalUploadPlan( + localPath, + remotePath, + hostPlatform, + signal, + plan, + budget, + depth + 1 + ) continue } + budget.record([localPath, remotePath], depth + 1) plan.files.push({ localPath, remotePath }) } return plan diff --git a/src/main/ssh/system-ssh-upload-plan-budget.test.ts b/src/main/ssh/system-ssh-upload-plan-budget.test.ts new file mode 100644 index 00000000000..d623c036992 --- /dev/null +++ b/src/main/ssh/system-ssh-upload-plan-budget.test.ts @@ -0,0 +1,61 @@ +import type * as FsPromises from 'node:fs/promises' +import { expect, it, vi } from 'vitest' +const { directory, stats } = vi.hoisted(() => ({ directory: vi.fn(), stats: vi.fn() })) +vi.mock('node:fs/promises', async (load) => ({ + ...(await load()), + opendir: directory, + lstat: stats +})) +import { collectLocalUploadPlan } from './system-ssh-file-transfer' +import { getRemoteHostPlatform } from './ssh-remote-platform' + +it('stops the upload producer at its retained-plan ceiling before any transfer starts', async () => { + let visited = 0 + let closed = false + stats.mockResolvedValue({ + isSymbolicLink: () => false, + isFile: () => true, + isDirectory: () => false + }) + directory.mockImplementation(async () => + (async function* () { + try { + for (let index = 0; index < 1_000_000; index++) { + visited++ + yield { name: `file-${index}.txt` } + } + } finally { + closed = true + } + })() + ) + await expect( + collectLocalUploadPlan('/local', 'C:/remote', getRemoteHostPlatform('win32-x64'), undefined) + ).rejects.toThrow('transfer plan is too large') + expect(visited).toBeLessThanOrEqual(100_000) + expect(closed).toBe(true) +}) + +it('closes an in-progress upload directory when canceled', async () => { + const controller = new AbortController() + let closed = false + directory.mockImplementation(async () => + (async function* () { + try { + controller.abort(new Error('canceled')) + yield { name: 'file' } + } finally { + closed = true + } + })() + ) + await expect( + collectLocalUploadPlan( + '/local', + 'C:/remote', + getRemoteHostPlatform('win32-x64'), + controller.signal + ) + ).rejects.toThrow('cancelled') + expect(closed).toBe(true) +}) diff --git a/src/relay/dispatcher-rpc-routing.ts b/src/relay/dispatcher-rpc-routing.ts index a6e6c24190c..84f8e71dfd5 100644 --- a/src/relay/dispatcher-rpc-routing.ts +++ b/src/relay/dispatcher-rpc-routing.ts @@ -138,7 +138,17 @@ export abstract class RelayDispatcherRpcRouting extends RelayDispatcherFrameCode } const message = err instanceof Error ? err.message : String(err) const errorCode = (err as { code?: unknown }).code - const code = typeof errorCode === 'number' ? errorCode : -32000 + const capacityCodes: Record = { + git_grep_record_capacity: RelayErrorCode.GitGrepRecordCapacity, + markdown_document_listing_capacity: RelayErrorCode.MarkdownListingCapacity, + directory_listing_capacity: RelayErrorCode.DirectoryListingCapacity + } + const code = + typeof errorCode === 'number' + ? errorCode + : typeof errorCode === 'string' + ? (capacityCodes[errorCode] ?? -32000) + : -32000 // Why an allowlist keyed on the error code: error `data` is otherwise dropped, so a // handler cannot leak internals by attaching them. Each published shape is validated // against its own schema before it crosses. diff --git a/src/relay/filesystem-capacity-error-wire.test.ts b/src/relay/filesystem-capacity-error-wire.test.ts new file mode 100644 index 00000000000..91c0554de5b --- /dev/null +++ b/src/relay/filesystem-capacity-error-wire.test.ts @@ -0,0 +1,37 @@ +import { expect, it, vi } from 'vitest' +import { RelayDispatcher } from './dispatcher' +import { encodeJsonRpcFrame, RelayErrorCode } from './protocol' +import { GitGrepRecordCapacityError } from '../shared/git-grep-record-limit' +import { MarkdownDocumentListingCapacityError } from '../shared/markdown-document-listing-limits' +import { DirectoryListingCapacityError } from '../shared/directory-listing-budget' + +it.each([ + [new GitGrepRecordCapacityError(), RelayErrorCode.GitGrepRecordCapacity], + [new MarkdownDocumentListingCapacityError(), RelayErrorCode.MarkdownListingCapacity], + [new DirectoryListingCapacityError(), RelayErrorCode.DirectoryListingCapacity] +])( + 'preserves typed filesystem capacity errors across the existing JSON-RPC envelope', + async (failure, code) => { + vi.useFakeTimers() + const frames: Buffer[] = [] + const dispatcher = new RelayDispatcher((frame) => { + frames.push(frame) + return true + }) + try { + dispatcher.onRequest('fs.fixture', async () => { + throw failure + }) + dispatcher.feed(encodeJsonRpcFrame({ jsonrpc: '2.0', id: 1, method: 'fs.fixture' }, 1, 0)) + await vi.advanceTimersByTimeAsync(0) + expect(frames).toHaveLength(1) + const frame = frames[0] + const response = JSON.parse(frame.subarray(13, 13 + frame.readUInt32BE(9)).toString()) + expect(response.error.code).toBe(code) + expect(response.error.message).toBe(failure.message) + } finally { + dispatcher.dispose() + vi.useRealTimers() + } + } +) diff --git a/src/relay/fs-directory-listing.ts b/src/relay/fs-directory-listing.ts new file mode 100644 index 00000000000..5580daf6ba8 --- /dev/null +++ b/src/relay/fs-directory-listing.ts @@ -0,0 +1,34 @@ +import { opendir, stat } from 'node:fs/promises' +import { join } from 'node:path' +import { DirectoryListingBudget } from '../shared/directory-listing-budget' +import type { DirEntry } from '../shared/filesystem-entry-types' +import { sortDirEntries } from '../shared/file-name-sort' +import { expandTilde } from './context' + +export async function readRelayDirectoryBounded( + dirPath: string, + signal?: AbortSignal +): Promise { + signal?.throwIfAborted() + const root = expandTilde(dirPath) + const budget = new DirectoryListingBudget() + const entries: DirEntry[] = [] + for await (const entry of await opendir(root)) { + signal?.throwIfAborted() + budget.record(entry.name) + const mapped = { + name: entry.name, + isDirectory: entry.isDirectory(), + isSymlink: entry.isSymbolicLink() + } + if (mapped.isSymlink && !mapped.isDirectory) { + try { + mapped.isDirectory = (await stat(join(root, entry.name))).isDirectory() + } catch { + // Broken links remain visible as links. + } + } + entries.push(mapped) + } + return sortDirEntries(entries) +} diff --git a/src/relay/fs-file-listing-paths.ts b/src/relay/fs-file-listing-paths.ts new file mode 100644 index 00000000000..0d0e380b92b --- /dev/null +++ b/src/relay/fs-file-listing-paths.ts @@ -0,0 +1,33 @@ +import { + normalizeQuickOpenRgLine, + shouldExcludeQuickOpenRelPath, + shouldIncludeQuickOpenPath +} from '../shared/quick-open-filter' +import type { FileInventoryBudget } from '../shared/file-inventory-budget' +import type { QuickOpenPathRanker } from '../shared/quick-open-path-search' + +export function retainRelayFileListingPath( + rawLine: string, + excludePathPrefixes: readonly string[], + ranker: QuickOpenPathRanker | null, + files: Set, + budget: FileInventoryBudget | null +): boolean { + const relativePath = normalizeQuickOpenRgLine(rawLine, { kind: 'cwd-relative' }) + if (relativePath === null) { + return false + } + if ( + !shouldIncludeQuickOpenPath(relativePath) || + shouldExcludeQuickOpenRelPath(relativePath, excludePathPrefixes) + ) { + return true + } + if (ranker) { + ranker.consider(relativePath) + } else if (!files.has(relativePath)) { + budget?.record(relativePath) + files.add(relativePath) + } + return true +} diff --git a/src/relay/fs-handler-git-fallback.ts b/src/relay/fs-handler-git-fallback.ts index 3298f369c9e..fd76bda75ef 100644 --- a/src/relay/fs-handler-git-fallback.ts +++ b/src/relay/fs-handler-git-fallback.ts @@ -1,3 +1,5 @@ +import { killSpawnedRipgrepProcess } from '../shared/ripgrep-process-availability' +import { FileInventoryBudget, FileInventoryCapacityError } from '../shared/file-inventory-budget' /** * Git-based fallbacks for file listing and text search. * @@ -37,6 +39,7 @@ export function listFilesWithGit( if (signal?.aborted) { return Promise.reject(fileListingCancellationError(signal)) } + const inventoryBudget = new FileInventoryBudget() const gitPaths = new Set() const directoryPaths = new Set() const directFileCandidates = new Set() @@ -57,6 +60,9 @@ export function listFilesWithGit( if (!path) { return false } + if (!gitPaths.has(path) && !directoryPaths.has(path)) { + inventoryBudget.record(path) + } if (path.endsWith('/')) { directoryPaths.add(path) } else { @@ -126,7 +132,18 @@ export function listFilesWithGit( let start = 0 let idx = buf.indexOf('\0', start) while (idx !== -1) { - if (processPath(buf.substring(start, idx))) { + let atLimit: boolean + try { + atLimit = processPath(buf.substring(start, idx)) + } catch (error) { + killSpawnedRipgrepProcess(child) + rejectPass(error instanceof Error ? error : new FileInventoryCapacityError()) + killSurvivors('git file inventory capacity exceeded') + gitPaths.clear() + directoryPaths.clear() + return + } + if (atLimit) { buf = '' finishAtLimit() return @@ -216,6 +233,9 @@ export function listFilesWithGit( // Why: ignored files are supplementary — a failed or timed-out ignored // pass must not discard the primary listing the user actually needs. runGitLsFiles(ignoredPass).catch((err: Error) => { + if (err instanceof FileInventoryCapacityError) { + throw err + } if (!signal?.aborted) { console.warn( '[relay quick-open] git ignored-file pass failed; keeping primary results:', @@ -242,6 +262,12 @@ export function listFilesWithGit( }) // Why: directory placeholders are expanded after Git exits; restore // Git's path order for empty queries and fuzzy-score ties over SSH. + if (maxResults === undefined) { + const outputBudget = new FileInventoryBudget() + for (const path of files) { + outputBudget.record(path) + } + } return files.sort().slice(0, maxResults) }) .catch((err) => { diff --git a/src/relay/fs-handler-git-search-capacity.test.ts b/src/relay/fs-handler-git-search-capacity.test.ts new file mode 100644 index 00000000000..34a22a98264 --- /dev/null +++ b/src/relay/fs-handler-git-search-capacity.test.ts @@ -0,0 +1,72 @@ +import { EventEmitter } from 'node:events' +import { PassThrough } from 'node:stream' +import { describe, expect, it, vi } from 'vitest' + +const { spawnMock } = vi.hoisted(() => ({ spawnMock: vi.fn() })) +vi.mock('../shared/child-process/run-process', () => ({ spawnProcess: spawnMock })) +import { searchWithGitGrep } from './fs-handler-git-search' +import { GitGrepRecordCapacityError } from '../shared/git-grep-record-limit' + +class SearchProcess extends EventEmitter { + stdout = new PassThrough() + stderr = new PassThrough() + kill = vi.fn(() => true) +} + +function start() { + const child = new SearchProcess() + spawnMock.mockReturnValue(child) + return { child, result: searchWithGitGrep('/repo', 'ok', { maxResults: 100 }) } +} + +describe('git search record capacity', () => { + it('rejects an unterminated record past 8 MiB and detaches even when kill fails', async () => { + const { child, result } = start() + child.kill.mockImplementation(() => { + throw new Error('kill refused') + }) + const outcome = expect(result).rejects.toThrow(GitGrepRecordCapacityError) + for (let chunk = 0; chunk < 129; chunk++) { + child.stdout.write(Buffer.alloc(64 * 1024, 'x')) + } + await outcome + expect(child.kill).toHaveBeenCalled() + expect(child.stdout.listenerCount('data')).toBe(0) + expect(child.stderr.listenerCount('data')).toBe(0) + expect(child.listenerCount('close')).toBe(0) + child.emit('close', 0) + child.stdout.write('later.ts\x001\x00ok\n') + }) + + it('accepts exactly 8 MiB followed by newline and recovers on the next request', async () => { + const { child, result } = start() + child.stdout.write('x'.repeat(8 * 1024 * 1024)) + child.stdout.write('\nvalid.ts\x001\x00ok\n') + child.emit('close', 0) + expect((await result).files).toHaveLength(1) + const next = start() + next.child.stdout.write('next.ts\x001\x00ok\n') + next.child.emit('close', 0) + expect((await next.result).files[0].relativePath).toBe('next.ts') + }) + + it('releases the carry on cancellation before reaching the cap', async () => { + const child = new SearchProcess() + spawnMock.mockReturnValue(child) + const controller = new AbortController() + const result = searchWithGitGrep('/repo', 'ok', { maxResults: 100, signal: controller.signal }) + child.stdout.write('x'.repeat(1024 * 1024)) + controller.abort(new Error('workspace switched')) + await expect(result).rejects.toThrow('workspace switched') + expect(child.stdout.listenerCount('data')).toBe(0) + }) +}) + +it('charges raw bytes before replacement decoding invalid UTF-8', async () => { + const { child, result } = start() + child.stdout.write(Buffer.alloc(3 * 1024 * 1024, 0xff)) + child.stdout.write('\nvalid.ts\x001\x00ok\n') + child.emit('close', 0) + expect((await result).files[0].relativePath).toBe('valid.ts') + expect(child.kill).not.toHaveBeenCalled() +}) diff --git a/src/relay/fs-handler-git-search-real-capacity.test.ts b/src/relay/fs-handler-git-search-real-capacity.test.ts new file mode 100644 index 00000000000..de980e6cf79 --- /dev/null +++ b/src/relay/fs-handler-git-search-real-capacity.test.ts @@ -0,0 +1,24 @@ +import { mkdtemp, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { describe, expect, it } from 'vitest' +import { runProcess } from '../shared/child-process/run-process' +import { searchWithGitGrep } from './fs-handler-git-search' +import { GitGrepRecordCapacityError } from '../shared/git-grep-record-limit' + +describe('real git search capacity and recovery', () => { + it('rejects a matching newline-free file and can search again in the same folder', async () => { + const root = await mkdtemp(join(tmpdir(), 'orca-git-record-')) + try { + await runProcess({ program: 'git', args: ['init', '--quiet'], cwd: root }) + await writeFile(join(root, 'record.txt'), `needle${'x'.repeat(9 * 1024 * 1024)}`) + await expect(searchWithGitGrep(root, 'needle', { maxResults: 10 })).rejects.toThrow( + GitGrepRecordCapacityError + ) + await writeFile(join(root, 'record.txt'), 'needle\n') + expect((await searchWithGitGrep(root, 'needle', { maxResults: 10 })).totalMatches).toBe(1) + } finally { + await rm(root, { recursive: true, force: true }) + } + }) +}) diff --git a/src/relay/fs-handler-git-search.ts b/src/relay/fs-handler-git-search.ts index c448c17d476..87252e2f215 100644 --- a/src/relay/fs-handler-git-search.ts +++ b/src/relay/fs-handler-git-search.ts @@ -1,3 +1,7 @@ +import { + GitGrepRecordCapacityError, + GIT_GREP_MAX_RECORD_BYTES +} from '../shared/git-grep-record-limit' import { SearchSubprocessLineAccumulator } from '../shared/search-subprocess-lines' import { spawnProcess } from '../shared/child-process/run-process' import { abortSignalReason } from '../shared/abort-signal-reason' @@ -32,7 +36,7 @@ export function searchWithGitGrep( const gitArgs = buildGitGrepArgs(query, opts) const matchRegex = buildSubmatchRegex(query, opts) const acc = createAccumulator() - const lines = new SearchSubprocessLineAccumulator(Number.MAX_SAFE_INTEGER) + const lines = new SearchSubprocessLineAccumulator(GIT_GREP_MAX_RECORD_BYTES) let done = false let processErrorObserved = false @@ -90,8 +94,15 @@ export function searchWithGitGrep( } } - function handleStdoutData(chunk: string): void { - lines.push(chunk, processLine) + function handleStdoutData(chunk: Buffer | string): void { + if (!lines.push(chunk, processLine) && settle()) { + try { + killSpawnedRipgrepProcess(child) + } catch { + // Release the request even when the host refuses the kill. + } + reject(new GitGrepRecordCapacityError()) + } } function handleStderrData(): void { @@ -111,7 +122,6 @@ export function searchWithGitGrep( resolveOnce() } - child.stdout!.setEncoding('utf-8') child.stdout!.on('data', handleStdoutData) child.stderr!.on('data', handleStderrData) child.once('error', handleError) diff --git a/src/relay/fs-handler-list-files-ignored.test.ts b/src/relay/fs-handler-list-files-ignored.test.ts index 36a3914d021..a7f06d62e93 100644 --- a/src/relay/fs-handler-list-files-ignored.test.ts +++ b/src/relay/fs-handler-list-files-ignored.test.ts @@ -86,6 +86,60 @@ describe('relay quick open ignored file listing', () => { } }) + it('rejects a full git inventory capacity failure and stops both passes', async () => { + const primary = createMockProcess() + const ignored = createMockProcess() + spawnMock.mockReturnValueOnce(primary).mockReturnValueOnce(ignored) + const result = listFilesWithGit('/remote/root') + const rejected = expect(result).rejects.toThrow('inventory is too large') + let produced = 0 + while (primary.stdout?.listenerCount('data') && produced < 100000) { + primary.stdout.emit( + 'data', + Array.from({ length: 100 }, () => `src/${'x'.repeat(1000)}-${produced++}.ts\0`).join('') + ) + } + await rejected + expect(produced).toBeLessThan(40000) + expect(primary.kill).toHaveBeenCalled() + expect(ignored.kill).toHaveBeenCalled() + expect(primary.stdout?.listenerCount('data')).toBe(0) + expect(ignored.stdout?.listenerCount('data')).toBe(0) + }) + + it('retains a late 25,002nd file in a complete inventory', async () => { + const child = createMockProcess() + spawnMock.mockReturnValue(child) + const result = listFilesWithRg('/remote/root') + child.stdout?.emit( + 'data', + Array.from({ length: 25002 }, (_, i) => `src/file-${i}.ts\0`).join('') + ) + child.emit('close', 0, null) + const paths = await result + expect(paths).toHaveLength(25002) + expect(paths.at(-1)).toBe('src/file-25001.ts') + }) + + it('stops a full-inventory producer at its aggregate retained-byte ceiling', async () => { + const child = createMockProcess() + spawnMock.mockReturnValue(child) + const result = listFilesWithRg('/remote/root') + const rejected = expect(result).rejects.toThrow('inventory is too large') + let produced = 0 + while (child.stdout?.listenerCount('data') && produced < 100000) { + child.stdout.emit( + 'data', + Array.from({ length: 100 }, () => `src/${'x'.repeat(1000)}-${produced++}.ts\0`).join('') + ) + } + await rejected + expect(produced).toBeLessThan(40000) + expect(child.kill).toHaveBeenCalled() + expect(child.stdout?.listenerCount('data')).toBe(0) + expect(child.listenerCount('close')).toBe(0) + }) + it('uses one broad rg pass for unbounded listings and keeps blocklists/excludes', async () => { const ignoredProc = createMockProcess() diff --git a/src/relay/fs-handler-list-files.ts b/src/relay/fs-handler-list-files.ts index 93c325c301a..dddb74fc5d3 100644 --- a/src/relay/fs-handler-list-files.ts +++ b/src/relay/fs-handler-list-files.ts @@ -1,3 +1,5 @@ +import { retainRelayFileListingPath } from './fs-file-listing-paths' +import { FileInventoryBudget } from '../shared/file-inventory-budget' import { RipgrepFilenameDecoder } from '../shared/ripgrep-filename-decoder' /** * Ripgrep-based file listing for Quick Open. @@ -16,12 +18,7 @@ import { RipgrepFilenameDecoder } from '../shared/ripgrep-filename-decoder' */ import { spawn, type ChildProcess } from 'node:child_process' import { fileListingCancellationError } from '../shared/file-listing-cancellation' -import { - buildRgArgsForQuickOpen, - normalizeQuickOpenRgLine, - shouldExcludeQuickOpenRelPath, - shouldIncludeQuickOpenPath -} from '../shared/quick-open-filter' +import { buildRgArgsForQuickOpen } from '../shared/quick-open-filter' import { absorbPendingRipgrepSpawnError, classifyRipgrepLaunchFailure, @@ -52,6 +49,8 @@ export function listFilesWithRg( return Promise.reject(fileListingCancellationError(signal)) } return new Promise((resolve, reject) => { + const inventoryBudget = + maxResults === undefined && searchQuery === undefined ? new FileInventoryBudget() : null const files = new Set() let rankedPaths: string[] | null = null let done = false @@ -71,25 +70,26 @@ export function listFilesWithRg( }) const processLine = (rawLine: string, attemptRanker: QuickOpenPathRanker | null): boolean => { - const relPath = normalizeQuickOpenRgLine(rawLine, { kind: 'cwd-relative' }) - if (relPath === null) { - return false - } - // Why: correctness backstop. The rg globs prune most blocklisted dirs, - // but a glob edge case could still surface e.g. a .git/ or .npm/ hit. - const excluded = shouldExcludeQuickOpenRelPath(relPath, excludePathPrefixes) - if (!shouldIncludeQuickOpenPath(relPath) || excluded) { + try { + const included = retainRelayFileListingPath( + rawLine, + excludePathPrefixes, + attemptRanker, + files, + inventoryBudget + ) + if (maxResults !== undefined && files.size >= maxResults) { + finishAtLimit() + } + return included + } catch (error) { + done = true + signal?.removeEventListener('abort', onAbort) + killSurvivors('File inventory capacity exceeded') + files.clear() + reject(error) return true } - if (attemptRanker) { - attemptRanker.consider(relPath) - return true - } - files.add(relPath) - if (maxResults !== undefined && files.size >= maxResults) { - finishAtLimit() - } - return true } const runPassOnce = (args: string[]): Promise => diff --git a/src/relay/fs-handler.ts b/src/relay/fs-handler.ts index 36765721ef3..542881abeb4 100644 --- a/src/relay/fs-handler.ts +++ b/src/relay/fs-handler.ts @@ -1,3 +1,7 @@ +import { readRelayDirectoryBounded } from './fs-directory-listing' +import { listRelayMarkdownDocuments } from './fs-markdown-document-listing' +import { markdownDocumentsFromRelativePaths } from '../shared/markdown-document-paths' +import { joinSearchRoot } from '../shared/text-search-paths' import { pathsExistOnRelay } from './fs-path-existence' import { tmpdir } from 'node:os' import type { RelayDispatcher, RequestContext } from './dispatcher' @@ -80,6 +84,15 @@ export class FsHandler { private registerHandlers(): void { this.dispatcher.onRequest('fs.readDir', (p) => readRelayDir(p)) + this.dispatcher.onRequest('fs.readDirBounded', async (p, c) => { + if (typeof p.dirPath !== 'string') { + throw new Error('Invalid directory path') + } + const entries = await readRelayDirectoryBounded(p.dirPath, c?.signal) + return this.responseStreams + ? maybeStreamRpcResponse(entries, p, c, this.responseStreams, this.dispatcher) + : entries + }) this.dispatcher.onRequest('fs.readFile', (p) => this.readFile(p)) this.dispatcher.onRequest('fs.readFileStream', (p, c) => this.readFileStream(p, c)) this.dispatcher.onRequest('fs.readFileRange', (p) => this.readFileRange(p)) @@ -108,6 +121,33 @@ export class FsHandler { pathExistenceBatchVersion: 1 })) this.dispatcher.onRequest('fs.listFiles', (p, c) => this.listFiles(p, c)) + this.dispatcher.onRequest('fs.listMarkdownDocuments', async (p, c) => { + if (typeof p.rootPath !== 'string') { + throw new Error('Invalid Markdown discovery root') + } + const rootPath = expandTilde(p.rootPath) + const documents = await listRelayMarkdownDocuments(rootPath, c?.signal).catch( + async (error) => { + if (!(error instanceof RipgrepUnavailableError)) { + throw error + } + const paths = await this.listFiles({ rootPath }, c) + if ( + !Array.isArray(paths) || + !paths.every((path): path is string => typeof path === 'string') + ) { + throw new Error('Invalid fallback file listing') + } + return markdownDocumentsFromRelativePaths(rootPath, paths).map((document) => ({ + ...document, + filePath: joinSearchRoot(rootPath, document.relativePath) + })) + } + ) + return this.responseStreams + ? maybeStreamRpcResponse(documents, p, c, this.responseStreams, this.dispatcher) + : documents + }) this.dispatcher.onRequest('fs.workspaceSpaceScan', (p, c) => this.workspaceSpaceScan(p, c)) this.dispatcher.onRequest('fs.watch', (p, context) => this.watchRegistry.watch( diff --git a/src/relay/fs-markdown-document-launch.test.ts b/src/relay/fs-markdown-document-launch.test.ts new file mode 100644 index 00000000000..fe39911228a --- /dev/null +++ b/src/relay/fs-markdown-document-launch.test.ts @@ -0,0 +1,101 @@ +import { EventEmitter } from 'node:events' +import { PassThrough } from 'node:stream' +import { beforeEach, expect, it, vi } from 'vitest' +import type * as RipgrepAvailability from '../shared/ripgrep-process-availability' + +const { spawnMock, cwdUsableMock } = vi.hoisted(() => ({ + spawnMock: vi.fn(), + cwdUsableMock: vi.fn() +})) +vi.mock('../shared/child-process/run-process', () => ({ spawnProcess: spawnMock })) +vi.mock('./relay-bundled-ripgrep', () => ({ resolveRelayRipgrepCommand: () => '/tools/rg' })) +vi.mock('../shared/ripgrep-process-availability', async (importOriginal) => ({ + ...(await importOriginal()), + isRipgrepSpawnCwdUsable: cwdUsableMock +})) + +import { RipgrepUnavailableError } from '../shared/ripgrep-process-availability' +import { listRelayMarkdownDocuments } from './fs-markdown-document-listing' + +class ListingProcess extends EventEmitter { + stdout = new PassThrough() + stderr = new PassThrough() + pid: number | undefined = undefined + exitCode: number | null = null + signalCode = null + kill = vi.fn(() => true) +} + +let child: ListingProcess +beforeEach(() => { + child = new ListingProcess() + spawnMock.mockReset().mockReturnValue(child) + cwdUsableMock.mockReset().mockResolvedValue(true) +}) + +it('tags only a missing launch in a usable root for the existing listing fallback', async () => { + const result = listRelayMarkdownDocuments('/repo') + child.emit('error', Object.assign(new Error('spawn ENOENT'), { code: 'ENOENT' })) + await expect(result).rejects.toThrow(RipgrepUnavailableError) +}) + +it('keeps an unreachable root out of the missing-binary fallback', async () => { + cwdUsableMock.mockResolvedValue(false) + const result = listRelayMarkdownDocuments('/repo') + child.emit('error', Object.assign(new Error('spawn ENOENT'), { code: 'ENOENT' })) + await expect(result).rejects.toThrow('Search root is not reachable') +}) + +it('keeps unusable native launchers on the existing listing fallback', async () => { + child.pid = 123 + child.exitCode = 127 + const result = listRelayMarkdownDocuments('/repo') + child.emit('close', 127, null) + await expect(result).rejects.toThrow(RipgrepUnavailableError) +}) + +it.each(['EMFILE', 'EAGAIN'])( + 'preserves %s pressure without retrying another scan', + async (code) => { + const error = Object.assign(new Error(`spawn ${code}`), { code }) + const result = listRelayMarkdownDocuments('/repo') + child.emit('error', error) + await expect(result).rejects.toBe(error) + expect(cwdUsableMock).not.toHaveBeenCalled() + } +) + +it('preserves readable SSH documents after an unreadable subtree', async () => { + child.pid = 123 + const result = listRelayMarkdownDocuments('/repo') + child.stdout.write('./README.md\0') + child.stderr.write('Permission denied') + child.emit('close', 2, null) + await expect(result).resolves.toEqual([ + { + filePath: '/repo/README.md', + relativePath: 'README.md', + basename: 'README.md', + name: 'README' + } + ]) + expect(cwdUsableMock).not.toHaveBeenCalled() +}) + +it('still rejects a permission failure that produced no readable documents', async () => { + child.pid = 123 + const result = listRelayMarkdownDocuments('/repo') + child.stderr.write('Permission denied') + child.emit('close', 2, null) + await expect(result).rejects.toThrow('Permission denied') + expect(cwdUsableMock).not.toHaveBeenCalled() +}) + +it('does not accept an incomplete record with the historical partial listing policy', async () => { + child.pid = 123 + const result = listRelayMarkdownDocuments('/repo') + child.stdout.write('./README.md\0./truncated') + child.emit('close', 2, null) + await expect(result).rejects.toThrow('Incomplete path') + expect(cwdUsableMock).not.toHaveBeenCalled() +}) diff --git a/src/relay/fs-markdown-document-listing.test.ts b/src/relay/fs-markdown-document-listing.test.ts new file mode 100644 index 00000000000..e54cc57e9b4 --- /dev/null +++ b/src/relay/fs-markdown-document-listing.test.ts @@ -0,0 +1,113 @@ +import { chmod, mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { dirname, join } from 'node:path' +import { expect, it, vi } from 'vitest' +import { bundledRipgrepCommand } from '../main/ripgrep/bundled-ripgrep-path' +import { isMarkdownDocumentName } from '../shared/markdown-document-paths' +import { configureRelayBundledRipgrep } from './relay-bundled-ripgrep' +import { listFilesWithRg } from './fs-handler-list-files' +import { listRelayMarkdownDocuments } from './fs-markdown-document-listing' +import { RelayContext } from './context' +import { FsHandler } from './fs-handler' +import type { RelayDispatcher } from './dispatcher' + +it.skipIf(process.platform === 'win32')( + 'keeps readable SSH Markdown documents when a child directory is unreadable', + async () => { + const root = await mkdtemp(join(tmpdir(), 'orca-relay-markdown-permissions-')) + const locked = join(root, 'locked') + configureRelayBundledRipgrep(bundledRipgrepCommand()) + try { + await mkdir(locked) + await writeFile(join(root, 'README.md'), '') + await writeFile(join(locked, 'private.md'), '') + await chmod(locked, 0) + expect(await listFilesWithRg(root)).toEqual(['README.md']) + await expect(listRelayMarkdownDocuments(root)).resolves.toEqual([ + { + filePath: join(root, 'README.md'), + relativePath: 'README.md', + basename: 'README.md', + name: 'README' + } + ]) + } finally { + await chmod(locked, 0o700) + configureRelayBundledRipgrep(undefined) + await rm(root, { recursive: true, force: true }) + } + } +) + +it('keeps Markdown discovery useful on folder hosts without uploaded or PATH ripgrep', async () => { + const root = await mkdtemp(join(tmpdir(), 'orca-relay-markdown-no-rg-')) + configureRelayBundledRipgrep(undefined) + vi.stubEnv('PATH', root) + vi.stubEnv('Path', root) + vi.stubEnv('CARGO_HOME', root) + const handlers = new Map) => Promise>() + const dispatcher = { + onRequest: (method: string, callback: (params: Record) => Promise) => + handlers.set(method, callback), + onNotification: vi.fn(), + onClientDetached: vi.fn(() => () => {}) + } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Filesystem registration uses only the three dispatcher hooks supplied by this fixture. + const handler = new FsHandler(dispatcher as unknown as RelayDispatcher, new RelayContext()) + try { + await mkdir(join(root, '.claude')) + await writeFile(join(root, '.claude', 'instructions.md'), '') + await writeFile(join(root, 'README.md'), '') + const listMarkdown = handlers.get('fs.listMarkdownDocuments') + if (!listMarkdown) { + throw new Error('Markdown discovery handler is missing') + } + await expect(listMarkdown({ rootPath: root })).resolves.toEqual( + expect.arrayContaining([ + expect.objectContaining({ relativePath: '.claude/instructions.md' }), + expect.objectContaining({ relativePath: 'README.md' }) + ]) + ) + } finally { + handler.dispose() + vi.unstubAllEnvs() + configureRelayBundledRipgrep(undefined) + await rm(root, { recursive: true, force: true }) + } +}) + +it('preserves SSH Markdown visibility when discovery moves off the full file inventory', async () => { + const root = await mkdtemp(join(tmpdir(), 'orca-relay-markdown-')) + configureRelayBundledRipgrep(bundledRipgrepCommand()) + try { + for (const path of [ + 'README.md', + '.config/settings.md', + '.claude/instructions.MDX', + '.github/template.md', + '.cache/hidden.md', + 'node_modules/dependency.md', + 'ignored.md', + 'excluded.md', + 'source.ts' + ]) { + await mkdir(dirname(join(root, path)), { recursive: true }) + await writeFile(join(root, path), '') + } + await writeFile(join(root, '.gitignore'), 'ignored.md\n') + await writeFile(join(root, '.ignore'), 'excluded.md\n') + const baseline = (await listFilesWithRg(root)).filter(isMarkdownDocumentName).sort() + expect(baseline).toEqual([ + '.claude/instructions.MDX', + '.config/settings.md', + '.github/template.md', + 'README.md', + 'ignored.md' + ]) + const documents = await listRelayMarkdownDocuments(root) + expect(documents.map((document) => document.relativePath).sort()).toEqual(baseline) + } finally { + configureRelayBundledRipgrep(undefined) + await rm(root, { recursive: true, force: true }) + } +}) diff --git a/src/relay/fs-markdown-document-listing.ts b/src/relay/fs-markdown-document-listing.ts new file mode 100644 index 00000000000..487e672789c --- /dev/null +++ b/src/relay/fs-markdown-document-listing.ts @@ -0,0 +1,58 @@ +import { spawnProcess } from '../shared/child-process/run-process' +import { + collectMarkdownDocuments, + MARKDOWN_DOCUMENT_GLOB +} from '../shared/node-markdown-document-listing' +import { buildRgArgsForQuickOpen } from '../shared/quick-open-filter' +import { + isRipgrepSpawnCwdUsable, + isRipgrepUnavailableExit, + isTransientRipgrepSpawnError, + ripgrepMissingCwdError, + RipgrepUnavailableError +} from '../shared/ripgrep-process-availability' +import { resolveRelayRipgrepCommand } from './relay-bundled-ripgrep' +import { expandTilde } from './context' + +export async function listRelayMarkdownDocuments(rootPath: string, signal?: AbortSignal) { + signal?.throwIfAborted() + const command = resolveRelayRipgrepCommand() + if (!command) { + throw new RipgrepUnavailableError() + } + const expandedRoot = expandTilde(rootPath) + const child = spawnProcess({ + program: command, + args: [ + '--type-add', + `orcamarkdown:${MARKDOWN_DOCUMENT_GLOB}`, + '--type', + 'orcamarkdown', + ...buildRgArgsForQuickOpen({ + searchRoot: '.', + excludePathPrefixes: [], + forceSlashSeparator: true + }).ignoredPass + ], + cwd: expandedRoot, + stdio: ['ignore', 'pipe', 'pipe'] + }) + try { + return await collectMarkdownDocuments(child, expandedRoot, false, signal, { + allowPartialListing: true + }) + } catch (error) { + signal?.throwIfAborted() + if ( + !isTransientRipgrepSpawnError(error) && + isRipgrepUnavailableExit(child, child.exitCode, child.signalCode, { + classifyNativeLauncherExit: true + }) + ) { + throw (await isRipgrepSpawnCwdUsable(expandedRoot)) + ? new RipgrepUnavailableError() + : ripgrepMissingCwdError(expandedRoot) + } + throw error + } +} diff --git a/src/relay/fs-search-line-fragments.test.ts b/src/relay/fs-search-line-fragments.test.ts index 74d17c428fe..fc584efb346 100644 --- a/src/relay/fs-search-line-fragments.test.ts +++ b/src/relay/fs-search-line-fragments.test.ts @@ -1,5 +1,5 @@ import { EventEmitter } from 'node:events' -import type { ChildProcess } from 'node:child_process' +import { PassThrough } from 'node:stream' import { afterEach, describe, expect, it, vi } from 'vitest' const { spawnMock } = vi.hoisted(() => ({ spawnMock: vi.fn() })) @@ -8,12 +8,12 @@ vi.mock('node:child_process', () => ({ spawn: spawnMock })) import { searchWithGitGrep } from './fs-handler-git-fallback' import { searchWithRg } from './fs-handler-utils' -function createProcess(): ChildProcess { +function createProcess() { return Object.assign(new EventEmitter(), { - stdout: Object.assign(new EventEmitter(), { setEncoding: vi.fn() }), + stdout: new PassThrough(), stderr: new EventEmitter(), kill: vi.fn() - }) as unknown as ChildProcess + }) } const searchCases = [ @@ -45,13 +45,12 @@ afterEach(() => { }) describe.each(searchCases)('relay $name line fragments', ({ search, encode }) => { - async function run(chunks: string[]) { + async function run(chunks: Buffer[]) { const child = createProcess() spawnMock.mockReturnValueOnce(child) const result = search('/remote/root', 'hit', { maxResults: 100 }) - expect(child.stdout!.setEncoding).toHaveBeenCalledWith('utf-8') for (const chunk of chunks) { - child.stdout!.emit('data', chunk) + child.stdout!.write(chunk) } child.emit('close', 0, null) const value = await result @@ -65,9 +64,12 @@ describe.each(searchCases)('relay $name line fragments', ({ search, encode }) => it('preserves decoded Unicode, batched lines, empty lines and the final unterminated match', async () => { const text = 'hit café 漢字 🐋' - const wire = `${encode(text, 1)}\n\n${encode('hit second', 2)}\n${encode(text, 3)}` + const wire = Buffer.from( + `${encode(text, 1)}\n\n${encode('hit second', 2)}\n${encode(text, 3)}`, + 'utf8' + ) const complete = await run([wire]) - const fragmented = await run(Array.from(wire)) + const fragmented = await run(Array.from(wire, (byte) => Buffer.from([byte]))) expect(fragmented).toEqual(complete) expect(fragmented.totalMatches).toBe(3) expect(fragmented.truncated).toBe(false) @@ -77,11 +79,11 @@ describe.each(searchCases)('relay $name line fragments', ({ search, encode }) => }) it('does not repeatedly split the growing partial output of a large matching line', async () => { - const wire = `${encode(`hit ${'x'.repeat(1024 * 1024)}`, 7)}\n` + const wire = Buffer.from(`${encode(`hit ${'x'.repeat(1024 * 1024)}`, 7)}\n`, 'utf8') const complete = await run([wire]) - const chunks: string[] = [] + const chunks: Buffer[] = [] for (let offset = 0; offset < wire.length; offset += 4096) { - chunks.push(wire.slice(offset, offset + 4096)) + chunks.push(wire.subarray(offset, offset + 4096)) } // Method-shaped type: a call-signature capture would reject `split`'s splitter-object overload. const originalSplit: { split(separator: unknown, limit?: number): string[] }['split'] = diff --git a/src/relay/protocol.ts b/src/relay/protocol.ts index 4a4f135f4ef..e0633c2d128 100644 --- a/src/relay/protocol.ts +++ b/src/relay/protocol.ts @@ -170,7 +170,10 @@ export const RelayErrorCode = { StreamProtocolError: -33007, /** Substituted for a response too large for the sink's frame capacity; the request fails * instead of the whole link, so a caller can retry with a narrower scope. */ - ResponseOverCapacity: -33008 + ResponseOverCapacity: -33008, + GitGrepRecordCapacity: -33009, + MarkdownListingCapacity: -33010, + DirectoryListingCapacity: -33011 } as const export type JsonRpcRequest = { diff --git a/src/renderer/src/components/editor/markdown-document-worktree-path-selector.test.ts b/src/renderer/src/components/editor/markdown-document-worktree-path-selector.test.ts index 20a387648db..0230113c026 100644 --- a/src/renderer/src/components/editor/markdown-document-worktree-path-selector.test.ts +++ b/src/renderer/src/components/editor/markdown-document-worktree-path-selector.test.ts @@ -104,3 +104,12 @@ describe('Markdown document worktree path selector', () => { expect(selectMarkdownDocumentWorktreePath({ worktreesByRepo: {} }, null)).toBeNull() }) }) + +it('resolves Markdown document listing roots for non-git folder workspaces', () => { + expect( + selectMarkdownDocumentWorktreePath( + { worktreesByRepo: {}, folderWorkspaces: [{ id: 'folder-id', folderPath: '/notes' }] }, + 'folder:folder-id' + ) + ).toBe('/notes') +}) diff --git a/src/renderer/src/components/editor/markdown-document-worktree-path-selector.ts b/src/renderer/src/components/editor/markdown-document-worktree-path-selector.ts index 2c342acff1f..7c8c740ce99 100644 --- a/src/renderer/src/components/editor/markdown-document-worktree-path-selector.ts +++ b/src/renderer/src/components/editor/markdown-document-worktree-path-selector.ts @@ -1,12 +1,23 @@ +import { parseWorkspaceKey } from '../../../../shared/workspace-scope' +import type { FolderWorkspace } from '../../../../shared/folder-workspace-types' import { getWorktreeMapFromState } from '@/store/selectors' import type { AppState } from '@/store/types' export function selectMarkdownDocumentWorktreePath( - state: Pick, + state: Pick & { + folderWorkspaces?: readonly Pick[] + }, worktreeId: string | null | undefined ): string | null { if (!worktreeId) { return null } + const scope = parseWorkspaceKey(worktreeId) + if (scope?.type === 'folder') { + return ( + state.folderWorkspaces?.find((workspace) => workspace.id === scope.folderWorkspaceId) + ?.folderPath ?? null + ) + } return getWorktreeMapFromState(state).get(worktreeId)?.path ?? null } diff --git a/src/renderer/src/components/editor/monaco-markdown-doc-completions-retention.test.ts b/src/renderer/src/components/editor/monaco-markdown-doc-completions-retention.test.ts new file mode 100644 index 00000000000..caa69321d9e --- /dev/null +++ b/src/renderer/src/components/editor/monaco-markdown-doc-completions-retention.test.ts @@ -0,0 +1,89 @@ +import { describe, expect, it, vi } from 'vitest' +import type { editor, languages, Position } from 'monaco-editor' +import type { OnMount } from '@monaco-editor/react' +import { + ensureMarkdownDocCompletionProvider, + setMarkdownDocCompletionDocuments, + clearMarkdownDocCompletionDocuments +} from './monaco-markdown-doc-completions' + +type MonacoApi = Parameters[1] + +function model(key: string): editor.ITextModel { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Completion uses only URI and line content from this model fixture. + return { + uri: { toString: () => key }, + getLineContent: () => '[[Ta' + } as unknown as editor.ITextModel +} + +function register() { + let current: languages.CompletionItemProvider | undefined + const api = { + languages: { + CompletionItemKind: { File: 1 }, + registerCompletionItemProvider: vi.fn( + (_language: string, provider: languages.CompletionItemProvider) => { + current = provider + return { dispose() {} } + } + ) + } + } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: The provider registers and reads only this languages API and completion kind. + ensureMarkdownDocCompletionProvider(api as unknown as MonacoApi) + return async (textModel: editor.ITextModel) => { + if (!current) { + throw new Error('Missing provider') + } + const position: Position = { + lineNumber: 1, + column: 5, + with: () => position, + delta: () => position, + equals: () => false, + isBefore: () => false, + isBeforeOrEqual: () => false, + clone: () => position, + toString: () => '(1,5)', + toJSON: () => ({ lineNumber: 1, column: 5 }) + } + const result = await current.provideCompletionItems( + textModel, + position, + { triggerKind: 0 }, + { isCancellationRequested: false, onCancellationRequested: () => ({ dispose() {} }) } + ) + return result?.suggestions ?? [] + } +} + +function documents(name: string) { + return [ + { filePath: `/repo/${name}.md`, relativePath: `${name}.md`, basename: `${name}.md`, name } + ] +} + +describe('live Markdown completion ownership', () => { + it('preserves every mounted model when many other scopes are supplied', async () => { + const complete = register() + const active = model('active') + setMarkdownDocCompletionDocuments(active, documents('Target')) + for (let scope = 0; scope < 300; scope++) { + setMarkdownDocCompletionDocuments(model(`scope-${scope}`), documents(`Target-${scope}`)) + } + expect((await complete(active)).map((item) => item.label)).toEqual(['Target']) + clearMarkdownDocCompletionDocuments(active) + expect(await complete(active)).toEqual([]) + }) + + it('keeps different model incarnations separate even when their URI is identical', async () => { + const complete = register() + const old = model('same-uri') + const current = model('same-uri') + setMarkdownDocCompletionDocuments(old, documents('Target old')) + setMarkdownDocCompletionDocuments(current, documents('Target current')) + clearMarkdownDocCompletionDocuments(old) + expect((await complete(current)).map((item) => item.label)).toEqual(['Target current']) + }) +}) diff --git a/src/renderer/src/components/editor/monaco-markdown-doc-completions.ts b/src/renderer/src/components/editor/monaco-markdown-doc-completions.ts index 3f6861c7897..30f903d17fc 100644 --- a/src/renderer/src/components/editor/monaco-markdown-doc-completions.ts +++ b/src/renderer/src/components/editor/monaco-markdown-doc-completions.ts @@ -1,5 +1,5 @@ import type { OnMount } from '@monaco-editor/react' -import type { IDisposable } from 'monaco-editor' +import type { IDisposable, editor } from 'monaco-editor' import type { MarkdownDocument } from '../../../../shared/filesystem-entry-types' import { getMarkdownDocCompletionContext, @@ -10,7 +10,7 @@ type MonacoApi = Parameters[1] let provider: IDisposable | null = null let providerMonaco: MonacoApi = null -const documentsByModel = new Map() +let documentsByModel = new WeakMap() export function ensureMarkdownDocCompletionProvider(monaco: MonacoApi): void { // Why: if Monaco was torn down and re-created (e.g. window reload), the old @@ -21,7 +21,7 @@ export function ensureMarkdownDocCompletionProvider(monaco: MonacoApi): void { } if (provider) { provider.dispose() - documentsByModel.clear() + documentsByModel = new WeakMap() } providerMonaco = monaco @@ -34,7 +34,7 @@ export function ensureMarkdownDocCompletionProvider(monaco: MonacoApi): void { return { suggestions: [] } } - const documents = documentsByModel.get(model.uri.toString()) ?? [] + const documents = documentsByModel.get(model) ?? [] const suffix = line.slice(position.column - 1) const range = { startLineNumber: position.lineNumber, @@ -59,12 +59,12 @@ export function ensureMarkdownDocCompletionProvider(monaco: MonacoApi): void { } export function setMarkdownDocCompletionDocuments( - modelKey: string, + model: editor.ITextModel, documents: MarkdownDocument[] ): void { - documentsByModel.set(modelKey, documents) + documentsByModel.set(model, documents) } -export function clearMarkdownDocCompletionDocuments(modelKey: string): void { - documentsByModel.delete(modelKey) +export function clearMarkdownDocCompletionDocuments(model: editor.ITextModel): void { + documentsByModel.delete(model) } diff --git a/src/renderer/src/components/editor/use-monaco-editor-decorations.ts b/src/renderer/src/components/editor/use-monaco-editor-decorations.ts index 9b2842e1704..2692929819d 100644 --- a/src/renderer/src/components/editor/use-monaco-editor-decorations.ts +++ b/src/renderer/src/components/editor/use-monaco-editor-decorations.ts @@ -32,23 +32,23 @@ export function useMonacoEditorDecorations(params: { conflictDecorationsEnabled } = params - const modelKeyRef = useRef(null) + const completionModelRef = useRef(null) const markdownDocLinkDecorationsRef = useRef(null) const conflictDecorationsRef = useRef(null) const updateMarkdownCompletionDocuments = useCallback((): void => { - const modelKey = editorRef.current?.getModel()?.uri.toString() ?? null - if (modelKeyRef.current && modelKeyRef.current !== modelKey) { - clearMarkdownDocCompletionDocuments(modelKeyRef.current) + const model = editorRef.current?.getModel() ?? null + if (completionModelRef.current && completionModelRef.current !== model) { + clearMarkdownDocCompletionDocuments(completionModelRef.current) } - modelKeyRef.current = modelKey - if (!modelKey) { + completionModelRef.current = model + if (!model) { return } if (language === 'markdown' && markdownDocuments) { - setMarkdownDocCompletionDocuments(modelKey, markdownDocuments) + setMarkdownDocCompletionDocuments(model, markdownDocuments) } else { - clearMarkdownDocCompletionDocuments(modelKey) + clearMarkdownDocCompletionDocuments(model) } }, [editorRef, language, markdownDocuments]) @@ -91,8 +91,8 @@ export function useMonacoEditorDecorations(params: { useEffect(() => { return () => { - if (modelKeyRef.current) { - clearMarkdownDocCompletionDocuments(modelKeyRef.current) + if (completionModelRef.current) { + clearMarkdownDocCompletionDocuments(completionModelRef.current) } markdownDocLinkDecorationsRef.current?.dispose() markdownDocLinkDecorationsRef.current = null diff --git a/src/renderer/src/components/editor/useMarkdownDocuments.navigation.test.ts b/src/renderer/src/components/editor/useMarkdownDocuments.navigation.test.ts index e15074341fa..25956dc5476 100644 --- a/src/renderer/src/components/editor/useMarkdownDocuments.navigation.test.ts +++ b/src/renderer/src/components/editor/useMarkdownDocuments.navigation.test.ts @@ -7,7 +7,9 @@ import { useMarkdownDocuments } from './useMarkdownDocuments' const runtime = vi.hoisted(() => ({ stat: vi.fn(), - list: vi.fn() + list: vi.fn(), + toastError: vi.fn(), + translate: vi.fn() })) let runtimeConnectionId: string | null = null const target = { @@ -36,6 +38,8 @@ vi.mock('@/runtime/runtime-rpc-client', () => ({ vi.mock('./markdown-document-list-request', () => ({ requestSharedMarkdownDocumentList: runtime.list })) +vi.mock('sonner', () => ({ toast: { error: runtime.toastError } })) +vi.mock('@/i18n/i18n', () => ({ translate: runtime.translate })) let root: Root let container: HTMLDivElement @@ -72,6 +76,7 @@ beforeEach(() => { runtimeConnectionId = null runtime.stat.mockResolvedValue({ isDirectory: false }) runtime.list.mockResolvedValue([target]) + runtime.translate.mockReturnValue('Localized listing failure') container = document.createElement('div') document.body.appendChild(container) root = createRoot(container) @@ -84,6 +89,55 @@ afterEach(() => { }) describe('Markdown document navigation', () => { + it('localizes a non-Error failure at settlement without restarting the request', async () => { + let rejectListing: (reason: unknown) => void = () => {} + runtime.list.mockReturnValueOnce( + new Promise((_resolve, reject) => { + rejectListing = reject + }) + ) + await render(sourceFile('edit'), 'source') + runtime.translate.mockReturnValue('Current language listing failure') + await render(sourceFile('edit'), 'source') + await act(async () => rejectListing(null)) + + expect(runtime.list).toHaveBeenCalledOnce() + expect(runtime.translate).toHaveBeenCalledWith( + 'auto.components.editor.useMarkdownDocuments.listFailed', + 'Failed to list Markdown documents.' + ) + expect(runtime.toastError).toHaveBeenCalledWith('Current language listing failure') + expect(controller.markdownDocuments).toEqual([]) + }) + + it('preserves actual listing error detail', async () => { + runtime.list.mockRejectedValueOnce(new Error('SSH listing timed out')) + await render(sourceFile('edit', 'runtime-owner'), 'source') + + expect(runtime.toastError).toHaveBeenCalledWith('SSH listing timed out') + expect(runtime.translate).not.toHaveBeenCalled() + }) + + it.each(['superseded', 'unmounted'])('ignores a %s listing failure', async (reason) => { + let rejectListing: (error: unknown) => void = () => {} + runtime.list.mockReturnValueOnce( + new Promise((_resolve, reject) => { + rejectListing = reject + }) + ) + await render(sourceFile('edit'), 'source') + await (reason === 'superseded' + ? render(sourceFile('edit', 'next-owner'), 'source') + : act(async () => root.render(null))) + await act(async () => rejectListing(null)) + + expect(runtime.toastError).not.toHaveBeenCalled() + expect(runtime.translate).not.toHaveBeenCalled() + if (reason === 'superseded') { + expect(controller.markdownDocuments).toEqual([target]) + } + }) + it.each([ ['markdown-preview', 'source'], ['edit', 'preview'], diff --git a/src/renderer/src/components/editor/useMarkdownDocuments.ts b/src/renderer/src/components/editor/useMarkdownDocuments.ts index d44c22447b9..6a7a087a9d2 100644 --- a/src/renderer/src/components/editor/useMarkdownDocuments.ts +++ b/src/renderer/src/components/editor/useMarkdownDocuments.ts @@ -1,6 +1,8 @@ +import { toast } from 'sonner' import { useCallback, useEffect, useMemo, useRef, useState } from 'react' import type { MarkdownDocument } from '../../../../shared/filesystem-entry-types' import { useAppStore } from '@/store' +import { translate } from '@/i18n/i18n' import { getConnectionId } from '@/lib/connection-context' import { statRuntimePath } from '@/runtime/runtime-file-client' import { settingsForRuntimeOwner } from '@/runtime/runtime-rpc-client' @@ -59,12 +61,17 @@ export function useMarkdownDocuments( const worktreePath = useAppStore((s) => selectMarkdownDocumentWorktreePath(s, worktreeId)) const openFile = useAppStore((s) => s.openFile) const openMarkdownPreview = useAppStore((s) => s.openMarkdownPreview) - const [markdownDocumentsByWorktree, setMarkdownDocumentsByWorktree] = useState< - Record - >({}) - const requestRef = useRef(0) - const connectionId = getConnectionId(worktreeId) + const scopeKey = JSON.stringify([ + activeFile.runtimeEnvironmentId, + connectionId, + worktreeId, + worktreePath + ]) + const [snapshot, setSnapshot] = useState<{ key: string; documents: MarkdownDocument[] } | null>( + null + ) + const requestRef = useRef(0) const refreshMarkdownDocuments = useCallback( async (requireFresh = false): Promise => { @@ -91,21 +98,25 @@ export function useMarkdownDocuments( if (requestRef.current !== requestId) { return } - setMarkdownDocumentsByWorktree((prev) => ({ - ...prev, - [worktreeId]: documents - })) + setSnapshot({ key: scopeKey, documents }) } catch (err) { console.error('Failed to list markdown documents:', err) if (requestRef.current === requestId) { - setMarkdownDocumentsByWorktree((prev) => ({ - ...prev, - [worktreeId]: [] - })) + toast.error( + err instanceof Error + ? err.message + : translate( + 'auto.components.editor.useMarkdownDocuments.listFailed', + 'Failed to list Markdown documents.' + ) + ) + } + if (requestRef.current === requestId) { + setSnapshot({ key: scopeKey, documents: [] }) } } }, - [activeFile.runtimeEnvironmentId, connectionId, worktreeId, worktreePath] + [activeFile.runtimeEnvironmentId, connectionId, worktreeId, worktreePath, scopeKey] ) const openMarkdownDocument = useCallback( @@ -180,11 +191,14 @@ export function useMarkdownDocuments( return } void refreshMarkdownDocuments() + return () => { + requestRef.current += 1 + } }, [activeFile.id, isMarkdown, viewMode, refreshMarkdownDocuments]) const markdownDocuments = useMemo( - () => (worktreeId ? (markdownDocumentsByWorktree[worktreeId] ?? []) : []), - [worktreeId, markdownDocumentsByWorktree] + () => (snapshot?.key === scopeKey ? snapshot.documents : []), + [scopeKey, snapshot] ) const previewProps = useMemo( diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 6fc6a042603..8770343ed37 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -15876,6 +15876,9 @@ "richMarkdownImageInsertionFeedback": { "importedFileKept": "Image insertion canceled because the destination changed. The imported file was kept.", "destinationChanged": "Image insertion canceled because the destination changed. Try again." + }, + "useMarkdownDocuments": { + "listFailed": "Failed to list Markdown documents." } }, "diff": { diff --git a/src/renderer/src/runtime/runtime-legacy-inventory-budget.test.ts b/src/renderer/src/runtime/runtime-legacy-inventory-budget.test.ts new file mode 100644 index 00000000000..d274e668fc0 --- /dev/null +++ b/src/renderer/src/runtime/runtime-legacy-inventory-budget.test.ts @@ -0,0 +1,64 @@ +import { describe, expect, it } from 'vitest' +import { + decodeLegacyQuickOpenInventory, + pruneLegacyInventoryCache +} from './runtime-legacy-inventory-budget' + +function inventory(count = 1) { + return { + worktree: 'folder:one', + rootPath: '/repo', + totalCount: count, + truncated: false, + files: Array.from({ length: count }, (_, index) => ({ + relativePath: `src/file-${index}.ts`, + basename: `file-${index}.ts`, + kind: 'text' + })) + } +} + +describe('complete legacy inventory retention', () => { + it('retains a full valid inventory beyond 20,000 and discards unknown payload fields', () => { + const value = inventory(25_002) + Object.assign(value.files[0], { extra: { nested: 'x'.repeat(1000) } }) + const { result, retainedBytes } = decodeLegacyQuickOpenInventory(value) + expect(result.files).toHaveLength(25_002) + expect(result.files[25_001].relativePath).toBe('src/file-25001.ts') + expect(result.files[0]).not.toHaveProperty('extra') + expect(retainedBytes).toBeGreaterThan(25_002 * 128) + }) + it('fails oversized complete responses instead of retaining a prefix', () => { + expect(() => decodeLegacyQuickOpenInventory(inventory(10), 1024)).toThrow('too large') + expect(() => + decodeLegacyQuickOpenInventory({ ...inventory(0), rootPath: 'x'.repeat(1000) }, 1024) + ).toThrow('too large') + const value = inventory() + value.files[0].relativePath = 'x'.repeat(65_537) + expect(() => decodeLegacyQuickOpenInventory(value)).toThrow('too large') + }) + it.each([ + null, + { files: [] }, + { ...inventory(), totalCount: 0 }, + { ...inventory(), files: [{ relativePath: 12 }] } + ])('rejects malformed responses %j', (value) => { + expect(() => decodeLegacyQuickOpenInventory(value)).toThrow('Invalid remote file inventory') + }) + it('evicts in access order under a shared byte allowance and releases removed accounting', () => { + const cache = new Map([ + ['one', { retainedBytes: 60 }], + ['two', { retainedBytes: 60 }] + ]) + pruneLegacyInventoryCache(cache, 8, 100) + expect([...cache.keys()]).toEqual(['two']) + cache.delete('two') + cache.set('three', { retainedBytes: 100 }) + pruneLegacyInventoryCache(cache, 8, 100) + expect([...cache.keys()]).toEqual(['three']) + cache.set('three', { retainedBytes: 10 }) + cache.set('owner-changed', { retainedBytes: 90 }) + pruneLegacyInventoryCache(cache, 8, 100) + expect(cache.size).toBe(2) + }) +}) diff --git a/src/renderer/src/runtime/runtime-legacy-inventory-budget.ts b/src/renderer/src/runtime/runtime-legacy-inventory-budget.ts new file mode 100644 index 00000000000..9bf8b086343 --- /dev/null +++ b/src/renderer/src/runtime/runtime-legacy-inventory-budget.ts @@ -0,0 +1,88 @@ +import type { RuntimeFileListEntry, RuntimeFileListResult } from '../../../shared/runtime-types' +import { + FILE_INVENTORY_MAX_BYTES, + FILE_INVENTORY_MAX_PATH_BYTES, + FileInventoryCapacityError +} from '../../../shared/file-inventory-budget' +import { isUtf8ByteLengthWithinLimit } from '../../../shared/utf8-byte-limits' + +export function decodeLegacyQuickOpenInventory( + value: unknown, + maxBytes = FILE_INVENTORY_MAX_BYTES +): { result: RuntimeFileListResult; retainedBytes: number } { + if ( + typeof value !== 'object' || + value === null || + !('files' in value) || + !Array.isArray(value.files) || + !('worktree' in value) || + typeof value.worktree !== 'string' || + !('rootPath' in value) || + typeof value.rootPath !== 'string' || + !('totalCount' in value) || + typeof value.totalCount !== 'number' || + !Number.isSafeInteger(value.totalCount) || + value.totalCount < value.files.length || + !('truncated' in value) || + typeof value.truncated !== 'boolean' + ) { + throw new Error('Invalid remote file inventory') + } + let retainedBytes = 128 + (value.worktree.length + value.rootPath.length) * 2 + const files: RuntimeFileListEntry[] = [] + for (const file of value.files) { + if ( + typeof file !== 'object' || + file === null || + !('relativePath' in file) || + typeof file.relativePath !== 'string' || + !('basename' in file) || + typeof file.basename !== 'string' || + !('kind' in file) || + (file.kind !== 'text' && file.kind !== 'binary') + ) { + throw new Error('Invalid remote file inventory entry') + } + retainedBytes += 128 + (file.relativePath.length + file.basename.length) * 2 + if ( + retainedBytes > maxBytes || + !isUtf8ByteLengthWithinLimit(file.relativePath, FILE_INVENTORY_MAX_PATH_BYTES) || + !isUtf8ByteLengthWithinLimit(file.basename, FILE_INVENTORY_MAX_PATH_BYTES) + ) { + throw new FileInventoryCapacityError() + } + files.push({ relativePath: file.relativePath, basename: file.basename, kind: file.kind }) + } + if (retainedBytes > maxBytes) { + throw new FileInventoryCapacityError() + } + return { + result: { + worktree: value.worktree, + rootPath: value.rootPath, + files, + totalCount: value.totalCount, + truncated: value.truncated, + ...('quickOpenSearchVersion' in value && typeof value.quickOpenSearchVersion === 'number' + ? { quickOpenSearchVersion: value.quickOpenSearchVersion } + : {}) + }, + retainedBytes + } +} + +export function pruneLegacyInventoryCache( + cache: Map, + maxEntries: number, + maxBytes = FILE_INVENTORY_MAX_BYTES +): void { + let bytes = Array.from(cache.values()).reduce((sum, entry) => sum + entry.retainedBytes, 0) + while (cache.size > maxEntries || bytes > maxBytes) { + const oldest = cache.entries().next().value + if (!oldest) { + break + } + bytes -= oldest[1].retainedBytes + cache.delete(oldest[0]) + } +} diff --git a/src/renderer/src/runtime/runtime-legacy-quick-open-inventory-expiry.test.ts b/src/renderer/src/runtime/runtime-legacy-quick-open-inventory-expiry.test.ts index 4b1c2d9ea0d..bba392e47b4 100644 --- a/src/renderer/src/runtime/runtime-legacy-quick-open-inventory-expiry.test.ts +++ b/src/renderer/src/runtime/runtime-legacy-quick-open-inventory-expiry.test.ts @@ -1,3 +1,4 @@ +import * as inventoryBudget from './runtime-legacy-inventory-budget' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { RuntimeFileListResult } from '../../../shared/runtime-types' import { MAX_TIMER_DELAY_MS } from '../../../shared/timer-delay' @@ -87,12 +88,18 @@ describe('legacy Quick Open inventory expiry', () => { it('releases an idle 5,000-file response at its deadline without another lookup', async () => { async function populate() { const response = listing('one', 5_000) - const retired = new WeakRef(response) + const decode = vi.spyOn(inventoryBudget, 'decodeLegacyQuickOpenInventory') call.mockImplementationOnce(async () => response) expect(await search()).toEqual({ files: ['src/feature-0/component.ts', 'src/feature-1/component.ts'], truncated: true }) + const decoded = decode.mock.results[0] + if (decoded.type !== 'return') { + throw new Error('Inventory did not decode') + } + const retired = new WeakRef(decoded.value.result) + decode.mockClear() call.mockReset() return retired } @@ -106,6 +113,42 @@ describe('legacy Quick Open inventory expiry', () => { expect(vi.getTimerCount()).toBe(0) }) + it('evicts old complete inventories when their aggregate retention exceeds the byte allowance', async () => { + call.mockImplementation(async (_target, _method, params) => { + const worktree = + typeof params === 'object' && + params !== null && + 'worktree' in params && + typeof params.worktree === 'string' + ? params.worktree + : 'one' + return listing(worktree, 100_000) + }) + for (let index = 0; index < 4; index += 1) { + await search(`large-${index}`) + } + expect(cached('large-0')).toBe(false) + expect(cached('large-3')).toBe(true) + vi.advanceTimersByTime(30_000) + expect(cached('large-3')).toBe(false) + expect(vi.getTimerCount()).toBe(0) + await search('new-owner') + expect(cached('new-owner')).toBe(true) + }) + + it('does not cache an oversized response and retries rather than treating recent paths as absent', async () => { + const value = listing() + value.files[0].relativePath = 'x'.repeat(65_537) + call.mockResolvedValueOnce(value) + await expect(search()).rejects.toThrow('too large') + expect(cached()).toBe(false) + expect(vi.getTimerCount()).toBe(0) + call.mockResolvedValueOnce(listing()) + await search() + expect(call).toHaveBeenCalledTimes(2) + expect(cached()).toBe(true) + }) + it('keeps one timer and strict expiry without renewing it on cache hits', async () => { call.mockImplementation(async () => listing()) const first = await search() diff --git a/src/renderer/src/runtime/runtime-legacy-quick-open-inventory.ts b/src/renderer/src/runtime/runtime-legacy-quick-open-inventory.ts index 4a1e3ccf6c6..8ac0de8a6df 100644 --- a/src/renderer/src/runtime/runtime-legacy-quick-open-inventory.ts +++ b/src/renderer/src/runtime/runtime-legacy-quick-open-inventory.ts @@ -1,3 +1,4 @@ +import { decodeLegacyQuickOpenInventory, pruneLegacyInventoryCache } from './runtime-legacy-inventory-budget' import type { RuntimeFileListResult } from '../../../shared/runtime-types' import { buildExcludePathPrefixes, @@ -19,6 +20,7 @@ type CacheEntry = { controller: AbortController activeConsumers: number settled: boolean + retainedBytes: number } const inventoryCache = new Map() @@ -111,7 +113,7 @@ async function loadLegacyQuickOpenInventory( controller.abort() } // Share one inventory request; abort it only after every caller detaches. - const load = callRuntimeRpc( + const load = callRuntimeRpc( target, 'files.list', { worktree: worktreeSelector }, @@ -121,7 +123,10 @@ async function loadLegacyQuickOpenInventory( expectedEnvironmentPairingRevision } ) - .then((result) => { + .then((value) => { + const { result, retainedBytes } = decodeLegacyQuickOpenInventory(value) + entry.retainedBytes = retainedBytes + pruneLegacyInventoryCache(inventoryCache, CACHE_LIMIT) entry.settled = true entry.expiresAt = Date.now() + CACHE_TTL_MS scheduleInventoryExpiry() @@ -139,17 +144,12 @@ async function loadLegacyQuickOpenInventory( load, controller, activeConsumers: 0, - settled: false + settled: false, + retainedBytes: 0 } inventoryCache.set(key, entry) scheduleInventoryExpiry() - while (inventoryCache.size > CACHE_LIMIT) { - const oldest = inventoryCache.keys().next().value as string | undefined - if (!oldest) { - break - } - inventoryCache.delete(oldest) - } + pruneLegacyInventoryCache(inventoryCache, CACHE_LIMIT) return awaitLegacyInventoryLoad(entry, signal) } diff --git a/src/shared/directory-listing-budget.ts b/src/shared/directory-listing-budget.ts new file mode 100644 index 00000000000..9922211e6e1 --- /dev/null +++ b/src/shared/directory-listing-budget.ts @@ -0,0 +1,50 @@ +import type { DirEntry } from './filesystem-entry-types' + +export const DIRECTORY_LISTING_MAX_ENTRIES = 100_000 +export const DIRECTORY_LISTING_MAX_RETAINED_BYTES = 8 * 1024 * 1024 + +export class DirectoryListingCapacityError extends Error { + readonly code = 'directory_listing_capacity' + constructor() { + super('Directory listing is too large to retain safely. Narrow the folder before retrying.') + this.name = 'DirectoryListingCapacityError' + } +} + +export class DirectoryListingBudget { + private entries = 0 + private retainedBytes = 0 + + record(name: string): void { + const bytes = name.length * 2 + 128 + if ( + this.entries >= DIRECTORY_LISTING_MAX_ENTRIES || + this.retainedBytes + bytes > DIRECTORY_LISTING_MAX_RETAINED_BYTES + ) { + throw new DirectoryListingCapacityError() + } + this.entries += 1 + this.retainedBytes += bytes + } +} + +export function validateDirectoryListing(result: unknown): DirEntry[] { + if (!Array.isArray(result)) { + throw new Error('Invalid directory listing') + } + const budget = new DirectoryListingBudget() + const entries: DirEntry[] = [] + for (const entry of result) { + if ( + !entry || + typeof entry.name !== 'string' || + typeof entry.isDirectory !== 'boolean' || + typeof entry.isSymlink !== 'boolean' + ) { + throw new Error('Invalid directory listing entry') + } + budget.record(entry.name) + entries.push({ name: entry.name, isDirectory: entry.isDirectory, isSymlink: entry.isSymlink }) + } + return entries +} diff --git a/src/shared/file-inventory-budget.test.ts b/src/shared/file-inventory-budget.test.ts new file mode 100644 index 00000000000..d54493a5016 --- /dev/null +++ b/src/shared/file-inventory-budget.test.ts @@ -0,0 +1,17 @@ +import { expect, it } from 'vitest' +import { FileInventoryBudget, FileInventoryCapacityError } from './file-inventory-budget' + +it('accepts the retained-byte boundary and rejects the next entry', () => { + const budget = new FileInventoryBudget(132) + budget.record('a') + budget.record('b') + expect(() => budget.record('c')).toThrow(FileInventoryCapacityError) +}) + +it('charges serialized escaping and rejects an oversized individual path', () => { + const escaped = new FileInventoryBudget(100) + expect(() => escaped.record('\u0001'.repeat(20))).toThrow(FileInventoryCapacityError) + expect(() => new FileInventoryBudget().record('x'.repeat(65537))).toThrow( + FileInventoryCapacityError + ) +}) diff --git a/src/shared/file-inventory-budget.ts b/src/shared/file-inventory-budget.ts new file mode 100644 index 00000000000..3eb1c7786ef --- /dev/null +++ b/src/shared/file-inventory-budget.ts @@ -0,0 +1,32 @@ +import { serializedQuickOpenPathBytes } from './quick-open-transport-budget' + +export const FILE_INVENTORY_MAX_BYTES = 64 * 1024 * 1024 +export const FILE_INVENTORY_MAX_PATH_BYTES = 64 * 1024 + +export class FileInventoryCapacityError extends Error { + readonly code = 'file_inventory_capacity' + constructor() { + super('File inventory is too large to retain safely. Use a filtered search.') + this.name = 'FileInventoryCapacityError' + } +} + +export class FileInventoryBudget { + private retainedBytes = 0 + private serializedBytes = 2 + constructor(private readonly maxBytes = FILE_INVENTORY_MAX_BYTES) {} + + record(path: string): void { + const retained = path.length * 2 + 64 + const serialized = serializedQuickOpenPathBytes(path) + 1 + if ( + Buffer.byteLength(path) > FILE_INVENTORY_MAX_PATH_BYTES || + this.retainedBytes + retained > this.maxBytes || + this.serializedBytes + serialized > this.maxBytes + ) { + throw new FileInventoryCapacityError() + } + this.retainedBytes += retained + this.serializedBytes += serialized + } +} diff --git a/src/shared/git-grep-record-limit.ts b/src/shared/git-grep-record-limit.ts new file mode 100644 index 00000000000..eba2c235b8e --- /dev/null +++ b/src/shared/git-grep-record-limit.ts @@ -0,0 +1,10 @@ +export const GIT_GREP_MAX_RECORD_BYTES = 8 * 1024 * 1024 + +export class GitGrepRecordCapacityError extends Error { + readonly code = 'git_grep_record_capacity' + + constructor() { + super('Git search record exceeds the 8 MiB limit.') + this.name = 'GitGrepRecordCapacityError' + } +} diff --git a/src/shared/markdown-document-listing-limits.test.ts b/src/shared/markdown-document-listing-limits.test.ts index 01e0809f5c1..d8cbd5719da 100644 --- a/src/shared/markdown-document-listing-limits.test.ts +++ b/src/shared/markdown-document-listing-limits.test.ts @@ -5,8 +5,7 @@ import { createMarkdownDocumentListingBudget, MARKDOWN_DOCUMENT_LISTING_ERROR_CODE, MarkdownDocumentListingCapacityError, - retainMarkdownDocument, - visitMarkdownDocumentListingEntry + retainMarkdownDocument } from './markdown-document-listing-limits' function document(path: string): MarkdownDocument { @@ -32,31 +31,25 @@ describe('Markdown document listing limits', () => { ) }) - it('rejects aggregate metadata, visited-entry, path, and depth overflow', () => { + it('rejects aggregate metadata overflow', () => { expect(() => assertMarkdownDocumentsWithinLimit([document('a'.repeat(100))], { maxMetadataBytes: 100 }) ).toThrow(MarkdownDocumentListingCapacityError) + }) - const visited = createMarkdownDocumentListingBudget({ - maxVisitedEntries: 1, - maxPathBytes: 4, - maxDepth: 1 - }) - visitMarkdownDocumentListingEntry(visited, 'a', 1) - expect(() => visitMarkdownDocumentListingEntry(visited, 'b', 1)).toThrow( + it('bounds UTF-8 bytes in both live document paths before retaining metadata', () => { + const budget = createMarkdownDocumentListingBudget({ maxPathBytes: 4 }) + expect(() => retainMarkdownDocument(budget, { ...document('a'), filePath: 'ééé' })).toThrow( MarkdownDocumentListingCapacityError ) - - const path = createMarkdownDocumentListingBudget({ maxPathBytes: 4 }) - expect(() => visitMarkdownDocumentListingEntry(path, 'ééé', 1)).toThrow( - MarkdownDocumentListingCapacityError - ) - - const depth = createMarkdownDocumentListingBudget({ maxDepth: 1 }) - expect(() => visitMarkdownDocumentListingEntry(depth, 'a/b', 2)).toThrow( + expect(() => retainMarkdownDocument(budget, { ...document('ééé'), filePath: 'a' })).toThrow( MarkdownDocumentListingCapacityError ) + expect(budget.documents).toBe(0) + expect(budget.metadataBytes).toBe(0) + retainMarkdownDocument(budget, { ...document('éé'), filePath: 'éé' }) + expect(budget.documents).toBe(1) }) }) diff --git a/src/shared/markdown-document-listing-limits.ts b/src/shared/markdown-document-listing-limits.ts index 14a4cc8ff8a..44ed87005a2 100644 --- a/src/shared/markdown-document-listing-limits.ts +++ b/src/shared/markdown-document-listing-limits.ts @@ -4,8 +4,6 @@ import { measureUtf8ByteLength } from './utf8-byte-limits' export const MARKDOWN_DOCUMENT_LISTING_MAX_DOCUMENTS = 20_000 export const MARKDOWN_DOCUMENT_LISTING_MAX_METADATA_BYTES = 8 * 1024 * 1024 export const MARKDOWN_DOCUMENT_LISTING_MAX_PATH_BYTES = 64 * 1024 -export const MARKDOWN_DOCUMENT_LISTING_MAX_VISITED_ENTRIES = 100_000 -export const MARKDOWN_DOCUMENT_LISTING_MAX_DEPTH = 256 export const MARKDOWN_DOCUMENT_LISTING_ERROR_CODE = 'markdown_document_listing_capacity' const MARKDOWN_DOCUMENT_LISTING_ERROR_MESSAGE = 'Workspace is too large for Markdown link completion.' @@ -16,14 +14,11 @@ export type MarkdownDocumentListingLimits = { maxDocuments: number maxMetadataBytes: number maxPathBytes: number - maxVisitedEntries: number - maxDepth: number } export type MarkdownDocumentListingBudget = { documents: number metadataBytes: number - visitedEntries: number limits: MarkdownDocumentListingLimits } @@ -42,19 +37,13 @@ export function createMarkdownDocumentListingBudget( return { documents: 0, metadataBytes: 0, - visitedEntries: 0, limits: { maxDocuments: clampLimit(requested.maxDocuments, MARKDOWN_DOCUMENT_LISTING_MAX_DOCUMENTS), maxMetadataBytes: clampLimit( requested.maxMetadataBytes, MARKDOWN_DOCUMENT_LISTING_MAX_METADATA_BYTES ), - maxPathBytes: clampLimit(requested.maxPathBytes, MARKDOWN_DOCUMENT_LISTING_MAX_PATH_BYTES), - maxVisitedEntries: clampLimit( - requested.maxVisitedEntries, - MARKDOWN_DOCUMENT_LISTING_MAX_VISITED_ENTRIES - ), - maxDepth: clampLimit(requested.maxDepth, MARKDOWN_DOCUMENT_LISTING_MAX_DEPTH) + maxPathBytes: clampLimit(requested.maxPathBytes, MARKDOWN_DOCUMENT_LISTING_MAX_PATH_BYTES) } } } @@ -68,18 +57,6 @@ export function assertMarkdownDocumentPathWithinLimit( } } -export function visitMarkdownDocumentListingEntry( - budget: MarkdownDocumentListingBudget, - path: string, - depth: number -): void { - assertMarkdownDocumentPathWithinLimit(path, budget.limits.maxPathBytes) - if (budget.visitedEntries >= budget.limits.maxVisitedEntries || depth > budget.limits.maxDepth) { - throw new MarkdownDocumentListingCapacityError() - } - budget.visitedEntries += 1 -} - export function estimateMarkdownDocumentRetainedBytes(document: MarkdownDocument): number { return ( (document.filePath.length + @@ -117,23 +94,6 @@ export function retainMarkdownDocument( budget.metadataBytes += retainedBytes } -export function retainMarkdownRelativePath( - budget: MarkdownDocumentListingBudget, - rootPath: string, - relativePath: string -): void { - const normalizedRoot = rootPath.replace(/[\\/]+$/, '') - const normalizedRelativePath = relativePath.replaceAll('\\', '/') - const basename = normalizedRelativePath.slice(normalizedRelativePath.lastIndexOf('/') + 1) - const extensionIndex = basename.lastIndexOf('.') - retainMarkdownDocument(budget, { - filePath: `${normalizedRoot}/${normalizedRelativePath}`, - relativePath: normalizedRelativePath, - basename, - name: extensionIndex > 0 ? basename.slice(0, extensionIndex) : basename - }) -} - export function assertMarkdownDocumentsWithinLimit( documents: unknown, requested: Partial = {} diff --git a/src/shared/markdown-document-paths.ts b/src/shared/markdown-document-paths.ts new file mode 100644 index 00000000000..10c65927f26 --- /dev/null +++ b/src/shared/markdown-document-paths.ts @@ -0,0 +1,103 @@ +import { + createMarkdownDocumentListingBudget, + retainMarkdownDocument +} from './markdown-document-listing-limits' +import { isWindowsAbsolutePathLike } from './cross-platform-path' +import { normalizeRelativePath } from './text-search-paths' +import { basename as pathBasename, extname, isAbsolute, posix, relative, resolve } from 'node:path' +import type { FileDocument, MarkdownDocument } from './filesystem-entry-types' +export function isMarkdownDocumentName(name: string): boolean { + return isMarkdownExtension(extname(name)) +} + +function isMarkdownExtension(extension: string): boolean { + const normalized = extension.toLowerCase() + return normalized === '.md' || normalized === '.mdx' || normalized === '.markdown' +} + +function basenameFromRelativePath(relativePath: string): string { + return relativePath.slice(relativePath.lastIndexOf('/') + 1) +} + +function isSafeRelativePath(relativePath: string): boolean { + return !relativePath.split('/').includes('..') +} + +function rootRelativePath(rootPath: string, filePath: string): string | null { + const resolvedRoot = resolve(rootPath) + const resolvedFile = resolve(filePath) + const relativePath = relative(resolvedRoot, resolvedFile) + if ( + !isSafeRelativePath(normalizeRelativePath(relativePath, rootPath)) || + isAbsolute(relativePath) + ) { + return null + } + return normalizeRelativePath(relativePath, rootPath) +} + +export function fileDocumentFromFilePath( + rootPath: string, + filePath: string, + options: { outsideRootRelativePath?: 'basename' | 'relative' } = {} +): FileDocument { + const basename = pathBasename(filePath) + const extension = extname(basename) + const relativePath = + rootRelativePath(rootPath, filePath) ?? + (options.outsideRootRelativePath === 'basename' + ? basename + : normalizeRelativePath(relative(rootPath, filePath), rootPath)) + return { + filePath, + relativePath, + basename, + name: extension ? basename.slice(0, -extension.length) : basename + } +} + +export const markdownDocumentFromFilePath = fileDocumentFromFilePath + +export function markdownDocumentFromRelativePath( + rootPath: string, + relativePath: string +): MarkdownDocument | null { + const normalizedRelativePath = normalizeRelativePath(relativePath, rootPath) + // Why: SSH providers should return root-relative paths; reject escape + // segments before building a synthetic absolute path for renderer use. + if (!isSafeRelativePath(normalizedRelativePath)) { + return null + } + const basename = basenameFromRelativePath(normalizedRelativePath) + // Remote separators are already normalized; a POSIX backslash stays part of the name. + const extension = posix.extname(basename) + if (!isMarkdownExtension(extension)) { + return null + } + const normalizedRoot = rootPath.replace( + isWindowsAbsolutePathLike(rootPath) ? /[\\/]+$/ : /\/+$/, + '' + ) + return { + filePath: `${normalizedRoot}/${normalizedRelativePath}`, + relativePath: normalizedRelativePath, + basename, + name: extension ? basename.slice(0, -extension.length) : basename + } +} + +export function markdownDocumentsFromRelativePaths( + rootPath: string, + relativePaths: string[] +): MarkdownDocument[] { + const budget = createMarkdownDocumentListingBudget() + const documents: MarkdownDocument[] = [] + for (const path of relativePaths) { + const document = markdownDocumentFromRelativePath(rootPath, path) + if (document) { + retainMarkdownDocument(budget, document) + documents.push(document) + } + } + return documents.sort((a, b) => a.relativePath.localeCompare(b.relativePath)) +} diff --git a/src/shared/node-markdown-document-discovery.test.ts b/src/shared/node-markdown-document-discovery.test.ts deleted file mode 100644 index 9c5e1bdfd14..00000000000 --- a/src/shared/node-markdown-document-discovery.test.ts +++ /dev/null @@ -1,77 +0,0 @@ -import type { Dirent } from 'node:fs' -import { join } from 'node:path' -import { describe, expect, it } from 'vitest' -import { MarkdownDocumentListingCapacityError } from './markdown-document-listing-limits' -import { discoverMarkdownRelativePaths } from './node-markdown-document-discovery' - -function entry(name: string, kind: 'directory' | 'file' | 'symlink' = 'file'): Dirent { - return { - name, - isDirectory: () => kind === 'directory', - isFile: () => kind === 'file', - isSymbolicLink: () => kind === 'symlink' - } as Dirent -} - -function reader(entriesByPath: Record) { - return async (path: string): Promise> => ({ - async *[Symbol.asyncIterator]() { - yield* entriesByPath[path] ?? [] - } - }) -} - -describe('bounded Markdown document discovery', () => { - it('preserves depth-first discovery and skips excluded and symlinked directories', async () => { - // Why join() for the child key: the subject descends with path.join, so a '/repo/docs' - // literal never matches on Windows and the walk silently stops at the root. - const result = await discoverMarkdownRelativePaths('/repo', { - readDirectory: reader({ - '/repo': [ - entry('README.md'), - entry('.git', 'directory'), - entry('docs', 'directory'), - entry('linked', 'symlink') - ], - [join('/repo', 'docs')]: [entry('guide.mdx'), entry('app.ts')] - }), - shouldDescend: (_relativePath, name) => name !== '.git' - }) - - expect(result).toEqual(['README.md', 'docs/guide.mdx']) - }) - - it('stops consuming a wide directory at the visited-entry limit', async () => { - let yielded = 0 - const readDirectory = async (): Promise> => ({ - async *[Symbol.asyncIterator]() { - for (let index = 0; index < 10_000; index += 1) { - yielded += 1 - yield entry(`source-${index}.ts`) - } - } - }) - - await expect( - discoverMarkdownRelativePaths('/repo', { - limits: { maxVisitedEntries: 2 }, - readDirectory, - shouldDescend: () => true - }) - ).rejects.toBeInstanceOf(MarkdownDocumentListingCapacityError) - expect(yielded).toBe(3) - }) - - it('rejects a directory deeper than the configured traversal limit', async () => { - await expect( - discoverMarkdownRelativePaths('/repo', { - limits: { maxDepth: 1 }, - readDirectory: reader({ - '/repo': [entry('one', 'directory')], - [join('/repo', 'one')]: [entry('two', 'directory')] - }), - shouldDescend: () => true - }) - ).rejects.toBeInstanceOf(MarkdownDocumentListingCapacityError) - }) -}) diff --git a/src/shared/node-markdown-document-discovery.ts b/src/shared/node-markdown-document-discovery.ts deleted file mode 100644 index 06631c6fc3b..00000000000 --- a/src/shared/node-markdown-document-discovery.ts +++ /dev/null @@ -1,86 +0,0 @@ -import { opendir } from 'node:fs/promises' -import type { Dirent, Dir } from 'node:fs' -import { join } from 'node:path' -import { - assertMarkdownDocumentPathWithinLimit, - createMarkdownDocumentListingBudget, - MarkdownDocumentListingCapacityError, - retainMarkdownRelativePath, - visitMarkdownDocumentListingEntry, - type MarkdownDocumentListingLimits -} from './markdown-document-listing-limits' - -type MarkdownDirectoryReader = (path: string) => Promise> - -export type MarkdownDocumentDiscoveryOptions = { - shouldDescend: (relativePath: string, name: string) => boolean - ignoreNestedDirectoryErrors?: boolean - limits?: Partial - readDirectory?: MarkdownDirectoryReader - signal?: AbortSignal -} - -export function isMarkdownDocumentPath(path: string): boolean { - const lowerPath = path.toLowerCase() - return lowerPath.endsWith('.md') || lowerPath.endsWith('.mdx') || lowerPath.endsWith('.markdown') -} - -export async function discoverMarkdownRelativePaths( - rootPath: string, - options: MarkdownDocumentDiscoveryOptions -): Promise { - const budget = createMarkdownDocumentListingBudget(options.limits) - const documents: string[] = [] - const readDirectory = options.readDirectory ?? opendir - assertMarkdownDocumentPathWithinLimit(rootPath, budget.limits.maxPathBytes) - - const visitDirectory = async ( - absoluteDirectoryPath: string, - relativeDirectoryPath: string, - depth: number - ): Promise => { - throwIfAborted(options.signal) - let directory: Dir | AsyncIterable - try { - directory = await readDirectory(absoluteDirectoryPath) - } catch (error) { - if (depth > 0 && options.ignoreNestedDirectoryErrors) { - return - } - throw error - } - - for await (const entry of directory) { - throwIfAborted(options.signal) - const relativePath = relativeDirectoryPath - ? `${relativeDirectoryPath}/${entry.name}` - : entry.name - const nextDepth = depth + 1 - const shouldDescend = entry.isDirectory() && options.shouldDescend(relativePath, entry.name) - visitMarkdownDocumentListingEntry(budget, relativePath, shouldDescend ? nextDepth : depth) - if (entry.isSymbolicLink()) { - continue - } - if (entry.isDirectory()) { - if (shouldDescend) { - await visitDirectory(join(absoluteDirectoryPath, entry.name), relativePath, nextDepth) - } - continue - } - if (entry.isFile() && isMarkdownDocumentPath(entry.name)) { - retainMarkdownRelativePath(budget, rootPath, relativePath) - documents.push(relativePath) - } - } - } - - await visitDirectory(rootPath, '', 0) - return documents -} - -function throwIfAborted(signal: AbortSignal | undefined): void { - if (!signal?.aborted) { - return - } - throw signal.reason instanceof Error ? signal.reason : new MarkdownDocumentListingCapacityError() -} diff --git a/src/shared/node-markdown-document-listing.ts b/src/shared/node-markdown-document-listing.ts new file mode 100644 index 00000000000..954486458ec --- /dev/null +++ b/src/shared/node-markdown-document-listing.ts @@ -0,0 +1,168 @@ +import type { ChildProcessHandle } from './child-process/process-spec' +import type { MarkdownDocument } from './filesystem-entry-types' +import { RipgrepFilenameDecoder } from './ripgrep-filename-decoder' +import { isRipgrepMissingCwdExit, ripgrepMissingCwdError } from './ripgrep-process-availability' +import { abortSignalReason } from './abort-signal-reason' +import { markdownDocumentFromRelativePath, isMarkdownDocumentName } from './markdown-document-paths' +import { joinSearchRoot } from './text-search-paths' +import { + createMarkdownDocumentListingBudget, + retainMarkdownDocument, + MarkdownDocumentListingCapacityError, + MARKDOWN_DOCUMENT_LISTING_MAX_PATH_BYTES +} from './markdown-document-listing-limits' + +const MARKDOWN_LISTING_TIMEOUT_MS = 15_000 +export const MARKDOWN_DOCUMENT_GLOB = '*.{[mM][dD],[mM][dD][xX],[mM][aA][rR][kK][dD][oO][wW][nN]}' +export const MARKDOWN_DOCUMENT_LISTING_ARGS = [ + '--files', + '--hidden', + '--no-ignore', + '--no-config', + '--null', + '--path-separator', + '/', + // Keep case variants in --glob: --iglob is applied after exclusions and can reopen hidden folders. + '--glob', + MARKDOWN_DOCUMENT_GLOB, + '--glob', + '!**/.*/', + '--glob', + '**/.github/', + '--glob', + '!**/node_modules/', + '.' +] + +export function collectMarkdownDocuments( + child: ChildProcessHandle, + rootPath: string, + windowsOutput = false, + signal?: AbortSignal, + options: { allowPartialListing?: boolean; stopProcess?: () => void } = {} +): Promise { + return new Promise((resolveListing, reject) => { + const filenameDecoder = new RipgrepFilenameDecoder((error) => finish(error), windowsOutput) + const documents: MarkdownDocument[] = [] + const budget = createMarkdownDocumentListingBudget() + let carry = '' + let stderr = '' + let settled = false + const finish = (error?: Error): void => { + if (settled) { + return + } + settled = true + clearTimeout(timer) + child.stdout?.off('data', onData) + child.stderr?.off('data', onStderr) + child.stdout?.off('error', onError) + child.stderr?.off('error', onError) + child.off('close', onClose) + child.off('error', onError) + // A spawn or pipe error can arrive after a timeout has already settled the listing. + child.on('error', ignoreLateError) + child.stdout?.on('error', ignoreLateError) + child.stderr?.on('error', ignoreLateError) + signal?.removeEventListener('abort', onAbort) + carry = '' + if (error) { + if (child.pid !== undefined) { + try { + if (options.stopProcess) { + options.stopProcess() + } else { + child.kill('SIGKILL') + } + } catch { + // The process may have exited before the timeout or stream error arrived. + } + } + documents.length = 0 + child.stdout?.resume() + child.stderr?.resume() + reject(error) + } else { + resolveListing(documents.sort((a, b) => a.relativePath.localeCompare(b.relativePath))) + } + } + const onAbort = (): void => finish(abortSignalReason(signal!)) + const onError = (error: Error): void => finish(error) + const onStderr = (chunk: string): void => { + stderr = (stderr + chunk).slice(0, 4096) + } + const onData = (chunk: Buffer | string): void => { + const decoded = filenameDecoder.decode(chunk) + if (decoded === null) { + return + } + carry += decoded + let start = 0 + let end: number + while ((end = carry.indexOf('\0', start)) !== -1) { + const path = carry.slice(start, end) + if (Buffer.byteLength(path) > MARKDOWN_DOCUMENT_LISTING_MAX_PATH_BYTES) { + finish(new MarkdownDocumentListingCapacityError()) + return + } + if (!path.startsWith('./') || path.split('/').includes('..')) { + finish(new Error('Invalid path in Markdown document listing')) + return + } + if (isMarkdownDocumentName(path)) { + const document = markdownDocumentFromRelativePath(rootPath, path.slice(2)) + if (document) { + document.filePath = joinSearchRoot(rootPath, document.relativePath) + try { + retainMarkdownDocument(budget, document) + documents.push(document) + } catch (error) { + finish(error instanceof Error ? error : new MarkdownDocumentListingCapacityError()) + return + } + } + } + start = end + 1 + } + carry = carry.slice(start) + if (Buffer.byteLength(carry) > MARKDOWN_DOCUMENT_LISTING_MAX_PATH_BYTES) { + finish(new MarkdownDocumentListingCapacityError()) + } + } + const onClose = (code: number | null, signal: NodeJS.Signals | null): void => { + if (isRipgrepMissingCwdExit(code)) { + finish(ripgrepMissingCwdError(rootPath)) + } else if ( + signal || + (code !== 0 && + code !== 1 && + !(code === 2 && options.allowPartialListing && documents.length > 0)) + ) { + finish(new Error(`Markdown document listing failed (${signal ?? code}): ${stderr.trim()}`)) + } else { + if (!filenameDecoder.finish()) { + return + } + finish(carry ? new Error('Incomplete path in Markdown document listing') : undefined) + } + } + const timer = setTimeout( + () => finish(new Error('Markdown document listing timed out')), + MARKDOWN_LISTING_TIMEOUT_MS + ) + timer.unref?.() + child.stderr?.setEncoding('utf8') + child.stdout?.on('data', onData) + child.stderr?.on('data', onStderr) + child.stdout?.on('error', onError) + child.stderr?.on('error', onError) + child.once('error', onError) + child.once('close', onClose) + signal?.addEventListener('abort', onAbort, { once: true }) + if (signal?.aborted) { + onAbort() + } + }) +} + +function ignoreLateError(): void {}