From 0417c14e41a7af61a56d9ba78a5e84eb94a036b1 Mon Sep 17 00:00:00 2001 From: Minsu Lee Date: Fri, 23 Oct 2026 19:13:15 +0900 Subject: [PATCH 1/3] fix(lsp): check a project-local pyright-langserver before PATH and bunx Resolve pyright-langserver from node_modules/.bin, walking up from the server root, before falling back to PATH and the pinned bunx run. This makes the pyright server honor the version a project already installs. Move findPackageBin and resolvePackageBin from typescript.ts into utils.ts so both servers share them. Closes #147 --- packages/lsp/src/server/pyright.ts | 5 ++- packages/lsp/src/server/typescript.ts | 65 +-------------------------- packages/lsp/src/server/utils.ts | 64 ++++++++++++++++++++++++++ packages/lsp/test/unit/server.test.ts | 44 ++++++++++++++++-- 4 files changed, 109 insertions(+), 69 deletions(-) diff --git a/packages/lsp/src/server/pyright.ts b/packages/lsp/src/server/pyright.ts index da32e07..90b4d49 100644 --- a/packages/lsp/src/server/pyright.ts +++ b/packages/lsp/src/server/pyright.ts @@ -5,7 +5,7 @@ import type { LSPServerInfo } from './types' import { spawn } from 'node:child_process' import process from 'node:process' -import { attachLSPProcessHandlers, log, nearestRoot } from './utils' +import { attachLSPProcessHandlers, findPackageBin, log, nearestRoot } from './utils' export const PYRIGHT_VERSION = '1.1.414' @@ -20,7 +20,8 @@ export const PyrightServer: LSPServerInfo = { ]), async spawn(root) { // Read PATH now rather than the copy Bun captured at startup - const pyright = Bun.which('pyright-langserver', { PATH: process.env.PATH ?? '' }) + const pyright = findPackageBin(root, 'pyright', 'pyright-langserver') + ?? Bun.which('pyright-langserver', { PATH: process.env.PATH ?? '' }) if (!pyright) { // pyright-langserver is a bin of the pyright package, not a package itself. // spawn reports a missing bunx only as an async 'error' event, so check for it first. diff --git a/packages/lsp/src/server/typescript.ts b/packages/lsp/src/server/typescript.ts index 7d8b25e..cda4b98 100644 --- a/packages/lsp/src/server/typescript.ts +++ b/packages/lsp/src/server/typescript.ts @@ -14,7 +14,7 @@ import fs from 'node:fs' import { createRequire } from 'node:module' import path from 'node:path' import process from 'node:process' -import { attachLSPProcessHandlers, findRunnableBin, log, nearestRoot, runsVersion } from './utils' +import { attachLSPProcessHandlers, findPackageBin, findRunnableBin, log, nearestRoot, resolvePackageBin, runsVersion } from './utils' const TS_EXTENSIONS = ['.ts', '.tsx', '.js', '.jsx', '.mjs', '.cjs', '.mts', '.cts'] @@ -33,52 +33,6 @@ function findTypeScriptPackageRoot(start: string): string | undefined { } } -/** Resolve a package-owned bin declared by the package and exposed by its shared shim. */ -function resolvePackageBin(packageRoot: string, name: string): string | undefined { - try { - const pkg = JSON.parse( - fs.readFileSync(path.join(packageRoot, 'package.json'), 'utf8'), - ) as { bin?: string | Record } - const relativeTarget = typeof pkg.bin === 'string' ? pkg.bin : pkg.bin?.[name] - if (!relativeTarget) { return undefined } - - const target = path.resolve(packageRoot, relativeTarget) - if (!fs.existsSync(target)) { return undefined } - const ext = process.platform === 'win32' ? '.cmd' : '' - const packageDir = path.dirname(packageRoot) - const nodeModules = path.basename(packageDir).startsWith('@') - ? path.dirname(packageDir) - : packageDir - const shim = path.join(nodeModules, '.bin', `${name}${ext}`) - if (!fs.existsSync(shim)) { return undefined } - - // npm-style symlinks and package-manager-generated wrapper scripts are both - // valid shims. For wrappers, require a reference to the package's declared - // target so a stale or unrelated shared shim is never selected. - const stat = fs.lstatSync(shim) - if (stat.isSymbolicLink()) { - const resolvedShim = path.resolve(path.dirname(shim), fs.readlinkSync(shim)) - return fs.existsSync(resolvedShim) - && fs.realpathSync(resolvedShim) === fs.realpathSync(target) - ? shim - : undefined - } - const normalizeShimPath = (value: string): string => { - const normalized = value.replaceAll('\\', '/') - return process.platform === 'win32' ? normalized.toLowerCase() : normalized - } - const wrapper = normalizeShimPath(fs.readFileSync(shim, 'utf8')) - const relativeFromBin = normalizeShimPath(path.relative(path.dirname(shim), target)) - const normalizedTarget = normalizeShimPath(target) - return wrapper.includes(relativeFromBin) || wrapper.includes(normalizedTarget) - ? shim - : undefined - } - catch { - return undefined - } -} - /** Whether the installed `typescript` package is the native (TS 7+) build. */ function isNativeTypeScriptPackage(packageRoot: string): boolean { try { @@ -199,23 +153,6 @@ function findWrapperPackageRoot(wrapper: string, bin: string): string | undefine return undefined } -function findPackageBin( - start: string, - packageName: string, - name: string, - isUsable: (packageRoot: string) => boolean = () => true, -): string | undefined { - let dir = path.resolve(start) - while (true) { - const packageRoot = path.join(dir, 'node_modules', ...packageName.split('/')) - const bin = resolvePackageBin(packageRoot, name) - if (bin && isUsable(packageRoot)) { return bin } - const parent = path.dirname(dir) - if (parent === dir) { return undefined } - dir = parent - } -} - /** * Resolve a native TypeScript 7 LSP command for `root`: a project-local one if * the workspace provides it, else a global `tsrs` on PATH. Exported for testing. diff --git a/packages/lsp/src/server/utils.ts b/packages/lsp/src/server/utils.ts index 955f493..2e7b23f 100644 --- a/packages/lsp/src/server/utils.ts +++ b/packages/lsp/src/server/utils.ts @@ -5,6 +5,7 @@ import type { ChildProcessWithoutNullStreams } from 'node:child_process' import type { RootFunction } from './types' +import { existsSync, lstatSync, readFileSync, readlinkSync, realpathSync } from 'node:fs' import fs from 'node:fs/promises' import path from 'node:path' import process from 'node:process' @@ -95,6 +96,69 @@ export async function runsVersion(bin: string, root: string): Promise { } } +/** Resolve a package-owned bin declared by the package and exposed by its shared shim. */ +export function resolvePackageBin(packageRoot: string, name: string): string | undefined { + try { + const pkg = JSON.parse( + readFileSync(path.join(packageRoot, 'package.json'), 'utf8'), + ) as { bin?: string | Record } + const relativeTarget = typeof pkg.bin === 'string' ? pkg.bin : pkg.bin?.[name] + if (!relativeTarget) { return undefined } + + const target = path.resolve(packageRoot, relativeTarget) + if (!existsSync(target)) { return undefined } + const ext = process.platform === 'win32' ? '.cmd' : '' + const packageDir = path.dirname(packageRoot) + const nodeModules = path.basename(packageDir).startsWith('@') + ? path.dirname(packageDir) + : packageDir + const shim = path.join(nodeModules, '.bin', `${name}${ext}`) + if (!existsSync(shim)) { return undefined } + + // npm-style symlinks and package-manager-generated wrapper scripts are both + // valid shims. For wrappers, require a reference to the package's declared + // target so a stale or unrelated shared shim is never selected. + const stat = lstatSync(shim) + if (stat.isSymbolicLink()) { + const resolvedShim = path.resolve(path.dirname(shim), readlinkSync(shim)) + return existsSync(resolvedShim) + && realpathSync(resolvedShim) === realpathSync(target) + ? shim + : undefined + } + const normalizeShimPath = (value: string): string => { + const normalized = value.replaceAll('\\', '/') + return process.platform === 'win32' ? normalized.toLowerCase() : normalized + } + const wrapper = normalizeShimPath(readFileSync(shim, 'utf8')) + const relativeFromBin = normalizeShimPath(path.relative(path.dirname(shim), target)) + const normalizedTarget = normalizeShimPath(target) + return wrapper.includes(relativeFromBin) || wrapper.includes(normalizedTarget) + ? shim + : undefined + } + catch { + return undefined + } +} + +export function findPackageBin( + start: string, + packageName: string, + name: string, + isUsable: (packageRoot: string) => boolean = () => true, +): string | undefined { + let dir = path.resolve(start) + while (true) { + const packageRoot = path.join(dir, 'node_modules', ...packageName.split('/')) + const bin = resolvePackageBin(packageRoot, name) + if (bin && isUsable(packageRoot)) { return bin } + const parent = path.dirname(dir) + if (parent === dir) { return undefined } + dir = parent + } +} + // ============================================================================= // Root Detection Utilities // ============================================================================= diff --git a/packages/lsp/test/unit/server.test.ts b/packages/lsp/test/unit/server.test.ts index 2d1f830..419c92c 100644 --- a/packages/lsp/test/unit/server.test.ts +++ b/packages/lsp/test/unit/server.test.ts @@ -466,11 +466,11 @@ describe.skipIf(process.platform === 'win32')('findRunnableBin', () => { // Each fake server records its name and arguments in `invoked` under the project root. const recorder = (name: string): string => `#!/bin/sh\necho "${name} $*" > invoked\n` -/** Spawn `server` with only `pathBins` on PATH and return what the launched binary recorded. */ async function spawnServerWith( server: LSPServerInfo, pathBins: Record, setupRoot: (rootDir: string) => Promise = async () => {}, + subdir = '', ): Promise { const binDir = await fs.mkdtemp(path.join(os.tmpdir(), 'lsp-fallback-bin-')) const rootDir = await fs.mkdtemp(path.join(os.tmpdir(), 'lsp-fallback-root-')) @@ -480,15 +480,17 @@ async function spawnServerWith( await fs.writeFile(path.join(binDir, name), script, { mode: 0o755 }) } await setupRoot(rootDir) + const spawnRoot = path.join(rootDir, subdir) + await fs.mkdir(spawnRoot, { recursive: true }) process.env.PATH = binDir - const handle = await server.spawn(rootDir) + const handle = await server.spawn(spawnRoot) process.env.PATH = savedPath expect(handle).toBeDefined() const proc = handle!.process if (proc.exitCode === null && proc.signalCode === null) { await new Promise(resolve => proc.once('exit', resolve)) } - return (await fs.readFile(path.join(rootDir, 'invoked'), 'utf8')).trim() + return (await fs.readFile(path.join(spawnRoot, 'invoked'), 'utf8')).trim() } finally { process.env.PATH = savedPath @@ -606,6 +608,42 @@ describe.skipIf(process.platform === 'win32')('PyrightServer fallback', () => { expect(invoked).toBe('path --stdio') }) + async function installLocalPyright(dir: string, script: string, strayShim?: string): Promise { + const packageDir = path.join(dir, 'node_modules', 'pyright') + await fs.mkdir(packageDir, { recursive: true }) + await fs.writeFile( + path.join(packageDir, 'package.json'), + JSON.stringify({ name: 'pyright', bin: { 'pyright': 'index.js', 'pyright-langserver': 'langserver.index.js' } }), + ) + await fs.writeFile(path.join(packageDir, 'langserver.index.js'), script, { mode: 0o755 }) + const binDir = path.join(dir, 'node_modules', '.bin') + await fs.mkdir(binDir) + const shim = path.join(binDir, 'pyright-langserver') + if (strayShim) { + await fs.writeFile(shim, strayShim, { mode: 0o755 }) + } + else { + await fs.symlink(path.join('..', 'pyright', 'langserver.index.js'), shim) + } + } + + const pathAndBunx = { 'pyright-langserver': recorder('path'), 'bunx': recorder('bunx') } + + test('prefers a project-local pyright-langserver over PATH and bunx', async () => { + const invoked = await spawnServerWith(PyrightServer, pathAndBunx, rootDir => installLocalPyright(rootDir, recorder('local'))) + expect(invoked).toBe('local --stdio') + }) + + test('finds a project-local pyright-langserver in a parent of the root', async () => { + const invoked = await spawnServerWith(PyrightServer, pathAndBunx, rootDir => installLocalPyright(rootDir, recorder('local')), 'backend') + expect(invoked).toBe('local --stdio') + }) + + test('ignores a .bin/pyright-langserver shim the pyright package does not own', async () => { + const invoked = await spawnServerWith(PyrightServer, pathAndBunx, rootDir => installLocalPyright(rootDir, recorder('local'), recorder('stray'))) + expect(invoked).toBe('path --stdio') + }) + test('returns undefined with the install hint when bunx is missing', async () => { const emptyDir = await fs.mkdtemp(path.join(os.tmpdir(), 'lsp-fallback-empty-')) const savedPath = process.env.PATH From 065b64ee0ee63ffd9578e9fb9bc9d7ff85024182 Mon Sep 17 00:00:00 2001 From: Minsu Lee Date: Fri, 23 Oct 2026 20:01:18 +0900 Subject: [PATCH 2/3] fix(lsp): run pyright shims through a shell on Windows and accept pnpm shims On Windows a project-local or npm-global pyright-langserver is a .cmd shim, which spawn can only run through a shell, so reuse nativeServerSpawnArgs as the classic typescript path does. pnpm .bin wrappers reference the package's real path in the .pnpm store, so resolvePackageBin also matches the wrapper against the realpath of the target. --- packages/lsp/src/server/pyright.ts | 7 ++++++- packages/lsp/src/server/utils.ts | 13 ++++++++++--- packages/lsp/test/unit/server.test.ts | 22 ++++++++++++++++++++++ 3 files changed, 38 insertions(+), 4 deletions(-) diff --git a/packages/lsp/src/server/pyright.ts b/packages/lsp/src/server/pyright.ts index 90b4d49..c33bb30 100644 --- a/packages/lsp/src/server/pyright.ts +++ b/packages/lsp/src/server/pyright.ts @@ -5,6 +5,7 @@ import type { LSPServerInfo } from './types' import { spawn } from 'node:child_process' import process from 'node:process' +import { nativeServerSpawnArgs } from './typescript' import { attachLSPProcessHandlers, findPackageBin, log, nearestRoot } from './utils' export const PYRIGHT_VERSION = '1.1.414' @@ -37,8 +38,12 @@ export const PyrightServer: LSPServerInfo = { return { process: proc } } - const proc = spawn(pyright, ['--stdio'], { + // On Windows both a project-local and an npm-global pyright-langserver are .cmd shims, + // which spawn can only run through a shell. + const launch = nativeServerSpawnArgs(pyright) + const proc = spawn(launch.command, ['--stdio'], { cwd: root, + shell: launch.shell, }) attachLSPProcessHandlers(proc, 'pyright') return { process: proc } diff --git a/packages/lsp/src/server/utils.ts b/packages/lsp/src/server/utils.ts index 2e7b23f..6b1582f 100644 --- a/packages/lsp/src/server/utils.ts +++ b/packages/lsp/src/server/utils.ts @@ -131,9 +131,16 @@ export function resolvePackageBin(packageRoot: string, name: string): string | u return process.platform === 'win32' ? normalized.toLowerCase() : normalized } const wrapper = normalizeShimPath(readFileSync(shim, 'utf8')) - const relativeFromBin = normalizeShimPath(path.relative(path.dirname(shim), target)) - const normalizedTarget = normalizeShimPath(target) - return wrapper.includes(relativeFromBin) || wrapper.includes(normalizedTarget) + // pnpm wrappers reference the package's real path in its .pnpm store, not the node_modules link. + const binDir = path.dirname(shim) + const realTarget = realpathSync(target) + const references = [ + path.relative(binDir, target), + target, + path.relative(realpathSync(binDir), realTarget), + realTarget, + ].map(normalizeShimPath) + return references.some(reference => wrapper.includes(reference)) ? shim : undefined } diff --git a/packages/lsp/test/unit/server.test.ts b/packages/lsp/test/unit/server.test.ts index 419c92c..8514490 100644 --- a/packages/lsp/test/unit/server.test.ts +++ b/packages/lsp/test/unit/server.test.ts @@ -639,6 +639,28 @@ describe.skipIf(process.platform === 'win32')('PyrightServer fallback', () => { expect(invoked).toBe('local --stdio') }) + test('accepts a pnpm wrapper shim that points into the .pnpm store', async () => { + const invoked = await spawnServerWith(PyrightServer, pathAndBunx, async (rootDir) => { + const storeRelative = path.join('.pnpm', 'pyright@1.1.414', 'node_modules', 'pyright') + const storeDir = path.join(rootDir, 'node_modules', storeRelative) + await fs.mkdir(storeDir, { recursive: true }) + await fs.writeFile( + path.join(storeDir, 'package.json'), + JSON.stringify({ name: 'pyright', bin: { 'pyright-langserver': 'langserver.index.js' } }), + ) + await fs.writeFile(path.join(storeDir, 'langserver.index.js'), recorder('local'), { mode: 0o755 }) + await fs.symlink(storeRelative, path.join(rootDir, 'node_modules', 'pyright')) + const binDir = path.join(rootDir, 'node_modules', '.bin') + await fs.mkdir(binDir) + await fs.writeFile( + path.join(binDir, 'pyright-langserver'), + `#!/bin/sh\nbasedir=\${0%/*}\nexec "$basedir/../${storeRelative}/langserver.index.js" "$@"\n`, + { mode: 0o755 }, + ) + }) + expect(invoked).toBe('local --stdio') + }) + test('ignores a .bin/pyright-langserver shim the pyright package does not own', async () => { const invoked = await spawnServerWith(PyrightServer, pathAndBunx, rootDir => installLocalPyright(rootDir, recorder('local'), recorder('stray'))) expect(invoked).toBe('path --stdio') From 0486614df4eccc7737bb983451ea9670b1acbe14 Mon Sep 17 00:00:00 2001 From: Minsu Lee Date: Fri, 23 Oct 2026 20:06:37 +0900 Subject: [PATCH 3/3] refactor(lsp): share Windows shim spawn handling via prepareSpawnCommand pyright no longer imports nativeServerSpawnArgs from the typescript server. Both servers use prepareSpawnCommand from utils, and the classic typescript fallback calls it directly since it never used the native --lsp args. --- packages/lsp/src/server/pyright.ts | 7 ++----- packages/lsp/src/server/typescript.ts | 7 +++---- packages/lsp/src/server/utils.ts | 12 ++++++++++++ 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/packages/lsp/src/server/pyright.ts b/packages/lsp/src/server/pyright.ts index c33bb30..03a0278 100644 --- a/packages/lsp/src/server/pyright.ts +++ b/packages/lsp/src/server/pyright.ts @@ -5,8 +5,7 @@ import type { LSPServerInfo } from './types' import { spawn } from 'node:child_process' import process from 'node:process' -import { nativeServerSpawnArgs } from './typescript' -import { attachLSPProcessHandlers, findPackageBin, log, nearestRoot } from './utils' +import { attachLSPProcessHandlers, findPackageBin, log, nearestRoot, prepareSpawnCommand } from './utils' export const PYRIGHT_VERSION = '1.1.414' @@ -38,9 +37,7 @@ export const PyrightServer: LSPServerInfo = { return { process: proc } } - // On Windows both a project-local and an npm-global pyright-langserver are .cmd shims, - // which spawn can only run through a shell. - const launch = nativeServerSpawnArgs(pyright) + const launch = prepareSpawnCommand(pyright) const proc = spawn(launch.command, ['--stdio'], { cwd: root, shell: launch.shell, diff --git a/packages/lsp/src/server/typescript.ts b/packages/lsp/src/server/typescript.ts index cda4b98..3c016f2 100644 --- a/packages/lsp/src/server/typescript.ts +++ b/packages/lsp/src/server/typescript.ts @@ -14,7 +14,7 @@ import fs from 'node:fs' import { createRequire } from 'node:module' import path from 'node:path' import process from 'node:process' -import { attachLSPProcessHandlers, findPackageBin, findRunnableBin, log, nearestRoot, resolvePackageBin, runsVersion } from './utils' +import { attachLSPProcessHandlers, findPackageBin, findRunnableBin, log, nearestRoot, prepareSpawnCommand, resolvePackageBin, runsVersion } from './utils' const TS_EXTENSIONS = ['.ts', '.tsx', '.js', '.jsx', '.mjs', '.cjs', '.mts', '.cts'] @@ -94,8 +94,7 @@ export function nativeServerSpawnArgs( command: string, platform: NodeJS.Platform = process.platform, ): { command: string, args: string[], shell: boolean } { - const win32 = platform === 'win32' - return { command: win32 ? `"${command}"` : command, args: ['--lsp', '--stdio'], shell: win32 } + return { ...prepareSpawnCommand(command, platform), args: ['--lsp', '--stdio'] } } function hasTsrsBinary(packageRoot: string, root: string): boolean { @@ -268,7 +267,7 @@ export const TypescriptServer: LSPServerInfo = { } // On Windows the bin is a .cmd shim, which only runs through a shell. - const launch = nativeServerSpawnArgs(tsserver) + const launch = prepareSpawnCommand(tsserver) const proc = spawn(launch.command, ['--stdio'], { cwd: root, env: { ...process.env, BUN_BE_BUN: '1' }, diff --git a/packages/lsp/src/server/utils.ts b/packages/lsp/src/server/utils.ts index 6b1582f..edf8ee1 100644 --- a/packages/lsp/src/server/utils.ts +++ b/packages/lsp/src/server/utils.ts @@ -96,6 +96,18 @@ export async function runsVersion(bin: string, root: string): Promise { } } +/** + * On Windows an npm bin is a .cmd shim, which spawn can only run through a shell, + * so the path is quoted for it. + */ +export function prepareSpawnCommand( + command: string, + platform: NodeJS.Platform = process.platform, +): { command: string, shell: boolean } { + const win32 = platform === 'win32' + return { command: win32 ? `"${command}"` : command, shell: win32 } +} + /** Resolve a package-owned bin declared by the package and exposed by its shared shim. */ export function resolvePackageBin(packageRoot: string, name: string): string | undefined { try {