Skip to content
Closed
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions packages/core/src/tools/ripGrep.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import {
import { RIP_GREP_DEFINITION } from './definitions/coreTools.js';
import { resolveToolDeclaration } from './definitions/resolver.js';
import { type GrepMatch, formatGrepResults } from './grep-utils.js';
import { isBinaryAvailable } from '../utils/binaryCheck.js';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

Instead of introducing a new isBinaryAvailable utility or using resolveExecutable, remove the pre-check entirely. Rely on the built-in error handling of spawn and spawnSync for command not found errors, as explicit pre-checks are discouraged in this repository.

References
  1. Rely on the built-in error handling of spawn and spawnSync for 'command not found' errors instead of adding an explicit commandExists pre-check.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

In this specific case, the pre check is needed because it must resolve the correct path (bundled vs system) during the init


const __dirname = path.dirname(fileURLToPath(import.meta.url));

Expand All @@ -61,6 +62,11 @@ export async function getRipgrepPath(): Promise<string | null> {
}
}

// 3. Fallback to system PATH
if (isBinaryAvailable('rg')) {
return 'rg';
}
Comment on lines +65 to +68

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

Avoid adding an explicit fallback check for the system ripgrep binary. It is preferred to rely on the environment's PATH and the built-in error handling of spawn/spawnSync to handle missing binaries rather than performing a pre-check.

References
  1. Rely on the built-in error handling of spawn and spawnSync for 'command not found' errors instead of adding an explicit commandExists pre-check.


return null;
}

Expand Down
Loading