fix(cli): rejected unloadable npm plugin installs

Validated npm plugin extension entry points before recording installs and rolled back fresh installs that cannot resolve their extension imports.

Fixes #2312

packages/coding-agent/CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## [Unreleased]
 
+### Fixed
+
+- Fixed npm plugin installs to reject packages whose declared extension entry points cannot load because imports or nested dependencies are unresolved ([#2312](https://github.com/can1357/oh-my-pi/issues/2312)).
+
 ## [15.11.2] - 2026-06-11
 
 ### Added

packages/coding-agent/src/extensibility/plugins/manager.ts

@@ -11,6 +11,8 @@ import {
 	logger,
 } from "@oh-my-pi/pi-utils";
 import { type GitSource, parseGitUrl } from "./git-url";
+import { installLegacyPiSpecifierShim, loadLegacyPiModule } from "./legacy-pi-compat";
+import { resolvePluginExtensionPaths } from "./loader";
 import { extractPackageName, parsePluginSpec } from "./parser";
 import type {
 	DoctorCheck,
@@ -74,6 +76,14 @@ function gitInstallSpec(original: string, source: GitSource): string {
 	return `${source.repo}#${source.ref}`;
 }
 
+function hasDefaultExport(value: unknown): value is { default?: unknown } {
+	return typeof value === "object" && value !== null && "default" in value;
+}
+
+function hasExtensionFactoryExport(module: unknown): boolean {
+	return typeof module === "function" || (hasDefaultExport(module) && typeof module.default === "function");
+}
+
 // =============================================================================
 // Plugin Manager
 // =============================================================================
@@ -173,6 +183,48 @@ export class PluginManager {
 		}
 	}
 
+	async #rollbackFailedInstall(
+		actualName: string,
+		packageJsonBefore: string,
+		wasInstalledBefore: boolean,
+	): Promise<void> {
+		try {
+			await Bun.write(getPluginsPackageJson(), packageJsonBefore);
+			if (!wasInstalledBefore) {
+				await fs.promises.rm(path.join(getPluginsNodeModules(), actualName), { recursive: true, force: true });
+			}
+		} catch (err) {
+			logger.warn("Failed to roll back invalid plugin install", { plugin: actualName, error: String(err) });
+		}
+	}
+
+	async #validateInstalledExtensions(plugin: InstalledPlugin): Promise<void> {
+		const extensionPaths = resolvePluginExtensionPaths(plugin);
+		if (plugin.manifest.extensions && plugin.manifest.extensions.length > 0 && extensionPaths.length === 0) {
+			throw new Error(`Plugin ${plugin.name} declares extension entries but none resolved to loadable files`);
+		}
+		if (extensionPaths.length === 0) {
+			return;
+		}
+
+		installLegacyPiSpecifierShim();
+		const errors: string[] = [];
+		for (const extensionPath of extensionPaths) {
+			try {
+				const module = await loadLegacyPiModule(extensionPath);
+				if (!hasExtensionFactoryExport(module)) {
+					errors.push(`${extensionPath}: extension does not export a valid factory function`);
+				}
+			} catch (err) {
+				const message = err instanceof Error ? err.message : String(err);
+				errors.push(`${extensionPath}: ${message}`);
+			}
+		}
+		if (errors.length > 0) {
+			throw new Error(`Plugin ${plugin.name} extension validation failed:\n${errors.join("\n")}`);
+		}
+	}
+
 	// ==========================================================================
 	// Install / Uninstall
 	// ==========================================================================
@@ -217,7 +269,8 @@ export class PluginManager {
 			};
 		}
 		const pkgJsonPath = getPluginsPackageJson();
-		const depsBefore = gitSource ? await this.#readDeps(pkgJsonPath) : {};
+		const packageJsonBefore = await Bun.file(pkgJsonPath).text();
+		const depsBefore = await this.#readDeps(pkgJsonPath);
 		const packageInstallSpec = gitSource ? gitInstallSpec(spec.packageName, gitSource) : spec.packageName;
 
 		// Run npm install
@@ -307,6 +360,22 @@ export class PluginManager {
 		}
 		// null = use defaults
 
