fix: preserve owned plugin dependencies during peer repair

This commit is contained in:
Shakker
2026-05-13 15:05:32 +01:00
committed by Shakker
parent f4cb20300f
commit 402b0df3b6
3 changed files with 170 additions and 24 deletions

View File

@@ -521,6 +521,7 @@ export async function syncManagedNpmRootPeerDependencies(params: {
const manifest = await readManagedNpmRootManifest(manifestPath);
const dependencies = readDependencyRecord(manifest.dependencies);
const previousManagedPeerDependencies = readManagedPeerDependencyKeys(manifest.openclaw);
const previousManagedPeerDependencySet = new Set(previousManagedPeerDependencies);
const peerPins = await collectManagedNpmRootPeerDependencyPins({ npmRoot: params.npmRoot });
const nextDependencies = { ...dependencies };
for (const packageName of previousManagedPeerDependencies) {
@@ -541,7 +542,13 @@ export async function syncManagedNpmRootPeerDependencies(params: {
delete overrides[key];
}
Object.assign(overrides, managedOverrides);
const managedPeerDependencyKeys = Object.keys(peerPins).toSorted();
const managedPeerDependencyKeys = Object.keys(peerPins)
.filter(
(packageName) =>
previousManagedPeerDependencySet.has(packageName) ||
!Object.hasOwn(dependencies, packageName),
)
.toSorted();
const openclawMetadata = buildManagedOpenClawMetadata({
current: manifest.openclaw,
managedOverrideKeys,
@@ -569,13 +576,6 @@ export async function syncManagedNpmRootPeerDependencies(params: {
return changed;
}
export async function readManagedNpmRootPeerDependencyNames(params: {
npmRoot: string;
}): Promise<Set<string>> {
const manifest = await readManagedNpmRootManifest(path.join(params.npmRoot, "package.json"));
return new Set(readManagedPeerDependencyKeys(manifest.openclaw));
}
export async function repairManagedNpmRootOpenClawPeer(params: {
npmRoot: string;
timeoutMs?: number;

View File

@@ -444,7 +444,7 @@ describe("installPluginFromNpmSpec e2e", () => {
).resolves.toBeTruthy();
});
it("does not attribute repaired pre-existing peer dependencies to later installs", async () => {
it("scans repaired pre-existing peer dependencies during later installs", async () => {
const rootDir = await makeTempDir("npm-plugin-repaired-peer-scan-e2e");
const npmRoot = path.join(rootDir, "managed-npm");
const pluginWithRuntimePeer = `existing-peer-plugin-${crypto.randomUUID().replace(/-/g, "").slice(0, 12)}`;
@@ -531,13 +531,27 @@ describe("installPluginFromNpmSpec e2e", () => {
logger: { info: () => {}, warn: () => {} },
timeoutMs: 120_000,
});
if (!later.ok) {
throw new Error(later.error);
expect(later.ok).toBe(false);
if (later.ok) {
throw new Error("expected repaired peer dependency scan to block installation");
}
expect(later.error).toContain("Dynamic code execution detected");
await expect(
fs.lstat(path.join(npmRoot, "node_modules", laterPlugin, "package.json")),
).rejects.toHaveProperty("code", "ENOENT");
await expect(
fs.lstat(path.join(npmRoot, "node_modules", runtimePeer, "package.json")),
).resolves.toBeTruthy();
).rejects.toHaveProperty("code", "ENOENT");
const rootManifest = JSON.parse(
await fs.readFile(path.join(npmRoot, "package.json"), "utf8"),
) as {
dependencies?: Record<string, string>;
openclaw?: { managedPeerDependencies?: string[] };
};
expect(rootManifest.dependencies?.[laterPlugin]).toBeUndefined();
expect(rootManifest.dependencies?.[runtimePeer]).toBeUndefined();
expect(rootManifest.openclaw?.managedPeerDependencies ?? []).not.toContain(runtimePeer);
});
it("bounds peer dependency discovery across repeated nested package realpaths", async () => {
@@ -680,6 +694,119 @@ describe("installPluginFromNpmSpec e2e", () => {
).rejects.toHaveProperty("code", "ENOENT");
});
it("does not take ownership of an existing root dependency observed as a peer", async () => {
const rootDir = await makeTempDir("npm-plugin-peer-existing-root-e2e");
const npmRoot = path.join(rootDir, "managed-npm");
const existingRootDependency = `existing-root-${crypto.randomUUID().replace(/-/g, "").slice(0, 12)}`;
const blockedPlugin = `blocked-plugin-${crypto.randomUUID().replace(/-/g, "").slice(0, 12)}`;
const runtimePeer = `runtime-peer-${crypto.randomUUID().replace(/-/g, "").slice(0, 12)}`;
const registry = await startStaticRegistry([
{
packageName: existingRootDependency,
latest: "1.0.0",
versions: [
await packPlugin({
packageName: existingRootDependency,
pluginId: existingRootDependency,
version: "1.0.0",
rootDir,
}),
],
},
{
packageName: blockedPlugin,
latest: "1.0.0",
versions: [
await packPlugin({
packageName: blockedPlugin,
peerDependencies: {
[existingRootDependency]: "^1.0.0",
[runtimePeer]: "^1.0.0",
},
peerDependenciesMeta: {},
pluginId: blockedPlugin,
version: "1.0.0",
rootDir,
}),
],
},
{
packageName: runtimePeer,
latest: "1.0.0",
versions: [
await packPlugin({
indexJs: "eval('1');\n",
packageName: runtimePeer,
pluginId: runtimePeer,
version: "1.0.0",
rootDir,
}),
],
},
]);
process.env.NPM_CONFIG_REGISTRY = registry;
process.env.npm_config_registry = registry;
await fs.mkdir(npmRoot, { recursive: true });
await fs.writeFile(
path.join(npmRoot, "package.json"),
`${JSON.stringify(
{
private: true,
dependencies: { [existingRootDependency]: "1.0.0" },
},
null,
2,
)}\n`,
"utf8",
);
await execFileAsync(
"npm",
[
"install",
"--omit=dev",
"--omit=peer",
"--legacy-peer-deps",
"--loglevel=error",
"--ignore-scripts",
"--no-audit",
"--no-fund",
],
{ cwd: npmRoot },
);
const result = await installPluginFromNpmSpec({
spec: `${blockedPlugin}@1.0.0`,
npmDir: npmRoot,
logger: { info: () => {}, warn: () => {} },
timeoutMs: 120_000,
});
expect(result.ok).toBe(false);
const rootManifest = JSON.parse(
await fs.readFile(path.join(npmRoot, "package.json"), "utf8"),
) as {
dependencies?: Record<string, string>;
openclaw?: { managedPeerDependencies?: string[] };
};
expect(rootManifest.dependencies?.[existingRootDependency]).toBe("1.0.0");
expect(rootManifest.dependencies?.[blockedPlugin]).toBeUndefined();
expect(rootManifest.dependencies?.[runtimePeer]).toBeUndefined();
expect(rootManifest.openclaw?.managedPeerDependencies ?? []).not.toContain(
existingRootDependency,
);
expect(rootManifest.openclaw?.managedPeerDependencies ?? []).not.toContain(runtimePeer);
await expect(
fs.lstat(path.join(npmRoot, "node_modules", existingRootDependency, "package.json")),
).resolves.toBeTruthy();
await expect(
fs.lstat(path.join(npmRoot, "node_modules", blockedPlugin, "package.json")),
).rejects.toHaveProperty("code", "ENOENT");
await expect(
fs.lstat(path.join(npmRoot, "node_modules", runtimePeer, "package.json")),
).rejects.toHaveProperty("code", "ENOENT");
});
it("scrubs host peers when installing beside an existing host-peer plugin", async () => {
const rootDir = await makeTempDir("npm-plugin-sibling-peer-e2e");
const npmRoot = path.join(rootDir, "managed-npm");

View File

@@ -13,7 +13,6 @@ import { resolveNpmIntegrityDriftWithDefaultMessage } from "../infra/npm-integri
import {
type ManagedNpmRootPeerDependencySnapshot,
readManagedNpmRootInstalledDependency,
readManagedNpmRootPeerDependencyNames,
readManagedNpmRootPeerDependencySnapshot,
readOpenClawManagedNpmRootOverrides,
repairManagedNpmRootOpenClawPeer,
@@ -380,11 +379,21 @@ async function rollbackManagedNpmPluginInstall(params: {
}
if (params.peerDependencySnapshot) {
try {
const preRestorePeerDependencySnapshot = await readManagedNpmRootPeerDependencySnapshot({
npmRoot: params.npmRoot,
});
const restoredPeerDependencyNames = new Set(
params.peerDependencySnapshot.managedPeerDependencies,
);
const addedPeerDependencyNames =
preRestorePeerDependencySnapshot.managedPeerDependencies.filter(
(packageName) => !restoredPeerDependencyNames.has(packageName),
);
await restoreManagedNpmRootPeerDependencySnapshot({
npmRoot: params.npmRoot,
snapshot: params.peerDependencySnapshot,
});
await runCommandWithTimeout(
const cleanupResult = await runCommandWithTimeout(
[
"npm",
"install",
@@ -406,6 +415,25 @@ async function rollbackManagedNpmPluginInstall(params: {
}),
},
);
if (cleanupResult.code !== 0) {
params.logger.warn?.(
`npm install cleanup after rollback for ${params.packageName} exited ${cleanupResult.code}: ${cleanupResult.stderr.trim() || cleanupResult.stdout.trim()}`,
);
await Promise.all(
addedPeerDependencyNames.map(async (packageName) => {
try {
await fs.rm(resolveManagedNpmRootPackageDir(params.npmRoot, packageName), {
recursive: true,
force: true,
});
} catch (error) {
params.logger.warn?.(
`Failed to remove rolled-back managed peer dependency ${packageName}: ${String(error)}`,
);
}
}),
);
}
} catch (error) {
params.logger.warn?.(
`Failed to restore managed npm peer dependencies after rollback for ${params.packageName}: ${String(error)}`,
@@ -504,16 +532,11 @@ function resolveManagedNpmRootPackageDir(npmRoot: string, packageName: string):
async function listNewManagedNpmRootPackageDirs(params: {
beforeInstallPackageNames: Set<string>;
excludePackageNames?: Set<string>;
npmRoot: string;
}): Promise<string[]> {
const afterInstallPackageNames = await listManagedNpmRootPackageNames(params.npmRoot);
return [...afterInstallPackageNames]
.filter(
(packageName) =>
!params.beforeInstallPackageNames.has(packageName) &&
!params.excludePackageNames?.has(packageName),
)
.filter((packageName) => !params.beforeInstallPackageNames.has(packageName))
.map((packageName) => resolveManagedNpmRootPackageDir(params.npmRoot, packageName))
.toSorted((left, right) => left.localeCompare(right));
}
@@ -619,9 +642,6 @@ async function installPluginFromManagedNpmRoot(
managedOverrides,
});
await syncManagedNpmRootPeerDependencies({ npmRoot, managedOverrides });
const preExistingManagedPeerDependencyNames = await readManagedNpmRootPeerDependencyNames({
npmRoot,
});
const npmInstallArgs = [
"npm",
...createSafeNpmInstallArgs({
@@ -808,7 +828,6 @@ async function installPluginFromManagedNpmRoot(
const newRootPackageDirs = await listNewManagedNpmRootPackageDirs({
beforeInstallPackageNames: preInstallRootPackageNames,
excludePackageNames: preExistingManagedPeerDependencyNames,
npmRoot,
});
const result = await installPluginFromInstalledPackageDir({