Skip to content

Commit 2671d1e

Browse files
committed
fix: drop the PATH fallback per review feedback
Per #248 review (edelauna): rely only on VS Code's bundled ripgrep — the fix keeps the @vscode/ripgrep-universal/bin/<platform>-<arch>/ resolution that VS Code Insiders' staged-install builds use (the original bug) but drops the system-PATH probe. This also addresses the Copilot trust-model note at the old line 111 (a PATH-resolved rg could be user-controlled) and clears the codecov gap — the uncovered lines were the PATH helper.
1 parent 051b67b commit 2671d1e

2 files changed

Lines changed: 8 additions & 70 deletions

File tree

src/services/ripgrep/__tests__/index.spec.ts

Lines changed: 2 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// npx vitest run src/services/ripgrep/__tests__/index.spec.ts
22

33
import path from "path"
4-
import { vi, describe, it, expect, beforeEach, afterEach } from "vitest"
4+
import { vi, describe, it, expect, beforeEach } from "vitest"
55

66
import { truncateLine, getBinPath } from "../index"
77
import { fileExistsAtPath } from "../../../utils/fs"
@@ -63,21 +63,12 @@ describe("getBinPath", () => {
6363
const appRoot = "/fake/vscode/appRoot"
6464
const binName = process.platform.startsWith("win") ? "rg.exe" : "rg"
6565
const platformDir = `${process.platform}-${process.arch}`
66-
const originalPath = process.env.PATH
6766

6867
beforeEach(() => {
6968
mockFileExists.mockReset()
7069
mockFileExists.mockResolvedValue(false)
7170
})
7271

73-
afterEach(() => {
74-
if (originalPath === undefined) {
75-
delete process.env.PATH
76-
} else {
77-
process.env.PATH = originalPath
78-
}
79-
})
80-
8172
it("resolves ripgrep from the classic @vscode/ripgrep layout", async () => {
8273
const rg = path.join(appRoot, "node_modules/@vscode/ripgrep/bin", binName)
8374
mockFileExists.mockImplementation(async (p: string) => p === rg)
@@ -92,25 +83,7 @@ describe("getBinPath", () => {
9283
expect(await getBinPath(appRoot)).toBe(rg)
9384
})
9485

95-
it("falls back to ripgrep on the system PATH when the VS Code copy is absent", async () => {
96-
process.env.PATH = ["/fake/empty", "/fake/tools"].join(path.delimiter)
97-
const rg = path.join("/fake/tools", binName)
98-
mockFileExists.mockImplementation(async (p: string) => p === rg)
99-
100-
expect(await getBinPath(appRoot)).toBe(rg)
101-
})
102-
103-
it("prefers the VS Code copy over the system PATH", async () => {
104-
process.env.PATH = "/fake/tools"
105-
const vscodeRg = path.join(appRoot, "node_modules/@vscode/ripgrep/bin", binName)
106-
const pathRg = path.join("/fake/tools", binName)
107-
mockFileExists.mockImplementation(async (p: string) => p === vscodeRg || p === pathRg)
108-
109-
expect(await getBinPath(appRoot)).toBe(vscodeRg)
110-
})
111-
112-
it("returns undefined when ripgrep cannot be found anywhere", async () => {
113-
process.env.PATH = "/fake/empty"
86+
it("returns undefined when ripgrep cannot be found", async () => {
11487
mockFileExists.mockResolvedValue(false)
11588

11689
expect(await getBinPath(appRoot)).toBeUndefined()

src/services/ripgrep/index.ts

Lines changed: 6 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -85,47 +85,13 @@ export function truncateLine(line: string, maxLength: number = MAX_LINE_LENGTH):
8585
return line.length > maxLength ? line.substring(0, maxLength) + " [truncated...]" : line
8686
}
8787
/**
88-
* Look up the ripgrep binary on the system PATH.
88+
* Get the path to the ripgrep binary shipped inside the VS Code installation.
8989
*
90-
* Used as a fallback after the VS Code installation has been checked. Covers
91-
* VS Code forks whose install layout is not recognized by getBinPath, headless
92-
* / CLI hosts with no VS Code installation, and machines where the user has
93-
* installed ripgrep themselves.
94-
*/
95-
async function findRipgrepOnPath(): Promise<string | undefined> {
96-
const pathEnv = process.env.PATH
97-
98-
if (!pathEnv) {
99-
return undefined
100-
}
101-
102-
for (const dir of pathEnv.split(path.delimiter)) {
103-
if (dir.length === 0) {
104-
continue
105-
}
106-
107-
const candidate = path.join(dir, binName)
108-
109-
if (await fileExistsAtPath(candidate)) {
110-
return candidate
111-
}
112-
}
113-
114-
return undefined
115-
}
116-
117-
/**
118-
* Get the path to the ripgrep binary.
119-
*
120-
* Resolution order:
121-
* 1. ripgrep shipped inside the VS Code installation. Both the long-standing
122-
* `@vscode/ripgrep` layout and the newer `@vscode/ripgrep-universal`
123-
* layout are checked — the latter is what VS Code Insiders' staged-install
124-
* builds use (see microsoft/vscode#252063).
125-
* 2. ripgrep on the system PATH — covers VS Code forks with an unrecognized
126-
* install layout, headless / CLI hosts, and a user-installed ripgrep.
90+
* Both the long-standing `@vscode/ripgrep` layout and the newer
91+
* `@vscode/ripgrep-universal` layout are checked — the latter is what VS Code
92+
* Insiders' staged-install builds use (see microsoft/vscode#252063).
12793
*
128-
* Returns `undefined` when ripgrep cannot be located anywhere.
94+
* Returns `undefined` when ripgrep cannot be located.
12995
*/
13096
export async function getBinPath(vscodeAppRoot: string): Promise<string | undefined> {
13197
const checkPath = async (pkgFolder: string) => {
@@ -139,8 +105,7 @@ export async function getBinPath(vscodeAppRoot: string): Promise<string | undefi
139105
(await checkPath("node_modules.asar.unpacked/vscode-ripgrep/bin/")) ||
140106
(await checkPath("node_modules.asar.unpacked/@vscode/ripgrep/bin/")) ||
141107
(await checkPath(`node_modules/@vscode/ripgrep-universal/${ripgrepUniversalBinDir}`)) ||
142-
(await checkPath(`node_modules.asar.unpacked/@vscode/ripgrep-universal/${ripgrepUniversalBinDir}`)) ||
143-
(await findRipgrepOnPath())
108+
(await checkPath(`node_modules.asar.unpacked/@vscode/ripgrep-universal/${ripgrepUniversalBinDir}`))
144109
)
145110
}
146111

0 commit comments

Comments
 (0)