From f59194d481f24dc9a67756f7edb049c5666c8f19 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Mon, 5 Oct 2026 21:45:52 -0700 Subject: [PATCH] Bound file-search inventories and release abandoned Markdown data (#25369) * Bound relay git-grep records and clean up capacity failures Credits @OrcaWin for the original bounded-record proposal. * Bound filesystem listing and transfer metadata at the execution host Apply mobile limits before transport, list Markdown through its semantic producer, and retain complete directory results only within explicit capacity budgets. Stream SFTP directory packets and reject oversized transfer plans before reporting success; preserve narrow older-peer fallbacks and validate streamed response retention. * Release inactive Markdown candidates and support folder scopes Keep one current document snapshot per consumer and attach completion candidates to the actual editor model lifetime. Resolve folder workspace roots through existing workspace identities so their previews and completions receive the same authorized listing as worktrees. * Preserve full runtime inventories under aggregate byte budgets Leave unqualified inventories complete beyond 20,000 files. Charge retained paths and serialized bytes at local and relay producers, reject oversized full inventories explicitly, and bound old-peer response reassembly; keep caller-specific mobile and explicit result limits. * Bound SFTP directory handle cleanup waits Send CLOSE even after cancellation, stop waiting after five seconds without an acknowledgement, and ignore late callbacks. Preserve the original capacity or cancellation error and close handles returned after an aborted OPENDIR. * Preserve registered root spelling in Markdown document paths * Reject incomplete SFTP directory cleanup and final cancellation * Bound pending response bytes before stream ownership arrives * Preserve bounded directory reads on legacy SSH hosts * Allow bounded per-chunk padding at response byte boundaries * Bound response payload retention after stream metadata * Bound complete legacy Quick Open inventories and cache retention by bytes * Localize Markdown document listing fallback * Exercise relay search decoding with real byte streams * Preserve concrete relay stream fixture types * test: compare distinct bundled ripgrep platforms on Windows * Preserve Markdown discovery across Windows and legacy SSH hosts * Make WSL process termination available to the bounds layer --- src/main/ipc/filesystem-list-files.test.ts | 37 +++ src/main/ipc/filesystem-list-files.ts | 17 +- ...lesystem-markdown-document-listing.test.ts | 84 ++++++ .../filesystem/filesystem-read-handlers.ts | 22 +- .../ipc/markdown-documents-ripgrep.test.ts | 78 +++++- src/main/ipc/markdown-documents.ts | 255 ++---------------- .../providers/filesystem-markdown-listing.ts | 18 ++ .../providers/filesystem-provider-contract.ts | 6 +- .../providers/sftp-directory-test-fixture.ts | 24 ++ ...ssh-directory-legacy-compatibility.test.ts | 96 +++++++ src/main/providers/ssh-directory-listing.ts | 63 +++++ .../ssh-directory-streaming-real.test.ts | 103 +++++++ .../providers/ssh-directory-streaming.test.ts | 216 +++++++++++++++ src/main/providers/ssh-file-listing.ts | 46 ++++ .../providers/ssh-filesystem-download.test.ts | 105 ++++++-- src/main/providers/ssh-filesystem-download.ts | 81 ++++-- ...ilesystem-provider-download-folder.test.ts | 13 +- .../providers/ssh-filesystem-provider-sftp.ts | 77 +++++- .../providers/ssh-filesystem-provider.test.ts | 6 +- src/main/providers/ssh-filesystem-provider.ts | 32 +-- .../ssh-listing-compatibility.test.ts | 98 +++++++ .../ssh-markdown-document-listing.ts | 34 +++ .../providers/ssh-sftp-directory-close.ts | 42 +++ .../providers/ssh-sftp-filesystem-channel.ts | 55 ++++ .../ssh-sftp-filesystem-provider.test.ts | 70 ++++- .../providers/ssh-sftp-filesystem-provider.ts | 78 ++---- src/main/ripgrep/bundled-ripgrep-path.test.ts | 3 +- src/main/ripgrep/bundled-ripgrep-stop.test.ts | 36 +++ src/main/ripgrep/bundled-ripgrep-stop.ts | 17 ++ .../runtime-file-commands-constructor.ts | 15 +- ...time-file-commands-search-runtime-files.ts | 9 +- ...time-file-listing-producer-budgets.test.ts | 81 ++++++ ...runtime-file-target-execution-host.test.ts | 16 +- src/main/ssh/ssh-directory-transfer-budget.ts | 43 +++ ...ssh-git-response-pending-retention.test.ts | 245 +++++++++++++++++ .../ssh/ssh-git-response-stream-reader.ts | 90 ++++--- .../ssh-listing-response-retention.test.ts | 60 +++++ src/main/ssh/ssh-response-payload.ts | 67 +++++ .../ssh/ssh-response-pending-frames.test.ts | 35 +++ src/main/ssh/ssh-response-pending-frames.ts | 101 +++++++ src/main/ssh/system-ssh-file-transfer.ts | 23 +- .../ssh/system-ssh-upload-plan-budget.test.ts | 61 +++++ src/relay/dispatcher-rpc-routing.ts | 12 +- .../filesystem-capacity-error-wire.test.ts | 37 +++ src/relay/fs-directory-listing.ts | 34 +++ src/relay/fs-file-listing-paths.ts | 33 +++ src/relay/fs-handler-git-fallback.ts | 28 +- .../fs-handler-git-search-capacity.test.ts | 72 +++++ ...s-handler-git-search-real-capacity.test.ts | 24 ++ src/relay/fs-handler-git-search.ts | 18 +- .../fs-handler-list-files-ignored.test.ts | 54 ++++ src/relay/fs-handler-list-files.ts | 46 ++-- src/relay/fs-handler.ts | 40 +++ src/relay/fs-markdown-document-launch.test.ts | 101 +++++++ .../fs-markdown-document-listing.test.ts | 113 ++++++++ src/relay/fs-markdown-document-listing.ts | 58 ++++ src/relay/fs-search-line-fragments.test.ts | 26 +- src/relay/protocol.ts | 5 +- ...wn-document-worktree-path-selector.test.ts | 9 + ...arkdown-document-worktree-path-selector.ts | 13 +- ...markdown-doc-completions-retention.test.ts | 89 ++++++ .../editor/monaco-markdown-doc-completions.ts | 16 +- .../editor/use-monaco-editor-decorations.ts | 20 +- .../useMarkdownDocuments.navigation.test.ts | 56 +++- .../components/editor/useMarkdownDocuments.ts | 46 ++-- src/renderer/src/i18n/locales/en.json | 3 + .../runtime-legacy-inventory-budget.test.ts | 64 +++++ .../runtime-legacy-inventory-budget.ts | 88 ++++++ ...legacy-quick-open-inventory-expiry.test.ts | 45 +++- .../runtime-legacy-quick-open-inventory.ts | 20 +- src/shared/directory-listing-budget.ts | 50 ++++ src/shared/file-inventory-budget.test.ts | 17 ++ src/shared/file-inventory-budget.ts | 32 +++ src/shared/git-grep-record-limit.ts | 10 + .../markdown-document-listing-limits.test.ts | 29 +- .../markdown-document-listing-limits.ts | 42 +-- src/shared/markdown-document-paths.ts | 103 +++++++ .../node-markdown-document-discovery.test.ts | 77 ------ .../node-markdown-document-discovery.ts | 86 ------ src/shared/node-markdown-document-listing.ts | 168 ++++++++++++ 80 files changed, 3689 insertions(+), 753 deletions(-) create mode 100644 src/main/providers/filesystem-markdown-listing.ts create mode 100644 src/main/providers/sftp-directory-test-fixture.ts create mode 100644 src/main/providers/ssh-directory-legacy-compatibility.test.ts create mode 100644 src/main/providers/ssh-directory-listing.ts create mode 100644 src/main/providers/ssh-directory-streaming-real.test.ts create mode 100644 src/main/providers/ssh-directory-streaming.test.ts create mode 100644 src/main/providers/ssh-file-listing.ts create mode 100644 src/main/providers/ssh-listing-compatibility.test.ts create mode 100644 src/main/providers/ssh-markdown-document-listing.ts create mode 100644 src/main/providers/ssh-sftp-directory-close.ts create mode 100644 src/main/providers/ssh-sftp-filesystem-channel.ts create mode 100644 src/main/ripgrep/bundled-ripgrep-stop.test.ts create mode 100644 src/main/ripgrep/bundled-ripgrep-stop.ts create mode 100644 src/main/runtime/runtime-file-listing-producer-budgets.test.ts create mode 100644 src/main/ssh/ssh-directory-transfer-budget.ts create mode 100644 src/main/ssh/ssh-git-response-pending-retention.test.ts create mode 100644 src/main/ssh/ssh-listing-response-retention.test.ts create mode 100644 src/main/ssh/ssh-response-payload.ts create mode 100644 src/main/ssh/ssh-response-pending-frames.test.ts create mode 100644 src/main/ssh/ssh-response-pending-frames.ts create mode 100644 src/main/ssh/system-ssh-upload-plan-budget.test.ts create mode 100644 src/relay/filesystem-capacity-error-wire.test.ts create mode 100644 src/relay/fs-directory-listing.ts create mode 100644 src/relay/fs-file-listing-paths.ts create mode 100644 src/relay/fs-handler-git-search-capacity.test.ts create mode 100644 src/relay/fs-handler-git-search-real-capacity.test.ts create mode 100644 src/relay/fs-markdown-document-launch.test.ts create mode 100644 src/relay/fs-markdown-document-listing.test.ts create mode 100644 src/relay/fs-markdown-document-listing.ts create mode 100644 src/renderer/src/components/editor/monaco-markdown-doc-completions-retention.test.ts create mode 100644 src/renderer/src/runtime/runtime-legacy-inventory-budget.test.ts create mode 100644 src/renderer/src/runtime/runtime-legacy-inventory-budget.ts create mode 100644 src/shared/directory-listing-budget.ts create mode 100644 src/shared/file-inventory-budget.test.ts create mode 100644 src/shared/file-inventory-budget.ts create mode 100644 src/shared/git-grep-record-limit.ts create mode 100644 src/shared/markdown-document-paths.ts delete mode 100644 src/shared/node-markdown-document-discovery.test.ts delete mode 100644 src/shared/node-markdown-document-discovery.ts create mode 100644 src/shared/node-markdown-document-listing.ts 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 {}