From eb4b6f7024d37552ae0065cb1e68a2e7b7cfebfe Mon Sep 17 00:00:00 2001 From: jayeshp19 Date: Fri, 3 Apr 2026 13:20:10 +0530 Subject: [PATCH] Use owning npm prefix for global updates --- src/cli/update-cli/update-command.ts | 8 ++-- src/infra/update-global.test.ts | 56 +++++++++++++++++++++++++ src/infra/update-global.ts | 62 ++++++++++++++++++++++++---- src/infra/update-runner.ts | 6 +-- 4 files changed, 118 insertions(+), 14 deletions(-) diff --git a/src/cli/update-cli/update-command.ts b/src/cli/update-cli/update-command.ts index 7e641b03f30..aa28178b947 100644 --- a/src/cli/update-cli/update-command.ts +++ b/src/cli/update-cli/update-command.ts @@ -351,7 +351,7 @@ async function runPackageInstallUpdate(params: { const installEnv = await createGlobalInstallEnv(); const runCommand = createGlobalCommandRunner(); - const pkgRoot = await resolveGlobalPackageRoot(manager, runCommand, params.timeoutMs); + const pkgRoot = await resolveGlobalPackageRoot(manager, runCommand, params.timeoutMs, params.root); const packageName = (pkgRoot ? await readPackageName(pkgRoot) : await readPackageName(params.root)) ?? DEFAULT_PACKAGE_NAME; @@ -371,7 +371,7 @@ async function runPackageInstallUpdate(params: { const updateStep = await runUpdateStep({ name: "global update", - argv: globalInstallArgs(manager, installSpec), + argv: globalInstallArgs(manager, installSpec, params.root), env: installEnv, timeoutMs: params.timeoutMs, progress: params.progress, @@ -381,7 +381,7 @@ async function runPackageInstallUpdate(params: { let afterVersion = beforeVersion; const verifiedPackageRoot = - (await resolveGlobalPackageRoot(manager, runCommand, params.timeoutMs)) ?? pkgRoot; + (await resolveGlobalPackageRoot(manager, runCommand, params.timeoutMs, params.root)) ?? pkgRoot; if (verifiedPackageRoot) { afterVersion = await readPackageVersion(verifiedPackageRoot); const expectedVersion = resolveExpectedInstalledVersionFromSpec(packageName, installSpec); @@ -484,7 +484,7 @@ async function runGitUpdate(params: { }); const installStep = await runUpdateStep({ name: "global install", - argv: globalInstallArgs(manager, updateRoot), + argv: globalInstallArgs(manager, updateRoot, params.root), cwd: updateRoot, env: installEnv, timeoutMs: effectiveTimeout, diff --git a/src/infra/update-global.test.ts b/src/infra/update-global.test.ts index f1dec69f987..d4e36aefa71 100644 --- a/src/infra/update-global.test.ts +++ b/src/infra/update-global.test.ts @@ -136,6 +136,62 @@ describe("update global helpers", () => { await expect(detectGlobalInstallManagerByPresence(runCommand, 1000)).resolves.toBe("bun"); }); + it("prefers the owning npm prefix when PATH npm points at a different global root", async () => { + const base = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-update-npm-prefix-")); + const brewPrefix = path.join(base, "opt", "homebrew"); + const brewBin = path.join(brewPrefix, "bin"); + const brewRoot = path.join(brewPrefix, "lib", "node_modules"); + const pkgRoot = path.join(brewRoot, "openclaw"); + const pathNpmRoot = path.join(base, "nvm", "lib", "node_modules"); + const brewNpm = path.join( + brewBin, + process.platform === "win32" ? "npm.cmd" : "npm", + ); + await fs.mkdir(pkgRoot, { recursive: true }); + await fs.mkdir(brewBin, { recursive: true }); + await fs.writeFile(brewNpm, "", "utf8"); + + const runCommand: CommandRunner = async (argv) => { + if (argv[0] === "npm") { + return { stdout: `${pathNpmRoot}\n`, stderr: "", code: 0 }; + } + if (argv[0] === brewNpm) { + return { stdout: `${brewRoot}\n`, stderr: "", code: 0 }; + } + if (argv[0] === "pnpm") { + return { stdout: "", stderr: "", code: 1 }; + } + throw new Error(`unexpected command: ${argv.join(" ")}`); + }; + + await expect(detectGlobalInstallManagerForRoot(runCommand, pkgRoot, 1000)).resolves.toBe( + "npm", + ); + await expect(resolveGlobalRoot("npm", runCommand, 1000, pkgRoot)).resolves.toBe(brewRoot); + await expect(resolveGlobalPackageRoot("npm", runCommand, 1000, pkgRoot)).resolves.toBe( + pkgRoot, + ); + expect(globalInstallArgs("npm", "openclaw@latest", pkgRoot)).toEqual([ + brewNpm, + "i", + "-g", + "openclaw@latest", + "--no-fund", + "--no-audit", + "--loglevel=error", + ]); + expect(globalInstallFallbackArgs("npm", "openclaw@latest", pkgRoot)).toEqual([ + brewNpm, + "i", + "-g", + "openclaw@latest", + "--omit=optional", + "--no-fund", + "--no-audit", + "--loglevel=error", + ]); + }); + it("builds install argv and npm fallback argv", () => { expect(globalInstallArgs("npm", "openclaw@latest")).toEqual([ "npm", diff --git a/src/infra/update-global.ts b/src/infra/update-global.ts index f636aa4d572..57ca5e388ad 100644 --- a/src/infra/update-global.ts +++ b/src/infra/update-global.ts @@ -1,3 +1,4 @@ +import fsSync from "node:fs"; import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; @@ -180,15 +181,50 @@ function resolveBunGlobalRoot(): string { return path.join(bunInstall, "install", "global", "node_modules"); } +function inferNpmPrefixFromPackageRoot(pkgRoot?: string | null): string | null { + const trimmed = pkgRoot?.trim(); + if (!trimmed) { + return null; + } + const normalized = path.resolve(trimmed); + const nodeModulesDir = path.dirname(normalized); + if (path.basename(nodeModulesDir) !== "node_modules") { + return null; + } + const libDir = path.dirname(nodeModulesDir); + if (path.basename(libDir) !== "lib") { + return null; + } + return path.dirname(libDir); +} + +function resolvePreferredGlobalManagerCommand( + manager: GlobalInstallManager, + pkgRoot?: string | null, +): string { + if (manager !== "npm") { + return manager; + } + const prefix = inferNpmPrefixFromPackageRoot(pkgRoot); + if (!prefix) { + return manager; + } + const candidate = + process.platform === "win32" ? path.join(prefix, "npm.cmd") : path.join(prefix, "bin", "npm"); + return fsSync.existsSync(candidate) ? candidate : manager; +} + export async function resolveGlobalRoot( manager: GlobalInstallManager, runCommand: CommandRunner, timeoutMs: number, + pkgRoot?: string | null, ): Promise { if (manager === "bun") { return resolveBunGlobalRoot(); } - const argv = manager === "pnpm" ? ["pnpm", "root", "-g"] : ["npm", "root", "-g"]; + const command = resolvePreferredGlobalManagerCommand(manager, pkgRoot); + const argv = [command, "root", "-g"]; const res = await runCommand(argv, { timeoutMs }).catch(() => null); if (!res || res.code !== 0) { return null; @@ -201,8 +237,9 @@ export async function resolveGlobalPackageRoot( manager: GlobalInstallManager, runCommand: CommandRunner, timeoutMs: number, + pkgRoot?: string | null, ): Promise { - const root = await resolveGlobalRoot(manager, runCommand, timeoutMs); + const root = await resolveGlobalRoot(manager, runCommand, timeoutMs, pkgRoot); if (!root) { return null; } @@ -253,6 +290,10 @@ export async function detectGlobalInstallManagerForRoot( } } + if (inferNpmPrefixFromPackageRoot(pkgRoot)) { + return "npm"; + } + return null; } @@ -281,24 +322,31 @@ export async function detectGlobalInstallManagerByPresence( return null; } -export function globalInstallArgs(manager: GlobalInstallManager, spec: string): string[] { +export function globalInstallArgs( + manager: GlobalInstallManager, + spec: string, + pkgRoot?: string | null, +): string[] { + const command = resolvePreferredGlobalManagerCommand(manager, pkgRoot); if (manager === "pnpm") { - return ["pnpm", "add", "-g", spec]; + return [command, "add", "-g", spec]; } if (manager === "bun") { - return ["bun", "add", "-g", spec]; + return [command, "add", "-g", spec]; } - return ["npm", "i", "-g", spec, ...NPM_GLOBAL_INSTALL_QUIET_FLAGS]; + return [command, "i", "-g", spec, ...NPM_GLOBAL_INSTALL_QUIET_FLAGS]; } export function globalInstallFallbackArgs( manager: GlobalInstallManager, spec: string, + pkgRoot?: string | null, ): string[] | null { if (manager !== "npm") { return null; } - return ["npm", "i", "-g", spec, ...NPM_GLOBAL_INSTALL_OMIT_OPTIONAL_FLAGS]; + const command = resolvePreferredGlobalManagerCommand(manager, pkgRoot); + return [command, "i", "-g", spec, ...NPM_GLOBAL_INSTALL_OMIT_OPTIONAL_FLAGS]; } export async function cleanupGlobalRenameDirs(params: { diff --git a/src/infra/update-runner.ts b/src/infra/update-runner.ts index 90df1408706..9f85efe0459 100644 --- a/src/infra/update-runner.ts +++ b/src/infra/update-runner.ts @@ -1018,7 +1018,7 @@ export async function runGatewayUpdate(opts: UpdateRunnerOptions = {}): Promise< const updateStep = await runStep({ runCommand, name: "global update", - argv: globalInstallArgs(globalManager, spec), + argv: globalInstallArgs(globalManager, spec, pkgRoot), cwd: pkgRoot, timeoutMs, env: globalInstallEnv, @@ -1030,7 +1030,7 @@ export async function runGatewayUpdate(opts: UpdateRunnerOptions = {}): Promise< let finalStep = updateStep; if (updateStep.exitCode !== 0) { - const fallbackArgv = globalInstallFallbackArgs(globalManager, spec); + const fallbackArgv = globalInstallFallbackArgs(globalManager, spec, pkgRoot); if (fallbackArgv) { const fallbackStep = await runStep({ runCommand, @@ -1049,7 +1049,7 @@ export async function runGatewayUpdate(opts: UpdateRunnerOptions = {}): Promise< } const verifiedPackageRoot = - (await resolveGlobalPackageRoot(globalManager, runCommand, timeoutMs)) ?? pkgRoot; + (await resolveGlobalPackageRoot(globalManager, runCommand, timeoutMs, pkgRoot)) ?? pkgRoot; const expectedVersion = resolveExpectedInstalledVersionFromSpec(packageName, spec); const verificationErrors = await collectInstalledGlobalPackageErrors({ packageRoot: verifiedPackageRoot,