fix(language-server): convert URI path to filesystem path for package manager detection - #147
Conversation
… manager detection `detectPackageManagerFromProject` receives a POSIX-style URI path from `WorkspaceContext`, but `package-manager-detector`'s `detect()` reads the actual filesystem and requires a platform-native path. Use `vscode-uri` to convert the path before passing it to `detect()`. Fixes path handling on Windows where URI paths and filesystem paths differ.
📝 WalkthroughWalkthroughPackage-manager detection now converts the project URI path to a native filesystem path before calling Merge Risk: ⚪ Minimal · up to This localized path-conversion fix should preserve package-manager detection on supported platforms, and no actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/language-server/src/workspace.integration.test.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. packages/language-server/src/workspace.test.tsESLint skipped: the matched ESLint configuration already failed (missing-dependency). 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 |
|
@9romise on windows |
|
@9romise 😭 |
|
@9romise 这问题真的存在 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: adf92e05-465f-4da5-b7a9-da300616a911
📒 Files selected for processing (2)
packages/language-server/src/workspace.integration.test.tspackages/language-server/src/workspace.test.ts
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.
| import { detect } from 'package-manager-detector/detect' | ||
| import { afterEach, describe, expect, it, vi } from 'vitest' | ||
| import { detectPackageManagerFromProject } from './workspace' | ||
|
|
||
| vi.mock('package-manager-detector/detect', () => ({ | ||
| detect: vi.fn(), | ||
| })) | ||
|
|
||
| const { detect } = await import('package-manager-detector/detect') | ||
|
|
||
| describe('detectPackageManagerFromProject', () => { | ||
| afterEach(() => { | ||
| vi.mocked(detect).mockReset() | ||
| }) | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- workspace.ts ---'
sed -n '1,120p' packages/language-server/src/workspace.ts
printf '%s\n' '--- workspace.test.ts ---'
sed -n '1,220p' packages/language-server/src/workspace.test.ts
printf '%s\n' '--- relevant diff ---'
git diff -- packages/language-server/src/workspace.ts packages/language-server/src/workspace.test.tsRepository: npmx-dev/vscode-npmx
Length of output: 4942
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import re
source = Path("packages/language-server/src/workspace.ts").read_text()
test = Path("packages/language-server/src/workspace.test.ts").read_text()
function = re.search(
r"export async function detectPackageManagerFromProject\(rootPath: string\).*?"
r"\n\}\n",
source,
re.S,
)
if not function:
raise SystemExit("detectPackageManagerFromProject was not found")
body = function.group(0)
required = [
"const fsPath = URI.file(rootPath).fsPath",
"cwd: fsPath",
"stopDir: fsPath",
"case 'bun':",
"case 'npm':",
"case 'pnpm':",
"case 'yarn':",
"return 'npm'",
]
missing = [item for item in required if item not in body]
if missing:
raise SystemExit(f"Missing expected function behaviour: {missing}")
supported_cases = re.findall(r"mockResolvedValue\(\{\s*name:\s*'([^']+)'", test)
print("supported detector names handled by source:", ["bun", "npm", "pnpm", "yarn"])
print("detector names mocked by current tests:", supported_cases)
print("native-path call contract in source: cwd and stopDir both receive fsPath")
print("supported-manager test present:", any(name in {"bun", "npm", "pnpm", "yarn"} for name in supported_cases))
PY
printf '%s\n' '--- package declarations for URI and detector ---'
rg -n '"(vscode-uri|package-manager-detector)"' package.json packages pnpm-lock.yaml yarn.lock package-lock.json 2>/dev/null || trueRepository: npmx-dev/vscode-npmx
Length of output: 830
Add a supported-manager test. Mock detect() to return pnpm, assert that the function resolves to pnpm, and assert that detect() receives URI.file('/repo').fsPath as both cwd and stopDir.
|
Sorry for the late reply. I've made some adjustments to the tests and merged the PR — thanks for your contribution! I've been quite busy with work lately. That said, in OSS communities, please avoid pinging maintainers casually, as it's generally considered impolite. |
2 participants
detectPackageManagerFromProjectreceives a POSIX-style URI path fromWorkspaceContext, butpackage-manager-detector'sdetect()reads the actual filesystem and requires a platform-native path. Usevscode-urito convert the path before passing it todetect().Without this fix, the path lookup silently fails on Windows (since the POSIX-style path doesn't exist on the local filesystem), causing the detector to always fall back to
npmregardless of the actual project configuration.Fixes path handling on Windows where URI paths and filesystem paths differ.
Note for reviewers
If you need to verify behaviors on Windows, feel free to reach out to me.