Uh oh!
There was an error while loading. Please reload this page.
fix(ripgrep): support @vscode/ripgrep >=1.18 platform-package layout (#1024) - #1032
Conversation
📝 WalkthroughWalkthroughThe change adds platform-specific VS Code ripgrep path probing for standard and unpacked package layouts, including architecture overrides. Workspace file-listing failures are now reported in environment details rather than propagated. ChangesRuntime resilience
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/services/ripgrep/__tests__/index.spec.ts`:
- Around line 120-138: The test suite lacks coverage for the swallowed
package-resolution error in getBinPath. Add a package-local test where the first
createRequire(...).resolve() call throws, configure the required mocks for that
resolution path, and assert that getBinPath(appRoot) returns undefined.
In `@src/services/ripgrep/index.ts`:
- Line 121: Update resolvePlatformRipgrepPath() to derive the architecture using
process.env.npm_config_arch || process.arch, matching the wrapper’s package
resolution, and use that value when constructing the `@vscode/ripgrep` package
name. Add a test covering npm_config_arch overrides.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f37a96eb-6392-4bcf-a0bb-b0a480980a40
📒 Files selected for processing (2)
src/services/ripgrep/__tests__/index.spec.tssrc/services/ripgrep/index.ts
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/core/environment/__tests__/getEnvironmentDetails.spec.ts`:
- Around line 454-457: Update the test “should not throw when listFiles rejects
with ripgrep not found” to await getEnvironmentDetails directly and assert that
the returned environment-details string contains the expected fallback message.
Remove the invalid `.resolves.not.toThrow()` assertion while preserving the
mocked ripgrep rejection.
In `@src/services/ripgrep/__tests__/index.spec.ts`:
- Around line 123-142: Update the npm_config_arch test around getBinPath so the
environment override is applied before the module captures its platform-package
constant, and ensure the module is reloaded or candidate paths are computed
dynamically. Keep the assertion targeted to the overridden architecture and
restore the environment and module state afterward.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f3af62a8-f7fa-4026-bda6-f992a487aa10
📒 Files selected for processing (4)
src/core/environment/__tests__/getEnvironmentDetails.spec.tssrc/core/environment/getEnvironmentDetails.tssrc/services/ripgrep/__tests__/index.spec.tssrc/services/ripgrep/index.ts
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…oo-Code-Org#1024) (Zoo-Code-Org#1032) * fix(ripgrep): resolve platform-specific binary for @vscode/ripgrep >=1.18 * fix(environment): gracefully handle ripgrep not found during file listing * fix(ripgrep): use npm_config_arch for platform-package resolution * refactor(ripgrep): replace createRequire resolver with static candidate paths * fix(ripgrep): evaluate npm_config_arch at call time in candidate paths --------- Co-authored-by: Naved Merchant <naved.merchant@gmail.com>
Related GitHub Issue
Closes: #1024
Description
VS Code 1.130+ ships
@vscode/ripgrep>=1.18, which changed the binary layout: instead of a single bundled binary,rgnow lives inside a platform-specific optional package (e.g.@vscode/ripgrep-win32-x64). Our candidate path list only covered the old layouts, sogetBinPathreturnedundefinedon affected installs.When that happened,
listFilesthrew"Could not find ripgrep binary", which propagated uncaught throughgetEnvironmentDetailsand stalled every task at 0% before the first API call.Changes:
@vscode/ripgrep-${platform}-${arch}and unpacked-asar variants toripgrepCandidatePaths, readingprocess.env.npm_config_arch || process.archat call time to match the wrapper's own resolution logiclistFilesin a try/catch ingetEnvironmentDetailsso a missing ripgrep binary degrades gracefully (shows a message in context) rather than hanging the taskTest Procedure
Unit tests cover all new candidate paths including the platform-package layout, arch override, and the graceful fallback when
listFilesthrows.Pre-Submission Checklist
Summary by CodeRabbit
Bug Fixes
Tests