diff --git a/src/domain/search/models.ts b/src/domain/search/models.ts index 1c8230e5..c00cbd76 100644 --- a/src/domain/search/models.ts +++ b/src/domain/search/models.ts @@ -1,4 +1,5 @@ import { execFileSync } from 'node:child_process'; +import { existsSync, realpathSync } from 'node:fs'; import { createRequire } from 'node:module'; import path from 'node:path'; import { createInterface } from 'node:readline'; @@ -6,6 +7,27 @@ import { info } from '../../infrastructure/logger.js'; import { ConfigError, EngineError } from '../../shared/errors.js'; const _require = createRequire(import.meta.url); +const NPM_BIN = process.platform === 'win32' ? 'npm.cmd' : 'npm'; + +/** Resolve a path to its real (symlink-free) form, or return it unchanged if that fails. */ +function tryRealpath(p: string): string { + try { + return realpathSync(p); + } catch { + return p; + } +} + +/** + * Normalize a path for equality comparison. Windows filesystems are + * case-insensitive (but case-preserving), and paths sourced from + * `execFileSync` output vs. `require.resolve` can differ in drive-letter or + * segment casing (e.g. `C:\Users\...` vs `c:\users\...`) despite pointing at + * the same directory — so comparisons must fold case on win32. + */ +function normalizeForComparison(p: string): string { + return process.platform === 'win32' ? p.toLowerCase() : p; +} /** * Resolve the directory where `npm install` should run so the installed @@ -35,6 +57,51 @@ export function resolveNpmInstallCwd(): string | undefined { } } +/** + * True when `dir` is npm's own global modules root — the directory whose + * `node_modules` is npm's global install target (and therefore already + * contains npm's own installation, `node_modules/npm`). + * + * Running `npm install` with this as `cwd` makes npm reify its *own* + * dependency tree as a side effect of installing the requested package. + * On at least one observed setup (Homebrew-managed Node/npm on macOS) this + * deleted npm's own installation and other co-located global packages + * before the install's lifecycle-script error even surfaced (#1720). + * Global installs of codegraph must never run `npm install` here. + * + * Authoritative check: ask npm itself for its global modules root + * (`npm root -g`) and compare against `dir`. This avoids misclassifying a + * normal project that merely happens to depend on the `npm` package itself + * (e.g. a tool that shells out to npm) — such a project's `node_modules/npm` + * would satisfy the old file-existence heuristic without actually being + * npm's global root. Falls back to the file-existence heuristic only if the + * `npm root -g` call itself fails (e.g. npm binary unavailable in PATH). + * + * @internal Exported for unit tests; not part of the public barrel. + */ +export function isNpmGlobalModulesRoot(dir: string | undefined): boolean { + if (!dir) return false; + + const candidate = normalizeForComparison(tryRealpath(path.join(dir, 'node_modules'))); + try { + const globalRoot = execFileSync(NPM_BIN, ['root', '-g'], { + encoding: 'utf8', + timeout: 10_000, + }).trim(); + if (globalRoot) { + return normalizeForComparison(tryRealpath(globalRoot)) === candidate; + } + } catch { + // npm unavailable/unresolvable — fall back to the heuristic below. + } + + try { + return existsSync(path.join(dir, 'node_modules', 'npm', 'package.json')); + } catch { + return false; + } +} + export interface ModelConfig { name: string; dim: number; @@ -138,7 +205,6 @@ export const MODELS: Record = { export const EMBEDDING_STRATEGIES: readonly string[] = ['structured', 'source']; export const DEFAULT_MODEL: string = 'nomic'; -const NPM_BIN = process.platform === 'win32' ? 'npm.cmd' : 'npm'; const BATCH_SIZE_MAP: Record = { minilm: 32, 'jina-small': 16, @@ -173,6 +239,14 @@ export function getModelConfig(modelKey?: string): ModelConfig { */ export function promptInstall(packageName: string): Promise { const installCwd = resolveNpmInstallCwd(); + + if (isNpmGlobalModulesRoot(installCwd)) { + info( + `${packageName} is missing, but codegraph is installed globally — auto-install is skipped to avoid modifying npm's own global installation.\nInstall it yourself with:\n npm install -g ${packageName}`, + ); + return Promise.resolve(false); + } + if (!process.stdin.isTTY) { info(`Installing ${packageName} (optional dependency for semantic search)…`); try { @@ -237,7 +311,10 @@ export async function loadTransformers(): Promise { ); } } - throw new EngineError(`Semantic search requires ${pkg}.\nInstall it with: npm install ${pkg}`); + const manualInstall = isNpmGlobalModulesRoot(resolveNpmInstallCwd()) + ? `npm install -g ${pkg}` + : `npm install ${pkg}`; + throw new EngineError(`Semantic search requires ${pkg}.\nInstall it with: ${manualInstall}`); } } diff --git a/tests/unit/prompt-install.test.ts b/tests/unit/prompt-install.test.ts index eb2b9a8b..5100c607 100644 --- a/tests/unit/prompt-install.test.ts +++ b/tests/unit/prompt-install.test.ts @@ -8,6 +8,8 @@ * so every test gets a fresh embedder module with its own mocks. */ +import fs from 'node:fs'; +import os from 'node:os'; import path from 'node:path'; import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'; @@ -232,3 +234,130 @@ describe('resolveNpmInstallCwd', () => { expect(resolveNpmInstallCwd()).toBeUndefined(); }); }); + +describe('isNpmGlobalModulesRoot', () => { + let tmpDir: string; + + beforeEach(() => { + vi.resetModules(); + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-npm-global-')); + }); + + afterEach(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); + vi.restoreAllMocks(); + }); + + test('returns true when `npm root -g` matches dir/node_modules', async () => { + const execMock = vi.fn(() => `${path.join(tmpDir, 'node_modules')}\n`); + vi.doMock('node:child_process', () => ({ execFileSync: execMock })); + + const { isNpmGlobalModulesRoot } = await import('../../src/domain/search/models.js'); + expect(isNpmGlobalModulesRoot(tmpDir)).toBe(true); + expect(execMock).toHaveBeenCalledWith( + expectedNpmBin, + ['root', '-g'], + expect.objectContaining({ encoding: 'utf8' }), + ); + }); + + test('returns false for a normal project dir even if it depends on the npm package', async () => { + // A project that happens to have `npm` as a dependency must NOT be + // misclassified as npm's global root — only `npm root -g` is authoritative. + fs.mkdirSync(path.join(tmpDir, 'node_modules', 'npm'), { recursive: true }); + fs.writeFileSync(path.join(tmpDir, 'node_modules', 'npm', 'package.json'), '{}'); + const execMock = vi.fn(() => `${path.join(os.tmpdir(), 'some-other-global-root')}\n`); + vi.doMock('node:child_process', () => ({ execFileSync: execMock })); + + const { isNpmGlobalModulesRoot } = await import('../../src/domain/search/models.js'); + expect(isNpmGlobalModulesRoot(tmpDir)).toBe(false); + }); + + test('falls back to node_modules/npm heuristic when `npm root -g` fails', async () => { + fs.mkdirSync(path.join(tmpDir, 'node_modules', 'npm'), { recursive: true }); + fs.writeFileSync(path.join(tmpDir, 'node_modules', 'npm', 'package.json'), '{}'); + const execMock = vi.fn(() => { + throw new Error('npm: command not found'); + }); + vi.doMock('node:child_process', () => ({ execFileSync: execMock })); + + const { isNpmGlobalModulesRoot } = await import('../../src/domain/search/models.js'); + expect(isNpmGlobalModulesRoot(tmpDir)).toBe(true); + }); + + test('falls back to false when `npm root -g` fails and heuristic dir is a normal project', async () => { + fs.mkdirSync(path.join(tmpDir, 'node_modules', '@optave', 'codegraph'), { recursive: true }); + const execMock = vi.fn(() => { + throw new Error('npm: command not found'); + }); + vi.doMock('node:child_process', () => ({ execFileSync: execMock })); + + const { isNpmGlobalModulesRoot } = await import('../../src/domain/search/models.js'); + expect(isNpmGlobalModulesRoot(tmpDir)).toBe(false); + }); + + test('returns false when dir is undefined', async () => { + const { isNpmGlobalModulesRoot } = await import('../../src/domain/search/models.js'); + expect(isNpmGlobalModulesRoot(undefined)).toBe(false); + }); +}); + +describe('promptInstall: global codegraph install', () => { + let tmpDir: string; + let origTTY: any; + + beforeEach(() => { + vi.resetModules(); + origTTY = process.stdin.isTTY; + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-global-install-')); + // Simulate npm's own global modules root: /node_modules/npm + the + // globally-installed codegraph package living alongside it. + fs.mkdirSync(path.join(tmpDir, 'node_modules', 'npm'), { recursive: true }); + fs.writeFileSync(path.join(tmpDir, 'node_modules', 'npm', 'package.json'), '{}'); + fs.mkdirSync(path.join(tmpDir, 'node_modules', '@optave', 'codegraph'), { recursive: true }); + + const fakePkg = path.join(tmpDir, 'node_modules', '@optave', 'codegraph', 'package.json'); + vi.doMock('node:module', () => ({ + createRequire: () => ({ + resolve: (req: string) => { + if (req === '@optave/codegraph/package.json') return fakePkg; + throw new Error(`Cannot find: ${req}`); + }, + }), + })); + }); + + afterEach(() => { + process.stdin.isTTY = origTTY; + fs.rmSync(tmpDir, { recursive: true, force: true }); + vi.restoreAllMocks(); + }); + + test('never invokes npm install and rejects with -g guidance', async () => { + process.stdin.isTTY = undefined; + + // `npm root -g` resolves to the same simulated global root set up in + // beforeEach, so isNpmGlobalModulesRoot() classifies it authoritatively + // rather than via the node_modules/npm fallback heuristic. + const execMock = vi.fn((_bin: string, args: string[]) => { + if (args[0] === 'root') return `${path.join(tmpDir, 'node_modules')}\n`; + throw new Error('npm install should never be invoked in this scenario'); + }); + vi.doMock('node:child_process', () => ({ execFileSync: execMock })); + vi.doMock('@huggingface/transformers', () => { + throw new Error('Cannot find package'); + }); + + const { embed } = await import('../../src/domain/search/index.js'); + + await expect(embed(['test'], 'minilm')).rejects.toThrow( + 'npm install -g @huggingface/transformers', + ); + // npm install must never have been attempted — only the read-only `npm root -g` probe. + expect(execMock).not.toHaveBeenCalledWith( + expect.anything(), + expect.arrayContaining(['install']), + expect.anything(), + ); + }); +});