From 2cafbd07746d7e27c44d9fcb052ef8e1e601f8e4 Mon Sep 17 00:00:00 2001 From: Alex Knight <15041791+amknight@users.noreply.github.com> Date: Tue, 9 Jun 2026 16:46:06 -0700 Subject: [PATCH] fix(plugins): reconcile managed npm root overrides with managed peer pins --- src/infra/npm-managed-root.test.ts | 388 +++++++++++++++++++++++++++++ src/infra/npm-managed-root.ts | 164 ++++++++++-- 2 files changed, 527 insertions(+), 25 deletions(-) diff --git a/src/infra/npm-managed-root.test.ts b/src/infra/npm-managed-root.test.ts index ae1d030f519a..1a46189d572d 100644 --- a/src/infra/npm-managed-root.test.ts +++ b/src/infra/npm-managed-root.test.ts @@ -15,6 +15,7 @@ import { readManagedNpmRootInstalledDependency, readOpenClawManagedNpmRootOverrides, resolveManagedNpmRootDependencySpec, + restoreManagedNpmRootPeerDependencySnapshot, syncManagedNpmRootPeerDependencies, upsertManagedNpmRootDependency, } from "./npm-managed-root.js"; @@ -294,6 +295,241 @@ describe("managed npm root", () => { }); }); + it("aligns stale managed peer pins with managed overrides when adding a plugin", async () => { + const npmRoot = await makeTempRoot(); + await fs.writeFile( + path.join(npmRoot, "package.json"), + `${JSON.stringify( + { + private: true, + dependencies: { + plugin: "1.0.0", + "runtime-peer": "4.12.23", + }, + openclaw: { + managedPeerDependencies: ["runtime-peer"], + }, + }, + null, + 2, + )}\n`, + ); + + await upsertManagedNpmRootDependency({ + npmRoot, + packageName: "plugin", + dependencySpec: "2.0.0", + managedOverrides: { + "runtime-peer": "4.12.18", + }, + }); + + await expect( + fs.readFile(path.join(npmRoot, "package.json"), "utf8").then((raw) => JSON.parse(raw)), + ).resolves.toEqual({ + private: true, + dependencies: { + plugin: "2.0.0", + "runtime-peer": "4.12.18", + }, + overrides: { + "runtime-peer": "4.12.18", + }, + openclaw: { + managedOverrides: ["runtime-peer"], + managedPeerDependencies: ["runtime-peer"], + }, + }); + }); + + it("drops managed overrides that conflict with an explicitly installed package", async () => { + const npmRoot = await makeTempRoot(); + + await upsertManagedNpmRootDependency({ + npmRoot, + packageName: "pinned-package", + dependencySpec: "2.0.0", + managedOverrides: { + "pinned-package": "1.0.0", + axios: "1.16.0", + }, + }); + + await expect( + fs.readFile(path.join(npmRoot, "package.json"), "utf8").then((raw) => JSON.parse(raw)), + ).resolves.toEqual({ + private: true, + dependencies: { + "pinned-package": "2.0.0", + }, + overrides: { + axios: "1.16.0", + }, + openclaw: { + managedOverrides: ["axios"], + }, + }); + }); + + it("keeps child override rules when only the root entry conflicts with an installed package", async () => { + const npmRoot = await makeTempRoot(); + + await upsertManagedNpmRootDependency({ + npmRoot, + packageName: "pinned-package", + dependencySpec: "2.0.0", + managedOverrides: { + "pinned-package": { ".": "1.0.0", "vuln-child": "3.0.0" }, + }, + }); + + await expect( + fs.readFile(path.join(npmRoot, "package.json"), "utf8").then((raw) => JSON.parse(raw)), + ).resolves.toEqual({ + private: true, + dependencies: { + "pinned-package": "2.0.0", + }, + overrides: { + "pinned-package": { "vuln-child": "3.0.0" }, + }, + openclaw: { + managedOverrides: ["pinned-package"], + }, + }); + }); + + it("does not treat wildcard overrides as root dependency conflicts", async () => { + const npmRoot = await makeTempRoot(); + await fs.writeFile( + path.join(npmRoot, "package.json"), + `${JSON.stringify( + { + private: true, + dependencies: { + plugin: "1.0.0", + "runtime-peer": "4.12.23", + }, + openclaw: { + managedPeerDependencies: ["runtime-peer"], + }, + }, + null, + 2, + )}\n`, + ); + + await upsertManagedNpmRootDependency({ + npmRoot, + packageName: "plugin", + dependencySpec: "2.0.0", + managedOverrides: { + "runtime-peer": "*", + }, + }); + + await expect( + fs.readFile(path.join(npmRoot, "package.json"), "utf8").then((raw) => JSON.parse(raw)), + ).resolves.toMatchObject({ + dependencies: { + plugin: "2.0.0", + "runtime-peer": "4.12.23", + }, + overrides: { + "runtime-peer": "*", + }, + }); + }); + + it("transfers ownership of a managed peer pin when it is explicitly installed", async () => { + const npmRoot = await makeTempRoot(); + await fs.writeFile( + path.join(npmRoot, "package.json"), + `${JSON.stringify( + { + private: true, + dependencies: { + plugin: "1.0.0", + "runtime-peer": "4.12.23", + }, + openclaw: { + managedPeerDependencies: ["runtime-peer"], + }, + }, + null, + 2, + )}\n`, + ); + + await upsertManagedNpmRootDependency({ + npmRoot, + packageName: "runtime-peer", + dependencySpec: "5.0.0", + }); + + // The installed package leaves managedPeerDependencies so the next peer sync + // cannot re-pin or delete the explicitly requested version. + await expect( + fs.readFile(path.join(npmRoot, "package.json"), "utf8").then((raw) => JSON.parse(raw)), + ).resolves.toEqual({ + private: true, + dependencies: { + plugin: "1.0.0", + "runtime-peer": "5.0.0", + }, + }); + }); + + it("realigns restored managed peer pins with manifest overrides", async () => { + const npmRoot = await makeTempRoot(); + await fs.writeFile( + path.join(npmRoot, "package.json"), + `${JSON.stringify( + { + private: true, + dependencies: { + plugin: "1.0.0", + "runtime-peer": "4.12.18", + }, + overrides: { + "runtime-peer": "4.12.18", + }, + openclaw: { + managedOverrides: ["runtime-peer"], + managedPeerDependencies: ["runtime-peer"], + }, + }, + null, + 2, + )}\n`, + ); + + await restoreManagedNpmRootPeerDependencySnapshot({ + npmRoot, + snapshot: { + dependencies: { "runtime-peer": "4.12.23" }, + managedPeerDependencies: ["runtime-peer"], + }, + }); + + await expect( + fs.readFile(path.join(npmRoot, "package.json"), "utf8").then((raw) => JSON.parse(raw)), + ).resolves.toEqual({ + private: true, + dependencies: { + plugin: "1.0.0", + "runtime-peer": "4.12.18", + }, + overrides: { + "runtime-peer": "4.12.18", + }, + openclaw: { + managedOverrides: ["runtime-peer"], + managedPeerDependencies: ["runtime-peer"], + }, + }); + }); + it("reads workspace pnpm overrides for managed plugin installs", async () => { const workspace = YAML.parse( await fs.readFile(path.resolve(process.cwd(), "pnpm-workspace.yaml"), "utf8"), @@ -596,6 +832,158 @@ describe("managed npm root", () => { }); }); + it("advances stale managed peer pins to the override-aware npm plan", async () => { + const npmRoot = await makeTempRoot(); + await fs.writeFile( + path.join(npmRoot, "package.json"), + `${JSON.stringify( + { + private: true, + dependencies: { + plugin: "1.0.0", + "runtime-peer": "4.12.23", + }, + overrides: { + "runtime-peer": "4.12.18", + }, + openclaw: { + managedOverrides: ["runtime-peer"], + managedPeerDependencies: ["runtime-peer"], + }, + }, + null, + 2, + )}\n`, + ); + + const runCommand = vi.fn(async (_args: string[], optionsOrTimeout: number | CommandOptions) => { + const options = requireCommandOptions(optionsOrTimeout, "npm peer plan"); + if (!options.cwd) { + throw new Error("expected npm peer plan cwd"); + } + const tempManifest = JSON.parse( + await fs.readFile(path.join(options.cwd, "package.json"), "utf8"), + ) as { + dependencies?: Record; + overrides?: Record; + }; + expect(tempManifest.dependencies).toEqual({ plugin: "1.0.0" }); + expect(tempManifest.overrides).toEqual({ "runtime-peer": "4.12.18" }); + await fs.writeFile( + path.join(options.cwd, "package-lock.json"), + `${JSON.stringify( + { + lockfileVersion: 3, + packages: { + "": { + dependencies: tempManifest.dependencies, + }, + "node_modules/plugin": { + peerDependencies: { + "runtime-peer": "^4.0.0", + }, + version: "1.0.0", + }, + "node_modules/runtime-peer": { + peer: true, + version: "4.12.18", + }, + }, + }, + null, + 2, + )}\n`, + ); + return successfulSpawn; + }); + + await expect( + syncManagedNpmRootPeerDependencies({ + npmRoot, + managedOverrides: { "runtime-peer": "4.12.18" }, + runCommand, + }), + ).resolves.toBe(true); + + await expect( + fs.readFile(path.join(npmRoot, "package.json"), "utf8").then((raw) => JSON.parse(raw)), + ).resolves.toEqual({ + private: true, + dependencies: { + plugin: "1.0.0", + "runtime-peer": "4.12.18", + }, + overrides: { + "runtime-peer": "4.12.18", + }, + openclaw: { + managedOverrides: ["runtime-peer"], + managedPeerDependencies: ["runtime-peer"], + }, + }); + }); + + it("reconciles preserved stale pins with managed overrides when peer planning fails", async () => { + const npmRoot = await makeTempRoot(); + await fs.writeFile( + path.join(npmRoot, "package.json"), + `${JSON.stringify( + { + private: true, + dependencies: { + "aliased-peer": "3.0.10", + plugin: "1.0.0", + "runtime-peer": "4.12.23", + }, + openclaw: { + managedPeerDependencies: ["aliased-peer", "runtime-peer"], + }, + }, + null, + 2, + )}\n`, + ); + + const runCommand = vi.fn(async () => ({ + code: 1, + stdout: "", + stderr: "npm ERR! network request failed", + signal: null, + killed: false, + termination: "exit" as const, + })); + + await expect( + syncManagedNpmRootPeerDependencies({ + npmRoot, + managedOverrides: { + "aliased-peer": "npm:@scope/real@3.0.10", + "runtime-peer": "4.12.18", + }, + runCommand, + }), + ).resolves.toBe(true); + + await expect( + fs.readFile(path.join(npmRoot, "package.json"), "utf8").then((raw) => JSON.parse(raw)), + ).resolves.toEqual({ + private: true, + dependencies: { + "aliased-peer": "npm:@scope/real@3.0.10", + plugin: "1.0.0", + "runtime-peer": "4.12.18", + }, + overrides: { + "aliased-peer": "npm:@scope/real@3.0.10", + "runtime-peer": "4.12.18", + }, + openclaw: { + managedOverrides: ["aliased-peer", "runtime-peer"], + managedPeerDependencies: ["aliased-peer", "runtime-peer"], + }, + }); + }); + it("preserves existing managed peer dependencies when npm cannot plan third-party peers", async () => { const npmRoot = await makeTempRoot(); await fs.writeFile( diff --git a/src/infra/npm-managed-root.ts b/src/infra/npm-managed-root.ts index 2220851b3406..98eb1faeeb40 100644 --- a/src/infra/npm-managed-root.ts +++ b/src/infra/npm-managed-root.ts @@ -203,6 +203,89 @@ function filterUnsupportedManagedNpmRootOverrides(value: unknown): Record; + overrides: Record; + managedDependencyNames: ReadonlySet; + managedOverrideNames: ReadonlySet; +}): void { + for (const [packageName, overrideValue] of Object.entries(params.overrides)) { + const dependencySpec = params.dependencies[packageName]; + if (dependencySpec === undefined) { + continue; + } + const overrideSpec = readRootOverrideSpec(overrideValue); + // npm allows "$dep" references on root direct dependencies and never applies "*". + if ( + overrideSpec === undefined || + overrideSpec === "*" || + overrideSpec.startsWith("$") || + overrideSpec === dependencySpec + ) { + continue; + } + if (params.managedDependencyNames.has(packageName)) { + params.dependencies[packageName] = overrideSpec; + continue; + } + if (!params.managedOverrideNames.has(packageName)) { + continue; + } + // Only the "." entry conflicts with the root edge; child rules stay valid. + if (isRecord(overrideValue)) { + const trimmed = { ...overrideValue }; + delete trimmed["."]; + if (Object.keys(trimmed).length > 0) { + params.overrides[packageName] = trimmed; + continue; + } + } + delete params.overrides[packageName]; + } +} + +/** Merge managed overrides into a managed root manifest's override record and keep the + * EOVERRIDE invariant plus metadata (keys actually written) consistent in one place. */ +function applyManagedNpmRootOverrides(params: { + manifest: ManagedNpmRootManifest; + managedOverrides: Record; + dependencies: Record; + managedDependencyNames: ReadonlySet; +}): { overrides: Record; managedOverrideKeys: string[] } { + const overrides = readOverrideRecord(params.manifest.overrides); + for (const key of readManagedOverrideKeys(params.manifest.openclaw)) { + delete overrides[key]; + } + Object.assign(overrides, params.managedOverrides); + reconcileManagedNpmRootOverrideConflicts({ + dependencies: params.dependencies, + overrides, + managedDependencyNames: params.managedDependencyNames, + managedOverrideNames: new Set(Object.keys(params.managedOverrides)), + }); + const managedOverrideKeys = Object.keys(params.managedOverrides) + .filter((key) => Object.hasOwn(overrides, key)) + .toSorted(); + return { overrides, managedOverrideKeys }; +} + /** Read host OpenClaw pnpm overrides for reuse inside a managed npm root. */ export async function readOpenClawManagedNpmRootOverrides(params?: { argv1?: string; @@ -263,23 +346,29 @@ export async function upsertManagedNpmRootDependency(params: { const managedOverrides = params.omitUnsupportedManagedOverrides ? filterUnsupportedManagedNpmRootOverrides(params.managedOverrides) : readOverrideRecord(params.managedOverrides); - const managedOverrideKeys = Object.keys(managedOverrides).toSorted(); - const overrides = readOverrideRecord(manifest.overrides); - for (const key of readManagedOverrideKeys(manifest.openclaw)) { - delete overrides[key]; - } - Object.assign(overrides, managedOverrides); + const nextDependencies = { + ...dependencies, + [params.packageName]: params.dependencySpec, + }; + // Explicit install transfers ownership: the package stops being a managed peer pin, + // so the installer's spec wins now and later syncs may not re-pin or delete it. + const managedDependencyNames = new Set(readManagedPeerDependencyKeys(manifest.openclaw)); + managedDependencyNames.delete(params.packageName); + const { overrides, managedOverrideKeys } = applyManagedNpmRootOverrides({ + manifest, + managedOverrides, + dependencies: nextDependencies, + managedDependencyNames, + }); const openclawMetadata = buildManagedOpenClawMetadata({ current: manifest.openclaw, managedOverrideKeys, + managedPeerDependencyKeys: [...managedDependencyNames].toSorted(), }); const next: ManagedNpmRootManifest = { ...manifest, private: true, - dependencies: { - ...dependencies, - [params.packageName]: params.dependencySpec, - }, + dependencies: nextDependencies, }; if (Object.keys(overrides).length > 0) { next.overrides = overrides; @@ -752,7 +841,19 @@ export async function restoreManagedNpmRootPeerDependencySnapshot(params: { delete dependencies[packageName]; } Object.assign(dependencies, params.snapshot.dependencies); - const managedOverrideKeys = readManagedOverrideKeys(manifest.openclaw).toSorted(); + const overrides = readOverrideRecord(manifest.overrides); + const currentManagedOverrideKeys = readManagedOverrideKeys(manifest.openclaw); + // Restored pins predate the overrides currently in the manifest; realign them so a + // rollback never persists a root npm rejects with EOVERRIDE. + reconcileManagedNpmRootOverrideConflicts({ + dependencies, + overrides, + managedDependencyNames: new Set(params.snapshot.managedPeerDependencies), + managedOverrideNames: new Set(currentManagedOverrideKeys), + }); + const managedOverrideKeys = currentManagedOverrideKeys + .filter((key) => Object.hasOwn(overrides, key)) + .toSorted(); const openclawMetadata = buildManagedOpenClawMetadata({ current: manifest.openclaw, managedOverrideKeys, @@ -763,6 +864,11 @@ export async function restoreManagedNpmRootPeerDependencySnapshot(params: { private: true, dependencies, }; + if (Object.keys(overrides).length > 0) { + next.overrides = overrides; + } else { + delete next.overrides; + } if (openclawMetadata) { next.openclaw = openclawMetadata; } else { @@ -789,6 +895,13 @@ export async function syncManagedNpmRootPeerDependencies(params: { runCommand: params.runCommand, timeoutMs: params.timeoutMs, }); + const managedPeerDependencyNames = new Set( + Object.keys(peerPins).filter( + (packageName) => + previousManagedPeerDependencySet.has(packageName) || + !Object.hasOwn(dependencies, packageName), + ), + ); const nextDependencies = { ...dependencies }; for (const packageName of previousManagedPeerDependencies) { if (!Object.hasOwn(peerPins, packageName)) { @@ -796,25 +909,26 @@ export async function syncManagedNpmRootPeerDependencies(params: { } } for (const [packageName, dependencySpec] of Object.entries(peerPins)) { - nextDependencies[packageName] = dependencies[packageName] ?? dependencySpec; + // Managed pins follow the fresh plan, which npm resolved with the managed overrides + // applied. Preserving a stale pin instead can contradict a managed override and npm + // then rejects the whole root with EOVERRIDE. Owned root deps keep their spec. + if (managedPeerDependencyNames.has(packageName)) { + nextDependencies[packageName] = dependencySpec; + } } const managedOverrides = params.omitUnsupportedManagedOverrides ? filterUnsupportedManagedNpmRootOverrides(params.managedOverrides) : readOverrideRecord(params.managedOverrides); - const managedOverrideKeys = Object.keys(managedOverrides).toSorted(); - const overrides = readOverrideRecord(manifest.overrides); - for (const key of readManagedOverrideKeys(manifest.openclaw)) { - delete overrides[key]; - } - Object.assign(overrides, managedOverrides); - const managedPeerDependencyKeys = Object.keys(peerPins) - .filter( - (packageName) => - previousManagedPeerDependencySet.has(packageName) || - !Object.hasOwn(dependencies, packageName), - ) - .toSorted(); + // Also catches the plan-failure fallback (stale pins reused) and alias overrides whose + // lock-resolved version can never string-match the override spec. + const { overrides, managedOverrideKeys } = applyManagedNpmRootOverrides({ + manifest, + managedOverrides, + dependencies: nextDependencies, + managedDependencyNames: managedPeerDependencyNames, + }); + const managedPeerDependencyKeys = [...managedPeerDependencyNames].toSorted(); const openclawMetadata = buildManagedOpenClawMetadata({ current: manifest.openclaw, managedOverrideKeys,