From 58bd15fa3ffa65942d94aee8137e35c27f4368c8 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 4 Oct 2026 01:27:30 -0700 Subject: [PATCH] Fix ripgrep result completeness, filename handling, and search errors (#25156) * Preserve ripgrep search results, filename identity, and failure diagnostics * Fix adversarial Unicode and Explorer filename findings * Register search failure localization fallback * Preserve host filename identity through document and watcher consumers --- .../patches/@streamparser__json@0.0.26.patch | 66 +++++++++ pnpm-lock.yaml | 5 +- pnpm-workspace.yaml | 1 + .../filesystem-list-files-name-filter.test.ts | 8 +- src/main/ipc/filesystem-list-files.test.ts | 71 ++++++--- src/main/ipc/filesystem-list-files.ts | 67 ++++----- .../ipc/filesystem-search-file-paths.test.ts | 76 ++++++++-- src/main/ipc/filesystem-search-file-paths.ts | 38 ++--- .../ipc/filesystem-search-rg-timeout.test.ts | 32 ++++- .../filesystem/filesystem-search-handlers.ts | 50 ++++--- .../markdown-documents-remote-paths.test.ts | 47 ++++++ .../ipc/markdown-documents-ripgrep.test.ts | 18 +++ src/main/ipc/markdown-documents.test.ts | 36 ++++- src/main/ipc/markdown-documents.ts | 71 +++++---- .../ripgrep-filename-identity-real.test.ts | 62 ++++++++ .../ripgrep/text-search-unicode-real.test.ts | 123 ++++++++++++++++ .../runtime/ripgrep-filename-identity.test.ts | 82 +++++++++++ ...ile-commands-search-local-runtime-files.ts | 41 ++++-- src/main/runtime/runtime-relative-paths.ts | 12 +- .../fs-handler-list-files-cancel.test.ts | 6 +- .../fs-handler-list-files-ignored.test.ts | 91 ++++++------ ...fs-handler-list-files-path-framing.test.ts | 57 ++++++++ src/relay/fs-handler-list-files.ts | 29 ++-- src/relay/fs-handler-utils.ts | 42 +++--- src/relay/fs-search-errors-real.test.ts | 39 +++++ src/relay/relay-bundled-ripgrep.test.ts | 2 +- src/relay/relay-bundled-ripgrep.ts | 34 +++-- src/relay/relay-ripgrep-cwd-env.test.ts | 2 +- .../relay-windows-path-ripgrep-cache.test.ts | 79 ++++++++++ src/relay/relay-windows-path-ripgrep.test.ts | 9 +- .../editor-external-watch-path-index.test.ts | 30 ++++ .../right-sidebar/SearchResultsPane.tsx | 16 ++- ...plorer-directory-filename-identity.test.ts | 54 +++++++ .../file-explorer-directory-listing.ts | 6 +- .../right-sidebar/file-explorer-entries.ts | 5 +- ...le-explorer-name-filter-projection.test.ts | 73 ++++++++++ .../file-explorer-name-filter-projection.ts | 17 +-- .../right-sidebar/file-explorer-paths.test.ts | 25 ++-- .../right-sidebar/file-explorer-paths.ts | 19 +-- ...e-explorer-watch-filename-identity.test.ts | 45 ++++++ .../right-sidebar/file-explorer-watch-path.ts | 11 +- .../file-explorer-watcher-reconcile.ts | 8 +- .../src/components/right-sidebar/path-tree.ts | 6 +- .../right-sidebar/status-display.ts | 9 +- .../useFileExplorerVisibleRowProjection.ts | 15 +- .../right-sidebar/useFileSearchPanel.ts | 4 +- .../useFileSearchRunner.test.tsx | 49 +++++++ .../right-sidebar/useFileSearchRunner.ts | 9 +- src/renderer/src/i18n/locales/en.json | 3 + src/renderer/src/lib/path.test.ts | 8 ++ src/renderer/src/lib/path.ts | 21 ++- .../editor/actions/file-search-actions.ts | 3 + .../slices/editor/search/file-search-state.ts | 1 + .../types/file-search-worktree-state.ts | 1 + src/shared/quick-open-filter.test.ts | 16 ++- src/shared/quick-open-filter.ts | 25 ++-- src/shared/quick-open-ripgrep-output-mode.ts | 14 ++ src/shared/ripgrep-dense-match-json.test.ts | 78 ++++++++++ src/shared/ripgrep-dense-match-json.ts | 135 ++++++++++++++++++ src/shared/ripgrep-filename-decoder.test.ts | 44 ++++++ src/shared/ripgrep-filename-decoder.ts | 51 +++++++ src/shared/ripgrep-line-decoding.test.ts | 61 ++++++++ src/shared/ripgrep-line-decoding.ts | 60 ++++++++ src/shared/ripgrep-match-offsets.test.ts | 36 +++++ src/shared/ripgrep-match-offsets.ts | 37 +++++ src/shared/ripgrep-search-diagnostics.test.ts | 42 ++++++ src/shared/ripgrep-search-diagnostics.ts | 34 +++++ src/shared/text-search-dense-matches.test.ts | 126 ++++++++++++++++ .../text-search-invalid-filename.test.ts | 19 +++ src/shared/text-search-paths.test.ts | 36 +++++ src/shared/text-search-paths.ts | 14 +- src/shared/text-search.ts | 59 ++++---- 72 files changed, 2260 insertions(+), 361 deletions(-) create mode 100644 config/patches/@streamparser__json@0.0.26.patch create mode 100644 src/main/ipc/markdown-documents-remote-paths.test.ts create mode 100644 src/main/ripgrep/ripgrep-filename-identity-real.test.ts create mode 100644 src/main/ripgrep/text-search-unicode-real.test.ts create mode 100644 src/main/runtime/ripgrep-filename-identity.test.ts create mode 100644 src/relay/fs-handler-list-files-path-framing.test.ts create mode 100644 src/relay/fs-search-errors-real.test.ts create mode 100644 src/relay/relay-windows-path-ripgrep-cache.test.ts create mode 100644 src/renderer/src/components/right-sidebar/file-explorer-directory-filename-identity.test.ts create mode 100644 src/renderer/src/components/right-sidebar/file-explorer-watch-filename-identity.test.ts create mode 100644 src/shared/quick-open-ripgrep-output-mode.ts create mode 100644 src/shared/ripgrep-dense-match-json.test.ts create mode 100644 src/shared/ripgrep-dense-match-json.ts create mode 100644 src/shared/ripgrep-filename-decoder.test.ts create mode 100644 src/shared/ripgrep-filename-decoder.ts create mode 100644 src/shared/ripgrep-line-decoding.test.ts create mode 100644 src/shared/ripgrep-line-decoding.ts create mode 100644 src/shared/ripgrep-match-offsets.test.ts create mode 100644 src/shared/ripgrep-match-offsets.ts create mode 100644 src/shared/ripgrep-search-diagnostics.test.ts create mode 100644 src/shared/ripgrep-search-diagnostics.ts create mode 100644 src/shared/text-search-dense-matches.test.ts create mode 100644 src/shared/text-search-invalid-filename.test.ts create mode 100644 src/shared/text-search-paths.test.ts diff --git a/config/patches/@streamparser__json@0.0.26.patch b/config/patches/@streamparser__json@0.0.26.patch new file mode 100644 index 00000000000..c93998f85fe --- /dev/null +++ b/config/patches/@streamparser__json@0.0.26.patch @@ -0,0 +1,66 @@ +diff --git a/dist/cjs/utils/bufferedString.js b/dist/cjs/utils/bufferedString.js +index 82f710a018f9771fe10335e2dcacd75d707d9062..f3cfa64cefa967d7a83c328e1abb9246135b067b 100644 +--- a/dist/cjs/utils/bufferedString.js ++++ b/dist/cjs/utils/bufferedString.js +@@ -16,7 +16,7 @@ class NonBufferedString { + constructor() { + // fatal: true makes invalid byte sequences (e.g. a lead byte followed by a + // non-continuation byte) throw instead of silently decoding to U+FFFD. +- this.decoder = new TextDecoder("utf-8", { fatal: true }); ++ this.decoder = new TextDecoder("utf-8", { fatal: true, ignoreBOM: true }); + // Pieces appended since the last toString(), not yet folded into `string`. + this.pending = []; + this.string = ""; +@@ -66,7 +66,7 @@ class BufferedString { + constructor(bufferSize) { + // fatal: true makes invalid byte sequences (e.g. a lead byte followed by a + // non-continuation byte) throw instead of silently decoding to U+FFFD. +- this.decoder = new TextDecoder("utf-8", { fatal: true }); ++ this.decoder = new TextDecoder("utf-8", { fatal: true, ignoreBOM: true }); + this.bufferOffset = 0; + this.string = ""; + this.byteLength = 0; +diff --git a/dist/mjs/utils/bufferedString.js b/dist/mjs/utils/bufferedString.js +index 0fb208d8615f5e20a086f75a37bef928b155c47c..0d8ca407d594d61798eca7f7253e1dfc1770d291 100644 +--- a/dist/mjs/utils/bufferedString.js ++++ b/dist/mjs/utils/bufferedString.js +@@ -13,7 +13,7 @@ export class NonBufferedString { + constructor() { + // fatal: true makes invalid byte sequences (e.g. a lead byte followed by a + // non-continuation byte) throw instead of silently decoding to U+FFFD. +- this.decoder = new TextDecoder("utf-8", { fatal: true }); ++ this.decoder = new TextDecoder("utf-8", { fatal: true, ignoreBOM: true }); + // Pieces appended since the last toString(), not yet folded into `string`. + this.pending = []; + this.string = ""; +@@ -62,7 +62,7 @@ export class BufferedString { + constructor(bufferSize) { + // fatal: true makes invalid byte sequences (e.g. a lead byte followed by a + // non-continuation byte) throw instead of silently decoding to U+FFFD. +- this.decoder = new TextDecoder("utf-8", { fatal: true }); ++ this.decoder = new TextDecoder("utf-8", { fatal: true, ignoreBOM: true }); + this.bufferOffset = 0; + this.string = ""; + this.byteLength = 0; +diff --git a/src/utils/bufferedString.ts b/src/utils/bufferedString.ts +index 482c7402899bb157249cfb7882d327b7d9477912..4e45ef5d5bd7ec66776ece54e917282441cededb 100644 +--- a/src/utils/bufferedString.ts ++++ b/src/utils/bufferedString.ts +@@ -40,7 +40,7 @@ export interface StringBuilder { + export class NonBufferedString implements StringBuilder { + // fatal: true makes invalid byte sequences (e.g. a lead byte followed by a + // non-continuation byte) throw instead of silently decoding to U+FFFD. +- private decoder = new TextDecoder("utf-8", { fatal: true }); ++ private decoder = new TextDecoder("utf-8", { fatal: true, ignoreBOM: true }); + // Pieces appended since the last toString(), not yet folded into `string`. + private pending: string[] = []; + private string = ""; +@@ -90,7 +90,7 @@ export class NonBufferedString implements StringBuilder { + export class BufferedString implements StringBuilder { + // fatal: true makes invalid byte sequences (e.g. a lead byte followed by a + // non-continuation byte) throw instead of silently decoding to U+FFFD. +- private decoder = new TextDecoder("utf-8", { fatal: true }); ++ private decoder = new TextDecoder("utf-8", { fatal: true, ignoreBOM: true }); + private buffer: Uint8Array; + private bufferOffset = 0; + private string = ""; diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9fb1428002d..2c58ae7a17a 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -166,6 +166,7 @@ overrides: monaco-editor>dompurify: 3.4.16 patchedDependencies: + '@streamparser/json@0.0.26': b2cf43861e5b4e485e97ffa7449ea65acab4d65dd983ab8c0508f1c68f7d80c9 '@vscode/windows-process-tree@0.8.0': 9da74aa3d17243aa53dcdc95c9f06e97437e7fbccf098aeb017579e2d24cbac2 '@xterm/addon-image@0.10.0-beta.300': e5254a46d6f57bef4a8a19683bfa685afa0ca0127545aea53b54a48104ca3562 '@xterm/addon-ligatures@0.11.0-beta.300': 47405b9994b5acf1b4e90b49250358c1ca03649854d59560e7732b72fe336920 @@ -198,7 +199,7 @@ importers: version: 2.5.6 '@streamparser/json': specifier: 0.0.26 - version: 0.0.26 + version: 0.0.26(patch_hash=b2cf43861e5b4e485e97ffa7449ea65acab4d65dd983ab8c0508f1c68f7d80c9) '@xterm/addon-serialize': specifier: 0.15.0-beta.300 version: 0.15.0-beta.300(patch_hash=b35533fe252e7e45433150170348889f4e08a6c17f7017ac34ea694d831fec7f)(@xterm/xterm@6.1.0-beta.303(patch_hash=dd0ccc59cd1ccf99f4d76e5aa2456da165fa0804dce19a833d7638bd07ffa393)) @@ -10208,7 +10209,7 @@ snapshots: '@standard-schema/spec@1.1.0': {} - '@streamparser/json@0.0.26': {} + '@streamparser/json@0.0.26(patch_hash=b2cf43861e5b4e485e97ffa7449ea65acab4d65dd983ab8c0508f1c68f7d80c9)': {} '@swc/core-darwin-arm64@1.15.46': optional: true diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index e3c133b29ee..f00e23d7f2f 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -63,3 +63,4 @@ patchedDependencies: lint-staged@16.4.0: config/patches/lint-staged@16.4.0.patch '@vscode/windows-process-tree@0.8.0': config/patches/@vscode__windows-process-tree@0.8.0.patch i18next-cli@1.74.2: config/patches/i18next-cli@1.74.2.patch + '@streamparser/json@0.0.26': config/patches/@streamparser__json@0.0.26.patch diff --git a/src/main/ipc/filesystem-list-files-name-filter.test.ts b/src/main/ipc/filesystem-list-files-name-filter.test.ts index b529dd34b39..e22043a4d69 100644 --- a/src/main/ipc/filesystem-list-files-name-filter.test.ts +++ b/src/main/ipc/filesystem-list-files-name-filter.test.ts @@ -62,7 +62,7 @@ describe('listQuickOpenFiles name filter', () => { it('counts only matches against the ripgrep cap', async () => { wslAwareSpawnMock - .mockImplementationOnce(() => fakeRipgrep('a.ts\nb.ts\nc.ts\nios/AppDelegate.swift\n')) + .mockImplementationOnce(() => fakeRipgrep('a.ts\0b.ts\0c.ts\0ios/AppDelegate.swift\0')) .mockImplementationOnce(() => fakeRipgrep('')) const files = await listQuickOpenFiles( @@ -80,7 +80,7 @@ describe('listQuickOpenFiles name filter', () => { it('rejects with the bundled-ripgrep error when the filtered ignored pass cannot start', async () => { wslAwareSpawnMock - .mockImplementationOnce(() => fakeRipgrep('ios/AppDelegate.swift\n')) + .mockImplementationOnce(() => fakeRipgrep('ios/AppDelegate.swift\0')) .mockImplementationOnce(() => fakeRipgrep('', null, -2)) await expect( @@ -98,7 +98,7 @@ describe('listQuickOpenFiles name filter', () => { it('keeps primary matches when the ignored-file pass fails during a filtered scan', async () => { wslAwareSpawnMock - .mockImplementationOnce(() => fakeRipgrep('ios/AppDelegate.swift\n')) + .mockImplementationOnce(() => fakeRipgrep('ios/AppDelegate.swift\0')) .mockImplementationOnce(() => fakeRipgrep('', 'SIGKILL')) await expect( @@ -116,7 +116,7 @@ describe('listQuickOpenFiles name filter', () => { it('still rejects an ignored-pass failure for unfiltered listings', async () => { wslAwareSpawnMock - .mockImplementationOnce(() => fakeRipgrep('a.ts\n')) + .mockImplementationOnce(() => fakeRipgrep('a.ts\0')) .mockImplementationOnce(() => fakeRipgrep('', 'SIGKILL')) await expect( diff --git a/src/main/ipc/filesystem-list-files.test.ts b/src/main/ipc/filesystem-list-files.test.ts index 84567c85419..b6fe3453c22 100644 --- a/src/main/ipc/filesystem-list-files.test.ts +++ b/src/main/ipc/filesystem-list-files.test.ts @@ -88,6 +88,33 @@ describe('filesystem-list-files', () => { ) }) + it.each(['invalid', 'incomplete'] as const)('rejects %s UTF-8 filename bytes', async (kind) => { + const child = createMockProcess() + spawnMock.mockReturnValue(child) + const store: Store = Object.create(null) + const promise = listQuickOpenFiles('/repo', store) + await vi.waitFor(() => expect(spawnMock).toHaveBeenCalledTimes(1)) + child.stdout?.emit('data', Buffer.from(kind === 'invalid' ? [0xff] : [0xe2, 0x82])) + if (kind === 'incomplete') { + child.emit('close', 0, null) + } + await expect(promise).rejects.toThrow('not valid UTF-8') + if (kind === 'invalid') { + expect(child.kill).toHaveBeenCalled() + } + }) + + it('counts NUL-delimited filenames containing newlines as one result each', async () => { + const child = createMockProcess() + spawnMock.mockReturnValue(child) + const store: Store = Object.create(null) + const promise = listQuickOpenFiles('/repo', store, undefined, undefined, 2) + await vi.waitFor(() => expect(spawnMock).toHaveBeenCalledTimes(1)) + child.stdout?.emit('data', 'first\nsecond.ts\0trailing\r\0third.ts\0') + await expect(promise).resolves.toEqual(['first\nsecond.ts', 'trailing\r']) + expect(child.kill).toHaveBeenCalled() + }) + it('rejects a synchronous launch failure before cleanup has been initialized', async () => { spawnMock.mockImplementationOnce(() => { throw Object.assign(new Error('spawn EMFILE'), { code: 'EMFILE' }) @@ -122,7 +149,7 @@ describe('filesystem-list-files', () => { ) setTimeout(() => { - ;(p1.stdout as unknown as EventEmitter).emit('data', 'one.ts\ntwo.ts') + p1.stdout?.emit('data', 'one.ts\0two.ts') p1.emit('close', 0, null) }, 0) const result = await promise @@ -154,12 +181,12 @@ describe('filesystem-list-files', () => { await flushMicrotasks() expect(spawnMock).toHaveBeenCalledTimes(1) expect(spawnMock.mock.calls[0]?.[1]).not.toContain('--no-ignore-vcs') - source.stdout?.emit('data', 'source.ts\n') + source.stdout?.emit('data', 'source.ts\0') source.emit('close', 0, null) await flushMicrotasks() expect(spawnMock).toHaveBeenCalledTimes(2) expect(spawnMock.mock.calls[1]?.[1]).toContain('--no-ignore-vcs') - broad.stdout?.emit('data', 'ignored-file.ts\n') + broad.stdout?.emit('data', 'ignored-file.ts\0') await expect(listing).resolves.toEqual(['source.ts']) expect(broad.kill).toHaveBeenCalledOnce() }) @@ -177,18 +204,18 @@ describe('filesystem-list-files', () => { // Simulate stdout output for normal files setTimeout(() => { - p1.stdout?.emit('data', 'file1.ts\n') - p1.stdout?.emit('data', 'node_modules/bad.js\n') - p1.stdout?.emit('data', '.git/config\n') - p1.stdout?.emit('data', '.github/workflows/ci.yml\n') + p1.stdout?.emit('data', 'file1.ts\0') + p1.stdout?.emit('data', 'node_modules/bad.js\0') + p1.stdout?.emit('data', '.git/config\0') + p1.stdout?.emit('data', '.github/workflows/ci.yml\0') p1.stdout?.emit('data', 'dir1/') // incomplete line - p1.stdout?.emit('data', 'file2.js\n') + p1.stdout?.emit('data', 'file2.js\0') // The broad pass includes ignored files too. - p1.stdout?.emit('data', '.env.local\n') - p1.stdout?.emit('data', 'dist/generated.js\n') - p1.stdout?.emit('data', 'file1.ts\n') // Duplicate - p1.stdout?.emit('data', 'node_modules/ignored.js\n') + p1.stdout?.emit('data', '.env.local\0') + p1.stdout?.emit('data', 'dist/generated.js\0') + p1.stdout?.emit('data', 'file1.ts\0') // Duplicate + p1.stdout?.emit('data', 'node_modules/ignored.js\0') p1.emit('close', 0, null) }, 10) @@ -213,7 +240,7 @@ describe('filesystem-list-files', () => { const promise = listQuickOpenFiles('C:\\repo', storeMock) setTimeout(() => { - ;(p1.stdout as unknown as EventEmitter).emit('data', 'src/index.ts\n') + p1.stdout?.emit('data', 'src/index.ts\0') p1.emit('close', 0, null) }, 10) @@ -237,7 +264,7 @@ describe('filesystem-list-files', () => { const promise = listQuickOpenFiles('C:\\repo', storeMock) setTimeout(() => { - ;(p1.stdout as unknown as EventEmitter).emit('data', '/mnt/c/repo/src/index.ts\n') + p1.stdout?.emit('data', '/mnt/c/repo/src/index.ts\0') p1.emit('close', 0, null) }, 10) @@ -310,7 +337,7 @@ describe('filesystem-list-files', () => { const promise = listQuickOpenFiles('/mock/root', storeMock) setTimeout(() => { - ;(p1.stdout as unknown as EventEmitter).emit('data', 'src/index.ts\n') + p1.stdout?.emit('data', 'src/index.ts\0') p1.emit('close', 2, null) }, 10) @@ -332,7 +359,7 @@ describe('filesystem-list-files', () => { await Promise.resolve() await Promise.resolve() - ;(p1.stdout as unknown as EventEmitter).emit('data', 'src/index.ts\npartial') + p1.stdout?.emit('data', 'src/index.ts\0partial') const rejection = expect(promise).rejects.toThrow('rg list timed out') await vi.advanceTimersByTimeAsync(10000) @@ -376,12 +403,12 @@ describe('filesystem-list-files', () => { const promise = listQuickOpenFiles('/mock/root', storeMock) setTimeout(() => { - ;(p1.stdout as unknown as EventEmitter).emit('data', '.next/cache/1.js\n') - ;(p1.stdout as unknown as EventEmitter).emit('data', '.cache/data.json\n') - ;(p1.stdout as unknown as EventEmitter).emit('data', '.stably/config.json\n') - ;(p1.stdout as unknown as EventEmitter).emit('data', '.vscode/settings.json\n') - ;(p1.stdout as unknown as EventEmitter).emit('data', '.idea/workspace.xml\n') - ;(p1.stdout as unknown as EventEmitter).emit('data', 'valid.ts\n') + p1.stdout?.emit('data', '.next/cache/1.js\0') + p1.stdout?.emit('data', '.cache/data.json\0') + p1.stdout?.emit('data', '.stably/config.json\0') + p1.stdout?.emit('data', '.vscode/settings.json\0') + p1.stdout?.emit('data', '.idea/workspace.xml\0') + p1.stdout?.emit('data', 'valid.ts\0') p1.emit('close', 0, null) }, 10) diff --git a/src/main/ipc/filesystem-list-files.ts b/src/main/ipc/filesystem-list-files.ts index 4e3ea49dbe8..9d9042584d9 100644 --- a/src/main/ipc/filesystem-list-files.ts +++ b/src/main/ipc/filesystem-list-files.ts @@ -1,3 +1,5 @@ +import { getQuickOpenRgOutputMode } from '../../shared/quick-open-ripgrep-output-mode' +import { RipgrepFilenameDecoder, RipgrepFilenameError } from '../../shared/ripgrep-filename-decoder' import { sep } from 'node:path' import type { ChildProcess } from 'node:child_process' import type { Store } from '../persistence' @@ -8,7 +10,6 @@ import { buildExcludePathPrefixes, buildRgArgsForQuickOpen, normalizeQuickOpenRgLine, - type RgOutputMode, shouldExcludeQuickOpenRelPath, shouldIncludeQuickOpenPath } from '../../shared/quick-open-filter' @@ -79,6 +80,10 @@ export async function listQuickOpenFiles( const runRg = (args: string[]): Promise => { return new Promise((resolve, reject) => { + const filenameDecoder = new RipgrepFilenameDecoder((error) => { + killSpawnedRipgrepProcess(child) + finish(error) + }, Boolean(wslDistroForOutput)) let buf = '' let done = false let parseablePathCount = 0 @@ -138,18 +143,22 @@ export async function listQuickOpenFiles( return } let timer: ReturnType - const handleStdoutData = (chunk: string): void => { - buf += chunk + const handleStdoutData = (chunk: Buffer | string): void => { + const decoded = filenameDecoder.decode(chunk) + if (decoded === null) { + return + } + buf += decoded let start = 0 - let newlineIdx = buf.indexOf('\n', start) - while (newlineIdx !== -1) { - if (processLine(buf.substring(start, newlineIdx))) { + let delimiterIdx = buf.indexOf('\0', start) + while (delimiterIdx !== -1) { + if (processLine(buf.substring(start, delimiterIdx))) { buf = '' finishAtLimit() return } - start = newlineIdx + 1 - newlineIdx = buf.indexOf('\n', start) + start = delimiterIdx + 1 + delimiterIdx = buf.indexOf('\0', start) } buf = start < buf.length ? buf.substring(start) : '' } @@ -211,14 +220,15 @@ export async function listQuickOpenFiles( finish(new Error(`rg killed by ${signal}`)) return } + if (!filenameDecoder.finish()) { + return + } if (buf && processLine(buf)) { buf = '' finishAtLimit() return } - if (code === 0 || code === 1) { - finish() - } else if (code === 2 && parseablePathCount > 0) { + if (code === 0 || code === 1 || (code === 2 && parseablePathCount > 0)) { // rg can return 2 for unreadable subdirectories while still listing // usable files from the rest of the root. finish() @@ -257,7 +267,6 @@ export async function listQuickOpenFiles( children.push({ child, isDone: () => done, finish }) - child.stdout?.setEncoding('utf-8') child.stdout?.on('data', handleStdoutData) child.stderr?.on('data', handleStderrData) child.once('error', handleError) @@ -290,16 +299,9 @@ export async function listQuickOpenFiles( } function finishAtLimit(): void { - for (const entry of children) { - if (entry.isDone()) { - continue - } - entry.finish() - if (entry.child.exitCode === null && entry.child.signalCode === null) { - killSpawnedRipgrepProcess(entry.child) - } - } + killSurvivors() } + try { if (maxResults === undefined && maxSerializedBytes === undefined) { // The broader pass already includes source files; an unbounded listing needs only one scan. @@ -314,7 +316,12 @@ export async function listQuickOpenFiles( ) { // Why: a filtered scan walks the whole tree; an ignored-pass timeout keeps primary matches. await runRg(ignoredPass).catch((err: unknown) => { - if (!pathFilter || signal?.aborted || err instanceof RipgrepUnavailableError) { + if ( + !pathFilter || + signal?.aborted || + err instanceof RipgrepUnavailableError || + err instanceof RipgrepFilenameError + ) { throw err } }) @@ -329,19 +336,3 @@ export async function listQuickOpenFiles( ? result : limitQuickOpenFilesBySerializedBytes(result, maxSerializedBytes) } - -function getQuickOpenRgOutputMode( - rawLine: string, - translatedLine: string, - rootPath: string -): RgOutputMode { - if ( - translatedLine !== rawLine || - rawLine.startsWith('/') || - /^[A-Za-z]:[\\/]/.test(rawLine) || - rawLine.startsWith('\\\\') - ) { - return { kind: 'absolute', rootPath } - } - return { kind: 'cwd-relative' } -} diff --git a/src/main/ipc/filesystem-search-file-paths.test.ts b/src/main/ipc/filesystem-search-file-paths.test.ts index e9415d66431..57c817c3b40 100644 --- a/src/main/ipc/filesystem-search-file-paths.test.ts +++ b/src/main/ipc/filesystem-search-file-paths.test.ts @@ -81,6 +81,64 @@ describe('searchQuickOpenFilePaths', () => { ) }) + it('preserves cancellation while an incomplete UTF-8 scalar is buffered', async () => { + const child = createMockProcess() + wslAwareSpawnMock.mockReturnValue(child) + const controller = new AbortController() + const promise = searchQuickOpenFilePaths('/repo', UNUSED_STORE, { + query: 'file', + limit: 2, + signal: controller.signal + }) + await flushMicrotasks() + child.stdout?.emit('data', Buffer.from([0xf0, 0x9f])) + controller.abort() + await expect(promise).rejects.toSatisfy(isFileListingCancellation) + }) + + it.each(['invalid', 'incomplete'] as const)('rejects %s UTF-8 filename bytes', async (kind) => { + const child = createMockProcess() + wslAwareSpawnMock.mockReturnValue(child) + const promise = searchQuickOpenFilePaths('/repo', UNUSED_STORE, { query: 'file', limit: 2 }) + await flushMicrotasks() + child.stdout?.emit('data', Buffer.from(kind === 'invalid' ? [0xff] : [0xe2, 0x82])) + if (kind === 'incomplete') { + child.emit('close', 0, null) + } + await expect(promise).rejects.toThrow('not valid UTF-8') + if (kind === 'invalid') { + expect(child.kill).toHaveBeenCalled() + } + }) + + it('preserves control characters within ranked paths', async () => { + const child = createMockProcess() + wslAwareSpawnMock.mockReturnValue(child) + const promise = searchQuickOpenFilePaths('/repo', UNUSED_STORE, { + query: 'target', + limit: 2 + }) + await flushMicrotasks() + child.stdout?.emit('data', 'first\ntarget.ts\0target.ts\r\0') + child.emit('close', 0, null) + expect((await promise).paths.sort()).toEqual(['first\ntarget.ts', 'target.ts\r'].sort()) + }) + + it('rejects and stops an oversized path rather than ranking a truncated suffix', async () => { + const child = createMockProcess() + wslAwareSpawnMock.mockReturnValue(child) + const promise = searchQuickOpenFilePaths('/repo', UNUSED_STORE, { + query: 'target', + limit: 2 + }) + await flushMicrotasks() + child.stdout?.emit('data', 'x'.repeat(64 * 1024 + 1)) + await expect(promise).rejects.toThrow('file path exceeds the listing limit') + expect(child.kill).toHaveBeenCalledTimes(1) + child.stdout?.emit('data', 'target.ts\0') + child.emit('close', 0, null) + }) + it('finds fuzzy matches after 100k paths without returning excluded worktrees', async () => { const child = createMockProcess() wslAwareSpawnMock.mockReturnValue(child) @@ -94,14 +152,11 @@ describe('searchQuickOpenFilePaths', () => { expect(wslAwareSpawnMock).toHaveBeenCalledTimes(1) expect(wslAwareSpawnMock.mock.calls[0][0]).toBe('/bundled/rg') expect(wslAwareSpawnMock.mock.calls[0][1]).toContain('--no-ignore-vcs') - ;(child.stdout as unknown as EventEmitter).emit( + child.stdout?.emit( 'data', - `${Array.from({ length: 100_100 }, (_, index) => `data/payload-${index}.bin`).join('\n')}\n` - ) - ;(child.stdout as unknown as EventEmitter).emit( - 'data', - 'nested/src/sta-4354-target.ts\nsrc/sta-4354-target.ts\n' + `${Array.from({ length: 100_100 }, (_, index) => `data/payload-${index}.bin`).join('\0')}\0` ) + child.stdout?.emit('data', 'nested/src/sta-4354-target.ts\0src/sta-4354-target.ts\0') child.emit('close', 0, null) await expect(promise).resolves.toEqual({ @@ -138,7 +193,7 @@ describe('searchQuickOpenFilePaths', () => { limit: 32 }) await flushMicrotasks() - ;(child.stdout as unknown as EventEmitter).emit('data', 'src/target.ts\n') + child.stdout?.emit('data', 'src/target.ts\0') child.emit('close', 0, null) await expect(promise).resolves.toMatchObject({ paths: ['src/target.ts'] }) @@ -164,10 +219,7 @@ describe('searchQuickOpenFilePaths', () => { await flushMicrotasks() expect(wslAwareSpawnMock).toHaveBeenCalledTimes(2) - ;(succeeded.stdout as unknown as EventEmitter).emit( - 'data', - 'data/chunk-077568/sta-4354-gitignored-target.bin\n' - ) + succeeded.stdout?.emit('data', 'data/chunk-077568/sta-4354-gitignored-target.bin\0') succeeded.emit('close', 0, null) await expect(promise).resolves.toEqual({ @@ -189,7 +241,7 @@ describe('searchQuickOpenFilePaths', () => { limit: 32 }) await flushMicrotasks() - ;(succeeded.stdout as unknown as EventEmitter).emit('data', 'src/target.ts\n') + succeeded.stdout?.emit('data', 'src/target.ts\0') succeeded.emit('close', 0, null) await expect(promise).resolves.toMatchObject({ paths: ['src/target.ts'] }) diff --git a/src/main/ipc/filesystem-search-file-paths.ts b/src/main/ipc/filesystem-search-file-paths.ts index 4a003aa17c5..3de6f4c6342 100644 --- a/src/main/ipc/filesystem-search-file-paths.ts +++ b/src/main/ipc/filesystem-search-file-paths.ts @@ -1,3 +1,5 @@ +import { getQuickOpenRgOutputMode } from '../../shared/quick-open-ripgrep-output-mode' +import { RipgrepFilenameDecoder } from '../../shared/ripgrep-filename-decoder' import { sep } from 'node:path' import type { Store } from '../persistence' import { fileListingCancellationError } from '../../shared/file-listing-cancellation' @@ -6,8 +8,7 @@ import { buildRgArgsForQuickOpen, normalizeQuickOpenRgLine, shouldExcludeQuickOpenRelPath, - shouldIncludeQuickOpenPath, - type RgOutputMode + shouldIncludeQuickOpenPath } from '../../shared/quick-open-filter' import { isQuickOpenQueryTooLarge, QuickOpenPathRanker } from '../../shared/quick-open-path-search' import { @@ -110,7 +111,11 @@ function scanRipgrepPaths(args: { return Promise.reject(fileListingCancellationError(args.signal)) } return new Promise((resolve, reject) => { - const pathAccumulator = new QuickOpenSubprocessPathAccumulator(0x0a) + const filenameDecoder = new RipgrepFilenameDecoder((error) => { + killSpawnedRipgrepProcess(child) + finish(error) + }, Boolean(args.wslDistroForOutput)) + const pathAccumulator = new QuickOpenSubprocessPathAccumulator(0) let done = false let parseablePathCount = 0 let processErrorObserved = false @@ -141,7 +146,7 @@ function scanRipgrepPaths(args: { : rawLine const relPath = normalizeQuickOpenRgLine( translated, - getOutputMode(rawLine, translated, args.authorizedRootPath) + getQuickOpenRgOutputMode(rawLine, translated, args.authorizedRootPath) ) if (relPath === null) { return @@ -178,11 +183,19 @@ function scanRipgrepPaths(args: { resolve() } } - const handleStdoutData = (chunk: string): void => { - pathAccumulator.push(chunk, (path) => { + const handleStdoutData = (chunk: Buffer | string): void => { + const decoded = filenameDecoder.decode(chunk) + if (decoded === null) { + return + } + const result = pathAccumulator.push(decoded, (path) => { processLine(path) return true }) + if (result === 'path-too-large') { + killSpawnedRipgrepProcess(child) + finish(new Error('Quick Open file path exceeds the listing limit')) + } } const handleStderrData = (): void => { /* drain */ @@ -233,6 +246,9 @@ function scanRipgrepPaths(args: { finish(new Error(`rg killed by ${signal}`)) return } + if (!filenameDecoder.finish()) { + return + } const trailingPath = pathAccumulator.finish() if (trailingPath) { processLine(trailingPath) @@ -249,7 +265,6 @@ function scanRipgrepPaths(args: { finish(fileListingCancellationError(args.signal)) } - child.stdout?.setEncoding('utf-8') child.stdout?.on('data', handleStdoutData) child.stderr?.on('data', handleStderrData) child.once('error', handleError) @@ -265,12 +280,3 @@ function scanRipgrepPaths(args: { } }) } - -function getOutputMode(rawLine: string, translatedLine: string, rootPath: string): RgOutputMode { - return translatedLine !== rawLine || - rawLine.startsWith('/') || - /^[A-Za-z]:[\\/]/.test(rawLine) || - rawLine.startsWith('\\\\') - ? { kind: 'absolute', rootPath } - : { kind: 'cwd-relative' } -} diff --git a/src/main/ipc/filesystem-search-rg-timeout.test.ts b/src/main/ipc/filesystem-search-rg-timeout.test.ts index 9c37bacb20a..d4096500381 100644 --- a/src/main/ipc/filesystem-search-rg-timeout.test.ts +++ b/src/main/ipc/filesystem-search-rg-timeout.test.ts @@ -199,7 +199,7 @@ describe('filesystem rg search timeout', () => { } ) - it('keeps post-spawn errors on the existing empty-result path', async () => { + it('rejects post-spawn errors instead of returning an empty result', async () => { const child = createMockProcess() Object.defineProperty(child, 'pid', { value: 1 }) wslAwareSpawnMock.mockReturnValue(child) @@ -212,7 +212,7 @@ describe('filesystem rg search timeout', () => { await flushMicrotasks() child.emit('error', new Error('post-spawn failure')) - await expect(promise).resolves.toMatchObject({ files: [] }) + await expect(promise).rejects.toThrow('post-spawn failure') }) // Why close(97): the WSL wrapper's "cd failed" code. It is above rg's own 0/1/2, so a handler @@ -298,6 +298,34 @@ describe('filesystem rg search timeout', () => { expect(wslAwareSpawnMock.mock.calls[0]?.[0]).toBe('/bundled/linux/rg') }) + it('marks WSL filenames that UNC cannot represent as incomplete', async () => { + const child = createMockProcess() + wslAwareSpawnMock.mockReturnValue(child) + getLocalGitOptionsForRegisteredWorktreeMock.mockReturnValue({ wslDistro: 'Ubuntu' }) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: all store access is mocked for this handler. + registerFilesystemHandlers({} as never) + const promise = handlers.get('fs:search')!( + { sender: { id: 7 } }, + { rootPath: 'C:\\repo', query: 'hello' } + ) + await flushMicrotasks() + child.stdout?.emit( + 'data', + `${JSON.stringify({ + type: 'match', + data: { + path: { text: './a\\b.txt' }, + lines: { text: 'hello\n' }, + line_number: 1, + submatches: [{ start: 0, end: 5 }] + } + })}\n` + ) + child.emit('close', 0, null) + await expect(promise).resolves.toMatchObject({ files: [], truncated: true }) + expect(toWindowsWslPathMock).not.toHaveBeenCalled() + }) + it('translates WSL rg output for Windows-path project search results', async () => { const child = createMockProcess() wslAwareSpawnMock.mockReturnValue(child) diff --git a/src/main/ipc/filesystem/filesystem-search-handlers.ts b/src/main/ipc/filesystem/filesystem-search-handlers.ts index 9212200e151..94cc07a2f1f 100644 --- a/src/main/ipc/filesystem/filesystem-search-handlers.ts +++ b/src/main/ipc/filesystem/filesystem-search-handlers.ts @@ -1,3 +1,4 @@ +import { RipgrepSearchDiagnostics } from '../../../shared/ripgrep-search-diagnostics' import { SearchSubprocessLineAccumulator } from '../../../shared/search-subprocess-lines' import { ipcMain } from 'electron' import type { ChildProcess } from 'node:child_process' @@ -65,7 +66,7 @@ export function registerFilesystemSearchHandlers(context: FilesystemHandlerConte const wslDistroForOutput = parseWslPath(rootPath)?.distro ?? localGitOptions.wslDistro return new Promise((resolvePromise, rejectPromise) => { - const rgArgs = buildRgArgs(args.query, rootPath, args) + const rgArgs = buildRgArgs(args.query, '.', args) // Why: kill the prior rg so it stops parsing thousands of matches on the main thread (the large-repo freeze) after the UI moved on. const previousChild = activeTextSearches.get(searchKey) if (previousChild) { @@ -73,7 +74,8 @@ export function registerFilesystemSearchHandlers(context: FilesystemHandlerConte } const acc = createAccumulator() - const lines = new SearchSubprocessLineAccumulator(Number.MAX_SAFE_INTEGER) + const lines = new SearchSubprocessLineAccumulator() + const diagnostics = new RipgrepSearchDiagnostics() let resolved = false let processErrorObserved = false let unavailableExitObserved = false @@ -81,8 +83,12 @@ export function registerFilesystemSearchHandlers(context: FilesystemHandlerConte let killTimeout: ReturnType const transformAbsPath = wslDistroForOutput - ? (path: string): string => - path.startsWith('/') ? toWindowsWslPath(path, wslDistroForOutput) : path + ? (path: string): string | null => + path.includes('\\') + ? null + : path.startsWith('/') + ? toWindowsWslPath(path, wslDistroForOutput) + : path : undefined const finish = (result: SearchResult | PromiseLike): void => { @@ -108,7 +114,10 @@ export function registerFilesystemSearchHandlers(context: FilesystemHandlerConte } resolvePromise(result) } - const resolveOnce = (): void => finish(finalize(acc)) + const resolveOnce = (code = 0, signal: NodeJS.Signals | null = null): void => { + const error = diagnostics.failure(code, signal, acc) + finish(error ? Promise.reject(error) : finalize(acc)) + } const rejectUnavailable = (): void => finish(Promise.reject(bundledRipgrepUnavailableError())) const processLine = (line: string): void => { @@ -138,10 +147,16 @@ export function registerFilesystemSearchHandlers(context: FilesystemHandlerConte activeTextSearches.set(searchKey, nextChild) const handleStdoutData = (chunk: string): void => { - lines.push(chunk, processLine) + if (!lines.push(chunk, processLine)) { + acc.truncated = true + if (child) { + killSpawnedRipgrepProcess(child) + } + resolveOnce() + } } - const handleStderrData = (): void => { - // Drain stderr so rg cannot block on a full pipe. + const handleStderrData = (chunk: Buffer): void => { + diagnostics.append(chunk) } const handleError = (error: NodeJS.ErrnoException): void => { processErrorObserved = true @@ -151,18 +166,12 @@ export function registerFilesystemSearchHandlers(context: FilesystemHandlerConte return } if (child && isRipgrepUnavailableExit(child, null, null)) { - // Why the cwd check first: spawn reports a missing cwd as ENOENT too, and blaming the - // binary for it tells the user to reinstall Orca over a workspace that simply moved. - // Why detach close first: a failed spawn emits error THEN close(code < 0), and - // close settles synchronously, so this probe would otherwise race it on a sub-ms - // margin -- two measurements disagreed on which wins. Detaching makes it deterministic. + // Distinguish a missing workspace from a missing binary before close can settle. child.off('close', handleClose) - // Why catch: a failed probe must not strand the search; fall back to the prior verdict. void isRipgrepSpawnCwdUsable(rootPath) .catch(() => true) .then((usable) => { - // Why re-check: finish() drops its argument once settled, so a rejected promise - // built after the close handler already won would go unhandled. + // A late rejected promise must not escape after close settles the search. if (resolved) { return } @@ -174,7 +183,10 @@ export function registerFilesystemSearchHandlers(context: FilesystemHandlerConte }) return } - resolveOnce() + finish(Promise.reject(error)) + if (child) { + killSpawnedRipgrepProcess(child) + } } const handleClose = (code: number | null, signal: NodeJS.Signals | null): void => { // Why first: this code is above rg's own 0/1/2, so the unavailable check would otherwise @@ -193,11 +205,11 @@ export function registerFilesystemSearchHandlers(context: FilesystemHandlerConte rejectUnavailable() return } - const tail = lines.finish() + const tail = !signal && (code === 0 || code === 1) ? lines.finish() : null if (tail !== null) { processLine(tail) } - resolveOnce() + resolveOnce(code ?? -1, signal) } nextChild.stdout?.setEncoding('utf-8') diff --git a/src/main/ipc/markdown-documents-remote-paths.test.ts b/src/main/ipc/markdown-documents-remote-paths.test.ts new file mode 100644 index 00000000000..0d5dc66983a --- /dev/null +++ b/src/main/ipc/markdown-documents-remote-paths.test.ts @@ -0,0 +1,47 @@ +import { describe, expect, it, vi } from 'vitest' +import type * as NodePath from 'node:path' + +vi.mock('node:path', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, extname: actual.win32.extname } +}) + +import { isMarkdownDocumentName, markdownDocumentFromRelativePath } from './markdown-documents' + +describe('remote Markdown filenames on a Windows client', () => { + it('keeps the local filename helper on Windows semantics', () => { + expect(isMarkdownDocumentName('notes\\.md')).toBe(false) + expect(isMarkdownDocumentName('notes.md\\')).toBe(true) + }) + + it.each(['notes\\.md', 'a\\b.MDX', '..\\a.markdown', 'nested/README.md'])( + 'preserves POSIX filename %s and its extension', + (relativePath) => { + const basename = relativePath.slice(relativePath.lastIndexOf('/') + 1) + expect(markdownDocumentFromRelativePath('/home/repo', relativePath)).toEqual({ + filePath: `/home/repo/${relativePath}`, + relativePath, + basename, + name: basename.slice(0, basename.lastIndexOf('.')) + }) + } + ) + + it.each(['notes.md\\', 'nested/.md', '../outside.md'])( + 'rejects non-Markdown or escaping POSIX filename %s', + (relativePath) => { + expect(markdownDocumentFromRelativePath('/home/repo', relativePath)).toBeNull() + } + ) + + it('normalizes Windows remote separators before reading the basename', () => { + expect(markdownDocumentFromRelativePath('C:\\repo', 'notes\\README.MD')).toEqual({ + filePath: 'C:\\repo/notes/README.MD', + relativePath: 'notes/README.MD', + basename: 'README.MD', + name: 'README' + }) + expect(markdownDocumentFromRelativePath('C:\\repo', 'notes\\.md')).toBeNull() + expect(markdownDocumentFromRelativePath('C:\\repo', '..\\outside.md')).toBeNull() + }) +}) diff --git a/src/main/ipc/markdown-documents-ripgrep.test.ts b/src/main/ipc/markdown-documents-ripgrep.test.ts index daee482bae1..13f48b78e17 100644 --- a/src/main/ipc/markdown-documents-ripgrep.test.ts +++ b/src/main/ipc/markdown-documents-ripgrep.test.ts @@ -47,6 +47,24 @@ describe('Markdown document ripgrep lifecycle', () => { expect(child.listenerCount('close')).toBe(0) }) + it('preserves timeout when an incomplete UTF-8 scalar is abandoned', async () => { + vi.useFakeTimers() + const result = listMarkdownDocuments(root) + const outcome = expect(result).rejects.toThrow('timed out') + child.stdout.write(Buffer.from([0xf0, 0x9f])) + await vi.advanceTimersByTimeAsync(15_000) + await outcome + }) + + it.each(['invalid', 'incomplete'] as const)('rejects %s UTF-8 filename bytes', async (kind) => { + const result = listMarkdownDocuments(root) + child.stdout.write(Buffer.from(kind === 'invalid' ? [0xff] : [0xe2, 0x82])) + if (kind === 'incomplete') { + child.emit('close', 0, null) + } + await expect(result).rejects.toThrow('not valid UTF-8') + }) + it('accepts an empty listing', async () => { const result = listMarkdownDocuments(root) child.emit('close', 1, null) diff --git a/src/main/ipc/markdown-documents.test.ts b/src/main/ipc/markdown-documents.test.ts index b23f5748cb4..698935b1f62 100644 --- a/src/main/ipc/markdown-documents.test.ts +++ b/src/main/ipc/markdown-documents.test.ts @@ -1,7 +1,41 @@ import { describe, expect, it } from 'vitest' -import { markdownDocumentFromFilePath } from './markdown-documents' +import { + markdownDocumentFromFilePath, + markdownDocumentFromRelativePath +} from './markdown-documents' describe('markdownDocumentFromFilePath', () => { + it.skipIf(process.platform === 'win32')('preserves local literal backslash names', () => { + expect(markdownDocumentFromFilePath('/repo\\', '/repo\\/a\\b.md')).toMatchObject({ + filePath: '/repo\\/a\\b.md', + relativePath: 'a\\b.md', + basename: 'a\\b.md' + }) + }) + + it('preserves POSIX remote filenames and trailing root backslashes', () => { + for (const name of ['a\\b.md', 'a/b.md', '..\\a.md']) { + expect(markdownDocumentFromRelativePath('/repo\\', name)).toMatchObject({ + filePath: `/repo\\/${name}`, + relativePath: name, + basename: name.slice(name.lastIndexOf('/') + 1) + }) + } + expect(markdownDocumentFromRelativePath('/repo\\', '../a.md')).toBeNull() + }) + + it.each(['C:\\repo\\', '\\\\server\\share\\repo\\'])( + 'preserves Windows separator handling under %s', + (root) => { + expect(markdownDocumentFromRelativePath(root, 'a\\b.md')).toMatchObject({ + filePath: `${root.slice(0, -1)}/a/b.md`, + relativePath: 'a/b.md', + basename: 'b.md' + }) + expect(markdownDocumentFromRelativePath(root, '..\\a.md')).toBeNull() + } + ) + it('keeps in-root path segments that merely start with parent traversal text', () => { expect(markdownDocumentFromFilePath('/workspace', '/workspace/..notes/file.md')).toMatchObject({ filePath: '/workspace/..notes/file.md', diff --git a/src/main/ipc/markdown-documents.ts b/src/main/ipc/markdown-documents.ts index 9bf263d56d4..1f457dba97e 100644 --- a/src/main/ipc/markdown-documents.ts +++ b/src/main/ipc/markdown-documents.ts @@ -1,4 +1,15 @@ -import { basename as pathBasename, extname, isAbsolute, join, relative, resolve } from 'node:path' +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 { parseWslPath } from '../wsl' @@ -7,36 +18,34 @@ import { ripgrepMissingCwdError } from '../../shared/ripgrep-process-availability' -function normalizeRelativePath(path: string): string { - return path.replace(/[\\/]+/g, '/').replace(/^\/+/, '') +export function isMarkdownDocumentName(name: string): boolean { + return isMarkdownExtension(extname(name)) } -export function isMarkdownDocumentName(name: string): boolean { - const extension = extname(name).toLowerCase() - return extension === '.md' || extension === '.mdx' || extension === '.markdown' +function isMarkdownExtension(extension: string): boolean { + const normalized = extension.toLowerCase() + return normalized === '.md' || normalized === '.mdx' || normalized === '.markdown' } function basenameFromRelativePath(relativePath: string): string { - const normalizedPath = relativePath.replaceAll('\\', '/') - return normalizedPath.slice(normalizedPath.lastIndexOf('/') + 1) + return relativePath.slice(relativePath.lastIndexOf('/') + 1) } function isSafeRelativePath(relativePath: string): boolean { return !relativePath.split('/').includes('..') } -function hasParentTraversalSegment(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 (hasParentTraversalSegment(relativePath) || isAbsolute(relativePath)) { + if ( + !isSafeRelativePath(normalizeRelativePath(relativePath, rootPath)) || + isAbsolute(relativePath) + ) { return null } - return normalizeRelativePath(relativePath) + return normalizeRelativePath(relativePath, rootPath) } export function fileDocumentFromFilePath( @@ -50,7 +59,7 @@ export function fileDocumentFromFilePath( rootRelativePath(rootPath, filePath) ?? (options.outsideRootRelativePath === 'basename' ? basename - : normalizeRelativePath(relative(rootPath, filePath))) + : normalizeRelativePath(relative(rootPath, filePath), rootPath)) return { filePath, relativePath, @@ -65,18 +74,22 @@ export function markdownDocumentFromRelativePath( rootPath: string, relativePath: string ): MarkdownDocument | null { - const normalizedRelativePath = normalizeRelativePath(relativePath) + 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) - if (!isMarkdownDocumentName(basename)) { + // Remote separators are already normalized; a POSIX backslash stays part of the name. + const extension = posix.extname(basename) + if (!isMarkdownExtension(extension)) { return null } - const extension = extname(basename) - const normalizedRoot = rootPath.replace(/[\\/]+$/, '') + const normalizedRoot = rootPath.replace( + isWindowsAbsolutePathLike(rootPath) ? /[\\/]+$/ : /\/+$/, + '' + ) return { filePath: `${normalizedRoot}/${normalizedRelativePath}`, relativePath: normalizedRelativePath, @@ -131,6 +144,10 @@ export async function listMarkdownDocuments( ) 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 = '' @@ -172,8 +189,12 @@ export async function listMarkdownDocuments( const onStderr = (chunk: string): void => { stderr = (stderr + chunk).slice(0, 4096) } - const onData = (chunk: string): void => { - carry += chunk + 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) { @@ -201,10 +222,11 @@ export async function listMarkdownDocuments( finish(ripgrepMissingCwdError(rootPath)) } else if (signal || (code !== 0 && code !== 1)) { finish(new Error(`Markdown document listing failed (${signal ?? code}): ${stderr.trim()}`)) - } else if (carry) { - finish(new Error('Incomplete path in Markdown document listing')) } else { - finish() + if (!filenameDecoder.finish()) { + return + } + finish(carry ? new Error('Incomplete path in Markdown document listing') : undefined) } } const timer = setTimeout( @@ -212,7 +234,6 @@ export async function listMarkdownDocuments( MARKDOWN_LISTING_TIMEOUT_MS ) timer.unref?.() - child.stdout?.setEncoding('utf8') child.stderr?.setEncoding('utf8') child.stdout?.on('data', onData) child.stderr?.on('data', onStderr) diff --git a/src/main/ripgrep/ripgrep-filename-identity-real.test.ts b/src/main/ripgrep/ripgrep-filename-identity-real.test.ts new file mode 100644 index 00000000000..6fc7116bd7d --- /dev/null +++ b/src/main/ripgrep/ripgrep-filename-identity-real.test.ts @@ -0,0 +1,62 @@ +import { mkdir, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { describe, expect, it } from 'vitest' +import { spawnBundledRipgrep } from './bundled-ripgrep-spawn' +import { buildRgArgsForQuickOpen, normalizeQuickOpenRgLine } from '../../shared/quick-open-filter' +import { buildRgArgs, createAccumulator, ingestRgJsonLine } from '../../shared/text-search' +import { joinWorktreeRelativePath } from '../runtime/runtime-relative-paths' + +async function capture(root: string, args: string[]): Promise { + const child = spawnBundledRipgrep(args, { cwd: root, stdio: ['ignore', 'pipe', 'pipe'] }) + let output = '' + child.stdout?.setEncoding('utf8').on('data', (chunk: string) => { + output += chunk + }) + child.stderr?.resume() + await new Promise((resolve, reject) => { + child.once('error', reject) + child.once('close', (code) => (code === 0 ? resolve() : reject(new Error(`rg exit ${code}`)))) + }) + return output +} + +describe.skipIf(process.platform === 'win32')('real ripgrep POSIX filename identities', () => { + it('lists, searches, and opens literal-backslash and nested names independently', async () => { + const parent = await mkdtemp(join(tmpdir(), 'orca-rg-identity-')) + const root = join(parent, 'repo\\root') + try { + await mkdir(join(root, 'a'), { recursive: true }) + await writeFile(join(root, 'a\\b.txt'), 'needle literal') + await writeFile(join(root, 'a/b.txt'), 'needle nested') + const args = buildRgArgsForQuickOpen({ + searchRoot: '.', + excludePathPrefixes: [], + forceSlashSeparator: false + }) + const listing = await capture(root, args.primary) + const names = listing + .split('\0') + .filter(Boolean) + .map((line) => normalizeQuickOpenRgLine(line, { kind: 'cwd-relative' })) + expect(names.sort()).toEqual(['a/b.txt', 'a\\b.txt'].sort()) + for (const name of names) { + expect(name).not.toBeNull() + if (name === null) { + throw new Error('invalid name') + } + expect(await readFile(joinWorktreeRelativePath(root, name), 'utf8')).toBe( + name === 'a\\b.txt' ? 'needle literal' : 'needle nested' + ) + } + const acc = createAccumulator() + for (const line of (await capture(root, buildRgArgs('needle', '.', {}))).split('\n')) { + ingestRgJsonLine(line, root, acc, 10) + } + expect([...acc.fileMap.values()].map((file) => file.relativePath).sort()).toEqual(names) + expect(acc.truncated).toBe(false) + } finally { + await rm(parent, { recursive: true, force: true }) + } + }) +}) diff --git a/src/main/ripgrep/text-search-unicode-real.test.ts b/src/main/ripgrep/text-search-unicode-real.test.ts new file mode 100644 index 00000000000..b56c1794a19 --- /dev/null +++ b/src/main/ripgrep/text-search-unicode-real.test.ts @@ -0,0 +1,123 @@ +import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { spawnBundledRipgrep } from './bundled-ripgrep-spawn' +import { + buildRgArgs, + createAccumulator, + finalize, + ingestRgJsonLine +} from '../../shared/text-search' + +describe('ripgrep Unicode match coordinates', () => { + let root: string + beforeEach(async () => { + root = await mkdtemp(join(tmpdir(), 'orca-rg-unicode-')) + }) + afterEach(async () => { + vi.unstubAllEnvs() + await rm(root, { recursive: true, force: true }) + }) + + async function search( + query: string, + useRegex = false, + includePattern?: string, + excludePattern?: string + ) { + const child = spawnBundledRipgrep( + buildRgArgs(query, '.', { useRegex, includePattern, excludePattern }), + { + cwd: root, + stdio: ['ignore', 'pipe', 'pipe'] + } + ) + let output = '' + child.stdout?.setEncoding('utf8').on('data', (chunk: string) => { + output += chunk + }) + child.stderr?.resume() + await new Promise((resolve, reject) => { + child.once('error', reject) + child.once('close', (code) => + code === 0 || code === 1 ? resolve() : reject(new Error(`rg exit ${code}`)) + ) + }) + const acc = createAccumulator() + for (const line of output.split('\n')) { + ingestRgJsonLine(line, root, acc, 2000) + } + return finalize(acc) + } + + it('uses UTF16 columns and lengths for multibyte prefixes and astral matches', async () => { + await writeFile(join(root, 'example.txt'), '😀 café 日本 needle 😀 needle\r\n') + const result = await search('needle|😀', true) + expect( + result.files[0]?.matches.map(({ column, matchLength }) => [column, matchLength]) + ).toEqual([ + [1, 2], + [12, 6], + [19, 2], + [22, 6] + ]) + }) + + it.each(['src/**', '/src/**'])('honors root-relative include glob %s', async (includePattern) => { + await mkdir(join(root, 'src')) + await mkdir(join(root, 'other')) + await writeFile(join(root, 'src', 'match.txt'), 'needle') + await writeFile(join(root, 'src', 'skip.txt'), 'needle') + await writeFile(join(root, 'other', 'match.txt'), 'needle') + const result = await search('needle', false, includePattern, '/src/skip.txt') + expect(result.files.map((file) => [file.filePath, file.relativePath])).toEqual([ + [join(root, 'src', 'match.txt'), 'src/match.txt'] + ]) + }) + + it('keeps navigation and clamped display coordinates aligned', async () => { + const prefix = '日本😀'.repeat(200) + await writeFile(join(root, 'long.txt'), `${prefix}needle`) + const match = (await search('needle')).files[0]?.matches[0] + expect(match?.column).toBe(prefix.length + 1) + expect( + match?.lineContent.slice( + (match.displayColumn ?? 1) - 1, + (match.displayColumn ?? 1) - 1 + match.matchLength + ) + ).toBe('needle') + }) + + it('decodes malformed UTF8 context and maps matches using the original bytes', async () => { + const bytes = Buffer.concat([ + Buffer.from('😀 '), + Buffer.from([0xe2, 0x82, 0xff]), + Buffer.from(' needle é needle\r\n') + ]) + await writeFile(join(root, 'malformed.txt'), bytes) + const content = bytes.toString('utf8').replace(/\n$/, '') + const result = await search('needle') + expect(result.totalMatches).toBe(2) + expect( + result.files[0]?.matches.map((match) => [match.column, match.matchLength, match.lineContent]) + ).toEqual([ + [content.indexOf('needle') + 1, 6, content], + [content.lastIndexOf('needle') + 1, 6, content] + ]) + expect(result.truncated).toBe(false) + }) + + it('ignores external rg config while retaining workspace ignore files', async () => { + const config = join(root, 'config') + await writeFile(config, '--invert-match\n') + vi.stubEnv('RIPGREP_CONFIG_PATH', config) + await writeFile(join(root, '.ignore'), 'ignored.txt\n') + await writeFile(join(root, 'ignored.txt'), 'needle\n') + await mkdir(join(root, 'docs')) + await writeFile(join(root, 'docs', 'visible.txt'), 'needle\nother\n') + const result = await search('needle') + expect(result.files.map((file) => file.relativePath)).toEqual(['docs/visible.txt']) + expect(result.files[0]?.matches[0]?.lineContent).toBe('needle') + }) +}) diff --git a/src/main/runtime/ripgrep-filename-identity.test.ts b/src/main/runtime/ripgrep-filename-identity.test.ts new file mode 100644 index 00000000000..b8ef50a76b6 --- /dev/null +++ b/src/main/runtime/ripgrep-filename-identity.test.ts @@ -0,0 +1,82 @@ +import { mkdir, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { describe, expect, it } from 'vitest' +import { joinWorktreeRelativePath, normalizeRuntimeRelativePath } from './runtime-relative-paths' +import { buildExcludePathPrefixes, normalizeQuickOpenRgLine } from '../../shared/quick-open-filter' +import { createAccumulator, ingestRgJsonLine } from '../../shared/text-search' + +it.each(['/native/repo\\root', '/ssh/repo\\root'])( + 'preserves POSIX search identities under %s independently of client platform', + (root) => { + const acc = createAccumulator() + for (const name of ['a\\b.txt', 'a/b.txt']) { + const relative = normalizeQuickOpenRgLine(`./${name}`, { kind: 'cwd-relative' }) + expect(relative).toBe(name) + expect( + normalizeQuickOpenRgLine(`${root}/${name}`, { kind: 'absolute', rootPath: root }) + ).toBe(name) + expect(joinWorktreeRelativePath(root, normalizeRuntimeRelativePath(name, root))).toBe( + `${root}/${name}` + ) + ingestRgJsonLine( + JSON.stringify({ + type: 'match', + data: { + path: { text: `${root}/${name}` }, + lines: { text: 'needle' }, + line_number: 1, + submatches: [{ start: 0, end: 6 }] + } + }), + root, + acc, + 10 + ) + } + expect([...acc.fileMap.values()].map((file) => file.relativePath)).toEqual([ + 'a\\b.txt', + 'a/b.txt' + ]) + expect(buildExcludePathPrefixes(root, [`${root}/a\\b`])).toEqual(['a\\b']) + } +) + +it.each(['C:\\repo', '\\\\server\\share\\repo'])( + 'preserves Windows separator compatibility under %s', + (root) => { + expect(joinWorktreeRelativePath(root, normalizeRuntimeRelativePath('a\\b.txt', root))).toBe( + `${root}\\a\\b.txt` + ) + expect( + normalizeQuickOpenRgLine(`${root}\\a\\b.txt`, { kind: 'absolute', rootPath: root }) + ).toBe('a/b.txt') + } +) + +describe.skipIf(process.platform === 'win32')('real POSIX filename collision', () => { + it('opens both literal-backslash and nested files without changing identity', async () => { + const parent = await mkdtemp(join(tmpdir(), 'orca-rg-path-')) + const root = join(parent, 'repo\\root') + try { + await mkdir(join(root, 'a'), { recursive: true }) + await writeFile(join(root, 'a\\b.txt'), 'literal') + await writeFile(join(root, 'a/b.txt'), 'nested') + for (const [name, expected] of [ + ['a\\b.txt', 'literal'], + ['a/b.txt', 'nested'] + ]) { + const relative = normalizeQuickOpenRgLine(`./${name}`, { kind: 'cwd-relative' }) + expect(relative).toBe(name) + expect( + await readFile( + joinWorktreeRelativePath(root, normalizeRuntimeRelativePath(name, root)), + 'utf8' + ) + ).toBe(expected) + } + } finally { + await rm(parent, { recursive: true, force: true }) + } + }) +}) diff --git a/src/main/runtime/runtime-file-commands-search-local-runtime-files.ts b/src/main/runtime/runtime-file-commands-search-local-runtime-files.ts index 8ed435fddc7..83e30a15f47 100644 --- a/src/main/runtime/runtime-file-commands-search-local-runtime-files.ts +++ b/src/main/runtime/runtime-file-commands-search-local-runtime-files.ts @@ -1,4 +1,5 @@ // @ts-nocheck -- mechanically split class members. +import { RipgrepSearchDiagnostics } from '../../shared/ripgrep-search-diagnostics' import { SearchSubprocessLineAccumulator } from '../../shared/search-subprocess-lines' import { RuntimeFileCommandsWithSearchRuntimeFiles } from './runtime-file-commands-search-runtime-files' import type { SearchOptions, SearchResult } from '../../shared/code-search-types' @@ -50,20 +51,26 @@ export class RuntimeFileCommandsWithSearchLocalRuntimeFiles extends RuntimeFileC return new Promise((resolvePromise, rejectPromise) => { const searchKey = `${this.host.getRuntimeId()}:${authorizedRootPath}` - const rgArgs = buildRgArgs(options.query, authorizedRootPath, options) + const rgArgs = buildRgArgs(options.query, '.', options) const previousChild = this.activeRuntimeTextSearches.get(searchKey) if (previousChild) { killSpawnedRipgrepProcess(previousChild) } const acc = createAccumulator() - const lines = new SearchSubprocessLineAccumulator(Number.MAX_SAFE_INTEGER) + const lines = new SearchSubprocessLineAccumulator() + const diagnostics = new RipgrepSearchDiagnostics() let resolved = false let processErrorObserved = false let unavailableExitObserved = false let child: ChildProcessHandle | null = null const transformAbsPath = wslDistroForOutput - ? (p: string): string => (p.startsWith('/') ? toWindowsWslPath(p, wslDistroForOutput) : p) + ? (p: string): string | null => + p.includes('\\') + ? null + : p.startsWith('/') + ? toWindowsWslPath(p, wslDistroForOutput) + : p : undefined const finish = (result: SearchResult | PromiseLike): void => { @@ -77,7 +84,10 @@ export class RuntimeFileCommandsWithSearchLocalRuntimeFiles extends RuntimeFileC cleanupListeners() resolvePromise(result) } - const resolveOnce = (): void => finish(finalize(acc)) + const resolveOnce = (code = 0, signal: NodeJS.Signals | null = null): void => { + const error = diagnostics.failure(code, signal, acc) + finish(error ? Promise.reject(error) : finalize(acc)) + } const rejectUnavailable = (): void => finish(Promise.reject(bundledRipgrepUnavailableError())) let killTimeout: ReturnType | null = null @@ -133,10 +143,16 @@ export class RuntimeFileCommandsWithSearchLocalRuntimeFiles extends RuntimeFileC nextChild.stdout?.setEncoding('utf-8') const onStdoutData = (chunk: string): void => { - lines.push(chunk, processLine) + if (!lines.push(chunk, processLine)) { + acc.truncated = true + if (child) { + killSpawnedRipgrepProcess(child) + } + resolveOnce() + } } - const onStderrData = (): void => { - // Drain stderr so rg cannot block on a full pipe. + const onStderrData = (chunk: Buffer): void => { + diagnostics.append(chunk) } const onError = (error: NodeJS.ErrnoException): void => { processErrorObserved = true @@ -171,7 +187,10 @@ export class RuntimeFileCommandsWithSearchLocalRuntimeFiles extends RuntimeFileC }) return } - resolveOnce() + finish(Promise.reject(error)) + if (child) { + killSpawnedRipgrepProcess(child) + } } const onClose = (code: number | null, signal: NodeJS.Signals | null): void => { // Why first: this code is above rg's own 0/1/2, so the unavailable check would otherwise @@ -190,11 +209,11 @@ export class RuntimeFileCommandsWithSearchLocalRuntimeFiles extends RuntimeFileC rejectUnavailable() return } - const tail = lines.finish() + const tail = !signal && (code === 0 || code === 1) ? lines.finish() : null if (tail !== null) { processLine(tail) } - resolveOnce() + resolveOnce(code ?? -1, signal) } nextChild.stdout?.on('data', onStdoutData) @@ -229,7 +248,7 @@ export class RuntimeFileCommandsWithSearchLocalRuntimeFiles extends RuntimeFileC worktree: target.worktree, path: joinWorktreeRelativePath( target.worktree.path, - normalizeRuntimeRelativePath(relativePath) + normalizeRuntimeRelativePath(relativePath, target.worktree.path) ), executionHostId: target.executionHostId })) diff --git a/src/main/runtime/runtime-relative-paths.ts b/src/main/runtime/runtime-relative-paths.ts index 1ec252e24f2..1f5ab8e71cb 100644 --- a/src/main/runtime/runtime-relative-paths.ts +++ b/src/main/runtime/runtime-relative-paths.ts @@ -2,15 +2,21 @@ import { posix, win32 } from 'node:path' import { isWindowsAbsolutePathLike } from '../../shared/cross-platform-path' export function joinWorktreeRelativePath(rootPath: string, relativePath: string): string { - const normalizedRelativePath = relativePath.replace(/\\/g, '/') + const normalizedRelativePath = isWindowsAbsolutePathLike(rootPath) + ? relativePath.replace(/\\/g, '/') + : relativePath if (isWindowsAbsolutePathLike(rootPath)) { return win32.join(rootPath.replace(/\//g, '\\'), ...normalizedRelativePath.split('/')) } return posix.join(rootPath, ...normalizedRelativePath.split('/')) } -export function normalizeRuntimeRelativePath(relativePath: string): string { - const normalized = relativePath.replace(/\\/g, '/').replace(/\/+$/, '') +export function normalizeRuntimeRelativePath(relativePath: string, rootPath?: string): string { + const path = + rootPath !== undefined && !isWindowsAbsolutePathLike(rootPath) + ? relativePath + : relativePath.replace(/\\/g, '/') + const normalized = path.replace(/\/+$/, '') if (normalized === '') { return '' } diff --git a/src/relay/fs-handler-list-files-cancel.test.ts b/src/relay/fs-handler-list-files-cancel.test.ts index 97bea1b0813..b6829921cb4 100644 --- a/src/relay/fs-handler-list-files-cancel.test.ts +++ b/src/relay/fs-handler-list-files-cancel.test.ts @@ -52,7 +52,7 @@ describe('relay list-files cancellation', () => { const promise = listFilesWithRg('/remote/root', [], { signal: controller.signal }) // Partial output before the abort — must be discarded, not resolved. - ignoredProc.stdout?.emit('data', 'src/index.ts\n') + ignoredProc.stdout?.emit('data', 'src/index.ts\0') controller.abort() await expect(promise).rejects.toSatisfy(isFileListingCancellation) @@ -81,8 +81,8 @@ describe('relay list-files cancellation', () => { const promise = listFilesWithRg('/remote/root', [], { signal: controller.signal }) setTimeout(() => { - ignoredProc.stdout?.emit('data', 'src/index.ts\n') - ;(ignoredProc.stdout as unknown as EventEmitter).emit('data', 'dist/out.js\n') + ignoredProc.stdout?.emit('data', 'src/index.ts\0') + ignoredProc.stdout?.emit('data', 'dist/out.js\0') ignoredProc.emit('close', 0, null) }, 5) diff --git a/src/relay/fs-handler-list-files-ignored.test.ts b/src/relay/fs-handler-list-files-ignored.test.ts index 3f8b8c2b883..36a3914d021 100644 --- a/src/relay/fs-handler-list-files-ignored.test.ts +++ b/src/relay/fs-handler-list-files-ignored.test.ts @@ -71,6 +71,21 @@ describe('relay quick open ignored file listing', () => { await Promise.all(tempDirs.splice(0).map((dir) => rm(dir, { recursive: true, force: true }))) }) + it.each(['invalid', 'incomplete'] as const)('rejects %s UTF-8 filename bytes', async (kind) => { + const child = createMockProcess() + spawnMock.mockReturnValue(child) + const promise = listFilesWithRg('/remote/root') + + child.stdout?.emit('data', Buffer.from(kind === 'invalid' ? [0xff] : [0xe2, 0x82])) + if (kind === 'incomplete') { + child.emit('close', 0, null) + } + await expect(promise).rejects.toThrow('not valid UTF-8') + if (kind === 'invalid') { + expect(child.kill).toHaveBeenCalled() + } + }) + it('uses one broad rg pass for unbounded listings and keeps blocklists/excludes', async () => { const ignoredProc = createMockProcess() @@ -80,10 +95,10 @@ describe('relay quick open ignored file listing', () => { expect(spawnMock).toHaveBeenCalledTimes(1) setTimeout(() => { - ignoredProc.stdout?.emit('data', 'src/index.ts\n') - ;(ignoredProc.stdout as unknown as EventEmitter).emit('data', 'dist/generated.js\n') - ;(ignoredProc.stdout as unknown as EventEmitter).emit('data', 'node_modules/pkg/index.js\n') - ;(ignoredProc.stdout as unknown as EventEmitter).emit('data', 'packages/other/src/x.ts\n') + ignoredProc.stdout?.emit('data', 'src/index.ts\0') + ignoredProc.stdout?.emit('data', 'dist/generated.js\0') + ignoredProc.stdout?.emit('data', 'node_modules/pkg/index.js\0') + ignoredProc.stdout?.emit('data', 'packages/other/src/x.ts\0') ignoredProc.emit('close', 0, null) }, 10) @@ -106,10 +121,7 @@ describe('relay quick open ignored file listing', () => { spawnMock.mockImplementation(() => (++callIndex === 1 ? primaryProc : ignoredProc)) const promise = listFilesWithRg('/remote/root', [], { maxResults: 2 }) - ;(primaryProc.stdout as unknown as EventEmitter).emit( - 'data', - 'src/one.ts\nsrc/two.ts\nsrc/three.ts\n' - ) + primaryProc.stdout?.emit('data', 'src/one.ts\0src/two.ts\0src/three.ts\0') await expect(promise).resolves.toEqual(['src/one.ts', 'src/two.ts']) expect(primaryProc.kill).toHaveBeenCalled() @@ -127,14 +139,11 @@ describe('relay quick open ignored file listing', () => { expect(spawnMock).toHaveBeenCalledTimes(1) expect(spawnMock.mock.calls[0][1]).toContain('--no-ignore-vcs') - ;(ignoredProc.stdout as unknown as EventEmitter).emit( + ignoredProc.stdout?.emit( 'data', - `${Array.from({ length: 100_100 }, (_, index) => `data/payload-${index}.bin`).join('\n')}\n` - ) - ;(ignoredProc.stdout as unknown as EventEmitter).emit( - 'data', - 'src/components/target.ts\nscripts/check-target.ts\n' + `${Array.from({ length: 100_100 }, (_, index) => `data/payload-${index}.bin`).join('\0')}\0` ) + ignoredProc.stdout?.emit('data', 'src/components/target.ts\0scripts/check-target.ts\0') ignoredProc.emit('close', 0, null) await expect(promise).resolves.toEqual(['scripts/check-target.ts', 'src/components/target.ts']) @@ -148,11 +157,11 @@ describe('relay quick open ignored file listing', () => { const promise = listFilesWithRg('/remote/root', [], { maxResults: 2 }) expect(spawnMock).toHaveBeenCalledTimes(1) expect(spawnMock.mock.calls[0][1]).not.toContain('--no-ignore-vcs') - primary.stdout?.emit('data', 'src/index.ts\n') + primary.stdout?.emit('data', 'src/index.ts\0') primary.emit('close', 0, null) await vi.waitFor(() => expect(spawnMock).toHaveBeenCalledTimes(2)) expect(spawnMock.mock.calls[1][1]).toContain('--no-ignore-vcs') - broad.stdout?.emit('data', 'src/index.ts\ndist/generated.js\ndist/extra.js\n') + broad.stdout?.emit('data', 'src/index.ts\0dist/generated.js\0dist/extra.js\0') await expect(promise).resolves.toEqual(['src/index.ts', 'dist/generated.js']) expect(broad.kill).toHaveBeenCalled() @@ -168,14 +177,11 @@ describe('relay quick open ignored file listing', () => { maxResults: 32, searchQuery: 'target' }) - ;(failed.stdout as unknown as EventEmitter).emit('data', 'src/target.ts\n') + failed.stdout?.emit('data', 'src/target.ts\0') failed.emit('error', Object.assign(new Error('spawn rg EAGAIN'), { code: 'EAGAIN' })) await vi.waitFor(() => expect(spawnMock).toHaveBeenCalledTimes(2)) - ;(succeeded.stdout as unknown as EventEmitter).emit( - 'data', - 'src/target.ts\nsrc/another-target.ts\n' - ) + succeeded.stdout?.emit('data', 'src/target.ts\0src/another-target.ts\0') succeeded.emit('close', 0, null) await expect(promise).resolves.toEqual(['src/target.ts', 'src/another-target.ts']) @@ -193,7 +199,7 @@ describe('relay quick open ignored file listing', () => { searchQuery: 'target' }) await vi.waitFor(() => expect(spawnMock).toHaveBeenCalledTimes(2)) - ;(succeeded.stdout as unknown as EventEmitter).emit('data', 'src/target.ts\n') + succeeded.stdout?.emit('data', 'src/target.ts\0') succeeded.emit('close', 0, null) await expect(promise).resolves.toEqual(['src/target.ts']) @@ -209,7 +215,7 @@ describe('relay quick open ignored file listing', () => { failed.emit('error', Object.assign(new Error('spawn rg EAGAIN'), { code: 'EAGAIN' })) await vi.waitFor(() => expect(spawnMock).toHaveBeenCalledTimes(2)) - succeeded.stdout?.emit('data', 'src/index.ts\ndist/generated.js\n') + succeeded.stdout?.emit('data', 'src/index.ts\0dist/generated.js\0') succeeded.emit('close', 0, null) await expect(promise).resolves.toEqual(['src/index.ts', 'dist/generated.js']) @@ -305,18 +311,12 @@ describe('relay quick open ignored file listing', () => { const promise = listFilesWithGit(root, ['packages/other']) setTimeout(() => { - ;(primaryProc.stdout as unknown as EventEmitter).emit( - 'data', - `${staged('100644', 'src/index.ts')}\0` - ) - ;(primaryProc.stdout as unknown as EventEmitter).emit( - 'data', - `${staged('100644', 'tab\tfile.txt')}\0` - ) + primaryProc.stdout?.emit('data', `${staged('100644', 'src/index.ts')}\0`) + primaryProc.stdout?.emit('data', `${staged('100644', 'tab\tfile.txt')}\0`) primaryProc.emit('close', 0, null) - ;(ignoredProc.stdout as unknown as EventEmitter).emit('data', 'dist/\0') - ;(ignoredProc.stdout as unknown as EventEmitter).emit('data', 'packages/other/src/x.ts\0') + ignoredProc.stdout?.emit('data', 'dist/\0') + ignoredProc.stdout?.emit('data', 'packages/other/src/x.ts\0') ignoredProc.emit('close', 0, null) }, 10) @@ -346,7 +346,7 @@ describe('relay quick open ignored file listing', () => { spawnMock.mockImplementation(() => (++callIndex === 1 ? primaryProc : ignoredProc)) const promise = listFilesWithGit('/remote/root', [], { maxResults: 2 }) - ;(primaryProc.stdout as unknown as EventEmitter).emit('data', 'src/one.ts\0src/two.ts') + primaryProc.stdout?.emit('data', 'src/one.ts\0src/two.ts') primaryProc.emit('close', 0, null) await expect(promise).resolves.toEqual(['src/one.ts', 'src/two.ts']) expect(primaryProc.kill).toHaveBeenCalled() @@ -359,10 +359,7 @@ describe('relay quick open ignored file listing', () => { spawnMock.mockReturnValue(primaryProc) const promise = listFilesWithGit('/remote/root', [], { maxResults: 1 }) - ;(primaryProc.stdout as unknown as EventEmitter).emit( - 'data', - `discarded/\0${staged('100644', 'src/kept.ts')}\0` - ) + primaryProc.stdout?.emit('data', `discarded/\0${staged('100644', 'src/kept.ts')}\0`) await expect(promise).resolves.toEqual(['src/kept.ts']) expect(primaryProc.kill).toHaveBeenCalled() @@ -389,7 +386,7 @@ describe('relay quick open ignored file listing', () => { const promise = listFilesWithGit(root) setTimeout(() => { - ;(primaryProc.stdout as unknown as EventEmitter).emit( + primaryProc.stdout?.emit( 'data', `${staged('100644', 'README.md')}\0${staged('160000', 'packages/app')}\0packages/lib/\0` ) @@ -419,11 +416,11 @@ describe('relay quick open ignored file listing', () => { const promise = listFilesWithGit('/remote/root') setTimeout(() => { - ;(primaryProc.stdout as unknown as EventEmitter).emit('data', 'src/index.ts\0') + primaryProc.stdout?.emit('data', 'src/index.ts\0') primaryProc.emit('close', 0, null) // Entries streamed before the kill are kept alongside the primary pass. - ;(ignoredProc.stdout as unknown as EventEmitter).emit('data', 'dist/generated.js\0') + ignoredProc.stdout?.emit('data', 'dist/generated.js\0') ignoredProc.emit('close', null, 'SIGTERM') }, 10) @@ -449,7 +446,7 @@ describe('relay quick open ignored file listing', () => { const promise = listFilesWithGit('/remote/root') setTimeout(() => { - ;(primaryProc.stdout as unknown as EventEmitter).emit('data', 'src/index.ts\0') + primaryProc.stdout?.emit('data', 'src/index.ts\0') primaryProc.emit('close', 0, null) ignoredProc.emit('close', 128, null) @@ -475,10 +472,10 @@ describe('relay quick open ignored file listing', () => { const promise = listFilesWithGit('/remote/root') setTimeout(() => { - ;(primaryProc.stdout as unknown as EventEmitter).emit('data', 'src/index.ts\0') + primaryProc.stdout?.emit('data', 'src/index.ts\0') primaryProc.emit('close', null, 'SIGTERM') - ;(ignoredProc.stdout as unknown as EventEmitter).emit('data', 'dist/generated.js\0') + ignoredProc.stdout?.emit('data', 'dist/generated.js\0') ignoredProc.emit('close', 0, null) }, 10) @@ -500,7 +497,7 @@ describe('relay quick open ignored file listing', () => { setTimeout(() => { primaryProc.emit('close', 128, null) - ;(ignoredProc.stdout as unknown as EventEmitter).emit('data', 'dist/generated.js\0') + ignoredProc.stdout?.emit('data', 'dist/generated.js\0') ignoredProc.emit('close', 0, null) }, 10) @@ -733,12 +730,12 @@ describe('relay quick open ignored file listing', () => { expect(() => probe.emit('error', error)).not.toThrow() }) - it('keeps post-spawn rg search errors on the existing empty-result path', async () => { + it('rejects post-spawn rg search errors instead of returning an empty result', async () => { const started = createMockProcess() Object.defineProperty(started, 'pid', { value: 1 }) spawnMock.mockReturnValueOnce(started) const ordinaryFailure = searchWithRg('/remote/root', 'ok', { maxResults: 100 }) started.emit('error', new Error('post-spawn failure')) - await expect(ordinaryFailure).resolves.toMatchObject({ files: [], totalMatches: 0 }) + await expect(ordinaryFailure).rejects.toThrow('post-spawn failure') }) }) diff --git a/src/relay/fs-handler-list-files-path-framing.test.ts b/src/relay/fs-handler-list-files-path-framing.test.ts new file mode 100644 index 00000000000..037bbc97a41 --- /dev/null +++ b/src/relay/fs-handler-list-files-path-framing.test.ts @@ -0,0 +1,57 @@ +import { mkdtemp, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, expect, it, vi } from 'vitest' +import { rgPath } from '@vscode/ripgrep-universal' +import { configureRelayBundledRipgrep } from './relay-bundled-ripgrep' +import { listFilesWithRg } from './fs-handler-list-files' + +let root: string | undefined + +afterEach(async () => { + configureRelayBundledRipgrep(undefined) + vi.unstubAllEnvs() + if (root) { + await rm(root, { recursive: true, force: true }) + } +}) + +it('ignores user rg configuration when listing files', async () => { + root = await mkdtemp(join(tmpdir(), 'orca-rg-config-')) + configureRelayBundledRipgrep(rgPath) + const config = join(root, 'config') + await writeFile(config, '--glob\n!*.ts\n') + await writeFile(join(root, 'visible.ts'), '') + vi.stubEnv('RIPGREP_CONFIG_PATH', config) + + expect(await listFilesWithRg(root)).toContain('visible.ts') + expect(await listFilesWithRg(root, [], { maxResults: 10 })).toContain('visible.ts') +}) + +it.skipIf(process.platform === 'win32')( + 'preserves newline, carriage return and Unicode filenames', + async () => { + root = await mkdtemp(join(tmpdir(), 'orca-rg-filenames-')) + configureRelayBundledRipgrep(rgPath) + const names = ['first\nsecond.ts', 'trailing\r', 'résumé-😀.ts', 'spaces .ts '] + const fixtureRoot = root + await Promise.all(names.map((name) => writeFile(join(fixtureRoot, name), ''))) + + expect((await listFilesWithRg(root)).sort()).toEqual([...names].sort()) + expect((await listFilesWithRg(root, [], { maxResults: 10 })).sort()).toEqual([...names].sort()) + expect(await listFilesWithRg(root, [], { searchQuery: 'second', maxResults: 1 })).toEqual([ + 'first\nsecond.ts' + ]) + } +) + +it.skipIf(process.platform !== 'linux')( + 'rejects real non-UTF8 filenames instead of fabricating paths', + async () => { + root = await mkdtemp(join(tmpdir(), 'orca-rg-invalid-name-')) + configureRelayBundledRipgrep(rgPath) + const invalidPath = Buffer.concat([Buffer.from(join(root, 'bad-')), Buffer.from([0xff])]) + await writeFile(invalidPath, '') + await expect(listFilesWithRg(root)).rejects.toThrow('not valid UTF-8') + } +) diff --git a/src/relay/fs-handler-list-files.ts b/src/relay/fs-handler-list-files.ts index 506693fa0c8..ca25cd85e9e 100644 --- a/src/relay/fs-handler-list-files.ts +++ b/src/relay/fs-handler-list-files.ts @@ -1,3 +1,4 @@ +import { RipgrepFilenameDecoder } from '../shared/ripgrep-filename-decoder' /** * Ripgrep-based file listing for Quick Open. * Why a full rewrite vs. the older execFile+maxBuffer version: on a home-dir @@ -97,6 +98,10 @@ export function listFilesWithRg( new Promise((passResolve, passReject) => { const attemptRanker = searchQuery === undefined ? null : new QuickOpenPathRanker(searchQuery, maxResults ?? 16) + const filenameDecoder = new RipgrepFilenameDecoder((error) => { + killSpawnedRipgrepProcess(child) + rejectPass(error) + }) let passBuf = '' let passDone = false let passFileCount = 0 @@ -126,11 +131,9 @@ export function listFilesWithRg( ) : error } - let timer: ReturnType | null = null const cleanup = (): void => { if (timer) { clearTimeout(timer) - timer = null } child.stdout?.off('data', handleStdoutData) child.stderr?.off('data', handleStderrData) @@ -188,17 +191,21 @@ export function listFilesWithRg( } children.push({ child, isDone: () => passDone, reject: rejectPass }) - timer = setTimeout(() => { + const timer = setTimeout(() => { // Discard residual buffer on abnormal exit — a truncated byte // sequence could look like a valid path. killSpawnedRipgrepProcess(child) rejectPass(new Error('rg list timed out')) }, LIST_FILES_TIMEOUT_MS) - function handleStdoutData(chunk: string): void { - passBuf += chunk + function handleStdoutData(chunk: Buffer | string): void { + const decoded = filenameDecoder.decode(chunk) + if (decoded === null) { + return + } + passBuf += decoded let start = 0 - let idx = passBuf.indexOf('\n', start) + let idx = passBuf.indexOf('\0', start) while (idx !== -1) { if (processLine(passBuf.substring(start, idx), attemptRanker)) { passFileCount++ @@ -207,7 +214,7 @@ export function listFilesWithRg( return } start = idx + 1 - idx = passBuf.indexOf('\n', start) + idx = passBuf.indexOf('\0', start) } passBuf = start < passBuf.length ? passBuf.substring(start) : '' } @@ -248,6 +255,9 @@ export function listFilesWithRg( rejectPass(new Error(`rg killed by ${signal}`)) return } + if (!filenameDecoder.finish()) { + return + } // Flush residual line only on clean exit. if (passBuf) { if (processLine(passBuf, attemptRanker)) { @@ -259,16 +269,13 @@ export function listFilesWithRg( // (e.g. EACCES on .ssh), but rg also returns 2 for fatal errors // (bad flag, invalid glob). Only trust exit 2 when rg emitted at // least one parseable path — otherwise treat it as a real failure. - if (code === 0 || code === 1) { - resolvePass() - } else if (code === 2 && passFileCount > 0) { + if (code === 0 || code === 1 || (code === 2 && passFileCount > 0)) { resolvePass() } else { rejectPass(new Error(`rg exited with code ${code}`)) } } - 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-utils.ts b/src/relay/fs-handler-utils.ts index a3b2234ced0..ec2715c9b1a 100644 --- a/src/relay/fs-handler-utils.ts +++ b/src/relay/fs-handler-utils.ts @@ -1,3 +1,4 @@ +import { RipgrepSearchDiagnostics } from '../shared/ripgrep-search-diagnostics' /** * Pure helpers and child-process search utilities extracted from fs-handler.ts. * @@ -111,18 +112,15 @@ export function searchWithRg( return Promise.reject(abortSignalReason(signal)) } return new Promise((resolve, reject) => { - const rgArgs = buildRgArgs(query, rootPath, opts) + const rgArgs = buildRgArgs(query, '.', opts) const acc = createAccumulator() - const lines = new SearchSubprocessLineAccumulator(Number.MAX_SAFE_INTEGER) + const lines = new SearchSubprocessLineAccumulator() + const diagnostics = new RipgrepSearchDiagnostics() let resolved = false let processErrorObserved = false let unavailableExitObserved = false let launchFailureCheck: Promise | null = null - // Why: spawn can throw synchronously on invalid options (e.g. bad cwd), - // which would leak out of the `new Promise` executor and leave the - // promise forever pending. Treat a synchronous throw as a clean - // "no results" fallback, the same way an async 'error' event is handled. const resolvedRgCommand = resolveRelayRipgrepCommand() // Why not spawn a bare name when this is null: on Windows CreateProcessW searches the spawn // cwd -- the user's repo -- before PATH, so a planted rg.exe would run instead. @@ -142,8 +140,8 @@ export function searchWithRg( env, stdio: ['ignore', 'pipe', 'pipe'] }) - } catch { - resolve(finalize(acc)) + } catch (error) { + reject(error) return } @@ -181,9 +179,14 @@ export function searchWithRg( } } - function resolveOnce(): void { + function resolveOnce(code = 0, signal: NodeJS.Signals | null = null): void { if (settle()) { - resolve(finalize(acc)) + const error = diagnostics.failure(code, signal, acc) + if (error) { + reject(error) + } else { + resolve(finalize(acc)) + } } } @@ -235,11 +238,15 @@ export function searchWithRg( } function handleStdoutData(chunk: string): void { - lines.push(chunk, processLine) + if (!lines.push(chunk, processLine)) { + acc.truncated = true + killSpawnedRipgrepProcess(child) + resolveOnce() + } } - function handleStderrData(): void { - /* drain */ + function handleStderrData(chunk: Buffer): void { + diagnostics.append(chunk) } function handleError(error: Error): void { @@ -248,7 +255,10 @@ export function searchWithRg( settleLaunchFailure(error) return } - resolveOnce() + if (settle()) { + reject(error) + } + killSpawnedRipgrepProcess(child) } function handleClose(code: number | null, signal: NodeJS.Signals | null): void { @@ -261,11 +271,11 @@ export function searchWithRg( settleLaunchFailure() return } - const tail = lines.finish() + const tail = !signal && (code === 0 || code === 1) ? lines.finish() : null if (tail !== null) { processLine(tail) } - resolveOnce() + resolveOnce(code ?? -1, signal) } child.stdout?.setEncoding('utf-8') diff --git a/src/relay/fs-search-errors-real.test.ts b/src/relay/fs-search-errors-real.test.ts new file mode 100644 index 00000000000..eaf7fb9633c --- /dev/null +++ b/src/relay/fs-search-errors-real.test.ts @@ -0,0 +1,39 @@ +import { mkdtemp, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, expect, it } from 'vitest' +import { rgPath } from '@vscode/ripgrep-universal' +import { configureRelayBundledRipgrep } from './relay-bundled-ripgrep' +import { searchWithRg } from './fs-handler-utils' + +let root: string +beforeEach(async () => { + root = await mkdtemp(join(tmpdir(), 'orca-search-errors-')) + configureRelayBundledRipgrep(rgPath) + await writeFile(join(root, 'source.txt'), 'needle\n') +}) +afterEach(async () => { + configureRelayBundledRipgrep(undefined) + await rm(root, { recursive: true, force: true }) +}) + +it('reports an invalid regular expression as an error', async () => { + await expect(searchWithRg(root, '[', { useRegex: true, maxResults: 100 })).rejects.toThrow( + /regex parse error|unclosed character class/ + ) +}) + +it('distinguishes no matches from invalid syntax', async () => { + await expect(searchWithRg(root, 'absent', { maxResults: 100 })).resolves.toEqual({ + files: [], + totalMatches: 0, + truncated: false + }) +}) + +it('retains intentional capped results after stopping the process', async () => { + await writeFile(join(root, 'source.txt'), 'needle\n'.repeat(1000)) + const result = await searchWithRg(root, 'needle', { maxResults: 2 }) + expect(result.totalMatches).toBe(2) + expect(result.truncated).toBe(true) +}) diff --git a/src/relay/relay-bundled-ripgrep.test.ts b/src/relay/relay-bundled-ripgrep.test.ts index 5f667ad1429..93c4e56490f 100644 --- a/src/relay/relay-bundled-ripgrep.test.ts +++ b/src/relay/relay-bundled-ripgrep.test.ts @@ -104,7 +104,7 @@ describe('relay bundled ripgrep', () => { return child } const child = createProcess(42) - succeedWith(child, 'src/index.ts\n') + succeedWith(child, 'src/index.ts\0') return child }) diff --git a/src/relay/relay-bundled-ripgrep.ts b/src/relay/relay-bundled-ripgrep.ts index 787176e3f42..5c2dd89f3be 100644 --- a/src/relay/relay-bundled-ripgrep.ts +++ b/src/relay/relay-bundled-ripgrep.ts @@ -4,12 +4,13 @@ * the answer whenever that binary is absent or cannot launch on this host. */ import { existsSync, statSync } from 'node:fs' -import { delimiter, join, win32 } from 'node:path' +import { win32 } from 'node:path' import { isRipgrepSpawnCwdUsable, isTransientRipgrepSpawnError } from '../shared/ripgrep-process-availability' import { relayLogLine } from './relay-diagnostic-log' +import { buildRelayCommandEnv } from './relay-command-env' export const PATH_RIPGREP_COMMAND = 'rg' @@ -31,13 +32,13 @@ export function isDriveRootedWindowsPath(dir: string): boolean { // Why only rg.exe, though libuv honours %PATHEXT%: `.bat`/`.cmd` shims are not spawnable without // `shell: true`, and every ripgrep installer (winget, choco, scoop, cargo) lays down rg.exe. -function resolveWindowsPathRipgrep(): string | null { - for (const entry of (process.env.PATH ?? '').split(delimiter)) { +function resolveWindowsPathRipgrep(path: string): string | null { + for (const entry of path.split(';')) { const dir = entry.replace(/^"|"$/g, '').trim() if (!dir || !isDriveRootedWindowsPath(dir)) { continue } - const candidate = join(dir, 'rg.exe') + const candidate = win32.join(dir, 'rg.exe') try { if (statSync(candidate).isFile()) { return candidate @@ -49,7 +50,8 @@ function resolveWindowsPathRipgrep(): string | null { return null } -let windowsPathRipgrep: string | null | undefined +let windowsPathRipgrep: { path: string; command: string | null; expiresAt: number } | undefined +const WINDOWS_PATH_MISS_RETRY_MS = 60_000 let bundledRipgrepPath: string | null = null // Why a back-off, not forever: Windows AV often locks a just-installed rg.exe for its first spawns. @@ -69,13 +71,23 @@ export function pathRipgrepCommand(): string | null { if (process.platform !== 'win32') { return PATH_RIPGREP_COMMAND } - // Why the explicit undefined check and not `??=`: a miss resolves to null, which is nullish, so - // `??=` would re-walk every PATH entry on each call -- and a miss is the expensive case, since - // it stats every directory instead of stopping at the first hit. - if (windowsPathRipgrep === undefined) { - windowsPathRipgrep = resolveWindowsPathRipgrep() + const env = buildRelayCommandEnv() + const path = env.PATH ?? env.Path ?? '' + const now = Date.now() + // Cache the effective PATH; retry misses so an install can become visible without restarting. + if ( + !windowsPathRipgrep || + windowsPathRipgrep.path !== path || + now >= windowsPathRipgrep.expiresAt + ) { + const command = resolveWindowsPathRipgrep(path) + windowsPathRipgrep = { + path, + command, + expiresAt: command === null ? now + WINDOWS_PATH_MISS_RETRY_MS : Infinity + } } - return windowsPathRipgrep + return windowsPathRipgrep.command } /** Null means this host has no usable rg, so the caller falls back to git/readdir. */ diff --git a/src/relay/relay-ripgrep-cwd-env.test.ts b/src/relay/relay-ripgrep-cwd-env.test.ts index 38734ca63e6..c30590b4ad1 100644 --- a/src/relay/relay-ripgrep-cwd-env.test.ts +++ b/src/relay/relay-ripgrep-cwd-env.test.ts @@ -16,7 +16,7 @@ describe.runIf(process.platform !== 'win32')( writeFileSync(join(dir, 'found.txt'), 'needle') const cargo = join(dir, 'cargo') mkdirSync(join(cargo, 'bin'), { recursive: true }) - writeFileSync(join(cargo, 'bin', 'rg'), '#!/bin/sh\necho found.txt\n', { mode: 0o755 }) + writeFileSync(join(cargo, 'bin', 'rg'), "#!/bin/sh\nprintf 'found.txt\\0'\n", { mode: 0o755 }) vi.stubEnv('PATH', join(dir, 'empty-path')) vi.stubEnv('CARGO_HOME', cargo) configureRelayBundledRipgrep(undefined) diff --git a/src/relay/relay-windows-path-ripgrep-cache.test.ts b/src/relay/relay-windows-path-ripgrep-cache.test.ts new file mode 100644 index 00000000000..bea305bd741 --- /dev/null +++ b/src/relay/relay-windows-path-ripgrep-cache.test.ts @@ -0,0 +1,79 @@ +import type * as fs from 'node:fs' +import { statSync } from 'node:fs' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { pathRipgrepCommand, resetRelayRipgrepPathCacheForTests } from './relay-bundled-ripgrep' + +vi.mock('node:fs', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, statSync: vi.fn(actual.statSync) } +}) + +const originalPlatform = process.platform +const present = new Set() +const statMock = vi.mocked(statSync) +const fileStats = statSync(new URL(import.meta.url)) + +describe('Windows relay ripgrep effective PATH cache', () => { + beforeEach(() => { + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + vi.stubEnv('Path', undefined) + vi.stubEnv('PATH', 'C:\\tools') + vi.stubEnv('CARGO_HOME', 'C:\\cargo') + vi.useFakeTimers() + vi.setSystemTime(1000) + present.clear() + resetRelayRipgrepPathCacheForTests() + statMock.mockImplementation((path) => { + if (!present.has(String(path))) { + throw Object.assign(new Error('missing'), { code: 'ENOENT' }) + } + return fileStats + }) + statMock.mockClear() + }) + + afterEach(() => { + Object.defineProperty(process, 'platform', { configurable: true, value: originalPlatform }) + vi.unstubAllEnvs() + vi.useRealTimers() + statMock.mockReset() + resetRelayRipgrepPathCacheForTests() + }) + + it('finds rg in the Cargo fallback appended to the command environment', () => { + present.add('C:\\cargo\\bin\\rg.exe') + expect(pathRipgrepCommand()).toBe('C:\\cargo\\bin\\rg.exe') + const calls = statMock.mock.calls.length + expect(pathRipgrepCommand()).toBe('C:\\cargo\\bin\\rg.exe') + expect(statMock).toHaveBeenCalledTimes(calls) + }) + + it('supports mixed-case Path and never probes cwd-relative entries', () => { + vi.stubEnv('PATH', undefined) + vi.stubEnv('Path', '.;node_modules\\.bin;\\tools;C:tools;C:\\safe') + present.add('C:\\safe\\rg.exe') + expect(pathRipgrepCommand()).toBe('C:\\safe\\rg.exe') + expect(statMock.mock.calls.map(([path]) => path)).toEqual(['C:\\safe\\rg.exe']) + }) + + it('invalidates both successful and missing resolutions when effective PATH changes', () => { + expect(pathRipgrepCommand()).toBeNull() + vi.stubEnv('CARGO_HOME', 'D:\\cargo') + present.add('D:\\cargo\\bin\\rg.exe') + expect(pathRipgrepCommand()).toBe('D:\\cargo\\bin\\rg.exe') + vi.stubEnv('PATH', 'E:\\tools') + present.add('E:\\tools\\rg.exe') + expect(pathRipgrepCommand()).toBe('E:\\tools\\rg.exe') + }) + + it('retries cached misses after a bounded delay without rescanning on each call', () => { + expect(pathRipgrepCommand()).toBeNull() + const calls = statMock.mock.calls.length + present.add('C:\\tools\\rg.exe') + vi.advanceTimersByTime(59_999) + expect(pathRipgrepCommand()).toBeNull() + expect(statMock).toHaveBeenCalledTimes(calls) + vi.advanceTimersByTime(1) + expect(pathRipgrepCommand()).toBe('C:\\tools\\rg.exe') + }) +}) diff --git a/src/relay/relay-windows-path-ripgrep.test.ts b/src/relay/relay-windows-path-ripgrep.test.ts index a6dee0e14e8..aa8601fdab5 100644 --- a/src/relay/relay-windows-path-ripgrep.test.ts +++ b/src/relay/relay-windows-path-ripgrep.test.ts @@ -1,7 +1,7 @@ import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { delimiter, join } from 'node:path' -import { afterEach, describe, expect, it } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' import { configureRelayBundledRipgrep, isDriveRootedWindowsPath, @@ -9,6 +9,9 @@ import { resetRelayRipgrepPathCacheForTests } from './relay-bundled-ripgrep' +// These tests isolate PATH safety; effective-environment coverage lives in the cache suite. +vi.mock('./relay-command-env', () => ({ buildRelayCommandEnv: () => ({ ...process.env }) })) + const originalPlatform = process.platform const originalPath = process.env.PATH @@ -68,7 +71,7 @@ describe('relay PATH ripgrep resolution', () => { // while the spawn -- running with the user's repo as cwd -- resolves them against the repo's. it('skips rooted PATH entries that carry no drive', () => { setPlatform('win32') - process.env.PATH = `\\tools${delimiter}/tools${delimiter}C:tools` + process.env.PATH = '\\tools;/tools;C:tools' resetRelayRipgrepPathCacheForTests() expect(pathRipgrepCommand()).toBeNull() @@ -77,7 +80,7 @@ describe('relay PATH ripgrep resolution', () => { // Why a relative PATH entry is skipped: it resolves against the cwd, the hazard being avoided. it('ignores relative PATH entries on Windows', () => { setPlatform('win32') - process.env.PATH = `.${delimiter}node_modules/.bin` + process.env.PATH = '.;node_modules/.bin' resetRelayRipgrepPathCacheForTests() expect(pathRipgrepCommand()).toBeNull() diff --git a/src/renderer/src/components/editor/editor-external-watch-path-index.test.ts b/src/renderer/src/components/editor/editor-external-watch-path-index.test.ts index 68e33bc6baf..18bc3b6ce2c 100644 --- a/src/renderer/src/components/editor/editor-external-watch-path-index.test.ts +++ b/src/renderer/src/components/editor/editor-external-watch-path-index.test.ts @@ -22,6 +22,36 @@ function file(overrides: Partial & Pick): } describe('editor external watch path batch index', () => { + it('routes POSIX literal-backslash updates only to the matching tab', () => { + const scope = { worktreeId: 'wt-posix', worktreePath: '/repo', runtimeEnvironmentId: null } + const literal = file({ + id: 'literal', + worktreeId: scope.worktreeId, + filePath: '/repo/a\\b.txt', + relativePath: 'a\\b.txt' + }) + const nested = file({ + id: 'nested', + worktreeId: scope.worktreeId, + filePath: '/repo/a/b.txt', + relativePath: 'a/b.txt' + }) + const index = indexEditorExternalWatchBatchPaths( + { + worktreePath: scope.worktreePath, + events: [ + { kind: 'update', absolutePath: literal.filePath }, + { kind: 'update', absolutePath: nested.filePath } + ] + }, + [literal, nested], + scope + ) + expect(index.changes.map((change) => change.relativePath)).toEqual(['a\\b.txt', 'a/b.txt']) + expect(index.matchingOpenFiles(index.changes[0])).toEqual([literal]) + expect(index.matchingOpenFiles(index.changes[1])).toEqual([nested]) + }) + it('matches UNC aliases for updates, deletes, and restored tombstones', () => { const restored = file({ id: 'restored', diff --git a/src/renderer/src/components/right-sidebar/SearchResultsPane.tsx b/src/renderer/src/components/right-sidebar/SearchResultsPane.tsx index b1fc60b7d5e..f0b5b27d486 100644 --- a/src/renderer/src/components/right-sidebar/SearchResultsPane.tsx +++ b/src/renderer/src/components/right-sidebar/SearchResultsPane.tsx @@ -13,6 +13,7 @@ const SEARCH_VIRTUAL_OVERSCAN = 12 type SearchResultsPaneProps = { results: SearchResult | null + error?: string | null hasCommittedResults: boolean query: string loading: boolean @@ -25,6 +26,7 @@ type SearchResultsPaneProps = { export function SearchResultsPane({ results, hasCommittedResults, + error, query, loading, rows, @@ -68,7 +70,7 @@ export function SearchResultsPane({ <> {/* Why: the summary is rendered outside the virtualizer so it stays pinned at the top while the user scrolls through results. */} - {results && rows.length > 0 && ( + {!error && results && (rows.length > 0 || results.truncated) && (
{results.totalMatches}{' '} {translate('auto.components.right.sidebar.Search.6aeda362ed', 'result')} @@ -83,7 +85,15 @@ export function SearchResultsPane({ )}
- {rows.length > 0 && ( + {error && ( +
+ {error} +
+ )} + {!error && rows.length > 0 && (
{virtualizer.getVirtualItems().map((virtualRow) => { const row = rows[virtualRow.index] @@ -117,7 +127,7 @@ export function SearchResultsPane({
)} - {!hasCommittedResults && query && !loading && ( + {!error && !hasCommittedResults && query && !loading && (
{translate('auto.components.right.sidebar.Search.d56d140747', 'Press Enter to search')}
diff --git a/src/renderer/src/components/right-sidebar/file-explorer-directory-filename-identity.test.ts b/src/renderer/src/components/right-sidebar/file-explorer-directory-filename-identity.test.ts new file mode 100644 index 00000000000..3dbc25998d2 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/file-explorer-directory-filename-identity.test.ts @@ -0,0 +1,54 @@ +import { describe, expect, it } from 'vitest' +import { fileExplorerEntriesToTreeNodes } from './file-explorer-directory-listing' + +describe('Explorer directory filename identity', () => { + it.each(['/', '/repo/', 'C:\\', 'C:\\repo\\', '\\\\server\\share\\'])( + 'preserves the first filename character when the root ends in a separator: %s', + (root) => { + const [node] = fileExplorerEntriesToTreeNodes( + [{ name: 'abc.txt', isDirectory: false, isSymlink: false }], + root, + -1, + root, + { kind: 'local' } + ) + expect(node.relativePath).toBe('abc.txt') + } + ) + + it.each(['/repo', '/ssh/repo\\root'])( + 'keeps literal and nested POSIX paths distinct under %s', + (root) => { + const literal = fileExplorerEntriesToTreeNodes( + [{ name: 'a\\b.txt', isDirectory: false, isSymlink: false }], + root, + -1, + root, + { kind: 'local' } + )[0] + const nested = fileExplorerEntriesToTreeNodes( + [{ name: 'b.txt', isDirectory: false, isSymlink: false }], + `${root}/a`, + 0, + root, + { kind: 'local' } + )[0] + expect(literal).toMatchObject({ path: `${root}/a\\b.txt`, relativePath: 'a\\b.txt' }) + expect(nested).toMatchObject({ path: `${root}/a/b.txt`, relativePath: 'a/b.txt' }) + } + ) + + it.each(['C:\\repo', '\\\\server\\share\\repo'])( + 'keeps Windows relative paths canonical under %s', + (root) => { + const [node] = fileExplorerEntriesToTreeNodes( + [{ name: 'b.txt', isDirectory: false, isSymlink: false }], + `${root}\\a`, + 0, + root, + { kind: 'local' } + ) + expect(node).toMatchObject({ path: `${root}\\a\\b.txt`, relativePath: 'a/b.txt' }) + } + ) +}) diff --git a/src/renderer/src/components/right-sidebar/file-explorer-directory-listing.ts b/src/renderer/src/components/right-sidebar/file-explorer-directory-listing.ts index d904c910e0b..e94a32e3fa5 100644 --- a/src/renderer/src/components/right-sidebar/file-explorer-directory-listing.ts +++ b/src/renderer/src/components/right-sidebar/file-explorer-directory-listing.ts @@ -1,4 +1,4 @@ -import { joinPath, normalizeRelativePath } from '@/lib/path' +import { getRelativePathInsideRoot, joinPath } from '@/lib/path' import type { DirEntry } from '../../../../shared/filesystem-entry-types' import { sortDirEntries } from '../../../../shared/file-name-sort' import { readRuntimeDirectory } from '@/runtime/runtime-file-client' @@ -27,9 +27,7 @@ export function fileExplorerEntriesToTreeNodes( return { name: entry.name, path, - relativePath: worktreePath - ? normalizeRelativePath(path.slice(worktreePath.length + 1)) - : entry.name, + relativePath: getRelativePathInsideRoot(path, worktreePath) ?? entry.name, isDirectory: entry.isDirectory, isSymlink: entry.isSymlink, depth: depth + 1, diff --git a/src/renderer/src/components/right-sidebar/file-explorer-entries.ts b/src/renderer/src/components/right-sidebar/file-explorer-entries.ts index f21116c7822..d909224675b 100644 --- a/src/renderer/src/components/right-sidebar/file-explorer-entries.ts +++ b/src/renderer/src/components/right-sidebar/file-explorer-entries.ts @@ -1,4 +1,5 @@ import type { DirEntry } from '../../../../shared/filesystem-entry-types' +import { splitPathSegments } from './path-tree' export function shouldIncludeFileExplorerEntry(entry: DirEntry): boolean { return entry.name !== '.git' && entry.name !== 'node_modules' @@ -8,6 +9,6 @@ function isDotfileSegment(segment: string): boolean { return segment.length > 1 && segment !== '..' && segment.startsWith('.') } -export function isDotfileRelativePath(relativePath: string): boolean { - return relativePath.split(/[\\/]+/).some(isDotfileSegment) +export function isDotfileRelativePath(relativePath: string, rootPath?: string | null): boolean { + return splitPathSegments(relativePath, rootPath).some(isDotfileSegment) } diff --git a/src/renderer/src/components/right-sidebar/file-explorer-name-filter-projection.test.ts b/src/renderer/src/components/right-sidebar/file-explorer-name-filter-projection.test.ts index 6075b8c36a5..f883ec8544a 100644 --- a/src/renderer/src/components/right-sidebar/file-explorer-name-filter-projection.test.ts +++ b/src/renderer/src/components/right-sidebar/file-explorer-name-filter-projection.test.ts @@ -1,8 +1,11 @@ import { describe, expect, it } from 'vitest' import { + createNameFilteredFileExplorerProjection, + getFileExplorerNameFilterIgnoredQueryRelativePaths, getNameFilterCollapsedPathsAfterExpand, getNextNameFilterCollapsedPaths } from './file-explorer-name-filter-projection' +import { buildIgnoredSet } from './status-display' describe('getNextNameFilterCollapsedPaths', () => { it('collapses expanded filtered folders and expands collapsed filtered folders', () => { @@ -22,3 +25,73 @@ describe('getNextNameFilterCollapsedPaths', () => { expect([...expanded]).toEqual(['/repo/docs']) }) }) + +describe('name-filter filename identity', () => { + function project( + worktreePath: string, + relativePaths: string[], + options: { displayRootPath?: string; ignoredPaths?: string[]; showDotfiles?: boolean } = {} + ) { + return createNameFilteredFileExplorerProjection({ + worktreePath, + displayRootPath: options.displayRootPath, + nameFilter: { query: 'txt', relativePaths }, + ignoredSet: buildIgnoredSet(options.ignoredPaths, worktreePath), + showDotfiles: options.showDotfiles ?? true, + showGitIgnoredFiles: false + }).getVisibleSlice(0, 100) + } + + it.each(['/native/repo', '/ssh/repo\\root'])( + 'keeps literal backslashes separate from directories under %s', + (root) => { + const paths = ['a\\b.txt', 'a/b.txt', 'C:\\foo/a\\b.txt', '\\\\server/a\\b.txt'] + const files = project(root, paths).filter((row) => !row.isDirectory) + expect(files.map((row) => row.relativePath).sort()).toEqual([...paths].sort()) + expect(files.map((row) => row.path).sort()).toEqual( + paths.map((path) => `${root}/${path}`).sort() + ) + } + ) + + it('scopes a literal-backslash directory without selecting its nested counterpart', () => { + const rows = project('/repo', ['a\\b/file.txt', 'a/b/other.txt'], { + displayRootPath: '/repo/a\\b' + }) + expect(rows.map((row) => row.relativePath)).toEqual(['a\\b/file.txt']) + }) + + it('keeps ignored and dotfile identities distinct on POSIX', () => { + const paths = ['a\\b.txt', 'a/b.txt', 'a\\.hidden.txt', 'a/.hidden.txt'] + const rows = project('/repo', paths, { ignoredPaths: ['a/b.txt'], showDotfiles: false }) + expect(rows.map((row) => row.relativePath).sort()).toEqual(['a\\.hidden.txt', 'a\\b.txt']) + expect( + getFileExplorerNameFilterIgnoredQueryRelativePaths( + { query: 'txt', relativePaths: paths }, + false, + '/repo' + ) + ).toEqual(['a\\b.txt', 'a/b.txt', 'a\\.hidden.txt']) + expect( + project('/repo', paths, { ignoredPaths: ['a\\b.txt'] }).map((row) => row.relativePath) + ).not.toContain('a\\b.txt') + }) + + it.each(['C:\\repo', '\\\\server\\share\\repo'])( + 'keeps Windows separator semantics under %s', + (root) => { + const paths = ['a\\b.txt', 'a/b.txt', 'a\\.hidden.txt'] + const files = project(root, paths, { showDotfiles: false }).filter((row) => !row.isDirectory) + expect(files.map((row) => row.relativePath)).toEqual(['a/b.txt']) + expect(files.map((row) => row.path)).toEqual([`${root}\\a\\b.txt`]) + expect(project(root, paths, { ignoredPaths: ['a\\'], showDotfiles: false })).toEqual([]) + expect( + getFileExplorerNameFilterIgnoredQueryRelativePaths( + { query: 'txt', relativePaths: paths }, + false, + root + ) + ).toEqual(['a/b.txt', 'a/b.txt']) + } + ) +}) diff --git a/src/renderer/src/components/right-sidebar/file-explorer-name-filter-projection.ts b/src/renderer/src/components/right-sidebar/file-explorer-name-filter-projection.ts index b8d9256219e..5aa8b3c73e9 100644 --- a/src/renderer/src/components/right-sidebar/file-explorer-name-filter-projection.ts +++ b/src/renderer/src/components/right-sidebar/file-explorer-name-filter-projection.ts @@ -65,7 +65,8 @@ export function getFileExplorerNameFilterTokens(query: string | undefined): stri export function getFileExplorerNameFilterIgnoredQueryRelativePaths( source: FileExplorerNameFilterProjectionSource, - showDotfiles: boolean + showDotfiles: boolean, + worktreePath?: string | null ): string[] { if (isFileExplorerNameFilterQueryTooLarge(source.query)) { return [] @@ -75,11 +76,11 @@ export function getFileExplorerNameFilterIgnoredQueryRelativePaths( } const tokens = getFileExplorerNameFilterTokens(source.query) return source.relativePaths - .map((relativePath) => normalizeRelativePath(relativePath)) + .map((relativePath) => normalizeRelativePath(relativePath, worktreePath)) .filter( (relativePath) => Boolean(relativePath) && - (showDotfiles || !isDotfileRelativePath(relativePath)) && + (showDotfiles || !isDotfileRelativePath(relativePath, worktreePath)) && pathMatchesFileNameFilterTokens(relativePath, tokens) ) } @@ -139,14 +140,14 @@ export function createNameFilteredFileExplorerProjection({ const rootChildren = new Map() for (const rawRelativePath of nameFilter.relativePaths) { - const relativePath = normalizeRelativePath(rawRelativePath) + const relativePath = normalizeRelativePath(rawRelativePath, worktreePath) if ( !relativePath || getRelativePathInsideRoot(joinPath(worktreePath, relativePath), displayRootPath) === null ) { continue } - if (!showDotfiles && isDotfileRelativePath(relativePath)) { + if (!showDotfiles && isDotfileRelativePath(relativePath, worktreePath)) { continue } if (!showGitIgnoredFiles && isPathIgnored(ignoredSet, relativePath)) { @@ -156,12 +157,12 @@ export function createNameFilteredFileExplorerProjection({ continue } - const segments = splitPathSegments(relativePath) + const segments = splitPathSegments(relativePath, worktreePath) let currentChildren = rootChildren let currentRelativePath = '' for (let index = 0; index < segments.length; index += 1) { const name = segments[index] - currentRelativePath = currentRelativePath ? joinPath(currentRelativePath, name) : name + currentRelativePath = currentRelativePath ? `${currentRelativePath}/${name}` : name const isDirectory = index < segments.length - 1 let entry = currentChildren.get(name) if (!entry) { @@ -186,7 +187,7 @@ export function createNameFilteredFileExplorerProjection({ let displayChildren = rootChildren const scope = getRelativePathInsideRoot(displayRootPath, worktreePath) - for (const segment of scope ? splitPathSegments(scope) : []) { + for (const segment of scope ? splitPathSegments(scope, worktreePath) : []) { const entry = displayChildren.get(segment) if (!entry) { return createFileExplorerRowProjectionFromParts(visibleFlatRows, rowsByPath) diff --git a/src/renderer/src/components/right-sidebar/file-explorer-paths.test.ts b/src/renderer/src/components/right-sidebar/file-explorer-paths.test.ts index bbc373d6ad9..fa96214d0a4 100644 --- a/src/renderer/src/components/right-sidebar/file-explorer-paths.test.ts +++ b/src/renderer/src/components/right-sidebar/file-explorer-paths.test.ts @@ -1,15 +1,7 @@ import { describe, expect, it } from 'vitest' -import { - getRevealAncestorDirs, - isPathEqualOrDescendant, - normalizeAbsolutePath -} from './file-explorer-paths' +import { getRevealAncestorDirs, isPathEqualOrDescendant } from './file-explorer-paths' describe('file explorer path helpers', () => { - it('preserves UNC roots while normalizing separators', () => { - expect(normalizeAbsolutePath('\\\\Server\\Share\\Repo\\')).toBe('//Server/Share/Repo') - }) - it('matches Windows drive paths case-insensitively with segment boundaries', () => { expect(isPathEqualOrDescendant('c:\\repo\\src\\a.ts', 'C:\\Repo')).toBe(true) expect(isPathEqualOrDescendant('C:\\Repository\\src\\a.ts', 'C:\\Repo')).toBe(false) @@ -42,4 +34,19 @@ describe('file explorer path helpers', () => { 'C:\\repo\\src' ]) }) + + it.each(['/repo', '/ssh/repo\\root'])( + 'reveals POSIX literal-backslash names without inventing directories under %s', + (root) => { + expect(getRevealAncestorDirs(root, `${root}/a\\b.txt`)).toEqual([]) + expect(getRevealAncestorDirs(root, `${root}/a/b.txt`)).toEqual([`${root}/a`]) + expect(getRevealAncestorDirs(root, `${root}/a\\b/c\\d.txt`)).toEqual([`${root}/a\\b`]) + } + ) + + it('reveals Windows UNC paths with native separators', () => { + expect( + getRevealAncestorDirs('\\\\server\\share\\repo', '\\\\server\\share\\repo\\a\\b.txt') + ).toEqual(['\\\\server\\share\\repo\\a']) + }) }) diff --git a/src/renderer/src/components/right-sidebar/file-explorer-paths.ts b/src/renderer/src/components/right-sidebar/file-explorer-paths.ts index 1c2fa044117..ce2f64d8fc3 100644 --- a/src/renderer/src/components/right-sidebar/file-explorer-paths.ts +++ b/src/renderer/src/components/right-sidebar/file-explorer-paths.ts @@ -1,26 +1,11 @@ -import { joinPath, normalizeRelativePath } from '@/lib/path' +import { joinPath } from '@/lib/path' import { isPathInsideOrEqual, normalizeRuntimePathForComparison, - normalizeRuntimePathSeparators, relativePathInsideRoot } from '../../../../shared/cross-platform-path' import { splitPathSegments } from './path-tree' -export function normalizeAbsolutePath(path: string): string { - const normalizedPath = normalizeRuntimePathSeparators(path) - - if (normalizedPath === '/') { - return normalizedPath - } - - if (/^[A-Za-z]:\/$/.test(normalizedPath)) { - return normalizedPath - } - - return normalizedPath.replace(/\/+$/, '') -} - export function normalizeAbsolutePathForComparison(path: string): string { return normalizeRuntimePathForComparison(path) } @@ -35,7 +20,7 @@ export function getRevealAncestorDirs(worktreePath: string, filePath: string): s return null } - const segments = splitPathSegments(normalizeRelativePath(relativePath)) + const segments = splitPathSegments(relativePath, worktreePath) const ancestorDirs: string[] = [] let currentPath = worktreePath diff --git a/src/renderer/src/components/right-sidebar/file-explorer-watch-filename-identity.test.ts b/src/renderer/src/components/right-sidebar/file-explorer-watch-filename-identity.test.ts new file mode 100644 index 00000000000..f28474f5913 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/file-explorer-watch-filename-identity.test.ts @@ -0,0 +1,45 @@ +import { describe, expect, it } from 'vitest' +import { useAppStore } from '@/store' +import { clearStalePendingReveal } from './file-explorer-watcher-reconcile' +import { + canonicalizeFileExplorerWatchPath, + getExternalFileChangeRelativePath, + normalizeExplorerAbsolutePath, + parentDirForWatchPath +} from './file-explorer-watch-path' + +describe('Explorer watcher filename identity', () => { + it('does not clear a nested pending reveal when a distinct literal-backslash path is deleted', () => { + const previous = useAppStore.getState().pendingExplorerReveal + const pending = { worktreeId: 'wt-posix', filePath: '/repo/a/b/file.txt', requestId: 1 } + try { + useAppStore.setState({ pendingExplorerReveal: pending }) + clearStalePendingReveal('/repo/a\\b') + expect(useAppStore.getState().pendingExplorerReveal).toEqual(pending) + clearStalePendingReveal('/repo/a/b') + expect(useAppStore.getState().pendingExplorerReveal).toBeNull() + } finally { + useAppStore.setState({ pendingExplorerReveal: previous }) + } + }) + + it.each(['/repo', '/ssh/repo\\'])( + 'preserves POSIX backslashes in watched roots, filenames and parents under %s', + (root) => { + expect(normalizeExplorerAbsolutePath(`${root}/`)).toBe(root) + expect(canonicalizeFileExplorerWatchPath(root, `${root}/a\\b.txt`)).toBe(`${root}/a\\b.txt`) + expect(getExternalFileChangeRelativePath(root, `${root}/a\\b.txt`, false)).toBe('a\\b.txt') + expect(parentDirForWatchPath(`${root}/a\\b.txt`)).toBe(root) + expect(parentDirForWatchPath(`${root}/a\\/b.txt`)).toBe(`${root}/a\\`) + } + ) + + it.each(['C:\\repo', '\\\\server\\share\\repo'])( + 'retains Windows watcher separator semantics under %s', + (root) => { + expect(normalizeExplorerAbsolutePath(`${root}\\`)).toBe(root) + expect(getExternalFileChangeRelativePath(root, `${root}\\a\\b.txt`, false)).toBe('a/b.txt') + expect(parentDirForWatchPath(`${root}\\a\\b.txt`)).toBe(`${root}\\a`) + } + ) +}) diff --git a/src/renderer/src/components/right-sidebar/file-explorer-watch-path.ts b/src/renderer/src/components/right-sidebar/file-explorer-watch-path.ts index 4066bca2d81..9988d9a9387 100644 --- a/src/renderer/src/components/right-sidebar/file-explorer-watch-path.ts +++ b/src/renderer/src/components/right-sidebar/file-explorer-watch-path.ts @@ -1,11 +1,14 @@ import { joinPath, dirname, normalizeRelativePath } from '@/lib/path' import { + isWindowsAbsolutePathLike, normalizeRuntimePathForComparison, relativePathInsideRoot } from '../../../../shared/cross-platform-path' export function normalizeExplorerAbsolutePath(path: string): string { - return path === '/' || /^[A-Za-z]:[\\/]$/.test(path) ? path : path.replace(/[\\/]+$/, '') + return path === '/' || /^[A-Za-z]:[\\/]$/.test(path) + ? path + : path.replace(isWindowsAbsolutePathLike(path) ? /[\\/]+$/ : /\/+$/, '') } export function getExternalFileChangeRelativePath( @@ -23,7 +26,7 @@ export function getExternalFileChangeRelativePath( } // Why: EditorPanel reloads tabs only from a worktree-relative path, not the watcher's absolute one; normalize or contents go stale. - return normalizeRelativePath(relativePath) + return normalizeRelativePath(relativePath, worktreePath) } export function canonicalizeFileExplorerWatchPath( @@ -87,6 +90,10 @@ export function resolveCachedDirPath( } export function parentDirForWatchPath(normalizedPath: string): string { + if (!isWindowsAbsolutePathLike(normalizedPath)) { + const separator = normalizedPath.lastIndexOf('/') + return separator === -1 ? '.' : normalizedPath.slice(0, separator) || '/' + } const parentPath = dirname(normalizedPath) if (/^[A-Za-z]:$/.test(parentPath)) { return `${parentPath}${normalizedPath.includes('\\') ? '\\' : '/'}` diff --git a/src/renderer/src/components/right-sidebar/file-explorer-watcher-reconcile.ts b/src/renderer/src/components/right-sidebar/file-explorer-watcher-reconcile.ts index 0f151c54295..6072cdb201f 100644 --- a/src/renderer/src/components/right-sidebar/file-explorer-watcher-reconcile.ts +++ b/src/renderer/src/components/right-sidebar/file-explorer-watcher-reconcile.ts @@ -1,6 +1,6 @@ import type { Dispatch, SetStateAction } from 'react' import type { DirCache } from './file-explorer-types' -import { normalizeAbsolutePath, isPathEqualOrDescendant } from './file-explorer-paths' +import { isPathEqualOrDescendant } from './file-explorer-paths' import { useAppStore } from '@/store' import { normalizeRuntimePathForComparison } from '../../../../shared/cross-platform-path' @@ -103,14 +103,10 @@ export function purgeExpandedDirsSubtrees( // would cause the reveal logic to expand stale ancestor directories. export function clearStalePendingReveal(deletedPath: string): void { - const normalized = normalizeAbsolutePath(deletedPath) useAppStore.setState((state) => { if ( state.pendingExplorerReveal && - isPathEqualOrDescendant( - normalizeAbsolutePath(state.pendingExplorerReveal.filePath), - normalized - ) + isPathEqualOrDescendant(state.pendingExplorerReveal.filePath, deletedPath) ) { return { pendingExplorerReveal: null } } diff --git a/src/renderer/src/components/right-sidebar/path-tree.ts b/src/renderer/src/components/right-sidebar/path-tree.ts index 133d2ec50a7..c983d4d4859 100644 --- a/src/renderer/src/components/right-sidebar/path-tree.ts +++ b/src/renderer/src/components/right-sidebar/path-tree.ts @@ -1,3 +1,5 @@ -export function splitPathSegments(path: string): string[] { - return path.split(/[\\/]+/).filter(Boolean) +import { normalizeRelativePath } from '@/lib/path' + +export function splitPathSegments(path: string, rootPath?: string | null): string[] { + return normalizeRelativePath(path, rootPath).split('/').filter(Boolean) } diff --git a/src/renderer/src/components/right-sidebar/status-display.ts b/src/renderer/src/components/right-sidebar/status-display.ts index 2af97edd531..978d4cc98aa 100644 --- a/src/renderer/src/components/right-sidebar/status-display.ts +++ b/src/renderer/src/components/right-sidebar/status-display.ts @@ -124,14 +124,17 @@ export function shouldShowIgnoredDecoration( return !nodeStatus && isPathIgnored(ignored, relativePath) } -export function buildIgnoredSet(ignoredPaths: readonly string[] | undefined): Set { +export function buildIgnoredSet( + ignoredPaths: readonly string[] | undefined, + rootPath?: string | null +): Set { const set = new Set() if (!ignoredPaths) { return set } for (const rawPath of ignoredPaths) { - const trimmed = rawPath.endsWith('/') ? rawPath.slice(0, -1) : rawPath - set.add(normalizeRelativePath(trimmed)) + const path = normalizeRelativePath(rawPath, rootPath) + set.add(path.endsWith('/') ? path.slice(0, -1) : path) } return set } diff --git a/src/renderer/src/components/right-sidebar/useFileExplorerVisibleRowProjection.ts b/src/renderer/src/components/right-sidebar/useFileExplorerVisibleRowProjection.ts index afefa839b41..8c0e039b187 100644 --- a/src/renderer/src/components/right-sidebar/useFileExplorerVisibleRowProjection.ts +++ b/src/renderer/src/components/right-sidebar/useFileExplorerVisibleRowProjection.ts @@ -49,7 +49,7 @@ export function getFileExplorerIgnoredQueryRelativePaths( return } for (const row of cached.children) { - if (!showDotfiles && isDotfileRelativePath(row.relativePath)) { + if (!showDotfiles && isDotfileRelativePath(row.relativePath, worktreePath)) { continue } relativePaths.push(row.relativePath) @@ -88,7 +88,7 @@ export function createVisibleFileExplorerRowProjection( } const shouldHideRow = (row: TreeNode): boolean => { - if (!options.showDotfiles && isDotfileRelativePath(row.relativePath)) { + if (!options.showDotfiles && isDotfileRelativePath(row.relativePath, worktreePath)) { return true } return !options.showGitIgnoredFiles && isPathIgnored(options.ignoredSet, row.relativePath) @@ -179,7 +179,11 @@ export function useFileExplorerVisibleRowProjection( () => activeRepoSupportsGit ? nameFilter - ? getFileExplorerNameFilterIgnoredQueryRelativePaths(nameFilter, showDotfiles) + ? getFileExplorerNameFilterIgnoredQueryRelativePaths( + nameFilter, + showDotfiles, + worktreePath + ) : getFileExplorerIgnoredQueryRelativePaths( { dirCache, expanded, worktreePath, displayRootPath }, showDotfiles @@ -211,7 +215,10 @@ export function useFileExplorerVisibleRowProjection( shouldDebounceIgnoredQuery, worktreePath }) - const ignoredSet = useMemo(() => buildIgnoredSet(effectiveIgnoredPaths), [effectiveIgnoredPaths]) + const ignoredSet = useMemo( + () => buildIgnoredSet(effectiveIgnoredPaths, worktreePath), + [effectiveIgnoredPaths, worktreePath] + ) const rowProjection = useMemo( () => createVisibleFileExplorerRowProjection( diff --git a/src/renderer/src/components/right-sidebar/useFileSearchPanel.ts b/src/renderer/src/components/right-sidebar/useFileSearchPanel.ts index e66e647c26f..a1ff15bf55b 100644 --- a/src/renderer/src/components/right-sidebar/useFileSearchPanel.ts +++ b/src/renderer/src/components/right-sidebar/useFileSearchPanel.ts @@ -21,6 +21,7 @@ export type FileSearchPanelModel = { filtersProps: SearchFiltersProps resultsProps: { results: SearchResult | null + error?: string | null hasCommittedResults: boolean query: string loading: boolean @@ -132,7 +133,7 @@ export function useFileSearchPanel(explorerView: 'files' | 'search'): FileSearch useEffect(() => { if (!worktreePath) { cancelPendingSearch() - updateActiveSearchState({ results: null, resultOwner: null }) + updateActiveSearchState({ results: null, resultOwner: null, error: null }) } }, [worktreePath, cancelPendingSearch, updateActiveSearchState]) @@ -281,6 +282,7 @@ export function useFileSearchPanel(explorerView: 'files' | 'search'): FileSearch }, resultsProps: { results: deferredSearchResults.results, + error: searchState?.error, hasCommittedResults: fileSearchResults !== null, query: fileSearchQuery, loading: fileSearchLoading, diff --git a/src/renderer/src/components/right-sidebar/useFileSearchRunner.test.tsx b/src/renderer/src/components/right-sidebar/useFileSearchRunner.test.tsx index c68f5effa4e..5615a0e4ab8 100644 --- a/src/renderer/src/components/right-sidebar/useFileSearchRunner.test.tsx +++ b/src/renderer/src/components/right-sidebar/useFileSearchRunner.test.tsx @@ -163,4 +163,53 @@ describe('useFileSearchRunner result ownership', () => { resultOwner: { worktreeId, runtimeEnvironmentId: null } }) }) + + it('shows an active failure and clears it when the next search succeeds or is empty', async () => { + const worktreeId = 'missing-repo::/repo' + const { hook, updates } = renderSearchRunner( + { settings: {}, repos: [], worktreesByRepo: {}, fileSearchStateByWorktree: {} }, + worktreeId + ) + const log = vi.spyOn(console, 'error').mockImplementation(() => {}) + mocks.searchRuntimeFiles.mockRejectedValueOnce( + new Error("Error invoking remote method 'search': Error: regex parse error\nUnclosed group") + ) + await finishSearch(hook.result.current.executeSearch) + expect(Object.assign({}, ...updates)).toMatchObject({ + error: 'regex parse error\nUnclosed group', + results: null, + loading: false + }) + + await finishSearch(hook.result.current.executeSearch) + expect(Object.assign({}, ...updates)).toMatchObject({ error: null, results: RESULTS }) + act(() => hook.result.current.executeSearch('')) + expect(Object.assign({}, ...updates)).toMatchObject({ error: null, results: null }) + log.mockRestore() + }) + + it('ignores an older rejection after a newer search has succeeded', async () => { + const { hook, updates } = renderSearchRunner( + { settings: {}, repos: [], worktreesByRepo: {}, fileSearchStateByWorktree: {} }, + 'missing-repo::/repo' + ) + const log = vi.spyOn(console, 'error').mockImplementation(() => {}) + let rejectOldSearch: (reason: Error) => void = () => {} + mocks.searchRuntimeFiles.mockImplementationOnce( + () => + new Promise((_resolve, reject) => { + rejectOldSearch = reject + }) + ) + await finishSearch(hook.result.current.executeSearch) + await finishSearch(hook.result.current.executeSearch) + await act(async () => rejectOldSearch(new Error('old failure'))) + expect(Object.assign({}, ...updates)).toMatchObject({ + error: null, + results: RESULTS, + loading: false + }) + expect(updates.some((update) => update.error === 'old failure')).toBe(false) + log.mockRestore() + }) }) diff --git a/src/renderer/src/components/right-sidebar/useFileSearchRunner.ts b/src/renderer/src/components/right-sidebar/useFileSearchRunner.ts index 7950a86114f..24d44d7a3aa 100644 --- a/src/renderer/src/components/right-sidebar/useFileSearchRunner.ts +++ b/src/renderer/src/components/right-sidebar/useFileSearchRunner.ts @@ -1,3 +1,5 @@ +import { readIpcErrorDetail } from '@/lib/ipc-error' +import { translate } from '@/i18n/i18n' import { useCallback, useEffect, useRef } from 'react' import { getConnectionId } from '@/lib/connection-context' import { @@ -17,6 +19,7 @@ const SEARCH_DEBOUNCE_MS = 300 const SEARCH_MAX_RESULTS = 2000 type UpdateSearchState = (updates: { + error?: string | null loading?: boolean results?: SearchResult | null resultOwner?: FileSearchResultOwner | null @@ -54,6 +57,7 @@ export function useFileSearchRunner({ (query: string) => { latestSearchIdRef.current += 1 const searchId = latestSearchIdRef.current + updateActiveSearchState({ error: null }) if (searchTimerRef.current) { clearTimeout(searchTimerRef.current) @@ -138,7 +142,10 @@ export function useFileSearchRunner({ console.error('Search failed:', err) if (latestSearchIdRef.current === searchId) { updateActiveSearchState({ - results: { files: [], totalMatches: 0, truncated: false }, + results: null, + error: + readIpcErrorDetail(err) ?? + translate('fileSearch.failed', 'Search failed. Try again.'), resultOwner }) } diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 6c160e408dc..9bc69d7501d 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -18888,5 +18888,8 @@ "removeDescription": "Stop agents using this profile first. Removal deletes its saved credentials and conversation data. Your system login stays unchanged.", "cancel": "Cancel" } + }, + "fileSearch": { + "failed": "Search failed. Try again." } } diff --git a/src/renderer/src/lib/path.test.ts b/src/renderer/src/lib/path.test.ts index 42f4d59581e..080427be04e 100644 --- a/src/renderer/src/lib/path.test.ts +++ b/src/renderer/src/lib/path.test.ts @@ -49,3 +49,11 @@ describe('getRelativePathInsideRoot', () => { ) }) }) + +it.each(['/native/repo\\root', '/ssh/repo\\root'])( + 'joins POSIX names without merging literal-backslash identities under %s', + (root) => { + expect(joinPath(root, 'a\\b.txt')).toBe(`${root}/a\\b.txt`) + expect(joinPath(root, 'a/b.txt')).toBe(`${root}/a/b.txt`) + } +) diff --git a/src/renderer/src/lib/path.ts b/src/renderer/src/lib/path.ts index 284ee247ad2..6e03992cd6e 100644 --- a/src/renderer/src/lib/path.ts +++ b/src/renderer/src/lib/path.ts @@ -1,4 +1,7 @@ -import { relativePathInsideRoot } from '../../../shared/cross-platform-path' +import { + isWindowsAbsolutePathLike, + relativePathInsideRoot +} from '../../../shared/cross-platform-path' function stripTrailingSeparators(path: string): string { return path.replace(/[\\/]+$/, '') @@ -12,7 +15,10 @@ function getSeparator(path: string): '/' | '\\' { return path.includes('\\') ? '\\' : '/' } -export function normalizeRelativePath(path: string): string { +export function normalizeRelativePath(path: string, rootPath?: string | null): string { + if (rootPath && !isWindowsAbsolutePathLike(rootPath)) { + return path.replace(/^\/+/, '').replace(/\/+/g, '/') + } return stripLeadingSeparators(path).replace(/[\\/]+/g, '/') } @@ -74,9 +80,14 @@ export function joinPath(basePath: string, relativePath: string): string { return basePath } - const separator = getSeparator(basePath) - const normalizedBasePath = stripTrailingSeparators(basePath) - const normalizedRelativePath = stripLeadingSeparators(relativePath).replace(/[\\/]+/g, separator) + const windowsPath = isWindowsAbsolutePathLike(basePath) + const separator = windowsPath ? getSeparator(basePath) : '/' + const normalizedBasePath = windowsPath + ? stripTrailingSeparators(basePath) + : basePath.replace(/\/+$/, '') + const normalizedRelativePath = windowsPath + ? stripLeadingSeparators(relativePath).replace(/[\\/]+/g, separator) + : relativePath.replace(/^\/+/, '').replace(/\/+/g, '/') return `${normalizedBasePath}${separator}${normalizedRelativePath}` } diff --git a/src/renderer/src/store/slices/editor/actions/file-search-actions.ts b/src/renderer/src/store/slices/editor/actions/file-search-actions.ts index d68a7618298..4e8ef519ceb 100644 --- a/src/renderer/src/store/slices/editor/actions/file-search-actions.ts +++ b/src/renderer/src/store/slices/editor/actions/file-search-actions.ts @@ -39,6 +39,7 @@ export function createFileSearchActions( results: null, resultOwner: null, loading: false, + error: null, collapsedFiles: new Set(), seedRequestId: (current.seedRequestId ?? 0) + 1 } @@ -57,6 +58,7 @@ export function createFileSearchActions( results: null, resultOwner: null, loading: false, + error: null, collapsedFiles: new Set(), seedRequestId: (current.seedRequestId ?? 0) + 1 } @@ -112,6 +114,7 @@ export function createFileSearchActions( results: null, resultOwner: null, loading: false, + error: null, collapsedFiles: new Set() } } diff --git a/src/renderer/src/store/slices/editor/search/file-search-state.ts b/src/renderer/src/store/slices/editor/search/file-search-state.ts index 063bb379fad..dba481e1f09 100644 --- a/src/renderer/src/store/slices/editor/search/file-search-state.ts +++ b/src/renderer/src/store/slices/editor/search/file-search-state.ts @@ -10,6 +10,7 @@ const DEFAULT_FILE_SEARCH_STATE = { results: null, resultOwner: null, loading: false, + error: null, collapsedFiles: new Set() } satisfies Omit diff --git a/src/renderer/src/store/slices/editor/types/file-search-worktree-state.ts b/src/renderer/src/store/slices/editor/types/file-search-worktree-state.ts index a59d9c1e5c6..d70c0399ce4 100644 --- a/src/renderer/src/store/slices/editor/types/file-search-worktree-state.ts +++ b/src/renderer/src/store/slices/editor/types/file-search-worktree-state.ts @@ -10,6 +10,7 @@ export type FileSearchWorktreeState = { excludePattern: string results: SearchResult | null resultOwner: FileSearchResultOwner | null + error?: string | null loading: boolean collapsedFiles: Set seedRequestId?: number diff --git a/src/shared/quick-open-filter.test.ts b/src/shared/quick-open-filter.test.ts index e142c8ef765..e6b65c8d2bb 100644 --- a/src/shared/quick-open-filter.test.ts +++ b/src/shared/quick-open-filter.test.ts @@ -225,6 +225,18 @@ describe('buildRgArgsForQuickOpen', () => { }) }) +it('pins both Quick Open passes to config-independent NUL output', () => { + const { primary, ignoredPass } = buildRgArgsForQuickOpen({ + searchRoot: '.', + excludePathPrefixes: [], + forceSlashSeparator: true + }) + for (const args of [primary, ignoredPass]) { + expect(args).toContain('--no-config') + expect(args).toContain('--null') + } +}) + describe('normalizeQuickOpenRgLine', () => { it('strips absolute root prefix', () => { expect( @@ -266,9 +278,9 @@ describe('normalizeQuickOpenRgLine', () => { expect(normalizeQuickOpenRgLine('..', { kind: 'cwd-relative' })).toBeNull() }) - it('strips CRLF', () => { + it('preserves a trailing carriage return in a NUL-delimited filename', () => { expect(normalizeQuickOpenRgLine('/root/a.ts\r', { kind: 'absolute', rootPath: '/root' })).toBe( - 'a.ts' + 'a.ts\r' ) }) diff --git a/src/shared/quick-open-filter.ts b/src/shared/quick-open-filter.ts index ffb1ac346ef..7c4da971a01 100644 --- a/src/shared/quick-open-filter.ts +++ b/src/shared/quick-open-filter.ts @@ -6,7 +6,7 @@ * Centralized to stop local/relay listFiles from drifting on blocklist, ignores, exclusions, * timeouts, and buffering. See docs/design/share-quick-open-file-listing.md. */ -import { relativePathInsideRoot } from './cross-platform-path' +import { isWindowsAbsolutePathLike, relativePathInsideRoot } from './cross-platform-path' // ─── Hidden-dir blocklist ──────────────────────────────────────────── @@ -93,7 +93,7 @@ export function buildExcludePathPrefixes(rootPath: string, excludePaths?: unknow if (relativePath === null) { continue } - let rel = relativePath.replace(/\\/g, '/') + let rel = isWindowsAbsolutePathLike(rootPath) ? relativePath.replace(/\\/g, '/') : relativePath if (!rel || isParentRelativePath(rel) || rel.startsWith('/')) { continue } @@ -204,6 +204,8 @@ export function buildRgArgsForQuickOpen(opts: RgArgsOptions): RgArgs { const primary = [ '--files', + '--no-config', + '--null', '--hidden', ...sepArgs, ...hiddenDirGlobs, @@ -214,6 +216,8 @@ export function buildRgArgsForQuickOpen(opts: RgArgsOptions): RgArgs { // Ignored pass: --no-ignore-vcs broadens to gitignored/parent/global ignored files; blocklist globs still guard. const ignoredPass = [ '--files', + '--no-config', + '--null', '--hidden', '--no-ignore-vcs', ...sepArgs, @@ -234,20 +238,18 @@ export type RgOutputMode = | { kind: 'cwd-relative' } /** - * Convert one rg --files stdout line into a root-relative, `/`-separated path. - * Returns `null` for lines that escape the root (symlink edge cases) or can't be normalized. + * Convert one NUL-delimited rg --files record into a root-relative, `/`-separated path. + * Returns `null` for records that escape the root (symlink edge cases) or can't be normalized. * Callers do any WSL translation first, keeping WSL out of the shared module. */ export function normalizeQuickOpenRgLine(rawLine: string, outputMode: RgOutputMode): string | null { - let line = rawLine - // Strip CR so CRLF from rg on Windows doesn't leak into results. - if (line.length > 0 && line.charCodeAt(line.length - 1) === 13) { - line = line.substring(0, line.length - 1) - } + const line = rawLine if (!line) { return null } - const normalized = line.replace(/\\/g, '/') + const windowsPath = + outputMode.kind === 'absolute' && isWindowsAbsolutePathLike(outputMode.rootPath) + const normalized = windowsPath ? line.replace(/\\/g, '/') : line if (outputMode.kind === 'cwd-relative') { let rel = normalized if (rel.startsWith('./')) { @@ -262,7 +264,8 @@ export function normalizeQuickOpenRgLine(rawLine: string, outputMode: RgOutputMo } // Absolute mode: strip the root prefix. // Why: only replace backslashes; collapsing repeated slashes would break Windows UNC roots (`\\server\share`). - const normalizedRoot = `${outputMode.rootPath.replace(/\\/g, '/').replace(/\/+$/, '')}/` + const root = windowsPath ? outputMode.rootPath.replace(/\\/g, '/') : outputMode.rootPath + const normalizedRoot = `${root.replace(/\/+$/, '')}/` if (normalized.startsWith(normalizedRoot)) { const rel = normalized.substring(normalizedRoot.length) if (!rel || isParentRelativePath(rel) || rel.startsWith('/')) { diff --git a/src/shared/quick-open-ripgrep-output-mode.ts b/src/shared/quick-open-ripgrep-output-mode.ts new file mode 100644 index 00000000000..e3a8f1541a1 --- /dev/null +++ b/src/shared/quick-open-ripgrep-output-mode.ts @@ -0,0 +1,14 @@ +import type { RgOutputMode } from './quick-open-filter' + +export function getQuickOpenRgOutputMode( + rawLine: string, + translatedLine: string, + rootPath: string +): RgOutputMode { + return translatedLine !== rawLine || + rawLine.startsWith('/') || + /^[A-Za-z]:[\\/]/.test(rawLine) || + rawLine.startsWith('\\\\') + ? { kind: 'absolute', rootPath } + : { kind: 'cwd-relative' } +} diff --git a/src/shared/ripgrep-dense-match-json.test.ts b/src/shared/ripgrep-dense-match-json.test.ts new file mode 100644 index 00000000000..5a4cdd41e29 --- /dev/null +++ b/src/shared/ripgrep-dense-match-json.test.ts @@ -0,0 +1,78 @@ +import { expect, it } from 'vitest' +import { JSONParser } from '@streamparser/json' +import { parseDenseRipgrepMatchJson } from './ripgrep-dense-match-json' + +it.each([0, 64 * 1024])('preserves literal and escaped BOMs with buffer size %i', (size) => { + const source = { '\ufeffkey': `\ufeffstart${'\n'.repeat(64 * 1024)}\ufeffend` } + const literal = JSON.stringify(source) + for (const record of [literal, literal.replaceAll('\ufeff', '\\uFEFF')]) { + const parser = new JSONParser({ stringBufferSize: size }) + let parsed: unknown + parser.onValue = ({ value, stack }) => { + if (stack.length === 0) { + parsed = value + } + } + parser.write(record) + expect(parsed).toEqual(JSON.parse(record)) + } +}) + +it.each(['', '\\', '\n', '\n'.repeat(64 * 1024), `${'x'.repeat(64 * 1024)}\n`])( + 'preserves U+FEFF in text and filenames across string-buffer boundaries (%#)', + (prefix) => { + const text = `${prefix}\ufeff😀x` + const source = { + type: 'match', + data: { + path: { text }, + lines: { text }, + line_number: 1, + submatches: [{ start: 0, end: 1 }] + } + } + const record = JSON.stringify(source) + expect(parseDenseRipgrepMatchJson(record, 1, 16)).toEqual(JSON.parse(record)) + } +) + +it('retains only exact match fields and the remaining range budget', () => { + const ranges = [ + { start: 0, end: 1 }, + { start: 2, end: 3 } + ] + const source = { + type: 'match', + submatches: [{ start: 99, end: 100 }], + data: { + nested: { submatches: [{ start: 88, end: 89 }] }, + lines: { text: 'a "submatches" b', bytes: 'YQ==' }, + path: { text: '/root/a.ts' }, + line_number: 12, + submatches: ranges + } + } + expect(parseDenseRipgrepMatchJson(JSON.stringify(source), 1, 16)).toEqual({ + type: 'match', + data: { + lines: source.data.lines, + path: source.data.path, + line_number: 12, + submatches: ranges.slice(0, 1) + } + }) +}) + +it('rejects depth overflow and invalid submatch shapes', () => { + expect(() => parseDenseRipgrepMatchJson('['.repeat(17), 2, 16)).toThrow() + expect(() => parseDenseRipgrepMatchJson('{"data":{"submatches":[null]}}', 2, 16)).toThrow() +}) + +it.each(['{}', '{"type":"begin","data":{}}', '{"data":null}', '{"data":[]}'])( + 'does not fabricate a match from %s', + (source) => { + const projected = parseDenseRipgrepMatchJson(source, 2, 16) + expect(projected.data?.path).toBeUndefined() + expect(projected.data?.submatches).toEqual([]) + } +) diff --git a/src/shared/ripgrep-dense-match-json.ts b/src/shared/ripgrep-dense-match-json.ts new file mode 100644 index 00000000000..821415a0748 --- /dev/null +++ b/src/shared/ripgrep-dense-match-json.ts @@ -0,0 +1,135 @@ +import { + assertJsonTextStructureWithinLimits, + JsonTextStructureCapacityError, + type JsonTextStructureLimits +} from './json-text-structure-limit' +import { JSONParser, TokenType } from '@streamparser/json' + +export type RipgrepMatchMessage = { + type?: string + data?: { + path?: { text?: string } + lines?: { text?: string; bytes?: string } + line_number?: number + submatches?: { start: number; end: number }[] + } +} + +export function parseRipgrepMatchJson( + line: string, + maxMatches: number, + limits: JsonTextStructureLimits +): RipgrepMatchMessage { + try { + assertJsonTextStructureWithinLimits(line, limits) + return JSON.parse(line) + } catch (error) { + if ( + !(error instanceof JsonTextStructureCapacityError) || + error.resource !== 'structuralTokens' + ) { + throw error + } + return parseDenseRipgrepMatchJson(line, maxMatches, limits.nestingDepth) + } +} + +/** Dense rg records retain only the requested ranges while validating the entire record. */ +export function parseDenseRipgrepMatchJson( + line: string, + maxMatches: number, + nestingDepth: number +): RipgrepMatchMessage { + const parser = new JSONParser({ + paths: [ + '$.type', + '$.data.path.text', + '$.data.lines.text', + '$.data.lines.bytes', + '$.data.line_number', + '$.data.submatches.*' + ], + keepStack: false, + stringBufferSize: 64 * 1024 + }) + const data: NonNullable = { submatches: [] } + const result: RipgrepMatchMessage = { data } + const containers: { object: boolean; expectingKey: boolean; keys?: Set }[] = [] + let elementTokens = 0 + parser.onToken = ({ token, value }) => { + const current = containers.at(-1) + if (current?.object && current.expectingKey && token === TokenType.STRING) { + if (typeof value !== 'string') { + throw new SyntaxError('Invalid rg object key') + } + if (current.keys?.has(value)) { + throw new SyntaxError('Duplicate rg object key') + } + current.keys?.add(value) + if ((current.keys?.size ?? 0) > 128) { + throw new Error('Too many rg record fields') + } + current.expectingKey = false + } + if (token === TokenType.LEFT_BRACE || token === TokenType.LEFT_BRACKET) { + containers.push({ + object: token === TokenType.LEFT_BRACE, + expectingKey: token === TokenType.LEFT_BRACE, + // rg's envelope keys are unique; reject duplicates instead of mixing projections. + keys: containers.length < 2 ? new Set() : undefined + }) + if (containers.length > nestingDepth) { + throw new Error('rg record nesting exceeds limit') + } + if (containers.length === 4) { + elementTokens = 0 + } + } else if (token === TokenType.RIGHT_BRACE || token === TokenType.RIGHT_BRACKET) { + containers.pop() + } else if (token === TokenType.COMMA && current?.object) { + current.expectingKey = true + } + if (containers.length >= 4 && ++elementTokens > 32 * 1024) { + throw new Error('rg submatch structure exceeds limit') + } + } + parser.onValue = ({ key, value, parent, stack }) => { + if (stack.length === 1 && key === 'type' && typeof value === 'string') { + result.type = value + } else if (stack.length === 2 && stack[1].key === 'data' && key === 'line_number') { + if (typeof value === 'number') { + data.line_number = value + } + } else if (stack.length === 3 && stack[1].key === 'data') { + if (stack[2].key === 'submatches' && Array.isArray(parent)) { + if ( + !value || + typeof value !== 'object' || + Array.isArray(value) || + typeof value.start !== 'number' || + typeof value.end !== 'number' + ) { + throw new SyntaxError('Invalid rg submatch') + } + if (data.submatches && data.submatches.length < maxMatches) { + data.submatches.push({ start: value.start, end: value.end }) + } + parent.pop() + } else if (stack[2].key === 'path' && key === 'text' && typeof value === 'string') { + data.path = { text: value } + } else if (stack[2].key === 'lines' && typeof value === 'string') { + if (key === 'text') { + data.lines = { ...data.lines, text: value } + } + if (key === 'bytes') { + data.lines = { ...data.lines, bytes: value } + } + } + } + } + parser.write(line) + if (!parser.isEnded) { + parser.end() + } + return result +} diff --git a/src/shared/ripgrep-filename-decoder.test.ts b/src/shared/ripgrep-filename-decoder.test.ts new file mode 100644 index 00000000000..57af67e1abe --- /dev/null +++ b/src/shared/ripgrep-filename-decoder.test.ts @@ -0,0 +1,44 @@ +import { describe, expect, it } from 'vitest' +import { RipgrepFilenameDecoder, RipgrepFilenameEncodingError } from './ripgrep-filename-decoder' + +describe('ripgrep filename decoding', () => { + it('preserves split scalars, BOMs and literal replacement characters', () => { + const decoder = new RipgrepFilenameDecoder() + const text = '\uFEFF日本語😀\uFFFD\0' + let decoded = '' + for (const byte of Buffer.from(text)) { + decoded += decoder.decode(Buffer.from([byte])) + } + decoder.finish() + expect(decoded).toBe(text) + }) + + it.each([[0xff], [0x80], [0xc0, 0xaf], [0xed, 0xa0, 0x80]])( + 'refuses invalid filename bytes %j', + (...bytes) => { + expect(() => new RipgrepFilenameDecoder().decode(Buffer.from(bytes))).toThrow( + RipgrepFilenameEncodingError + ) + } + ) + + it('rejects an incomplete scalar at EOF', () => { + const decoder = new RipgrepFilenameDecoder() + expect(decoder.decode(Buffer.from([0xf0, 0x9f]))).toBe('') + expect(() => decoder.finish()).toThrow('not valid UTF-8') + }) + + it('accepts string fixtures without losing decoder state', () => { + const decoder = new RipgrepFilenameDecoder() + expect(decoder.decode('literal-�\0')).toBe('literal-�\0') + decoder.finish() + }) +}) + +it('refuses literal WSL backslashes only when Windows translation is required', () => { + const errors: Error[] = [] + const decoder = new RipgrepFilenameDecoder((error) => errors.push(error), true) + expect(decoder.decode(Buffer.from('./a\\b.txt\0'))).toBeNull() + expect(errors[0]?.message).toContain('WSL filenames containing a backslash') + expect(new RipgrepFilenameDecoder().decode(Buffer.from('./a\\b.txt\0'))).toBe('./a\\b.txt\0') +}) diff --git a/src/shared/ripgrep-filename-decoder.ts b/src/shared/ripgrep-filename-decoder.ts new file mode 100644 index 00000000000..7f903b3832d --- /dev/null +++ b/src/shared/ripgrep-filename-decoder.ts @@ -0,0 +1,51 @@ +export class RipgrepFilenameError extends Error {} + +export class RipgrepFilenameEncodingError extends RipgrepFilenameError { + constructor() { + super('File listing contains a filename that is not valid UTF-8') + this.name = 'RipgrepFilenameEncodingError' + } +} + +/** Replacement decoding would return a different, potentially existing filename. */ +export class RipgrepFilenameDecoder { + private readonly decoder = new TextDecoder('utf-8', { fatal: true, ignoreBOM: true }) + + constructor( + private readonly onError: (error: Error) => void = throwFilenameError, + private readonly rejectBackslash = false + ) {} + + decode(chunk: Buffer | string): string | null { + try { + const decoded = this.decoder.decode(typeof chunk === 'string' ? Buffer.from(chunk) : chunk, { + stream: true + }) + if (this.rejectBackslash && decoded.includes('\\')) { + throw new RipgrepFilenameError( + 'WSL filenames containing a backslash cannot be opened through Windows paths' + ) + } + return decoded + } catch (error) { + this.onError( + error instanceof RipgrepFilenameError ? error : new RipgrepFilenameEncodingError() + ) + return null + } + } + + finish(): boolean { + try { + this.decoder.decode() + return true + } catch { + this.onError(new RipgrepFilenameEncodingError()) + return false + } + } +} + +function throwFilenameError(error: Error): never { + throw error +} diff --git a/src/shared/ripgrep-line-decoding.test.ts b/src/shared/ripgrep-line-decoding.test.ts new file mode 100644 index 00000000000..3c28d76a37c --- /dev/null +++ b/src/shared/ripgrep-line-decoding.test.ts @@ -0,0 +1,61 @@ +import { describe, expect, it } from 'vitest' +import { decodeRipgrepLine } from './ripgrep-line-decoding' +import { ripgrepMatchRanges } from './ripgrep-match-offsets' + +describe('ripgrep byte line decoding', () => { + it.each([ + [0xff], + [0xc0, 0xaf], + [0xe2, 0x82], + [0xf0, 0x90, 0x80], + [0xed, 0xa0, 0x80], + [0xf4, 0x90, 0x80, 0x80], + [0x80, 0xbf], + [0xe0, 0x80, 0xaf] + ])('maps text after malformed sequence %j like Node UTF8 decoding', (...prefix) => { + const bytes = Buffer.concat([ + Buffer.from('😀é'), + Buffer.from(prefix), + Buffer.from('needle\r\n') + ]) + const decoded = decodeRipgrepLine({ bytes: bytes.toString('base64') }) + expect(decoded.text).toBe(bytes.toString('utf8').replace(/\n$/, '')) + expect(decoded.readOffset(bytes.length - 8)).toBe(decoded.text.indexOf('needle')) + expect(decoded.readOffset(bytes.length - 2)).toBe(decoded.text.indexOf('needle') + 6) + }) + + it('rejects partial replacement boundaries and overlapping ranges without inventing coordinates', () => { + const decoded = decodeRipgrepLine({ + bytes: Buffer.from([0xe2, 0x82, 0x20, 0x78]).toString('base64') + }) + let invalid = 0 + const ranges = [ + ...ripgrepMatchRanges( + decoded.text, + [ + { start: 0, end: 1 }, + { start: 2, end: 3 }, + { start: 2, end: 4 }, + { start: 4, end: 4 } + ], + decoded.readOffset, + () => invalid++ + ) + ] + expect(ranges).toEqual([ + { start: 1, end: 2 }, + { start: 3, end: 3 } + ]) + expect(invalid).toBe(2) + }) + + it('agrees with Node on all two-byte prefixes followed by ASCII', () => { + for (let first = 0; first < 256; first++) { + for (let second = 0; second < 256; second++) { + const bytes = Buffer.from([first, second, 0x78]) + const decoded = decodeRipgrepLine({ bytes: bytes.toString('base64') }) + expect(decoded.readOffset(2)).toBe(bytes.subarray(0, 2).toString('utf8').length) + } + } + }) +}) diff --git a/src/shared/ripgrep-line-decoding.ts b/src/shared/ripgrep-line-decoding.ts new file mode 100644 index 00000000000..c94091855f4 --- /dev/null +++ b/src/shared/ripgrep-line-decoding.ts @@ -0,0 +1,60 @@ +import { createRipgrepOffsetReader } from './ripgrep-match-offsets' + +function utf8SequenceWidth(bytes: Buffer, start: number): number { + const lead = bytes[start]! + const expected = + lead >= 0xc2 && lead <= 0xdf + ? 2 + : lead >= 0xe0 && lead <= 0xef + ? 3 + : lead >= 0xf0 && lead <= 0xf4 + ? 4 + : 1 + for (let offset = 1; offset < expected; offset++) { + const next = bytes[start + offset] + if ( + next === undefined || + next < 0x80 || + next > 0xbf || + (offset === 1 && + ((lead === 0xe0 && next < 0xa0) || + (lead === 0xed && next >= 0xa0) || + (lead === 0xf0 && next < 0x90) || + (lead === 0xf4 && next >= 0x90))) + ) { + return offset + } + } + return expected +} + +function createDecodedByteOffsetReader(bytes: Buffer): (offset: number) => number | null { + let position = 0 + let column = 0 + return (offset) => { + if (!Number.isSafeInteger(offset) || offset < position) { + return null + } + while (position < offset && position < bytes.length) { + const width = utf8SequenceWidth(bytes, position) + position += width + column += width === 4 ? 2 : 1 + } + return offset === position ? column : null + } +} + +/** Decode once; malformed sequences retain their original byte widths for match coordinates. */ +export function decodeRipgrepLine(data: { text?: string; bytes?: string } | undefined): { + text: string + readOffset: (offset: number) => number | null +} { + if (typeof data?.text === 'string') { + return { text: data.text.replace(/\n$/, ''), readOffset: createRipgrepOffsetReader(data.text) } + } + const bytes = Buffer.from(data?.bytes ?? '', 'base64') + return { + text: bytes.toString('utf8').replace(/\n$/, ''), + readOffset: createDecodedByteOffsetReader(bytes) + } +} diff --git a/src/shared/ripgrep-match-offsets.test.ts b/src/shared/ripgrep-match-offsets.test.ts new file mode 100644 index 00000000000..198af0b9ab8 --- /dev/null +++ b/src/shared/ripgrep-match-offsets.test.ts @@ -0,0 +1,36 @@ +import { describe, expect, it } from 'vitest' +import { createRipgrepOffsetReader, ripgrepMatchRanges } from './ripgrep-match-offsets' + +describe('ripgrep match offsets', () => { + it('converts ordered ASCII, accented, CJK and astral boundaries to UTF16', () => { + const text = 'aé日😀z' + const read = createRipgrepOffsetReader(text) + expect([0, 1, 3, 6, 10, 11].map(read)).toEqual([0, 1, 2, 3, 5, 6]) + expect(read(11)).toBe(6) + }) + + it('rejects invalid, backwards and partial codepoint offsets', () => { + const read = createRipgrepOffsetReader('é😀') + expect(read(-1)).toBeNull() + expect(read(0.5)).toBeNull() + expect(read(1)).toBeNull() + expect(read(2)).toBe(1) + expect(read(0)).toBeNull() + expect(read(6)).toBe(3) + expect(read(7)).toBeNull() + }) + + it('handles many adjacent matches in one pass', () => { + const read = createRipgrepOffsetReader('😀x'.repeat(10_000)) + for (let index = 0; index < 10_000; index++) { + expect(read(index * 5)).toBe(index * 3) + expect(read(index * 5 + 4)).toBe(index * 3 + 2) + } + }) + + it('preserves zero-length matches and whole-codepoint line fallbacks', () => { + expect([...ripgrepMatchRanges('😀é', [{ start: 4, end: 4 }])]).toEqual([{ start: 2, end: 2 }]) + expect([...ripgrepMatchRanges('😀é', [])]).toEqual([{ start: 0, end: 2 }]) + expect([...ripgrepMatchRanges('', [])]).toEqual([{ start: 0, end: 0 }]) + }) +}) diff --git a/src/shared/ripgrep-match-offsets.ts b/src/shared/ripgrep-match-offsets.ts new file mode 100644 index 00000000000..fe667349701 --- /dev/null +++ b/src/shared/ripgrep-match-offsets.ts @@ -0,0 +1,37 @@ +/** Converts ordered ripgrep byte offsets without rescanning each match's prefix. */ +export function createRipgrepOffsetReader(text: string): (byteOffset: number) => number | null { + let bytePosition = 0 + let column = 0 + return (byteOffset) => { + if (!Number.isSafeInteger(byteOffset) || byteOffset < bytePosition) { + return null + } + while (bytePosition < byteOffset && column < text.length) { + const point = text.codePointAt(column)! + bytePosition += point <= 0x7f ? 1 : point <= 0x7ff ? 2 : point <= 0xffff ? 3 : 4 + column += point > 0xffff ? 2 : 1 + } + return bytePosition === byteOffset ? column : null + } +} + +export function* ripgrepMatchRanges( + text: string, + submatches: readonly { start: number; end: number }[], + readOffset = createRipgrepOffsetReader(text), + onInvalidRange?: () => void +): Generator<{ start: number; end: number }> { + if (submatches.length === 0) { + yield { start: 0, end: text.length > 0 ? (text.codePointAt(0)! > 0xffff ? 2 : 1) : 0 } + return + } + for (const submatch of submatches) { + const start = readOffset(submatch.start) + const end = readOffset(submatch.end) + if (start !== null && end !== null) { + yield { start, end } + } else { + onInvalidRange?.() + } + } +} diff --git a/src/shared/ripgrep-search-diagnostics.test.ts b/src/shared/ripgrep-search-diagnostics.test.ts new file mode 100644 index 00000000000..3bacf4678b8 --- /dev/null +++ b/src/shared/ripgrep-search-diagnostics.test.ts @@ -0,0 +1,42 @@ +import { describe, expect, it } from 'vitest' +import { createAccumulator } from './text-search' +import { RipgrepSearchDiagnostics } from './ripgrep-search-diagnostics' + +describe('ripgrep search diagnostics', () => { + it('reports invalid regex diagnostics instead of an empty successful search', () => { + const diagnostics = new RipgrepSearchDiagnostics() + diagnostics.append(Buffer.from('regex parse error: unclosed character class')) + expect(diagnostics.failure(2, null, createAccumulator())?.message).toContain( + 'unclosed character class' + ) + }) + + it('limits retained diagnostics even for large stderr chunks', () => { + const diagnostics = new RipgrepSearchDiagnostics() + diagnostics.append('x'.repeat(100_000)) + diagnostics.append('must not be retained') + const error = diagnostics.failure(2, null, createAccumulator()) + expect(error?.message).toBe(`Search failed (2): ${'x'.repeat(4096)}`) + }) + + it('marks permission-error results as incomplete', () => { + const acc = createAccumulator() + acc.totalMatches = 1 + expect(new RipgrepSearchDiagnostics().failure(2, null, acc)).toBeNull() + expect(acc.truncated).toBe(true) + }) + + it('distinguishes a killed search from intentional truncation', () => { + const diagnostics = new RipgrepSearchDiagnostics() + const acc = createAccumulator() + expect(diagnostics.failure(null, 'SIGTERM', acc)).toBeInstanceOf(Error) + acc.truncated = true + expect(diagnostics.failure(null, 'SIGTERM', acc)).toBeNull() + }) + + it.each([0, 1])('accepts normal exit %i without inventing truncation', (code) => { + const acc = createAccumulator() + expect(new RipgrepSearchDiagnostics().failure(code, null, acc)).toBeNull() + expect(acc.truncated).toBe(false) + }) +}) diff --git a/src/shared/ripgrep-search-diagnostics.ts b/src/shared/ripgrep-search-diagnostics.ts new file mode 100644 index 00000000000..4b5445a79eb --- /dev/null +++ b/src/shared/ripgrep-search-diagnostics.ts @@ -0,0 +1,34 @@ +import type { SearchAccumulator } from './text-search' + +const MAX_SEARCH_ERROR_BYTES = 4096 + +export class RipgrepSearchDiagnostics { + private readonly bytes = Buffer.alloc(MAX_SEARCH_ERROR_BYTES) + private length = 0 + + append(chunk: Buffer | string): void { + if (this.length >= this.bytes.length) { + return + } + this.length += + typeof chunk === 'string' + ? this.bytes.write(chunk, this.length, this.bytes.length - this.length, 'utf8') + : chunk.copy(this.bytes, this.length, 0, this.bytes.length - this.length) + } + + failure( + code: number | null, + signal: NodeJS.Signals | null, + acc: SearchAccumulator + ): Error | null { + if (code === 0 || code === 1 || (signal && acc.truncated)) { + return null + } + if (acc.totalMatches > 0) { + acc.truncated = true + return null + } + const detail = this.bytes.toString('utf8', 0, this.length).trim() + return new Error(`Search failed (${signal ?? code})${detail ? `: ${detail}` : ''}`) + } +} diff --git a/src/shared/text-search-dense-matches.test.ts b/src/shared/text-search-dense-matches.test.ts new file mode 100644 index 00000000000..e32cd6b35e4 --- /dev/null +++ b/src/shared/text-search-dense-matches.test.ts @@ -0,0 +1,126 @@ +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 './child-process/run-process' +import { buildRgArgs, createAccumulator, ingestRgJsonLine } from './text-search' + +describe('text search match budgets', () => { + it('keeps dense-line columns after a leading U+FEFF from real rg', async () => { + const { rgPath } = await import('@vscode/ripgrep-universal') + const root = await mkdtemp(join(tmpdir(), 'orca-rg-dense-bom-')) + const filename = join(root, '\ufeffdense.txt') + try { + await writeFile(filename, `header\n\ufeff${'x '.repeat(10_000)}`) + const result = await runProcess({ + program: rgPath, + args: buildRgArgs('x', '.', {}), + cwd: root + }) + expect(result.code).toBe(0) + const accumulator = createAccumulator() + for (const line of result.stdout.split('\n')) { + if (ingestRgJsonLine(line, root, accumulator, 2000) === 'stop') { + break + } + } + expect(accumulator.totalMatches).toBe(2000) + expect(accumulator.fileMap.get(filename)?.matches[0]).toMatchObject({ + line: 2, + column: 2, + matchLength: 1 + }) + expect(accumulator.fileMap.get(filename)?.matches[1999]?.column).toBe(4000) + } finally { + await rm(root, { recursive: true, force: true }) + } + }) + + it('preserves the requested budget from real rg dense-line output', async () => { + const { rgPath } = await import('@vscode/ripgrep-universal') + const root = await mkdtemp(join(tmpdir(), 'orca-rg-dense-')) + try { + await writeFile(join(root, 'dense.txt'), 'x '.repeat(10_000)) + const result = await runProcess({ + program: rgPath, + args: buildRgArgs('x', root, {}), + cwd: root + }) + expect(result.code).toBe(0) + const accumulator = createAccumulator() + for (const line of result.stdout.split('\n')) { + if (ingestRgJsonLine(line, root, accumulator, 2000) === 'stop') { + break + } + } + expect(accumulator.totalMatches).toBe(2000) + expect(accumulator.truncated).toBe(true) + } finally { + await rm(root, { recursive: true, force: true }) + } + }) + + it('allows more than 100 matching lines in one file under the global budget', async () => { + const { rgPath } = await import('@vscode/ripgrep-universal') + const root = await mkdtemp(join(tmpdir(), 'orca-rg-lines-')) + try { + await writeFile(join(root, 'many.txt'), 'needle\n'.repeat(150)) + const result = await runProcess({ + program: rgPath, + args: buildRgArgs('needle', root, {}), + cwd: root + }) + expect(result.code).toBe(0) + const accumulator = createAccumulator() + for (const line of result.stdout.split('\n')) { + ingestRgJsonLine(line, root, accumulator, 2000) + } + expect(accumulator.totalMatches).toBe(150) + expect(accumulator.truncated).toBe(false) + } finally { + await rm(root, { recursive: true, force: true }) + } + }) +}) + +function denseRecord(count: number): string { + return JSON.stringify({ + type: 'match', + data: { + path: { text: '/root/dense.txt' }, + lines: { text: 'x '.repeat(count) }, + line_number: 1, + submatches: Array.from({ length: count }, (_, index) => ({ + match: { text: 'x' }, + start: index * 2, + end: index * 2 + 1 + })) + } + }) +} + +it('retains the first 2000 matches from a dense line beyond the normal JSON budget', () => { + const accumulator = createAccumulator() + expect(ingestRgJsonLine(denseRecord(10_000), '/root', accumulator, 2000)).toBe('stop') + expect(accumulator.totalMatches).toBe(2000) + expect(accumulator.truncated).toBe(true) + const matches = accumulator.fileMap.get('/root/dense.txt')?.matches + expect(matches?.map((match) => match.column)).toEqual( + Array.from({ length: 2000 }, (_, index) => index * 2 + 1) + ) +}) + +it.each([ + (record: string) => record.slice(0, -1), + (record: string) => `${record.slice(0, -2)},invalid}`, + (record: string) => `${record.slice(0, -1)},"data":{}}`, + (record: string) => `${record.slice(0, -2)},"submatches":[]}}` +])( + 'rejects invalid tails or duplicate envelope keys without retaining early matches', + (corrupt) => { + const accumulator = createAccumulator() + ingestRgJsonLine(corrupt(denseRecord(10_000)), '/root', accumulator, 2000) + expect(accumulator.totalMatches).toBe(0) + expect(accumulator.truncated).toBe(true) + } +) diff --git a/src/shared/text-search-invalid-filename.test.ts b/src/shared/text-search-invalid-filename.test.ts new file mode 100644 index 00000000000..d9faa51efde --- /dev/null +++ b/src/shared/text-search-invalid-filename.test.ts @@ -0,0 +1,19 @@ +import { expect, it } from 'vitest' +import { createAccumulator, ingestRgJsonLine } from './text-search' + +it('reports byte-only filenames as incomplete without inventing a replacement-character path', () => { + const acc = createAccumulator() + const line = JSON.stringify({ + type: 'match', + data: { + path: { bytes: Buffer.from([0xff, 0x2e, 0x74, 0x78, 0x74]).toString('base64') }, + lines: { text: 'needle\n' }, + line_number: 1, + submatches: [{ start: 0, end: 6 }] + } + }) + expect(ingestRgJsonLine(line, '/root', acc, 20)).toBe('continue') + expect(acc.truncated).toBe(true) + expect(acc.totalMatches).toBe(0) + expect(acc.fileMap.size).toBe(0) +}) diff --git a/src/shared/text-search-paths.test.ts b/src/shared/text-search-paths.test.ts new file mode 100644 index 00000000000..b6fdc3bb1c2 --- /dev/null +++ b/src/shared/text-search-paths.test.ts @@ -0,0 +1,36 @@ +import { describe, expect, it } from 'vitest' +import { createAccumulator, ingestRgJsonLine } from './text-search' +import { resolveSearchResultPath } from './text-search-paths' + +describe('text search result paths', () => { + it.each([ + ['/root/repo', './src/a.ts', '/root/repo/src/a.ts'], + ['/root/repo', '/root/repo/src/a.ts', '/root/repo/src/a.ts'], + ['C:\\repo', './src/a.ts', 'C:\\repo\\src\\a.ts'], + ['C:\\repo', 'C:/repo/src/a.ts', 'C:/repo/src/a.ts'], + ['\\\\wsl.localhost\\Ubuntu\\repo', './src/a.ts', '\\\\wsl.localhost\\Ubuntu\\repo\\src\\a.ts'] + ])('resolves %s and %s on the owning host', (root, reported, expected) => { + expect(resolveSearchResultPath(root, reported)).toBe(expected) + }) + + it('translates an absolute WSL path before resolving its host path', () => { + const acc = createAccumulator() + ingestRgJsonLine( + JSON.stringify({ + type: 'match', + data: { + path: { text: '/repo/src/a.ts' }, + lines: { text: 'match\n' }, + line_number: 1, + submatches: [{ start: 0, end: 5 }] + } + }), + '\\\\wsl.localhost\\Ubuntu\\repo', + acc, + 20, + (path) => `\\\\wsl.localhost\\Ubuntu${path.replaceAll('/', '\\')}` + ) + expect([...acc.fileMap.values()][0]?.relativePath).toBe('src/a.ts') + expect([...acc.fileMap.keys()]).toEqual(['\\\\wsl.localhost\\Ubuntu\\repo\\src\\a.ts']) + }) +}) diff --git a/src/shared/text-search-paths.ts b/src/shared/text-search-paths.ts index c556a10dedf..4e7cd154158 100644 --- a/src/shared/text-search-paths.ts +++ b/src/shared/text-search-paths.ts @@ -1,20 +1,28 @@ import { posix, win32 } from 'node:path' +import { isWindowsAbsolutePathLike } from './cross-platform-path' function pathFlavor(rootPath: string): typeof posix | typeof win32 { - if (/^[a-zA-Z]:[\\/]/.test(rootPath) || rootPath.startsWith('\\\\')) { + if (isWindowsAbsolutePathLike(rootPath)) { return win32 } return posix } -export function normalizeRelativePath(path: string): string { - return path.replace(/[\\/]+/g, '/').replace(/^\/+/, '') +export function normalizeRelativePath(path: string, rootPath?: string): string { + const separators = + rootPath !== undefined && !isWindowsAbsolutePathLike(rootPath) ? /\/+/g : /[\\/]+/g + return path.replace(separators, '/').replace(/^\/+/, '') } export function relativeToSearchRoot(rootPath: string, absolutePath: string): string { return pathFlavor(rootPath).relative(rootPath, absolutePath) } +export function resolveSearchResultPath(rootPath: string, reportedPath: string): string { + const paths = pathFlavor(rootPath) + return paths.isAbsolute(reportedPath) ? reportedPath : paths.resolve(rootPath, reportedPath) +} + export function joinSearchRoot(rootPath: string, relativePath: string): string { return pathFlavor(rootPath).join(rootPath, relativePath) } diff --git a/src/shared/text-search.ts b/src/shared/text-search.ts index b6f9e9e6b23..c811425a3d0 100644 --- a/src/shared/text-search.ts +++ b/src/shared/text-search.ts @@ -7,13 +7,20 @@ * can't re-diverge (notably the relay's old execFile maxBuffer that dropped matches). * Design doc: docs/design/share-text-search.md. */ -import { assertJsonTextStructureWithinLimits } from './json-text-structure-limit' +import { parseRipgrepMatchJson, type RipgrepMatchMessage } from './ripgrep-dense-match-json' import { normalizeSearchResult } from './search-match-count' import { escapeRegex } from './string-utils' import type { SearchFileResult, SearchOptions, SearchResult } from './code-search-types' import { pushSearchMatch } from './text-search-match-accumulator' import { splitSearchGlobPatterns, toGitGlobPathspecs } from './text-search-glob-patterns' -import { joinSearchRoot, normalizeRelativePath, relativeToSearchRoot } from './text-search-paths' +import { + joinSearchRoot, + normalizeRelativePath, + relativeToSearchRoot, + resolveSearchResultPath +} from './text-search-paths' +import { ripgrepMatchRanges } from './ripgrep-match-offsets' +import { decodeRipgrepLine } from './ripgrep-line-decoding' export type SearchAccumulator = { fileMap: Map @@ -27,7 +34,6 @@ export function createAccumulator(): SearchAccumulator { // ─── Constants shared by both callers ──────────────────────────────── -export const MAX_MATCHES_PER_FILE = 100 export const DEFAULT_SEARCH_MAX_RESULTS = 2000 export const SEARCH_TIMEOUT_MS = 15_000 export const SEARCH_JSON_STRUCTURE_LIMITS = { @@ -51,17 +57,15 @@ export type SearchOptionsLike = Pick< /** * Build the complete rg argv (flags + `--` + query + target) for both callers to spawn as-is. * - * Constraint: pass `rootPath` unchanged as `target` — do NOT WSL-translate it; only the rg - * invocation is routed through `wslAwareSpawn`, and output paths are translated back in `ingestRgJsonLine`. + * Use target `.` with cwd set to the search root so anchored globs match root-relative paths. */ export function buildRgArgs(query: string, target: string, opts: SearchOptionsLike): string[] { const args: string[] = [ + '--no-config', '--json', '--hidden', '--glob', '!.git', - '--max-count', - String(MAX_MATCHES_PER_FILE), '--max-filesize', `${Math.floor(SEARCH_MAX_FILE_SIZE / 1024 / 1024)}M` ] @@ -101,7 +105,7 @@ export function ingestRgJsonLine( rootPath: string, acc: SearchAccumulator, maxResults: number, - transformAbsPath?: (p: string) => string + transformAbsPath?: (p: string) => string | null ): 'continue' | 'stop' { if (acc.totalMatches >= maxResults) { return 'stop' @@ -109,19 +113,11 @@ export function ingestRgJsonLine( if (!line) { return 'continue' } - let msg: { - type?: string - data?: { - path?: { text?: string } - submatches?: { start: number; end: number }[] - line_number?: number - lines?: { text?: string } - } - } + let msg: RipgrepMatchMessage try { - assertJsonTextStructureWithinLimits(line, SEARCH_JSON_STRUCTURE_LIMITS) - msg = JSON.parse(line) + msg = parseRipgrepMatchJson(line, maxResults - acc.totalMatches, SEARCH_JSON_STRUCTURE_LIMITS) } catch { + acc.truncated = true return 'continue' } if (msg.type !== 'match' || !msg.data) { @@ -130,19 +126,22 @@ export function ingestRgJsonLine( const data = msg.data const rawPath = data.path?.text if (typeof rawPath !== 'string') { + // File APIs accept strings, so byte-only filenames cannot be opened losslessly. + acc.truncated = true return 'continue' } - const absPath = transformAbsPath ? transformAbsPath(rawPath) : rawPath - const relPath = normalizeRelativePath(relativeToSearchRoot(rootPath, absPath)) - const lineContent = (data.lines?.text ?? '').replace(/\n$/, '') - const lineNumber = data.line_number ?? 0 - let submatches = data.submatches ?? [] - if (submatches.length === 0) { - // Why: some rg matches report a line but no submatch ranges; surface a navigable line-level result instead of a count-0 row. - submatches = [{ start: 0, end: lineContent.length > 0 ? 1 : 0 }] + const mappedPath = transformAbsPath ? transformAbsPath(rawPath) : rawPath + if (mappedPath === null) { + acc.truncated = true + return 'continue' } - - for (const sub of submatches) { + const absPath = resolveSearchResultPath(rootPath, mappedPath) + const relPath = normalizeRelativePath(relativeToSearchRoot(rootPath, absPath), rootPath) + const { text: lineContent, readOffset } = decodeRipgrepLine(data.lines) + const lineNumber = data.line_number ?? 0 + for (const sub of ripgrepMatchRanges(lineContent, data.submatches ?? [], readOffset, () => { + acc.truncated = true + })) { let fileResult = acc.fileMap.get(absPath) if (!fileResult) { fileResult = { filePath: absPath, relativePath: relPath, matches: [], matchCount: 0 } @@ -263,7 +262,7 @@ export function ingestGitGrepLine( if (nullIdx === -1) { return 'continue' } - const relPath = normalizeRelativePath(line.substring(0, nullIdx)) + const relPath = normalizeRelativePath(line.substring(0, nullIdx), rootPath) const rest = line.substring(nullIdx + 1) const secondNullIdx = rest.indexOf('\0') let lineNumberText: string