+		const installedPlugin: InstalledPlugin = {
+			name: pkg.name,
+			version: pkg.version,
+			path: path.join(getPluginsNodeModules(), actualName),
+			manifest,
+			enabledFeatures,
+			enabled: true,
+		};
+
+		try {
+			await this.#validateInstalledExtensions(installedPlugin);
+		} catch (err) {
+			await this.#rollbackFailedInstall(actualName, packageJsonBefore, actualName in depsBefore);
+			throw err;
+		}
+
 		// Update runtime config
 		const config = await this.#ensureConfigLoaded();
 		config.plugins[pkg.name] = {
@@ -316,14 +385,7 @@ export class PluginManager {
 		};
 		await this.#saveRuntimeConfig();
 
-		return {
-			name: pkg.name,
-			version: pkg.version,
-			path: path.join(getPluginsNodeModules(), actualName),
-			manifest,
-			enabledFeatures,
-			enabled: true,
-		};
+		return installedPlugin;
 	}
 
 	/**

packages/coding-agent/test/plugin-install-validation.test.ts

@@ -0,0 +1,92 @@
+import { afterEach, beforeEach, describe, expect, test, vi } from "bun:test";
+import * as fs from "node:fs/promises";
+import * as os from "node:os";
+import * as path from "node:path";
+import { PluginManager } from "@oh-my-pi/pi-coding-agent/extensibility/plugins/manager";
+import * as piUtils from "@oh-my-pi/pi-utils";
+import type { Subprocess } from "bun";
+
+function emptyStream(): ReadableStream<Uint8Array> {
+	const body = new Response("").body;
+	if (!body) {
+		throw new Error("Failed to create empty response stream");
+	}
+	return body;
+}
+
+describe("PluginManager.install load validation", () => {
+	let tmpRoot: string;
+	let pluginsDir: string;
+	let pluginsNodeModules: string;
+	let pluginsPkgJson: string;
+
+	beforeEach(async () => {
+		tmpRoot = await fs.mkdtemp(path.join(os.tmpdir(), "omp-plugin-validation-"));
+		pluginsDir = path.join(tmpRoot, "plugins");
+		pluginsNodeModules = path.join(pluginsDir, "node_modules");
+		pluginsPkgJson = path.join(pluginsDir, "package.json");
+		await fs.mkdir(pluginsNodeModules, { recursive: true });
+
+		vi.spyOn(piUtils, "getPluginsDir").mockReturnValue(pluginsDir);
+		vi.spyOn(piUtils, "getPluginsNodeModules").mockReturnValue(pluginsNodeModules);
+		vi.spyOn(piUtils, "getPluginsPackageJson").mockReturnValue(pluginsPkgJson);
+		vi.spyOn(piUtils, "getPluginsLockfile").mockReturnValue(path.join(tmpRoot, "omp-plugins.lock.json"));
+		vi.spyOn(piUtils, "getProjectDir").mockReturnValue(tmpRoot);
+		vi.spyOn(piUtils, "getProjectPluginOverridesPath").mockReturnValue(path.join(tmpRoot, "plugin-overrides.json"));
+	});
+
+	afterEach(async () => {
+		vi.restoreAllMocks();
+		await fs.rm(tmpRoot, { recursive: true, force: true });
+	});
+
+	test("rejects an install whose extension entry cannot resolve its dependencies", async () => {
+		vi.spyOn(Bun, "spawn").mockImplementation(((cmd: string[]) => {
+			expect(cmd).toEqual(["bun", "install", "broken-plugin"]);
+
+			const prepare = (async () => {
+				await Bun.write(
+					pluginsPkgJson,
+					JSON.stringify(
+						{ name: "omp-plugins", private: true, dependencies: { "broken-plugin": "1.0.0" } },
+						null,
+						2,
+					),
+				);
+				const installedDir = path.join(pluginsNodeModules, "broken-plugin");
+				await fs.mkdir(path.join(installedDir, "dist"), { recursive: true });
+				await Bun.write(
+					path.join(installedDir, "package.json"),
+					JSON.stringify(
+						{
+							name: "broken-plugin",
+							version: "1.0.0",
+							peerDependencies: { "missing-peer": "^1.0.0" },
+							omp: { extensions: ["./dist/extension.ts"] },
+						},
+						null,
+						2,
+					),
+				);
+				await Bun.write(
+					path.join(installedDir, "dist", "extension.ts"),
+					'import { missing } from "missing-peer";\nexport default function(pi) { pi.registerCommand(String(missing), { handler: async () => {} }); }\n',
+				);
+			})();
+
+			return {
+				pid: 1,
+				stdout: emptyStream(),
+				stderr: emptyStream(),
+				exited: prepare.then(() => 0),
+			} as Subprocess;
+		}) as typeof Bun.spawn);
+
+		await expect(new PluginManager(tmpRoot).install("broken-plugin")).rejects.toThrow(/missing-peer/);
+
+		const pluginsPackage = await Bun.file(pluginsPkgJson).json();
+		expect(pluginsPackage.dependencies ?? {}).toEqual({});
+		expect(await Bun.file(path.join(pluginsNodeModules, "broken-plugin", "package.json")).exists()).toBe(false);
+		expect(await Bun.file(path.join(tmpRoot, "omp-plugins.lock.json")).exists()).toBe(false);
+	});
+});