[issues/541] Fix Go to RangeLink falsely reporting "Multiple files match" when root-level file matches#585
Conversation
…tch" when root-level file matches
## Summary
The bare filename resolution path in `resolveWorkspacePath()` always delegated to `findFiles('**/filename', undefined, 2)`, returning `FILENAME_AMBIGUOUS` as soon as 2 matches existed anywhere in the workspace. In monorepos, common root-level files like `package.json` appear in multiple subdirectories, so typing a bare filename always triggered the ambiguity warning. This fix checks workspace root(s) via `fs.stat` before falling back to `findFiles`, matching the most likely user intent. `node_modules` is now excluded from the findFiles fallback to avoid dependency-directory noise.
## Changes
- Reordered bare-filename resolution in `resolveWorkspacePath.ts`: check workspace root(s) via `fs.stat` before `findFiles`. Single root match returns `workspace-relative`; multiple root matches across workspace folders returns `FILENAME_AMBIGUOUS`
- Added `{**/node_modules/**}` exclude pattern to `findFiles` fallback
- Updated 5 test assertions to reflect new resolution order and node_modules exclusion
- Added multi-root ambiguity test for bare filename existing at multiple workspace roots
- Documentation: CHANGELOG added; README not needed (no new feature or setting)
## Test Plan
- [x] All existing tests pass (1923)
- [x] New tests added for: root-first resolution, multi-root ambiguity, findFiles node_modules exclusion
- [ ] Manual testing: test `package.json` bare filename navigation in monorepo workspace
## Related
Closes #541
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughUpdates ChangesBare-filename resolution refactor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
|
The PR modifies the behavior of the Suggested test cases:
Generated by QA Gap Check (GPT-4o-mini via GitHub Models) |
|
@coderabbitai review I was rate-limited in #585 (comment) |
|
(｡•̀ᴗ-)✧ ✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/rangelink-vscode-extension/src/__tests__/utils/resolveWorkspacePath.test.ts (1)
307-329: 💤 Low valueDuplicate tests: lines 307-317 and 319-329 are functionally identical.
Both tests:
- Override
mockStatto resolve successfully- Call
resolveWorkspacePath('auth.ts', mockVscode)- Assert identical expectations (
workspace-relative, same fsPath, stat called once, findFiles not called)The second test's title ("even if findFiles would fail") suggests a distinct scenario, but since
findFilesis never invoked when the root match succeeds, the test doesn't actually verify failure handling—it's the same test with a different name.Consider removing one or making test 319-329 genuinely distinct (e.g., by verifying a scenario where root resolution doesn't short-circuit).
🤖 Prompt for 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. In `@packages/rangelink-vscode-extension/src/__tests__/utils/resolveWorkspacePath.test.ts` around lines 307 - 329, The two tests calling resolveWorkspacePath('auth.ts', mockVscode) are duplicates; remove one or change the second ("should return root match when bare filename exists at workspace root even if findFiles would fail") to exercise the intended failure path by making mockStat reject (so workspace-relative doesn't short-circuit) and then set mockFindFiles to reject or return no results, then assert the behavior you expect when findFiles fails; reference the resolveWorkspacePath function and the mocks mockStat and mockFindFiles to locate and adjust the tests accordingly.
🤖 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.
Nitpick comments:
In
`@packages/rangelink-vscode-extension/src/__tests__/utils/resolveWorkspacePath.test.ts`:
- Around line 307-329: The two tests calling resolveWorkspacePath('auth.ts',
mockVscode) are duplicates; remove one or change the second ("should return root
match when bare filename exists at workspace root even if findFiles would fail")
to exercise the intended failure path by making mockStat reject (so
workspace-relative doesn't short-circuit) and then set mockFindFiles to reject
or return no results, then assert the behavior you expect when findFiles fails;
reference the resolveWorkspacePath function and the mocks mockStat and
mockFindFiles to locate and adjust the tests accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0e6ee404-6835-4ab8-912d-f0b85d7a7a02
📒 Files selected for processing (2)
packages/rangelink-vscode-extension/src/__tests__/utils/resolveWorkspacePath.test.tspackages/rangelink-vscode-extension/src/utils/resolveWorkspacePath.ts
The two tests at lines 307-317 and 319-329 were functionally identical: both set mockStat to resolve, called resolveWorkspacePath with the same input, and asserted the same expectations. The removed test's stated scenario ("even if findFiles would fail") is impossible: when mockStat resolves, the root-match branch short-circuits before findFiles is ever invoked. The remaining test at line 307 already covers the root-match-only case, and findFiles rejection is independently tested at line 221.
Ref: #585 (review)
CI / Integration Tests (with extensions) — run summary
|
CI / Test & Validate — run summary
|
1 participant
Summary
The bare filename resolution path in
resolveWorkspacePath()always delegated tofindFiles('**/filename', undefined, 2), returningFILENAME_AMBIGUOUSas soon as 2 matches existed anywhere in the workspace. In monorepos, common root-level files likepackage.jsonappear in multiple subdirectories, so typing a bare filename always triggered the ambiguity warning. This fix checks workspace root(s) viafs.statbefore falling back tofindFiles, matching the most likely user intent.node_modulesis now excluded from the findFiles fallback to avoid dependency-directory noise.Changes
resolveWorkspacePath.ts: check workspace root(s) viafs.statbeforefindFiles. Single root match returnsworkspace-relative; multiple root matches across workspace folders returnsFILENAME_AMBIGUOUS{**/node_modules/**}exclude pattern tofindFilesfallbackTest Plan
package.jsonbare filename navigation in monorepo workspaceRelated
Closes #541
Summary by CodeRabbit
Bug Fixes
Tests