Skip to content

fix: Windows build compatibility #286

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 3, 2025
Merged

fix: Windows build compatibility #286

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
"testemonials"
],
"scripts": {
"postinstall": "node dist/track-installation.js && node dist/npm-scripts/verify-ripgrep.js || true",
"postinstall": "node dist/track-installation.js && node dist/npm-scripts/verify-ripgrep.js || node -e \"process.exit(0)\"",
"open-chat": "open -n /Applications/Claude.app",
"sync-version": "node scripts/sync-version.js",
"bump": "node scripts/sync-version.js --bump",
Expand All @@ -32,7 +32,7 @@
"watch": "tsc --watch",
"start": "node dist/index.js",
"start:debug": "node --inspect-brk=9229 dist/index.js",
"setup": "npm install && npm run build && node setup-claude-server.js",
"setup": "npm install --include=dev && npm run build && node setup-claude-server.js",
"setup:debug": "npm install && npm run build && node setup-claude-server.js --debug",
"remove": "npm install && npm run build && node uninstall-claude-server.js",
"prepare": "npm run build",
Expand Down
5 changes: 3 additions & 2 deletions src/utils/ripgrep-resolver.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,11 @@ export async function getRipgrepPath(): Promise<string> {
// @vscode/ripgrep import or binary resolution failed, continue to fallbacks
}

// Strategy 2: Try system ripgrep using 'which' command
// Strategy 2: Try system ripgrep using 'which' (Unix) or 'where' (Windows)
try {
const systemRg = process.platform === 'win32' ? 'rg.exe' : 'rg';
Copy link
Contributor

@codeant-ai codeant-ai bot Dec 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Using the platform-specific literal 'rg.exe' for Windows may miss valid installs where the executable is registered as 'rg' (Windows 'where' will find 'rg' just as well), causing the lookup to fail; use the generic 'rg' name so the locator command can find both 'rg' and 'rg.exe'. [logic error]

Severity Level: Minor ⚠️

Suggested change
const systemRg = process.platform === 'win32' ? 'rg.exe' : 'rg';
const systemRg = 'rg';
Why it matters? ⭐

The current code special-cases Windows by using 'rg.exe'. While that usually works,
using the generic 'rg' is more robust: Windows 'where' can resolve executables
via PATHEXT and will find 'rg' whether it's listed as 'rg' or 'rg.exe' (and it
also covers shims/aliases that may not include the .exe suffix). This is a small,
safe improvement that reduces the chance of a false negative when locating ripgrep.

Prompt for AI Agent 🤖 
This is a comment left during a code review.

**Path:** src/utils/ripgrep-resolver.ts
**Line:** 38:38
**Comment:**
	*Logic Error: Using the platform-specific literal 'rg.exe' for Windows may miss valid installs where the executable is registered as 'rg' (Windows 'where' will find 'rg' just as well), causing the lookup to fail; use the generic 'rg' name so the locator command can find both 'rg' and 'rg.exe'.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.

const result = execSync(`which ${systemRg}`, { encoding: 'utf-8' }).trim();
const whichCmd = process.platform === 'win32' ? 'where' : 'which';
const result = execSync(`${whichCmd} ${systemRg}`, { encoding: 'utf-8' }).trim().split(/\r?\n/)[0];
if (result && existsSync(result)) {
cachedRgPath = result;
Comment on lines +40 to 42
Copy link
Contributor

@codeant-ai codeant-ai bot Dec 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: The code assumes execSync returns a single path and picks the first line; on Windows 'where' can return multiple paths (including shims) or quoted paths. Replace the single-line extraction with robust parsing: capture full stdout, split into candidate lines, trim quotes/whitespace, and iterate to return the first existing candidate (also add a short timeout and maxBuffer to avoid blocking indefinitely). [possible bug]

Severity Level: Critical 🚨

Suggested change
const result = execSync(`${whichCmd} ${systemRg}`, { encoding: 'utf-8' }).trim().split(/\r?\n/)[0];
if (result && existsSync(result)) {
cachedRgPath = result;
const output = execSync(`${whichCmd} ${systemRg}`, { encoding: 'utf-8', timeout: 2000, maxBuffer: 10 * 1024 * 1024 }).trim();
const candidates = output.split(/\r?\n/).map(s => s.trim().replace(/^"|"$/g, '')).filter(Boolean);
for (const candidate of candidates) {
try {
if (existsSync(candidate)) {
cachedRgPath = candidate;
return candidate;
}
} catch (e) {
// ignore invalid candidate and try next
}
Why it matters? ⭐

The PR currently grabs only the first line of execSync output. On Windows 'where'
may return multiple candidates (shims, different installs) and some lines can be
quoted. The proposed change to parse all stdout lines, trim quotes/whitespace,
and pick the first candidate that actually exists fixes a real robustness issue.
Adding a short timeout and reasonable maxBuffer is also sensible to avoid hangs
or huge output. This materially improves correctness over the existing implementation.

Prompt for AI Agent 🤖 
This is a comment left during a code review.

**Path:** src/utils/ripgrep-resolver.ts
**Line:** 40:42
**Comment:**
	*Possible Bug: The code assumes execSync returns a single path and picks the first line; on Windows 'where' can return multiple paths (including shims) or quoted paths. Replace the single-line extraction with robust parsing: capture full stdout, split into candidate lines, trim quotes/whitespace, and iterate to return the first existing candidate (also add a short timeout and maxBuffer to avoid blocking indefinitely).

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.

return result;
Expand Down