fix(plugins): remove managed peers after npm override errors (#116675)

* fix(plugins): retry npm override cleanup on uninstall

* fix(plugins): satisfy uninstall cleanup checks

src/plugins/uninstall-managed-npm.ts

@@ -0,0 +1,85 @@
+import {
+  type ManagedNpmOverrideOmissions,
+  syncManagedNpmRootPeerDependencies,
+} from "../infra/npm-managed-root.js";
+import { createSafeNpmInstallEnv } from "../infra/safe-package-install.js";
+import { runCommandWithTimeout } from "../process/exec.js";
+import { classifyNpmManagedOverrideCompatibilityError } from "./install-managed-npm-state.js";
+
+const MANAGED_NPM_PEER_CLEANUP_ARGS = [
+  "npm",
+  "install",
+  "--omit=dev",
+  "--omit=peer",
+  "--loglevel=error",
+  "--legacy-peer-deps",
+  "--ignore-scripts",
+  "--no-audit",
+  "--no-fund",
+] as const;
+
+export async function pruneManagedNpmPeerDependenciesAfterUninstall(params: {
+  npmRoot: string;
+  packageName: string;
+  managedOverrides: Record<string, unknown>;
+  runCommand?: typeof runCommandWithTimeout;
+}): Promise<string | undefined> {
+  const command = params.runCommand ?? runCommandWithTimeout;
+  const commandOptions = {
+    cwd: params.npmRoot,
+    timeoutMs: 300_000,
+    env: createSafeNpmInstallEnv(process.env, {
+      legacyPeerDeps: true,
+      npmConfigCwd: params.npmRoot,
+      packageLock: true,
+      quiet: true,
+    }),
+  };
+  let overrideOmissions: Required<ManagedNpmOverrideOmissions> = {
+    npmAliases: false,
+    pnpmParentChildSelectors: false,
+  };
+  const syncPeerDependencies = async () =>
+    await syncManagedNpmRootPeerDependencies({
+      npmRoot: params.npmRoot,
+      managedOverrides: params.managedOverrides,
+      overrideOmissions,
+      runCommand: command,
+    });
+
+  if (!(await syncPeerDependencies())) {
+    return undefined;
+  }
+
+  let cleanup = await command([...MANAGED_NPM_PEER_CLEANUP_ARGS], commandOptions);
+  while (cleanup.code !== 0) {
+    const compatibility = classifyNpmManagedOverrideCompatibilityError(cleanup);
+    if (!compatibility) {
+      break;
+    }
+    const nextOverrideOmissions = {
+      npmAliases: overrideOmissions.npmAliases || compatibility.npmAliases,
+      pnpmParentChildSelectors:
+        overrideOmissions.pnpmParentChildSelectors || compatibility.pnpmParentChildSelectors,
+    };
+    if (
+      nextOverrideOmissions.npmAliases === overrideOmissions.npmAliases &&
+      nextOverrideOmissions.pnpmParentChildSelectors === overrideOmissions.pnpmParentChildSelectors
+    ) {
+      break;
+    }
+    overrideOmissions = nextOverrideOmissions;
+    // The first sync can only rewrite a manifest that npm rejected. Run the
+    // plan again against that compatible manifest so peer pins are refreshed.
+    await syncPeerDependencies();
+    await syncPeerDependencies();
+    cleanup = await command([...MANAGED_NPM_PEER_CLEANUP_ARGS], commandOptions);
+  }
+
+  if (cleanup.code === 0) {
+    return undefined;
+  }
+  return `Failed to prune managed peer dependencies after uninstalling ${params.packageName}: ${
+    cleanup.stderr.trim() || cleanup.stdout.trim() || `npm exited with code ${cleanup.code}`
+  }`;
+}

src/plugins/uninstall.test.ts

@@ -3,6 +3,7 @@ import fs from "node:fs/promises";
 import path from "node:path";
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 import type { OpenClawConfig } from "../config/config.js";
+import type { runCommandWithTimeout } from "../process/exec.js";
 import { toRepoRelativePath } from "../test-utils/repo-files.js";
 import { resolvePluginNpmProjectDir } from "./install-paths.js";
 import { resolvePluginInstallDir } from "./install.js";
@@ -10,6 +11,7 @@ import {
   cleanupTrackedTempDirsAsync,
   makeTrackedTempDirAsync,
 } from "./test-helpers/fs-fixtures.js";
+import { pruneManagedNpmPeerDependenciesAfterUninstall } from "./uninstall-managed-npm.js";
 import {
   applyPluginUninstallDirectoryRemoval,
   removePluginFromConfig,
@@ -1316,6 +1318,199 @@ describe("uninstallPlugin", () => {
     expect(runCommandWithTimeoutMock).toHaveBeenCalledTimes(3);
   });
 
+  it("retries managed peer cleanup without npm-incompatible override kinds", async () => {
+    const npmRoot = path.join(tempDir, "npm-override-cleanup");
+    await fs.mkdir(npmRoot, { recursive: true });
+    await fs.writeFile(
+      path.join(npmRoot, "package.json"),
+      `${JSON.stringify(
+        {
+          private: true,
+          dependencies: { "stale-peer": "1.0.0" },
+          overrides: {
+            axios: "1.18.1",
+            "node-domexception": "npm:@nolyfill/domexception@1.0.28",
+            "werift-ice@0.2.2>ip": "npm:neoip@3.1.0",
+          },
+          openclaw: {
+            managedOverrides: ["axios", "node-domexception", "werift-ice@0.2.2>ip"],
+            managedPeerDependencies: ["stale-peer"],
+          },
+        },
+        null,
+        2,
+      )}\n`,
+    );
+
+    let cleanupAttempts = 0;
+    const runCommand: typeof runCommandWithTimeout = vi.fn(async (argv, optionsOrTimeout) => {
+      const cwd = typeof optionsOrTimeout === "number" ? undefined : optionsOrTimeout.cwd;
+      if (argv.includes("--package-lock-only")) {
+        expect(cwd).toBeTruthy();
+        const manifest = JSON.parse(
+          await fs.readFile(path.join(cwd as string, "package.json"), "utf8"),
+        ) as { overrides?: Record<string, unknown> };
+        if (manifest.overrides?.["werift-ice@0.2.2>ip"]) {
+          return {
+            code: 1,
+            stdout: "",
+            stderr:
+              'npm error code EINVALIDTAGNAME\nnpm error Invalid tag name "0.2.2>ip" of package "werift-ice@0.2.2>ip"',
+            signal: null,
+            killed: false,
+            termination: "exit" as const,
+          };
+        }
+        if (manifest.overrides?.["node-domexception"]) {
+          return {
+            code: 1,
+            stdout: "",
+            stderr: "npm ERR! Invalid comparator: npm:@nolyfill/domexception@1.0.28",
+            signal: null,
+            killed: false,
+            termination: "exit" as const,
+          };
+        }
+        await fs.writeFile(
+          path.join(cwd as string, "package-lock.json"),
+          `${JSON.stringify({ lockfileVersion: 3, packages: { "": {} } }, null, 2)}\n`,
+        );
+        return {
+          code: 0,
+          stdout: "",
+          stderr: "",
+          signal: null,
+          killed: false,
+          termination: "exit" as const,
+        };
+      }
+      cleanupAttempts += 1;
+      const manifest = JSON.parse(
+        await fs.readFile(path.join(npmRoot, "package.json"), "utf8"),
+      ) as { overrides?: Record<string, unknown> };
+      if (cleanupAttempts === 1) {
+        expect(manifest.overrides?.["werift-ice@0.2.2>ip"]).toBe("npm:neoip@3.1.0");
+        return {
+          code: 1,
+          stdout: "",
+          stderr:
+            'npm error code EINVALIDTAGNAME\nnpm error Invalid tag name "0.2.2>ip" of package "werift-ice@0.2.2>ip"',
+          signal: null,
+          killed: false,
+          termination: "exit" as const,
+        };
+      }
+      if (cleanupAttempts === 2) {
+        expect(manifest.overrides?.["werift-ice@0.2.2>ip"]).toBeUndefined();
+        expect(manifest.overrides?.["node-domexception"]).toBe("npm:@nolyfill/domexception@1.0.28");
+        return {
+          code: 1,
+          stdout: "",
+          stderr: "npm ERR! Invalid comparator: npm:@nolyfill/domexception@1.0.28",
+          signal: null,
+          killed: false,
+          termination: "exit" as const,
+        };
+      }
+      expect(manifest.overrides).toEqual({ axios: "1.18.1", hono: "4.12.32" });
+      return {
+        code: 0,
+        stdout: "",
+        stderr: "",
+        signal: null,
+        killed: false,
+        termination: "exit" as const,
+      };
+    });
+
+    await expect(
+      pruneManagedNpmPeerDependenciesAfterUninstall({
+        npmRoot,
+        packageName: "@openclaw/kitchen-sink",
+        managedOverrides: {
+          axios: "1.18.1",
+          hono: "4.12.32",
+          "node-domexception": "npm:@nolyfill/domexception@1.0.28",
+          "werift-ice@0.2.2>ip": "npm:neoip@3.1.0",
+        },
+        runCommand,
+      }),
+    ).resolves.toBeUndefined();
+    expect(cleanupAttempts).toBe(3);
+    const manifest = JSON.parse(await fs.readFile(path.join(npmRoot, "package.json"), "utf8")) as {
+      dependencies?: Record<string, string>;
+      overrides?: Record<string, unknown>;
+      openclaw?: {
+        managedOverrides?: string[];
+        managedPeerDependencies?: string[];
+      };
+    };
+    expect(manifest.dependencies).toEqual({});
+    expect(manifest.overrides).toEqual({ axios: "1.18.1", hono: "4.12.32" });
+    expect(manifest.openclaw?.managedOverrides).toEqual(["axios", "hono"]);
+    expect(manifest.openclaw?.managedPeerDependencies).toBeUndefined();
+  });
+
+  it("stops retrying when an incompatible unmanaged override remains", async () => {
+    const npmRoot = path.join(tempDir, "npm-unmanaged-override-cleanup");
+    await fs.mkdir(npmRoot, { recursive: true });
+    await fs.writeFile(
+      path.join(npmRoot, "package.json"),
+      `${JSON.stringify(
+        {
+          private: true,
+          overrides: {
+            "unmanaged-parent@1.0.0>child": "2.0.0",
+          },
+        },
+        null,
+        2,
+      )}\n`,
+    );
+
+    let cleanupAttempts = 0;
+    const runCommand: typeof runCommandWithTimeout = vi.fn(async (argv, optionsOrTimeout) => {
+      const cwd = typeof optionsOrTimeout === "number" ? undefined : optionsOrTimeout.cwd;
+      if (argv.includes("--package-lock-only")) {
+        expect(cwd).toBeTruthy();
+        await fs.writeFile(
+          path.join(cwd as string, "package-lock.json"),
+          `${JSON.stringify({ lockfileVersion: 3, packages: { "": {} } }, null, 2)}\n`,
+        );
+        return {
+          code: 0,
+          stdout: "",
+          stderr: "",
+          signal: null,
+          killed: false,
+          termination: "exit" as const,
+        };
+      }
+      cleanupAttempts += 1;
+      return {
+        code: 1,
+        stdout: "",
+        stderr:
+          'npm error code EINVALIDTAGNAME\nnpm error Invalid tag name "1.0.0>child" of package "unmanaged-parent@1.0.0>child"',
+        signal: null,
+        killed: false,
+        termination: "exit" as const,
+      };
+    });
+
+    await expect(
+      pruneManagedNpmPeerDependenciesAfterUninstall({
+        npmRoot,
+        packageName: "@openclaw/kitchen-sink",
+        managedOverrides: { axios: "1.18.1" },
+        runCommand,
+      }),
+    ).resolves.toContain(
+      "Failed to prune managed peer dependencies after uninstalling @openclaw/kitchen-sink: npm error code EINVALIDTAGNAME",
+    );
+    expect(cleanupAttempts).toBe(2);
+  });
+
   it("runs npm cleanup when the managed package directory is already absent", async () => {
     const stateDir = path.join(tempDir, "state");
     const npmRoot = path.join(stateDir, "npm");

src/plugins/uninstall.ts

@@ -5,10 +5,7 @@ import path from "node:path";
 import type { OpenClawConfig } from "../config/types.openclaw.js";
 import type { PluginInstallRecord } from "../config/types.plugins.js";
 import { formatErrorMessage } from "../infra/errors.js";
-import {
-  readOpenClawManagedNpmRootOverrides,
-  syncManagedNpmRootPeerDependencies,
-} from "../infra/npm-managed-root.js";
+import { readOpenClawManagedNpmRootOverrides } from "../infra/npm-managed-root.js";
 import { createSafeNpmInstallEnv } from "../infra/safe-package-install.js";
 import { runCommandWithTimeout } from "../process/exec.js";
 import {
@@ -19,6 +16,7 @@ import {
 } from "./install-paths.js";
 import { relinkOpenClawPeerDependenciesInManagedNpmRoot } from "./plugin-peer-link.js";
 import { defaultSlotIdForKey, resetPluginSlotsToDefaults } from "./slots.js";
+import { pruneManagedNpmPeerDependenciesAfterUninstall } from "./uninstall-managed-npm.js";
 
 type UninstallActions = {
   entry: boolean;
@@ -683,43 +681,13 @@ export async function applyPluginUninstallDirectoryRemoval(
     }
     try {
       const managedOverrides = await readOpenClawManagedNpmRootOverrides();
-      const syncedPeerDependencies = await syncManagedNpmRootPeerDependencies({
+      const warning = await pruneManagedNpmPeerDependenciesAfterUninstall({
         npmRoot: removal.cleanup.npmRoot,
+        packageName: removal.cleanup.packageName,
         managedOverrides,
       });
-      if (syncedPeerDependencies) {
-        const cleanup = await runCommandWithTimeout(
-          [
-            "npm",
-            "install",
-            "--omit=dev",
-            "--omit=peer",
-            "--loglevel=error",
-            "--legacy-peer-deps",
-            "--ignore-scripts",
-            "--no-audit",
-            "--no-fund",
-          ],
-          {
-            cwd: removal.cleanup.npmRoot,
-            timeoutMs: 300_000,
-            env: createSafeNpmInstallEnv(process.env, {
-              legacyPeerDeps: true,
-              npmConfigCwd: removal.cleanup.npmRoot,
-              packageLock: true,
-              quiet: true,
-            }),
-          },
-        );
-        if (cleanup.code !== 0) {
-          warnings.push(
-            `Failed to prune managed peer dependencies after uninstalling ${removal.cleanup.packageName}: ${
-              cleanup.stderr.trim() ||
-              cleanup.stdout.trim() ||
-              `npm exited with code ${cleanup.code}`
-            }`,
-          );
-        }
+      if (warning) {
+        warnings.push(warning);
       }
     } catch (error) {
       warnings.push(