fix(coding-agent): repair self-update detection

Fixes #3942 Fixes #3980 Fixes #3922
2026-04-30 19:10:06 +02:00
parent def47ece92
commit ade08de14c
6 changed files with 349 additions and 117 deletions
--- a/packages/coding-agent/src/config.ts
+++ b/packages/coding-agent/src/config.ts
@@ -1,8 +1,9 @@
 import { spawnSync } from "child_process";
-import { existsSync, readFileSync, realpathSync } from "fs";
+import { accessSync, constants, existsSync, readFileSync, realpathSync } from "fs";
 import { homedir } from "os";
-import { dirname, join, resolve, sep } from "path";
+import { basename, dirname, join, resolve, sep, win32 } from "path";
 import { fileURLToPath } from "url";
+import { shouldUseWindowsShell } from "./utils/child-process.js";

 // =============================================================================
 // Package Detection
@@ -46,7 +47,7 @@ export function detectInstallMethod(): InstallMethod {
 	if (resolvedPath.includes("/yarn/") || resolvedPath.includes("/.yarn/")) {
 		return "yarn";
 	}
-	if (isBunRuntime) {
+	if (isBunRuntime || resolvedPath.includes("/install/global/node_modules/")) {
 		return "bun";
 	}
 	if (resolvedPath.includes("/npm/") || resolvedPath.includes("/node_modules/")) {
@@ -56,58 +57,99 @@ export function detectInstallMethod(): InstallMethod {
 	return "unknown";
 }

-function getSelfUpdateCommandForMethod(method: InstallMethod, packageName: string): SelfUpdateCommand | undefined {
+function getNpmCommand(npmCommand?: string[]): { command: string; args: string[] } {
+	const [command = "npm", ...args] = npmCommand ?? [];
+	return { command, args };
+}
+
+function getInferredNpmInstall(packageName: string): { root: string; prefix: string } | undefined {
+	const packageDir = getPackageDir();
+	const path = process.platform === "win32" || packageDir.includes("\\") ? win32 : { basename, dirname };
+	const [scope, name] = packageName.split("/");
+	let root: string | undefined;
+	if (
+		name &&
+		scope?.startsWith("@") &&
+		path.basename(path.dirname(packageDir)) === scope &&
+		path.basename(packageDir) === name
+	) {
+		root = path.dirname(path.dirname(packageDir));
+	} else if (!name && path.basename(packageDir) === packageName) {
+		root = path.dirname(packageDir);
+	}
+	if (!root || path.basename(root) !== "node_modules") return undefined;
+	const parent = path.dirname(root);
+	if (path.basename(parent) === "lib") return { root, prefix: path.dirname(parent) };
+	// Windows global npm prefixes use `<prefix>\\node_modules`, which is
+	// indistinguishable from local project installs by path shape alone. Do not
+	// infer unsupported Windows custom prefixes without `npm root -g` evidence.
+	return undefined;
+}
+
+function getSelfUpdateCommandForMethod(
+	method: InstallMethod,
+	packageName: string,
+	npmCommand?: string[],
+): SelfUpdateCommand | undefined {
 	switch (method) {
 		case "bun-binary":
 			return undefined;
 		case "pnpm":
-			return {
-				command: "pnpm",
-				args: ["install", "-g", packageName],
-				display: `pnpm install -g ${packageName}`,
-			};
+			return { command: "pnpm", args: ["install", "-g", packageName], display: `pnpm install -g ${packageName}` };
 		case "yarn":
-			return {
-				command: "yarn",
-				args: ["global", "add", packageName],
-				display: `yarn global add ${packageName}`,
-			};
+			return { command: "yarn", args: ["global", "add", packageName], display: `yarn global add ${packageName}` };
 		case "bun":
-			return {
-				command: "bun",
-				args: ["install", "-g", packageName],
-				display: `bun install -g ${packageName}`,
-			};
-		case "npm":
-			return {
-				command: "npm",
-				args: ["install", "-g", packageName],
-				display: `npm install -g ${packageName}`,
-			};
+			return { command: "bun", args: ["install", "-g", packageName], display: `bun install -g ${packageName}` };
+		case "npm": {
+			const npm = getNpmCommand(npmCommand);
+			const inferred = npmCommand?.length ? undefined : getInferredNpmInstall(packageName);
+			const args = [...npm.args, ...(inferred ? ["--prefix", inferred.prefix] : []), "install", "-g", packageName];
+			const display = [npm.command, ...args].map((arg) => (/\s/.test(arg) ? `"${arg}"` : arg)).join(" ");
+			return { command: npm.command, args, display };
+		}
 		case "unknown":
 			return undefined;
 	}
 }

-function readCommandOutput(command: string, args: string[]): string | undefined {
+function readCommandOutput(
+	command: string,
+	args: string[],
+	options: { requireSuccess?: boolean } = {},
+): string | undefined {
 	const result = spawnSync(command, args, {
 		encoding: "utf-8",
-		stdio: ["ignore", "pipe", "ignore"],
-		timeout: 2000,
-		// Windows package managers are commonly .cmd shims. Use the shell so Node can execute them;
-		// command and args are fixed literals from getGlobalPackageRoots(), not user input.
-		shell: process.platform === "win32",
+		stdio: ["ignore", "pipe", "pipe"],
+		shell: shouldUseWindowsShell(command),
 	});
-	if (result.status !== 0) return undefined;
-	const stdout = result.stdout.trim();
-	return stdout || undefined;
+	if (result.status === 0) return result.stdout.trim() || undefined;
+	if (options.requireSuccess) {
+		const reason = result.error?.message || result.stderr.trim() || `exit code ${result.status ?? "unknown"}`;
+		throw new Error(`Failed to run ${[command, ...args].join(" ")}: ${reason}`);
+	}
+	return undefined;
 }

-function getGlobalPackageRoots(method: InstallMethod): string[] {
+function getGlobalPackageRoots(method: InstallMethod, packageName: string, npmCommand?: string[]): string[] {
 	switch (method) {
 		case "npm": {
-			const root = readCommandOutput("npm", ["root", "-g"]);
-			return root ? [root] : [];
+			const npm = getNpmCommand(npmCommand);
+			const configured = !!npmCommand?.length;
+			if (configured && npm.command === "bun") {
+				const bunBin = readCommandOutput(npm.command, [...npm.args, "pm", "bin", "-g"], {
+					requireSuccess: true,
+				});
+				const roots = [join(homedir(), ".bun", "install", "global", "node_modules")];
+				if (bunBin) {
+					roots.push(join(dirname(bunBin), "install", "global", "node_modules"));
+				}
+				return roots;
+			}
+			const root = readCommandOutput(npm.command, [...npm.args, "root", "-g"], {
+				requireSuccess: configured,
+			});
+			const inferred = configured ? undefined : getInferredNpmInstall(packageName);
+			return [root, inferred?.root].filter((x): x is string => !!x);
 		}
 		case "pnpm": {
 			const root = readCommandOutput("pnpm", ["root", "-g"]);
@@ -121,7 +163,7 @@ function getGlobalPackageRoots(method: InstallMethod): string[] {
 			const bunBin = readCommandOutput("bun", ["pm", "bin", "-g"]);
 			const roots = [join(homedir(), ".bun", "install", "global", "node_modules")];
 			if (bunBin) {
-				roots.push(join(dirname(dirname(bunBin)), "install", "global", "node_modules"));
+				roots.push(join(dirname(bunBin), "install", "global", "node_modules"));
 			}
 			return roots;
 		}
@@ -148,36 +190,50 @@ function normalizeExistingPathForComparison(path: string): string | undefined {
 	return normalizedPath;
 }

-function isManagedByGlobalPackageManager(method: InstallMethod): boolean {
-	const packageDir = normalizeExistingPathForComparison(getPackageDir());
-	if (!packageDir) {
+function isSelfUpdatePathWritable(): boolean {
+	const packageDir = getPackageDir();
+	try {
+		accessSync(packageDir, constants.W_OK);
+		accessSync(dirname(packageDir), constants.W_OK);
+		return true;
+	} catch {
 		return false;
 	}
-	return getGlobalPackageRoots(method).some((root) => {
-		const normalizedRoot = normalizeExistingPathForComparison(root);
-		return (
-			normalizedRoot !== undefined &&
-			(packageDir === normalizedRoot ||
-				packageDir.startsWith(normalizedRoot.endsWith(sep) ? normalizedRoot : `${normalizedRoot}${sep}`))
-		);
-	});
 }

-export function getSelfUpdateCommand(packageName: string): SelfUpdateCommand | undefined {
+function isManagedByGlobalPackageManager(method: InstallMethod, packageName: string, npmCommand?: string[]): boolean {
+	const packageDir = normalizeExistingPathForComparison(getPackageDir());
+	return (
+		!!packageDir &&
+		getGlobalPackageRoots(method, packageName, npmCommand).some((root) => {
+			const normalizedRoot = normalizeExistingPathForComparison(root);
+			return (
+				!!normalizedRoot &&
+				packageDir.startsWith(normalizedRoot.endsWith(sep) ? normalizedRoot : `${normalizedRoot}${sep}`)
+			);
+		})
+	);
+}
+
+export function getSelfUpdateCommand(packageName: string, npmCommand?: string[]): SelfUpdateCommand | undefined {
 	const method = detectInstallMethod();
-	const command = getSelfUpdateCommandForMethod(method, packageName);
-	if (!command || !isManagedByGlobalPackageManager(method)) {
+	const command = getSelfUpdateCommandForMethod(method, packageName, npmCommand);
+	if (!command || !isManagedByGlobalPackageManager(method, packageName, npmCommand) || !isSelfUpdatePathWritable()) {
 		return undefined;
 	}
 	return command;
 }

-export function getSelfUpdateUnavailableInstruction(packageName: string): string {
+export function getSelfUpdateUnavailableInstruction(packageName: string, npmCommand?: string[]): string {
 	const method = detectInstallMethod();
 	if (method === "bun-binary") {
 		return `Download from: https://github.com/badlogic/pi-mono/releases/latest`;
 	}
-	if (getSelfUpdateCommandForMethod(method, packageName)) {
+	const command = getSelfUpdateCommandForMethod(method, packageName, npmCommand);
+	if (command) {
+		if (isManagedByGlobalPackageManager(method, packageName, npmCommand) && !isSelfUpdatePathWritable()) {
+			return `This installation is managed by a global ${method} install, but the install path is not writable. Update it yourself with: ${command.display}`;
+		}
 		return `This installation is not managed by a global ${method} install. Update it with the package manager, wrapper, or source checkout that provides it.`;
 	}
 	return `Update ${packageName} using the package manager, wrapper, or source checkout that provides this installation.`;
--- a/packages/coding-agent/src/core/package-manager.ts
+++ b/packages/coding-agent/src/core/package-manager.ts
@@ -28,6 +28,7 @@ import { globSync } from "glob";
 import ignore from "ignore";
 import { minimatch } from "minimatch";
 import { CONFIG_DIR_NAME } from "../config.js";
+import { shouldUseWindowsShell } from "../utils/child-process.js";
 import { type GitSource, parseGitUrl } from "../utils/git.js";
 import { canonicalizePath, isLocalPath } from "../utils/paths.js";
 import { isStdoutTakenOver } from "./output-guard.js";
@@ -2329,28 +2330,11 @@ export class DefaultPackageManager implements PackageManager {
 		};
 	}

-	private shouldUseWindowsShell(command: string): boolean {
-		if (process.platform !== "win32") {
-			return false;
-		}
-		const commandName = basename(command).toLowerCase();
-		return (
-			commandName === "npm" ||
-			commandName === "npx" ||
-			commandName === "pnpm" ||
-			commandName === "yarn" ||
-			commandName === "yarnpkg" ||
-			commandName === "corepack" ||
-			commandName.endsWith(".cmd") ||
-			commandName.endsWith(".bat")
-		);
-	}
-
 	private spawnCommand(command: string, args: string[], options?: { cwd?: string }): ChildProcess {
 		return spawn(command, args, {
 			cwd: options?.cwd,
 			stdio: isStdoutTakenOver() ? ["ignore", 2, 2] : "inherit",
-			shell: this.shouldUseWindowsShell(command),
+			shell: shouldUseWindowsShell(command),
 			env: getEnv(),
 		});
 	}
@@ -2364,7 +2348,7 @@ export class DefaultPackageManager implements PackageManager {
 		return spawn(command, args, {
 			cwd: options?.cwd,
 			stdio: ["ignore", "pipe", "pipe"],
-			shell: this.shouldUseWindowsShell(command),
+			shell: shouldUseWindowsShell(command),
 			env: options?.env ? { ...baseEnv, ...options.env } : baseEnv,
 		});
 	}
@@ -2431,7 +2415,7 @@ export class DefaultPackageManager implements PackageManager {
 		const result = spawnSync(command, args, {
 			stdio: ["ignore", "pipe", "pipe"],
 			encoding: "utf-8",
-			shell: this.shouldUseWindowsShell(command),
+			shell: shouldUseWindowsShell(command),
 			env: getEnv(),
 		});
 		if (result.error || result.status !== 0) {
--- a/packages/coding-agent/src/package-manager-cli.ts
+++ b/packages/coding-agent/src/package-manager-cli.ts
@@ -7,10 +7,12 @@ import {
 	getSelfUpdateCommand,
 	getSelfUpdateUnavailableInstruction,
 	PACKAGE_NAME,
+	type SelfUpdateCommand,
 	VERSION,
 } from "./config.js";
 import { DefaultPackageManager } from "./core/package-manager.js";
 import { SettingsManager } from "./core/settings-manager.js";
+import { shouldUseWindowsShell } from "./utils/child-process.js";
 import { getLatestPiVersion, isNewerPackageVersion } from "./utils/version-check.js";

 export type PackageCommand = "install" | "remove" | "update" | "list";
@@ -271,13 +273,9 @@ function updateTargetIncludesExtensions(target: UpdateTarget): boolean {
 	return target.type === "all" || target.type === "extensions";
 }

-function canSelfUpdate(): boolean {
-	return getSelfUpdateCommand(PACKAGE_NAME) !== undefined;
-}
-
-function printSelfUpdateUnavailable(): void {
+function printSelfUpdateUnavailable(npmCommand?: string[]): void {
 	console.error(`error: ${APP_NAME} cannot self-update this installation.`);
-	console.error(getSelfUpdateUnavailableInstruction(PACKAGE_NAME));
+	console.error(getSelfUpdateUnavailableInstruction(PACKAGE_NAME, npmCommand));

 	const entrypoint = process.argv[1];
 	if (entrypoint) {
@@ -286,9 +284,7 @@ function printSelfUpdateUnavailable(): void {
 	}
 }

-function printSelfUpdateFallback(): void {
-	const command = getSelfUpdateCommand(PACKAGE_NAME);
-	if (!command) return;
+function printSelfUpdateFallback(command: SelfUpdateCommand): void {
 	console.error(chalk.dim(`If this keeps failing, run this command yourself: ${command.display}`));
 }

@@ -312,19 +308,14 @@ async function shouldRunSelfUpdate(force: boolean): Promise<boolean> {
 	return false;
 }

-async function runSelfUpdate(): Promise<void> {
-	const command = getSelfUpdateCommand(PACKAGE_NAME);
-	if (!command) {
-		throw new Error(
-			`${APP_NAME} cannot self-update this installation. ${getSelfUpdateUnavailableInstruction(PACKAGE_NAME)}`,
-		);
-	}
-
+async function runSelfUpdate(command: SelfUpdateCommand): Promise<void> {
 	console.log(chalk.dim(`Updating ${APP_NAME} with ${command.display}...`));
 	await new Promise<void>((resolve, reject) => {
-		// Windows package managers are commonly .cmd shims. Use the shell so Node can execute them;
-		// command and args come from getSelfUpdateCommandForMethod(), not user input.
-		const child = spawn(command.command, command.args, { stdio: "inherit", shell: process.platform === "win32" });
+		// Windows package managers are commonly .cmd shims. Use the shell so Node can execute them.
+		const child = spawn(command.command, command.args, {
+			stdio: "inherit",
+			shell: shouldUseWindowsShell(command.command),
+		});
 		child.on("error", (error) => {
 			reject(error);
 		});
@@ -409,16 +400,12 @@ export async function handlePackageCommand(args: string[]): Promise<boolean> {
 		return true;
 	}

-	if (options.command === "update" && options.updateTarget?.type === "self" && !canSelfUpdate()) {
-		printSelfUpdateUnavailable();
-		process.exitCode = 1;
-		return true;
-	}
-
 	const cwd = process.cwd();
 	const agentDir = getAgentDir();
 	const settingsManager = SettingsManager.create(cwd, agentDir);
 	reportSettingsErrors(settingsManager, "package command");
+	const selfUpdateNpmCommand = settingsManager.getGlobalSettings().npmCommand;
+
 	const packageManager = new DefaultPackageManager({ cwd, agentDir, settingsManager });

 	packageManager.setProgressCallback((event) => {
@@ -493,24 +480,25 @@ export async function handlePackageCommand(args: string[]): Promise<boolean> {
 					}
 				}
 				if (updateTargetIncludesSelf(target)) {
-					if (canSelfUpdate()) {
-						if (!(await shouldRunSelfUpdate(options.force))) {
-							return true;
-						}
-						try {
-							await runSelfUpdate();
-						} catch (error: unknown) {
-							const message = error instanceof Error ? error.message : "Unknown package command error";
-							console.error(chalk.red(`Error: ${message}`));
-							printSelfUpdateFallback();
-							process.exitCode = 1;
-							return true;
-						}
-						console.log(chalk.green(`Updated ${APP_NAME}`));
-					} else {
-						printSelfUpdateUnavailable();
+					const selfUpdateCommand = getSelfUpdateCommand(PACKAGE_NAME, selfUpdateNpmCommand);
+					if (!selfUpdateCommand) {
+						printSelfUpdateUnavailable(selfUpdateNpmCommand);
 						process.exitCode = 1;
+						return true;
 					}
+					if (!(await shouldRunSelfUpdate(options.force))) {
+						return true;
+					}
+					try {
+						await runSelfUpdate(selfUpdateCommand);
+					} catch (error: unknown) {
+						const message = error instanceof Error ? error.message : "Unknown package command error";
+						console.error(chalk.red(`Error: ${message}`));
+						printSelfUpdateFallback(selfUpdateCommand);
+						process.exitCode = 1;
+						return true;
+					}
+					console.log(chalk.green(`Updated ${APP_NAME}`));
 				}
 				return true;
 			}
--- a/packages/coding-agent/src/utils/child-process.ts
+++ b/packages/coding-agent/src/utils/child-process.ts
@@ -1,7 +1,16 @@
 import type { ChildProcess } from "node:child_process";
+import { basename } from "node:path";

 const EXIT_STDIO_GRACE_MS = 100;

+const WINDOWS_SHELL_COMMANDS = new Set(["npm", "npx", "pnpm", "yarn", "yarnpkg", "corepack"]);
+
+export function shouldUseWindowsShell(command: string): boolean {
+	if (process.platform !== "win32") return false;
+	const commandName = basename(command).toLowerCase();
+	return commandName.endsWith(".cmd") || commandName.endsWith(".bat") || WINDOWS_SHELL_COMMANDS.has(commandName);
+}
+
 /**
 * Wait for a child process to terminate without hanging on inherited stdio handles.
 *