fix(cli): restored plugin tree after invalid reinstall
Backed up existing npm plugin package trees before reinstall validation so failed extension loads restore the previous working package contents.
This commit is contained in:
roboomp
@@ -1,4 +1,5 @@
|
||||
import * as fs from "node:fs";
|
||||
import * as os from "node:os";
|
||||
import * as path from "node:path";
|
||||
import {
|
||||
getPluginsDir,
|
||||
@@ -76,6 +77,16 @@ function gitInstallSpec(original: string, source: GitSource): string {
|
||||
return `${source.repo}#${source.ref}`;
|
||||
}
|
||||
|
||||
function findExistingGitPackageName(packageInstallSpec: string, deps: Record<string, string>): string | undefined {
|
||||
const needle = packageInstallSpec.replace(/^git\+/i, "");
|
||||
for (const [key, value] of Object.entries(deps)) {
|
||||
if (typeof value === "string" && value.includes(needle)) {
|
||||
return key;
|
||||
}
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
function hasDefaultExport(value: unknown): value is { default?: unknown } {
|
||||
return typeof value === "object" && value !== null && "default" in value;
|
||||
}
|
||||
@@ -84,6 +95,13 @@ function hasExtensionFactoryExport(module: unknown): boolean {
|
||||
return typeof module === "function" || (hasDefaultExport(module) && typeof module.default === "function");
|
||||
}
|
||||
|
||||
interface PluginPackageSnapshot {
|
||||
readonly actualName: string;
|
||||
readonly packagePath: string;
|
||||
readonly backupRoot: string;
|
||||
readonly backupPath: string;
|
||||
}
|
||||
|
||||
// =============================================================================
|
||||
// Plugin Manager
|
||||
// =============================================================================
|
||||
@@ -183,19 +201,50 @@ export class PluginManager {
|
||||
}
|
||||
}
|
||||
|
||||
async #snapshotInstalledPackage(actualName: string | undefined): Promise<PluginPackageSnapshot | null> {
|
||||
if (!actualName) {
|
||||
return null;
|
||||
}
|
||||
const packagePath = path.join(getPluginsNodeModules(), actualName);
|
||||
try {
|
||||
await fs.promises.lstat(packagePath);
|
||||
} catch (err) {
|
||||
if (isEnoent(err)) {
|
||||
return null;
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
|
||||
const backupRoot = await fs.promises.mkdtemp(path.join(os.tmpdir(), "omp-plugin-backup-"));
|
||||
const backupPath = path.join(backupRoot, "package");
|
||||
await fs.promises.cp(packagePath, backupPath, { recursive: true, verbatimSymlinks: true });
|
||||
return { actualName, packagePath, backupRoot, backupPath };
|
||||
}
|
||||
|
||||
async #cleanupSnapshot(snapshot: PluginPackageSnapshot | null): Promise<void> {
|
||||
if (!snapshot) {
|
||||
return;
|
||||
}
|
||||
try {
|
||||
await fs.promises.rm(snapshot.backupRoot, { recursive: true, force: true });
|
||||
} catch (err) {
|
||||
logger.warn("Failed to remove plugin install backup", { plugin: snapshot.actualName, error: String(err) });
|
||||
}
|
||||
}
|
||||
|
||||
async #rollbackFailedInstall(
|
||||
actualName: string,
|
||||
packageJsonBefore: string,
|
||||
wasInstalledBefore: boolean,
|
||||
snapshot: PluginPackageSnapshot | null,
|
||||
): 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) });
|
||||
await Bun.write(getPluginsPackageJson(), packageJsonBefore);
|
||||
const packagePath = path.join(getPluginsNodeModules(), actualName);
|
||||
await fs.promises.rm(packagePath, { recursive: true, force: true });
|
||||
if (!snapshot) {
|
||||
return;
|
||||
}
|
||||
await fs.promises.mkdir(path.dirname(snapshot.packagePath), { recursive: true });
|
||||
await fs.promises.cp(snapshot.backupPath, snapshot.packagePath, { recursive: true, verbatimSymlinks: true });
|
||||
}
|
||||
|
||||
async #validateInstalledExtensions(plugin: InstalledPlugin): Promise<void> {
|
||||
@@ -272,120 +321,128 @@ export class PluginManager {
|
||||
const packageJsonBefore = await Bun.file(pkgJsonPath).text();
|
||||
const depsBefore = await this.#readDeps(pkgJsonPath);
|
||||
const packageInstallSpec = gitSource ? gitInstallSpec(spec.packageName, gitSource) : spec.packageName;
|
||||
const existingActualName = gitSource
|
||||
? findExistingGitPackageName(packageInstallSpec, depsBefore)
|
||||
: extractPackageName(spec.packageName);
|
||||
const packageSnapshot = await this.#snapshotInstalledPackage(existingActualName);
|
||||
|
||||
// Run npm install
|
||||
const proc = Bun.spawn(["bun", "install", packageInstallSpec], {
|
||||
cwd: getPluginsDir(),
|
||||
stdin: "ignore",
|
||||
stdout: "pipe",
|
||||
stderr: "pipe",
|
||||
windowsHide: true,
|
||||
});
|
||||
try {
|
||||
// Run npm install
|
||||
const proc = Bun.spawn(["bun", "install", packageInstallSpec], {
|
||||
cwd: getPluginsDir(),
|
||||
stdin: "ignore",
|
||||
stdout: "pipe",
|
||||
stderr: "pipe",
|
||||
windowsHide: true,
|
||||
});
|
||||
|
||||
const exitCode = await proc.exited;
|
||||
if (exitCode !== 0) {
|
||||
const stderr = await new Response(proc.stderr).text();
|
||||
throw new Error(`npm install failed: ${stderr}`);
|
||||
}
|
||||
// Resolve actual package name. npm specs encode the name (strip version);
|
||||
// git specs do not, so diff plugins/package.json deps to find the new entry.
|
||||
let actualName: string;
|
||||
if (gitSource) {
|
||||
const depsAfter = await this.#readDeps(pkgJsonPath);
|
||||
let resolved: string | undefined;
|
||||
for (const key of Object.keys(depsAfter)) {
|
||||
if (!(key in depsBefore)) {
|
||||
resolved = key;
|
||||
break;
|
||||
}
|
||||
const exitCode = await proc.exited;
|
||||
if (exitCode !== 0) {
|
||||
const stderr = await new Response(proc.stderr).text();
|
||||
throw new Error(`npm install failed: ${stderr}`);
|
||||
}
|
||||
// Fallback: a force-reinstall of an already-present git plugin will not
|
||||
// add a new key, just rewrite the existing one to the new spec value.
|
||||
// Match by the install value for force-reinstalls where no new key is
|
||||
// added (non-GitHub shorthands are normalized before bun sees them).
|
||||
if (!resolved) {
|
||||
const needle = packageInstallSpec.replace(/^git\+/i, "");
|
||||
for (const [key, value] of Object.entries(depsAfter)) {
|
||||
if (typeof value === "string" && value.includes(needle)) {
|
||||
// Resolve actual package name. npm specs encode the name (strip version);
|
||||
// git specs do not, so diff plugins/package.json deps to find the new entry.
|
||||
let actualName: string;
|
||||
if (gitSource) {
|
||||
const depsAfter = await this.#readDeps(pkgJsonPath);
|
||||
let resolved: string | undefined;
|
||||
for (const key of Object.keys(depsAfter)) {
|
||||
if (!(key in depsBefore)) {
|
||||
resolved = key;
|
||||
break;
|
||||
}
|
||||
}
|
||||
// Fallback: a force-reinstall of an already-present git plugin will not
|
||||
// add a new key, just rewrite the existing one to the new spec value.
|
||||
// Match by the install value for force-reinstalls where no new key is
|
||||
// added (non-GitHub shorthands are normalized before bun sees them).
|
||||
if (!resolved) {
|
||||
resolved = findExistingGitPackageName(packageInstallSpec, depsAfter);
|
||||
}
|
||||
if (!resolved) {
|
||||
throw new Error(
|
||||
`Installed ${spec.packageName} but could not determine package name from plugins/package.json`,
|
||||
);
|
||||
}
|
||||
actualName = resolved;
|
||||
} else {
|
||||
actualName = extractPackageName(spec.packageName);
|
||||
}
|
||||
if (!resolved) {
|
||||
throw new Error(
|
||||
`Installed ${spec.packageName} but could not determine package name from plugins/package.json`,
|
||||
);
|
||||
}
|
||||
actualName = resolved;
|
||||
} else {
|
||||
actualName = extractPackageName(spec.packageName);
|
||||
}
|
||||
const pkgPath = path.join(getPluginsNodeModules(), actualName, "package.json");
|
||||
const pkgPath = path.join(getPluginsNodeModules(), actualName, "package.json");
|
||||
|
||||
let pkg: { name: string; version: string; omp?: PluginManifest; pi?: PluginManifest };
|
||||
try {
|
||||
pkg = await Bun.file(pkgPath).json();
|
||||
} catch (err) {
|
||||
if (isEnoent(err)) {
|
||||
throw new Error(`Package installed but package.json not found at ${pkgPath}`);
|
||||
let pkg: { name: string; version: string; omp?: PluginManifest; pi?: PluginManifest };
|
||||
try {
|
||||
pkg = await Bun.file(pkgPath).json();
|
||||
} catch (err) {
|
||||
if (isEnoent(err)) {
|
||||
throw new Error(`Package installed but package.json not found at ${pkgPath}`);
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
const manifest: PluginManifest = pkg.omp || pkg.pi || { version: pkg.version };
|
||||
manifest.version = pkg.version;
|
||||
const manifest: PluginManifest = pkg.omp || pkg.pi || { version: pkg.version };
|
||||
manifest.version = pkg.version;
|
||||
|
||||
// Resolve enabled features
|
||||
let enabledFeatures: string[] | null = null;
|
||||
if (spec.features === "*") {
|
||||
// All features
|
||||
enabledFeatures = manifest.features ? Object.keys(manifest.features) : null;
|
||||
} else if (Array.isArray(spec.features)) {
|
||||
if (spec.features.length > 0) {
|
||||
// Validate requested features exist
|
||||
if (manifest.features) {
|
||||
for (const feat of spec.features) {
|
||||
if (!(feat in manifest.features)) {
|
||||
throw new Error(
|
||||
`Unknown feature "${feat}" in ${actualName}. Available: ${Object.keys(manifest.features).join(", ")}`,
|
||||
);
|
||||
// Resolve enabled features
|
||||
let enabledFeatures: string[] | null = null;
|
||||
if (spec.features === "*") {
|
||||
// All features
|
||||
enabledFeatures = manifest.features ? Object.keys(manifest.features) : null;
|
||||
} else if (Array.isArray(spec.features)) {
|
||||
if (spec.features.length > 0) {
|
||||
// Validate requested features exist
|
||||
if (manifest.features) {
|
||||
for (const feat of spec.features) {
|
||||
if (!(feat in manifest.features)) {
|
||||
throw new Error(
|
||||
`Unknown feature "${feat}" in ${actualName}. Available: ${Object.keys(manifest.features).join(", ")}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
enabledFeatures = spec.features;
|
||||
} else {
|
||||
// Empty array = no optional features
|
||||
enabledFeatures = [];
|
||||
}
|
||||
enabledFeatures = spec.features;
|
||||
} else {
|
||||
// Empty array = no optional features
|
||||
enabledFeatures = [];
|
||||
}
|
||||
// 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) {
|
||||
try {
|
||||
await this.#rollbackFailedInstall(actualName, packageJsonBefore, packageSnapshot);
|
||||
} catch (rollbackErr) {
|
||||
const message = err instanceof Error ? err.message : String(err);
|
||||
const rollbackMessage = rollbackErr instanceof Error ? rollbackErr.message : String(rollbackErr);
|
||||
throw new Error(`${message}\nRollback failed: ${rollbackMessage}`);
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
|
||||
// Update runtime config
|
||||
const config = await this.#ensureConfigLoaded();
|
||||
config.plugins[pkg.name] = {
|
||||
version: pkg.version,
|
||||
enabledFeatures,
|
||||
enabled: true,
|
||||
};
|
||||
await this.#saveRuntimeConfig();
|
||||
|
||||
return installedPlugin;
|
||||
} finally {
|
||||
await this.#cleanupSnapshot(packageSnapshot);
|
||||
}
|
||||
// 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] = {
|
||||
version: pkg.version,
|
||||
enabledFeatures,
|
||||
enabled: true,
|
||||
};
|
||||
await this.#saveRuntimeConfig();
|
||||
|
||||
return installedPlugin;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -14,6 +14,33 @@ function emptyStream(): ReadableStream<Uint8Array> {
|
||||
return body;
|
||||
}
|
||||
|
||||
interface PluginFixture {
|
||||
readonly version: string;
|
||||
readonly source: string;
|
||||
readonly dependencyVersion?: string;
|
||||
readonly peerDependencies?: Record<string, string>;
|
||||
}
|
||||
|
||||
async function writePluginPackage(pluginsNodeModules: string, name: string, fixture: PluginFixture): Promise<string> {
|
||||
const installedDir = path.join(pluginsNodeModules, name);
|
||||
await fs.mkdir(path.join(installedDir, "dist"), { recursive: true });
|
||||
await Bun.write(
|
||||
path.join(installedDir, "package.json"),
|
||||
JSON.stringify(
|
||||
{
|
||||
name,
|
||||
version: fixture.version,
|
||||
...(fixture.peerDependencies ? { peerDependencies: fixture.peerDependencies } : {}),
|
||||
omp: { extensions: ["./dist/extension.ts"] },
|
||||
},
|
||||
null,
|
||||
2,
|
||||
),
|
||||
);
|
||||
await Bun.write(path.join(installedDir, "dist", "extension.ts"), fixture.source);
|
||||
return installedDir;
|
||||
}
|
||||
|
||||
describe("PluginManager.install load validation", () => {
|
||||
let tmpRoot: string;
|
||||
let pluginsDir: string;
|
||||
@@ -53,25 +80,12 @@ describe("PluginManager.install load validation", () => {
|
||||
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',
|
||||
);
|
||||
await writePluginPackage(pluginsNodeModules, "broken-plugin", {
|
||||
version: "1.0.0",
|
||||
peerDependencies: { "missing-peer": "^1.0.0" },
|
||||
source:
|
||||
'import { missing } from "missing-peer";\nexport default function(pi) { pi.registerCommand(String(missing), { handler: async () => {} }); }\n',
|
||||
});
|
||||
})();
|
||||
|
||||
return {
|
||||
@@ -89,4 +103,65 @@ describe("PluginManager.install load validation", () => {
|
||||
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);
|
||||
});
|
||||
|
||||
test("restores the previous package tree when reinstall validation fails", async () => {
|
||||
await Bun.write(
|
||||
pluginsPkgJson,
|
||||
JSON.stringify({ name: "omp-plugins", private: true, dependencies: { "broken-plugin": "1.0.0" } }, null, 2),
|
||||
);
|
||||
await Bun.write(
|
||||
path.join(tmpRoot, "omp-plugins.lock.json"),
|
||||
JSON.stringify(
|
||||
{ plugins: { "broken-plugin": { version: "1.0.0", enabledFeatures: null, enabled: true } }, settings: {} },
|
||||
null,
|
||||
2,
|
||||
),
|
||||
);
|
||||
await writePluginPackage(pluginsNodeModules, "broken-plugin", {
|
||||
version: "1.0.0",
|
||||
source: 'export default function(pi) { pi.registerCommand("old-ok", { handler: async () => {} }); }\n',
|
||||
});
|
||||
|
||||
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": "2.0.0" } },
|
||||
null,
|
||||
2,
|
||||
),
|
||||
);
|
||||
await writePluginPackage(pluginsNodeModules, "broken-plugin", {
|
||||
version: "2.0.0",
|
||||
peerDependencies: { "missing-peer": "^1.0.0" },
|
||||
source:
|
||||
'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({ "broken-plugin": "1.0.0" });
|
||||
const restoredPackage = await Bun.file(path.join(pluginsNodeModules, "broken-plugin", "package.json")).json();
|
||||
expect(restoredPackage.version).toBe("1.0.0");
|
||||
const restoredExtension = await Bun.file(
|
||||
path.join(pluginsNodeModules, "broken-plugin", "dist", "extension.ts"),
|
||||
).text();
|
||||
expect(restoredExtension).toContain("old-ok");
|
||||
expect(restoredExtension).not.toContain("missing-peer");
|
||||
const lock = await Bun.file(path.join(tmpRoot, "omp-plugins.lock.json")).json();
|
||||
expect(lock.plugins["broken-plugin"]).toEqual({ version: "1.0.0", enabledFeatures: null, enabled: true });
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user