fix(upgrade): detect npm install method from node_modules path#723
Conversation
When the CLI is installed via npm/pnpm and run on Windows (especially via NVM), spawning package manager binaries to detect the install method fails with ENOENT because .cmd files aren't found without shell: true. This left the method as "unknown", blocking upgrades. Add detectPackageManagerFromPath() — a fast, authoritative check that examines process.argv[1] for a node_modules segment. Placed in the detection cascade after Homebrew (which uses /Cellar/) and before the DB lookup, it overrides stale stored info and avoids subprocess calls entirely. Detects pnpm via its .pnpm directory; defaults to npm for other layouts. Fixes CLI-Y1
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. Bug Fixes 🐛Upgrade
Other
Internal Changes 🔧
🤖 This preview updates automatically when you update the PR. |
|
Codecov Results 📊✅ 134 passed | Total: 134 | Pass Rate: 100% | Execution Time: 0ms 📊 Comparison with Base Branch
✨ No test changes detected All tests are passing successfully. ✅ Patch coverage is 100.00%. Project has 1603 uncovered lines. Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
- Coverage 95.30% 95.28% -0.02%
==========================================
Files 232 232 —
Lines 33970 33992 +22
Branches 0 0 —
==========================================
+ Hits 32376 32389 +13
- Misses 1594 1603 +9
- Partials 0 0 —Generated by Codecov Action |
Address review feedback: - Replace PATH_SEPARATOR_RE regex with path.sep (already imported) - Detect bun global installs via .bun path segment - Use join() in tests for platform-native path construction
|
Addressed both review comments in 387e801:
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 387e801. Configure here.
Address Cursor BugBot feedback: - Move detectPackageManagerFromPath() to after subprocess calls in detectLegacyInstallationMethod() so stored yarn info is preserved and subprocess-based yarn detection still works on macOS/Linux - Wrap auto-save setInstallInfo() in try-catch so a read-only DB doesn't crash detection after it already succeeded - Update tests: stored info now takes priority over path detection
1 participant
Summary
detectPackageManagerFromPath()— a fast path-based check that detects npm/pnpm installation by examiningprocess.argv[1]for anode_modulessegment/Cellar/) and before the DB lookup, mirroring the same authoritative-override patternProblem
When the CLI is installed via npm and run on Windows (especially via NVM/Laravel Herd), spawning
npm,pnpm,bun,yarnto detect the install method fails with ENOENT because.cmdfiles aren't found byspawn()withoutshell: true. This left the method as"unknown", blockingsentry cli upgradeentirely — even though the CLI was clearly running from anode_modulesdirectory.Approach
Rather than fixing the Windows spawn issue (which would still be slow and fragile), add a fast path check that examines the script path for a
node_modulessegment — the same pattern used by the Homebrew/Cellar/check. Cross-platform (splits on both/and\), handles Windows 8.3 short paths, and distinguishes pnpm via its.pnpmdirectory.Fixes CLI-Y1