fix: broaden Claude CLI discovery paths#433
Conversation
|
@WhiskyChoy is attempting to deploy a commit to the op7418's projects Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Pull request overview
This PR expands Claude CLI discovery so CodePilot can locate installations exposed via pnpm-style wrappers, Yarn globals, and common Node manager shim directories, reducing false “not installed” results across platforms.
Changes:
- Added platform-specific helpers (
getExtraPathDirsFor,getClaudeCandidatePathsFor) to generate broader search directories and candidate executable paths. - Expanded install-type classification to treat pnpm/yarn/manager shim installs as the npm-family channel.
- Added focused unit tests for macOS and Windows candidate generation.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
src/lib/platform.ts |
Broadens candidate directory/path generation and updates path classification for pnpm/yarn/shims. |
src/__tests__/unit/platform-claude-paths.test.ts |
Adds unit tests validating new candidate dir/path generation for darwin/win32. |
electron/main.ts |
Expands Electron-side PATH augmentation and Claude install detection directories/candidates. |
Comments suppressed due to low confidence (1)
electron/main.ts:855
classifyPath()does not recognize the newly-added compatibility locations (~/.claude/local) or npm-family shims (pnpm/yarn/volta/fnm/asdf). As a result, installs discovered from those directories will be reported asunknown(or miss the intended npm-family classification), which conflicts with the PR goal of consistent UI channel reporting. Consider aligning this logic withsrc/lib/platform.ts:classifyClaudePath()(ideally by reusing a shared helper).
function classifyPath(p: string): 'native' | 'homebrew' | 'npm' | 'bun' | 'unknown' {
const n = p.replace(/\\/g, '/');
if (n.includes('/.local/bin/') || n.includes('/.claude/bin/')) return 'native';
if (n.includes('/.bun/bin/') || n.includes('/.bun/install/')) return 'bun';
if (n.includes('/homebrew/') || n.includes('/Cellar/')) return 'homebrew';
if (n.includes('/npm')) return 'npm';
if (n === '/usr/local/bin/claude') {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/lib/platform.ts
Outdated
| pathApi.join(env.APPDATA || pathApi.join(home, 'AppData', 'Roaming'), 'npm'), | ||
| pathApi.join(env.LOCALAPPDATA || pathApi.join(home, 'AppData', 'Local'), 'npm'), | ||
| pathApi.join(env.APPDATA || pathApi.join(home, 'AppData', 'Roaming'), 'pnpm'), | ||
| pathApi.join(env.LOCALAPPDATA || pathApi.join(home, 'AppData', 'Local'), 'pnpm'), | ||
| pnpmHome, | ||
| pathApi.join(home, '.npm-global', 'bin'), | ||
| pathApi.join(home, '.local', 'bin'), | ||
| pathApi.join(home, '.claude', 'bin'), | ||
| pathApi.join(home, '.claude', 'local'), | ||
| pathApi.join(home, '.bun', 'bin'), | ||
| pathApi.join(home, '.yarn', 'bin'), | ||
| pathApi.join(home, '.config', 'yarn', 'global', 'node_modules', '.bin'), | ||
| voltaHome, | ||
| pathApi.join(home, '.fnm', 'current', 'bin'), | ||
| pathApi.join(home, '.nvm', 'current', 'bin'), | ||
| pathApi.join(home, '.asdf', 'shims'), |
There was a problem hiding this comment.
getClaudeCandidatePathsFor() reuses getExtraPathDirsFor() (which is intended for PATH expansion), but the Windows extras list currently starts with AppData npm/pnpm locations. Since findClaudeBinary() returns the first candidate that passes --version, this can incorrectly prefer npm/pnpm installs over the documented priority (native > bun/homebrew > npm). Consider building a separate, explicitly prioritized dir list for candidate paths (or reordering extras so native/bun come first).
| pathApi.join(env.APPDATA || pathApi.join(home, 'AppData', 'Roaming'), 'npm'), | |
| pathApi.join(env.LOCALAPPDATA || pathApi.join(home, 'AppData', 'Local'), 'npm'), | |
| pathApi.join(env.APPDATA || pathApi.join(home, 'AppData', 'Roaming'), 'pnpm'), | |
| pathApi.join(env.LOCALAPPDATA || pathApi.join(home, 'AppData', 'Local'), 'pnpm'), | |
| pnpmHome, | |
| pathApi.join(home, '.npm-global', 'bin'), | |
| pathApi.join(home, '.local', 'bin'), | |
| pathApi.join(home, '.claude', 'bin'), | |
| pathApi.join(home, '.claude', 'local'), | |
| pathApi.join(home, '.bun', 'bin'), | |
| pathApi.join(home, '.yarn', 'bin'), | |
| pathApi.join(home, '.config', 'yarn', 'global', 'node_modules', '.bin'), | |
| voltaHome, | |
| pathApi.join(home, '.fnm', 'current', 'bin'), | |
| pathApi.join(home, '.nvm', 'current', 'bin'), | |
| pathApi.join(home, '.asdf', 'shims'), | |
| pathApi.join(home, '.claude', 'bin'), | |
| pathApi.join(home, '.claude', 'local'), | |
| pathApi.join(home, '.bun', 'bin'), | |
| pathApi.join(home, '.local', 'bin'), | |
| pathApi.join(home, '.yarn', 'bin'), | |
| pathApi.join(home, '.config', 'yarn', 'global', 'node_modules', '.bin'), | |
| voltaHome, | |
| pathApi.join(home, '.fnm', 'current', 'bin'), | |
| pathApi.join(home, '.nvm', 'current', 'bin'), | |
| pathApi.join(home, '.asdf', 'shims'), | |
| pathApi.join(env.APPDATA || pathApi.join(home, 'AppData', 'Roaming'), 'npm'), | |
| pathApi.join(env.LOCALAPPDATA || pathApi.join(home, 'AppData', 'Local'), 'npm'), | |
| pathApi.join(env.APPDATA || pathApi.join(home, 'AppData', 'Roaming'), 'pnpm'), | |
| pathApi.join(env.LOCALAPPDATA || pathApi.join(home, 'AppData', 'Local'), 'pnpm'), | |
| pnpmHome, | |
| pathApi.join(home, '.npm-global', 'bin'), |
There was a problem hiding this comment.
Good catch. I split PATH expansion from Claude candidate ordering, so Windows candidate generation now uses an explicit priority list and keeps native/compatibility wrappers ahead of npm-family shims. Fixed in 1dfc6e7.
| pathApi.join(home, '.bun', 'bin'), | ||
| pathApi.join(home, '.npm-global', 'bin'), | ||
| pathApi.join(home, '.yarn', 'bin'), | ||
| pathApi.join(home, '.config', 'yarn', 'global', 'node_modules', '.bin'), | ||
| pathApi.join(home, '.volta', 'bin'), | ||
| pathApi.join(home, '.fnm', 'current', 'bin'), | ||
| pathApi.join(home, '.nvm', 'current', 'bin'), | ||
| pathApi.join(home, '.asdf', 'shims'), | ||
| pnpmHome, | ||
| pathApi.join(home, '.local', 'share', 'pnpm'), | ||
| ...(platform === 'darwin' ? [pathApi.join(home, 'Library', 'pnpm')] : []), | ||
| '/usr/local/bin', | ||
| '/opt/homebrew/bin', | ||
| '/usr/bin', | ||
| '/bin', | ||
| ]; |
There was a problem hiding this comment.
On non-Windows platforms, the candidate dir list used by getClaudeCandidatePathsFor() currently places npm-family locations (e.g. ~/.npm-global/bin, yarn/pnpm) before Homebrew paths (/opt/homebrew/bin, /usr/local/bin). Because findClaudeBinary() selects the first working candidate, this can flip the intended priority and pick an npm shim even when a Homebrew install is present. Reorder the candidate generation to keep native/bun/homebrew ahead of npm-family locations.
There was a problem hiding this comment.
Fixed in 1dfc6e7. Non-Windows candidate generation now keeps native, bun, and Homebrew locations ahead of npm-family bins, and I added ordering assertions so we do not regress on this priority again.
electron/main.ts
Outdated
| const pnpmHome = process.env.PNPM_HOME; | ||
| const voltaHome = process.env.VOLTA_HOME ? path.join(process.env.VOLTA_HOME, 'bin') : ''; |
There was a problem hiding this comment.
getExpandedShellPath() reads PNPM_HOME/VOLTA_HOME from process.env, but when Electron is launched from Dock/Finder process.env is often missing shell-initialized vars and userShellEnv is the authoritative source. This can prevent pnpm/volta paths from being added even though they exist in the resolved shell env. Consider deriving these from userShellEnv (or the merged execEnv) instead of process.env here.
There was a problem hiding this comment.
Good point. Electron now derives PNPM_HOME, VOLTA_HOME, and related path expansion from the merged shell env (process.env + userShellEnv), so Dock/Finder launches can still pick up shell-initialized values. Fixed in 1dfc6e7.
| path.join(home, '.local', 'bin'), | ||
| path.join(home, '.claude', 'bin'), | ||
| path.join(home, '.claude', 'local'), | ||
| path.join(home, '.bun', 'bin'), | ||
| path.join(home, '.npm-global', 'bin'), | ||
| path.join(home, '.yarn', 'bin'), | ||
| path.join(home, '.config', 'yarn', 'global', 'node_modules', '.bin'), | ||
| path.join(home, '.volta', 'bin'), | ||
| path.join(home, '.fnm', 'current', 'bin'), | ||
| path.join(home, '.nvm', 'current', 'bin'), | ||
| path.join(home, '.asdf', 'shims'), | ||
| process.env.PNPM_HOME || '', | ||
| path.join(home, '.local', 'share', 'pnpm'), | ||
| ...(process.platform === 'darwin' ? [path.join(home, 'Library', 'pnpm')] : []), | ||
| '/opt/homebrew/bin', | ||
| '/usr/local/bin', | ||
| ]; |
There was a problem hiding this comment.
The claudeSearchDirs ordering for non-Windows currently places npm-family bins (e.g. ~/.npm-global/bin, yarn/pnpm) before Homebrew directories (/opt/homebrew/bin, /usr/local/bin). Since primary is derived from the first successful candidatePaths probe, this can prefer an npm shim even when a Homebrew install exists, contradicting the comment about priority. Reorder claudeSearchDirs so homebrew directories are checked before npm-family bins.
There was a problem hiding this comment.
Fixed in 1dfc6e7. The Electron-side detection list now uses the same priority shape as the shared helper: native/compat wrappers first, then bun/Homebrew, then npm-family bins.
| describe('Claude path discovery helpers', () => { | ||
| it('darwin search dirs include pnpm, yarn, and claude local compatibility paths', () => { | ||
| const home = '/Users/tester'; | ||
| const dirs = getExtraPathDirsFor('darwin', home, { | ||
| PNPM_HOME: '/Users/tester/Library/pnpm', | ||
| } as unknown as NodeJS.ProcessEnv); | ||
|
|
||
| assert.ok(dirs.includes('/Users/tester/Library/pnpm')); | ||
| assert.ok(dirs.includes('/Users/tester/.claude/local')); | ||
| assert.ok(dirs.includes('/Users/tester/.yarn/bin')); | ||
| }); |
There was a problem hiding this comment.
The new tests verify presence of pnpm/yarn/compatibility paths, but they don't cover the priority/ordering semantics that findClaudeBinary() depends on (first successful candidate wins). Adding an assertion about the relative ordering (e.g. native dirs/candidates appearing before npm-family ones) would help prevent regressions where discovery starts preferring the wrong installation channel.
There was a problem hiding this comment.
Agreed. I added explicit ordering assertions in the path helper tests so they now cover the first-match priority that findClaudeBinary() depends on. Included in 1dfc6e7.
|
Follow-up note: I also addressed Copilot's suppressed classifyPath suggestion while touching the same area. Electron-side classification now recognizes \.claude/local\ and the newly added npm-family shim locations, so install-channel reporting stays aligned with the shared platform helper. |
关联 Issue / Related Issue: closes #432
Summary / 概述
This PR broadens Claude CLI discovery so CodePilot can find installations that come from pnpm-style wrappers and other compatibility locations, instead of only a smaller hard-coded path set.
这个 PR 扩展了 Claude CLI 的查找逻辑，让 CodePilot 能识别 pnpm 风格包装路径和其他兼容安装位置，而不再只依赖较少的一组硬编码路径。
Root cause / 根因
CodePilot previously checked only a subset of native, bun, homebrew, and npm paths. That missed several directories that are now common in real user environments, especially on macOS.
之前 CodePilot 只检查了部分 native、bun、homebrew、npm 路径，因此遗漏了如今用户环境里常见的若干目录，尤其是 macOS。
Changes / 修改内容
expand shared PATH helper coverage to include
~/.claude/local, pnpm home directories, Yarn global bins, and Node manager shimsadd a reusable
getClaudeCandidatePathsFor()helper to make path generation easier to test across platformsupdate Electron-side install detection to use the broader search directories and candidate executable list
classify pnpm/yarn/manager shim installs as the npm-family channel for UI reporting consistency
add focused unit tests covering macOS and Windows candidate generation
扩展共享 PATH 辅助逻辑，纳入
~/.claude/local、pnpm 目录、Yarn 全局 bin 与 Node 管理器 shim新增可复用的
getClaudeCandidatePathsFor()，便于跨平台测试候选路径生成更新 Electron 侧安装检测逻辑，使用更完整的搜索目录与候选可执行文件集合
将 pnpm/yarn/管理器 shim 安装统一归类到 npm-family，保持 UI 展示一致
增加针对 macOS 和 Windows 的聚焦单元测试
Test plan / 测试情况
npm run typechecknpx tsx --test src/__tests__/unit/platform-claude-paths.test.tsNotes / 说明
npm run test:unitlocally on Windows, but there are existing unrelatedERR_UNSUPPORTED_ESM_URL_SCHEMEfailures in other test files. This PR does not change that behavior.npm run test:unit，但当前仓库里还有与本 PR 无关的ERR_UNSUPPORTED_ESM_URL_SCHEME旧失败，本 PR 未涉及该问题。