From 3947c23e4804fdfbd84f28bd167fef61a9af3adc Mon Sep 17 00:00:00 2001 From: carlos-alm Date: Thu, 2 Jul 2026 00:54:14 -0600 Subject: [PATCH 1/3] fix(search): skip auto-install when embed targets npm's own global root resolveNpmInstallCwd() resolves to npm's own global modules root for a globally-installed codegraph (e.g. /opt/homebrew/lib on Homebrew Node). Running `npm install` there makes npm reify its own dependency tree as a side effect, which was observed to delete npm's own installation and co-located global packages before the lifecycle-script error surfaced. Add isNpmGlobalModulesRoot() to detect this case (presence of node_modules/npm) and skip the npm install entirely, pointing the user to `npm install -g ` instead of running an install that can nuke their global npm state. docs check acknowledged: this is an internal safety guard on an existing auto-install fallback path, not a documented feature or architectural change; no README/CLAUDE.md/ROADMAP.md content covers this behavior. Closes #1720 Impact: 3 functions changed, 6 callers affected Impact: 3 functions changed, 6 affected --- src/domain/search/models.ts | 36 ++++++++++++- tests/unit/prompt-install.test.ts | 85 +++++++++++++++++++++++++++++++ 2 files changed, 120 insertions(+), 1 deletion(-) diff --git a/src/domain/search/models.ts b/src/domain/search/models.ts index 1c8230e5d..b3d830675 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 } from 'node:fs'; import { createRequire } from 'node:module'; import path from 'node:path'; import { createInterface } from 'node:readline'; @@ -35,6 +36,28 @@ export function resolveNpmInstallCwd(): string | undefined { } } +/** + * True when `dir` is npm's own global modules root — i.e. it contains + * `node_modules/npm` (npm ships itself as a package inside its global root). + * + * 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. + * + * @internal Exported for unit tests; not part of the public barrel. + */ +export function isNpmGlobalModulesRoot(dir: string | undefined): boolean { + if (!dir) return false; + try { + return existsSync(path.join(dir, 'node_modules', 'npm', 'package.json')); + } catch { + return false; + } +} + export interface ModelConfig { name: string; dim: number; @@ -173,6 +196,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 +268,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 eb2b9a8b6..ea7db4739 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,86 @@ 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 dir contains node_modules/npm (npm global modules root)', async () => { + fs.mkdirSync(path.join(tmpDir, 'node_modules', 'npm'), { recursive: true }); + fs.writeFileSync(path.join(tmpDir, 'node_modules', 'npm', 'package.json'), '{}'); + + const { isNpmGlobalModulesRoot } = await import('../../src/domain/search/models.js'); + expect(isNpmGlobalModulesRoot(tmpDir)).toBe(true); + }); + + test('returns false for a normal project directory', async () => { + fs.mkdirSync(path.join(tmpDir, 'node_modules', '@optave', 'codegraph'), { recursive: true }); + + 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; + + const execMock = vi.fn(); + 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', + ); + expect(execMock).not.toHaveBeenCalled(); + }); +}); From ffa1adfcc12b020e1b7a33b3697f04e2a9b35c47 Mon Sep 17 00:00:00 2001 From: carlos-alm Date: Thu, 2 Jul 2026 01:48:22 -0600 Subject: [PATCH 2/3] fix(search): use npm root -g as authoritative global-root check (#1722) isNpmGlobalModulesRoot() previously flagged any directory containing node_modules/npm/package.json as npm's global install root, which misclassifies a normal local project that merely depends on the npm package itself (e.g. a tool that shells out to npm). Ask npm directly via `npm root -g` and compare its real path against the candidate directory; fall back to the old file-existence heuristic only if that call fails. Impact: 2 functions changed, 5 affected --- src/domain/search/models.ts | 40 +++++++++++++++++++++--- tests/unit/prompt-install.test.ts | 52 ++++++++++++++++++++++++++++--- 2 files changed, 84 insertions(+), 8 deletions(-) diff --git a/src/domain/search/models.ts b/src/domain/search/models.ts index b3d830675..e0ee5170c 100644 --- a/src/domain/search/models.ts +++ b/src/domain/search/models.ts @@ -1,5 +1,5 @@ import { execFileSync } from 'node:child_process'; -import { existsSync } from 'node:fs'; +import { existsSync, realpathSync } from 'node:fs'; import { createRequire } from 'node:module'; import path from 'node:path'; import { createInterface } from 'node:readline'; @@ -7,6 +7,16 @@ 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; + } +} /** * Resolve the directory where `npm install` should run so the installed @@ -37,8 +47,9 @@ export function resolveNpmInstallCwd(): string | undefined { } /** - * True when `dir` is npm's own global modules root — i.e. it contains - * `node_modules/npm` (npm ships itself as a package inside its global root). + * 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. @@ -47,10 +58,32 @@ export function resolveNpmInstallCwd(): string | undefined { * 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 = tryRealpath(path.join(dir, 'node_modules')); + try { + const globalRoot = execFileSync(NPM_BIN, ['root', '-g'], { + encoding: 'utf8', + timeout: 10_000, + }).trim(); + if (globalRoot) { + return 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 { @@ -161,7 +194,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, diff --git a/tests/unit/prompt-install.test.ts b/tests/unit/prompt-install.test.ts index ea7db4739..5100c607d 100644 --- a/tests/unit/prompt-install.test.ts +++ b/tests/unit/prompt-install.test.ts @@ -248,16 +248,49 @@ describe('isNpmGlobalModulesRoot', () => { vi.restoreAllMocks(); }); - test('returns true when dir contains node_modules/npm (npm global modules root)', async () => { + 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('returns false for a normal project directory', async () => { + 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); @@ -303,7 +336,13 @@ describe('promptInstall: global codegraph install', () => { test('never invokes npm install and rejects with -g guidance', async () => { process.stdin.isTTY = undefined; - const execMock = vi.fn(); + // `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'); @@ -314,6 +353,11 @@ describe('promptInstall: global codegraph install', () => { await expect(embed(['test'], 'minilm')).rejects.toThrow( 'npm install -g @huggingface/transformers', ); - expect(execMock).not.toHaveBeenCalled(); + // 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(), + ); }); }); From 9d93383fb79689515e99f92a1922b210092459ca Mon Sep 17 00:00:00 2001 From: carlos-alm Date: Thu, 2 Jul 2026 01:52:01 -0600 Subject: [PATCH 3/3] fix(search): fold path case on Windows when comparing npm global root (#1722) isNpmGlobalModulesRoot() compares a realpath'd candidate directory against npm's reported global root. On Windows, paths from execFileSync output and require.resolve can differ in drive-letter or segment casing despite referring to the same directory (NTFS is case-insensitive but case-preserving), which would cause a false-negative comparison. Fold case on win32 before comparing. Impact: 2 functions changed, 5 affected --- src/domain/search/models.ts | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/domain/search/models.ts b/src/domain/search/models.ts index e0ee5170c..c00cbd76e 100644 --- a/src/domain/search/models.ts +++ b/src/domain/search/models.ts @@ -18,6 +18,17 @@ function tryRealpath(p: string): string { } } +/** + * 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 * package ends up reachable by `await import(pkg)` from inside this module. @@ -71,14 +82,14 @@ export function resolveNpmInstallCwd(): string | undefined { export function isNpmGlobalModulesRoot(dir: string | undefined): boolean { if (!dir) return false; - const candidate = tryRealpath(path.join(dir, 'node_modules')); + 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 tryRealpath(globalRoot) === candidate; + return normalizeForComparison(tryRealpath(globalRoot)) === candidate; } } catch { // npm unavailable/unresolvable — fall back to the heuristic below.