refactor: clean up update and plugin uninstall helpers

2026-05-06 10:40:43 +00:00 · 2026-04-26 10:07:34 +01:00
parent d58ede1b34
commit 6cd047e7c2
7 changed files with 268 additions and 209 deletions
--- a/docs/install/updating.md
+++ b/docs/install/updating.md
@@ -73,6 +73,12 @@ the installer, pass `--install-method git --no-onboard` or
 npm i -g openclaw@latest
 ```

+When `openclaw update` manages a global npm install, it first runs the normal
+global install command. If that command fails, OpenClaw retries once with
+`--omit=optional`. That retry helps hosts where native optional dependencies
+cannot compile, while keeping the original failure visible if the fallback also
+fails.
+
 ```bash
 pnpm add -g openclaw@latest
 ```
--- a/src/cli/plugins-cli-test-helpers.ts
+++ b/src/cli/plugins-cli-test-helpers.ts
@@ -3,6 +3,7 @@ import type { Mock } from "vitest";
 import { vi } from "vitest";
 import type { OpenClawConfig } from "../config/types.openclaw.js";
 import type { PluginInstallRecord } from "../config/types.plugins.js";
+import { createEmptyUninstallActions } from "../plugins/uninstall.js";
 import { createCliRuntimeCapture } from "./test-runtime-capture.js";

 type UnknownMock = Mock<(...args: unknown[]) => unknown>;
@@ -309,20 +310,24 @@ vi.mock("../plugins/slots.js", async (importOriginal) => {
  };
 });

-vi.mock("../plugins/uninstall.js", () => ({
-  uninstallPlugin: ((
-    ...args: Parameters<(typeof import("../plugins/uninstall.js"))["uninstallPlugin"]>
-  ) =>
-    invokeMock<
-      Parameters<(typeof import("../plugins/uninstall.js"))["uninstallPlugin"]>,
-      ReturnType<(typeof import("../plugins/uninstall.js"))["uninstallPlugin"]>
-    >(uninstallPlugin, ...args)) as (typeof import("../plugins/uninstall.js"))["uninstallPlugin"],
-  resolveUninstallDirectoryTarget: ({
-    installRecord,
-  }: {
-    installRecord?: { installPath?: string; sourcePath?: string };
-  }) => installRecord?.installPath ?? installRecord?.sourcePath ?? null,
-}));
+vi.mock("../plugins/uninstall.js", async (importOriginal) => {
+  const actual = await importOriginal<typeof import("../plugins/uninstall.js")>();
+  return {
+    ...actual,
+    uninstallPlugin: ((
+      ...args: Parameters<(typeof import("../plugins/uninstall.js"))["uninstallPlugin"]>
+    ) =>
+      invokeMock<
+        Parameters<(typeof import("../plugins/uninstall.js"))["uninstallPlugin"]>,
+        ReturnType<(typeof import("../plugins/uninstall.js"))["uninstallPlugin"]>
+      >(uninstallPlugin, ...args)) as (typeof import("../plugins/uninstall.js"))["uninstallPlugin"],
+    resolveUninstallDirectoryTarget: ({
+      installRecord,
+    }: {
+      installRecord?: { installPath?: string; sourcePath?: string };
+    }) => installRecord?.installPath ?? installRecord?.sourcePath ?? null,
+  };
+});

 vi.mock("../plugins/update.js", () => ({
  updateNpmInstalledPlugins: ((
@@ -588,15 +593,7 @@ export function resetPluginsCliTestState() {
    ok: true,
    config: {} as OpenClawConfig,
    warnings: [],
-    actions: {
-      entry: false,
-      install: false,
-      allowlist: false,
-      loadPath: false,
-      memorySlot: false,
-      contextEngineSlot: false,
-      directory: false,
-    },
+    actions: createEmptyUninstallActions(),
  });
  updateNpmInstalledPlugins.mockResolvedValue({
    outcomes: [],
--- a/src/cli/plugins-cli.ts
+++ b/src/cli/plugins-cli.ts
@@ -14,7 +14,6 @@ import {
 } from "../plugins/installed-plugin-index-records.js";
 import { listMarketplacePlugins } from "../plugins/marketplace.js";
 import { inspectPluginRegistry, refreshPluginRegistry } from "../plugins/plugin-registry.js";
-import { defaultSlotIdForKey } from "../plugins/slots.js";
 import { formatPluginSourceForTable, resolvePluginSourceRoots } from "../plugins/source-display.js";
 import {
  buildAllPluginInspectReports,
@@ -26,8 +25,11 @@ import {
 } from "../plugins/status.js";
 import type { PluginLogger } from "../plugins/types.js";
 import {
+  formatUninstallActionLabels,
+  formatUninstallSlotResetPreview,
  resolveUninstallChannelConfigKeys,
  resolveUninstallDirectoryTarget,
+  UNINSTALL_ACTION_LABELS,
  uninstallPlugin,
 } from "../plugins/uninstall.js";
 import { defaultRuntime } from "../runtime.js";
@@ -616,35 +618,33 @@ export function registerPluginsCli(program: Command) {
      const isLinked = install?.source === "path";
      const preview: string[] = [];
      if (hasEntry) {
-        preview.push("config entry");
+        preview.push(UNINSTALL_ACTION_LABELS.entry);
      }
      if (hasInstall) {
-        preview.push("install record");
+        preview.push(UNINSTALL_ACTION_LABELS.install);
      }
      if (cfg.plugins?.allow?.includes(pluginId)) {
-        preview.push("allowlist entry");
+        preview.push(UNINSTALL_ACTION_LABELS.allowlist);
      }
      if (
        isLinked &&
        install?.sourcePath &&
        cfg.plugins?.load?.paths?.includes(install.sourcePath)
      ) {
-        preview.push("load path");
+        preview.push(UNINSTALL_ACTION_LABELS.loadPath);
      }
      if (cfg.plugins?.slots?.memory === pluginId) {
-        preview.push(`memory slot (will reset to "${defaultSlotIdForKey("memory")}")`);
+        preview.push(formatUninstallSlotResetPreview("memory"));
      }
      if (cfg.plugins?.slots?.contextEngine === pluginId) {
-        preview.push(
-          `context engine slot (will reset to "${defaultSlotIdForKey("contextEngine")}")`,
-        );
+        preview.push(formatUninstallSlotResetPreview("contextEngine"));
      }
      const channelIds = plugin?.status === "loaded" ? plugin.channelIds : undefined;
      const channels = cfg.channels as Record<string, unknown> | undefined;
      if (hasInstall && channels) {
        for (const key of resolveUninstallChannelConfigKeys(pluginId, { channelIds })) {
          if (Object.hasOwn(channels, key)) {
-            preview.push(`channel config (channels.${key})`);
+            preview.push(`${UNINSTALL_ACTION_LABELS.channelConfig} (channels.${key})`);
          }
        }
      }
@@ -712,31 +712,7 @@ export function registerPluginsCli(program: Command) {
        },
      });

-      const removed: string[] = [];
-      if (result.actions.entry) {
-        removed.push("config entry");
-      }
-      if (result.actions.install) {
-        removed.push("install record");
-      }
-      if (result.actions.allowlist) {
-        removed.push("allowlist");
-      }
-      if (result.actions.loadPath) {
-        removed.push("load path");
-      }
-      if (result.actions.memorySlot) {
-        removed.push("memory slot");
-      }
-      if (result.actions.contextEngineSlot) {
-        removed.push("context engine slot");
-      }
-      if (result.actions.channelConfig) {
-        removed.push("channel config");
-      }
-      if (result.actions.directory) {
-        removed.push("directory");
-      }
+      const removed = formatUninstallActionLabels(result.actions);

      defaultRuntime.log(
        `Uninstalled plugin "${pluginId}". Removed: ${removed.length > 0 ? removed.join(", ") : "nothing"}.`,
--- a/src/cli/update-cli/update-command.ts
+++ b/src/cli/update-cli/update-command.ts
@@ -19,6 +19,7 @@ import { resolveGatewayInstallEntrypoint } from "../../daemon/gateway-entrypoint
 import { resolveGatewayRestartLogPath } from "../../daemon/restart-logs.js";
 import { resolveGatewayService } from "../../daemon/service.js";
 import { createLowDiskSpaceWarning } from "../../infra/disk-space.js";
+import { runGlobalPackageUpdateSteps } from "../../infra/package-update-steps.js";
 import { nodeVersionSatisfiesEngine } from "../../infra/runtime-guard.js";
 import {
  channelToNpmTag,
@@ -33,13 +34,10 @@ import {
  checkUpdateStatus,
 } from "../../infra/update-check.js";
 import {
-  collectInstalledGlobalPackageErrors,
  canResolveRegistryVersionForPackageTarget,
  createGlobalInstallEnv,
  cleanupGlobalRenameDirs,
-  globalInstallFallbackArgs,
  globalInstallArgs,
-  resolveExpectedInstalledVersionFromSpec,
  resolveGlobalInstallTarget,
  resolveGlobalInstallSpec,
 } from "../../infra/update-global.js";
@@ -399,86 +397,45 @@ async function runPackageInstallUpdate(params: {
    }
  }

-  const updateStep = await runUpdateStep({
-    name: "global update",
-    argv: globalInstallArgs(installTarget, installSpec),
-    env: installEnv,
+  const packageUpdate = await runGlobalPackageUpdateSteps({
+    installTarget,
+    installSpec,
+    packageName,
+    packageRoot: pkgRoot,
+    runCommand,
    timeoutMs: params.timeoutMs,
-    progress: params.progress,
+    ...(installEnv === undefined ? {} : { env: installEnv }),
+    runStep: (stepParams) =>
+      runUpdateStep({
+        ...stepParams,
+        progress: params.progress,
+      }),
+    postVerifyStep: async (verifiedPackageRoot) => {
+      const entryPath = await resolveGatewayInstallEntrypoint(verifiedPackageRoot);
+      if (entryPath) {
+        return await runUpdateStep({
+          name: `${CLI_NAME} doctor`,
+          argv: [resolveNodeRunner(), entryPath, "doctor", "--non-interactive", "--fix"],
+          env: {
+            ...process.env,
+            OPENCLAW_UPDATE_IN_PROGRESS: "1",
+          },
+          timeoutMs: params.timeoutMs,
+          progress: params.progress,
+        });
+      }
+      return null;
+    },
  });

-  const steps = [updateStep];
-  let finalInstallStep = updateStep;
-  if (updateStep.exitCode !== 0) {
-    const fallbackArgv = globalInstallFallbackArgs(installTarget, installSpec);
-    if (fallbackArgv) {
-      const fallbackStep = await runUpdateStep({
-        name: "global update (omit optional)",
-        argv: fallbackArgv,
-        env: installEnv,
-        timeoutMs: params.timeoutMs,
-        progress: params.progress,
-      });
-      steps.push(fallbackStep);
-      finalInstallStep = fallbackStep;
-    }
-  }
-  let afterVersion = beforeVersion;
-
-  const verifiedPackageRoot =
-    (
-      await resolveGlobalInstallTarget({
-        manager: installTarget,
-        runCommand,
-        timeoutMs: params.timeoutMs,
-      })
-    ).packageRoot ?? pkgRoot;
-  if (verifiedPackageRoot) {
-    afterVersion = await readPackageVersion(verifiedPackageRoot);
-    const expectedVersion = resolveExpectedInstalledVersionFromSpec(packageName, installSpec);
-    const verificationErrors = await collectInstalledGlobalPackageErrors({
-      packageRoot: verifiedPackageRoot,
-      expectedVersion,
-    });
-    if (verificationErrors.length > 0) {
-      steps.push({
-        name: "global install verify",
-        command: `verify ${verifiedPackageRoot}`,
-        cwd: verifiedPackageRoot,
-        durationMs: 0,
-        exitCode: 1,
-        stderrTail: verificationErrors.join("\n"),
-        stdoutTail: null,
-      });
-    }
-    const entryPath = await resolveGatewayInstallEntrypoint(verifiedPackageRoot);
-    if (entryPath) {
-      const doctorStep = await runUpdateStep({
-        name: `${CLI_NAME} doctor`,
-        argv: [resolveNodeRunner(), entryPath, "doctor", "--non-interactive", "--fix"],
-        env: {
-          ...process.env,
-          OPENCLAW_UPDATE_IN_PROGRESS: "1",
-        },
-        timeoutMs: params.timeoutMs,
-        progress: params.progress,
-      });
-      steps.push(doctorStep);
-    }
-  }
-
-  const failedStep =
-    finalInstallStep.exitCode !== 0
-      ? finalInstallStep
-      : (steps.find((step) => step !== updateStep && step.exitCode !== 0) ?? null);
  return {
-    status: failedStep ? "error" : "ok",
+    status: packageUpdate.failedStep ? "error" : "ok",
    mode: manager,
-    root: verifiedPackageRoot ?? params.root,
-    reason: failedStep ? failedStep.name : undefined,
+    root: packageUpdate.verifiedPackageRoot ?? params.root,
+    reason: packageUpdate.failedStep ? packageUpdate.failedStep.name : undefined,
    before: { version: beforeVersion },
-    after: { version: afterVersion },
-    steps,
+    after: { version: packageUpdate.afterVersion ?? beforeVersion },
+    steps: packageUpdate.steps,
    durationMs: Date.now() - params.startedAt,
  };
 }
--- a/src/infra/package-update-steps.ts
+++ b/src/infra/package-update-steps.ts
@@ -0,0 +1,124 @@
+import { readPackageVersion } from "./package-json.js";
+import {
+  collectInstalledGlobalPackageErrors,
+  globalInstallArgs,
+  globalInstallFallbackArgs,
+  resolveExpectedInstalledVersionFromSpec,
+  resolveGlobalInstallTarget,
+  type CommandRunner,
+  type ResolvedGlobalInstallTarget,
+} from "./update-global.js";
+
+export type PackageUpdateStepResult = {
+  name: string;
+  command: string;
+  cwd: string;
+  durationMs: number;
+  exitCode: number | null;
+  stdoutTail?: string | null;
+  stderrTail?: string | null;
+};
+
+export type PackageUpdateStepRunner = (params: {
+  name: string;
+  argv: string[];
+  cwd?: string;
+  timeoutMs: number;
+  env?: NodeJS.ProcessEnv;
+}) => Promise<PackageUpdateStepResult>;
+
+export async function runGlobalPackageUpdateSteps(params: {
+  installTarget: ResolvedGlobalInstallTarget;
+  installSpec: string;
+  packageName: string;
+  packageRoot?: string | null;
+  runCommand: CommandRunner;
+  runStep: PackageUpdateStepRunner;
+  timeoutMs: number;
+  env?: NodeJS.ProcessEnv;
+  installCwd?: string;
+  postVerifyStep?: (packageRoot: string) => Promise<PackageUpdateStepResult | null>;
+}): Promise<{
+  steps: PackageUpdateStepResult[];
+  verifiedPackageRoot: string | null;
+  afterVersion: string | null;
+  failedStep: PackageUpdateStepResult | null;
+}> {
+  const installCwd = params.installCwd === undefined ? {} : { cwd: params.installCwd };
+  const installEnv = params.env === undefined ? {} : { env: params.env };
+  const updateStep = await params.runStep({
+    name: "global update",
+    argv: globalInstallArgs(params.installTarget, params.installSpec),
+    ...installCwd,
+    ...installEnv,
+    timeoutMs: params.timeoutMs,
+  });
+
+  const steps = [updateStep];
+  let finalInstallStep = updateStep;
+  if (updateStep.exitCode !== 0) {
+    const fallbackArgv = globalInstallFallbackArgs(params.installTarget, params.installSpec);
+    if (fallbackArgv) {
+      const fallbackStep = await params.runStep({
+        name: "global update (omit optional)",
+        argv: fallbackArgv,
+        ...installCwd,
+        ...installEnv,
+        timeoutMs: params.timeoutMs,
+      });
+      steps.push(fallbackStep);
+      finalInstallStep = fallbackStep;
+    }
+  }
+
+  const verifiedPackageRoot =
+    (
+      await resolveGlobalInstallTarget({
+        manager: params.installTarget,
+        runCommand: params.runCommand,
+        timeoutMs: params.timeoutMs,
+      })
+    ).packageRoot ??
+    params.packageRoot ??
+    null;
+
+  let afterVersion: string | null = null;
+  if (verifiedPackageRoot) {
+    afterVersion = await readPackageVersion(verifiedPackageRoot);
+    const expectedVersion = resolveExpectedInstalledVersionFromSpec(
+      params.packageName,
+      params.installSpec,
+    );
+    const verificationErrors = await collectInstalledGlobalPackageErrors({
+      packageRoot: verifiedPackageRoot,
+      expectedVersion,
+    });
+    if (verificationErrors.length > 0) {
+      steps.push({
+        name: "global install verify",
+        command: `verify ${verifiedPackageRoot}`,
+        cwd: verifiedPackageRoot,
+        durationMs: 0,
+        exitCode: 1,
+        stderrTail: verificationErrors.join("\n"),
+        stdoutTail: null,
+      });
+    }
+    const postVerifyStep = await params.postVerifyStep?.(verifiedPackageRoot);
+    if (postVerifyStep) {
+      steps.push(postVerifyStep);
+    }
+  }
+
+  const failedStep =
+    finalInstallStep.exitCode !== 0
+      ? finalInstallStep
+      : (steps.find((step) => step !== updateStep && step.exitCode !== 0) ?? null);
+
+  return {
+    steps,
+    verifiedPackageRoot,
+    afterVersion,
+    failedStep,
+  };
+}
--- a/src/infra/update-runner.ts
+++ b/src/infra/update-runner.ts
@@ -8,6 +8,7 @@ import {
 } from "./control-ui-assets.js";
 import { readPackageName, readPackageVersion } from "./package-json.js";
 import { normalizePackageTagInput } from "./package-tag.js";
+import { runGlobalPackageUpdateSteps } from "./package-update-steps.js";
 import { trimLogTail } from "./restart-sentinel.js";
 import { resolveStableNodePath } from "./stable-node-path.js";
 import {
@@ -20,13 +21,9 @@ import {
 } from "./update-channels.js";
 import { compareSemverStrings } from "./update-check.js";
 import {
-  collectInstalledGlobalPackageErrors,
  cleanupGlobalRenameDirs,
  createGlobalInstallEnv,
  detectGlobalInstallManagerForRoot,
-  globalInstallArgs,
-  globalInstallFallbackArgs,
-  resolveExpectedInstalledVersionFromSpec,
  resolveGlobalInstallTarget,
  resolveGlobalInstallSpec,
 } from "./update-global.js";
@@ -1297,83 +1294,39 @@ export async function runGatewayUpdate(opts: UpdateRunnerOptions = {}): Promise<
    });
    const channel = opts.channel ?? DEFAULT_PACKAGE_CHANNEL;
    const tag = normalizeTag(opts.tag ?? channelToNpmTag(channel));
-    const steps: UpdateStepResult[] = [];
    const globalInstallEnv = await createGlobalInstallEnv();
    const spec = resolveGlobalInstallSpec({
      packageName,
      tag,
      env: globalInstallEnv,
    });
-    const updateStep = await runStep({
+    const packageUpdate = await runGlobalPackageUpdateSteps({
+      installTarget,
+      installSpec: spec,
+      packageName,
+      packageRoot: pkgRoot,
      runCommand,
-      name: "global update",
-      argv: globalInstallArgs(installTarget, spec),
-      cwd: pkgRoot,
      timeoutMs,
-      env: globalInstallEnv,
-      progress,
-      stepIndex: 0,
-      totalSteps: 1,
-    });
-    steps.push(updateStep);
-
-    let finalStep = updateStep;
-    if (updateStep.exitCode !== 0) {
-      const fallbackArgv = globalInstallFallbackArgs(installTarget, spec);
-      if (fallbackArgv) {
-        const fallbackStep = await runStep({
+      ...(globalInstallEnv === undefined ? {} : { env: globalInstallEnv }),
+      installCwd: pkgRoot,
+      runStep: (stepParams) =>
+        runStep({
          runCommand,
-          name: "global update (omit optional)",
-          argv: fallbackArgv,
-          cwd: pkgRoot,
-          timeoutMs,
-          env: globalInstallEnv,
+          ...stepParams,
+          cwd: stepParams.cwd ?? pkgRoot,
          progress,
          stepIndex: 0,
          totalSteps: 1,
-        });
-        steps.push(fallbackStep);
-        finalStep = fallbackStep;
-      }
-    }
-
-    const verifiedPackageRoot =
-      (
-        await resolveGlobalInstallTarget({
-          manager: installTarget,
-          runCommand,
-          timeoutMs,
-        })
-      ).packageRoot ?? pkgRoot;
-    const expectedVersion = resolveExpectedInstalledVersionFromSpec(packageName, spec);
-    const verificationErrors = await collectInstalledGlobalPackageErrors({
-      packageRoot: verifiedPackageRoot,
-      expectedVersion,
+        }),
    });
-    if (verificationErrors.length > 0) {
-      steps.push({
-        name: "global install verify",
-        command: `verify ${verifiedPackageRoot}`,
-        cwd: verifiedPackageRoot,
-        durationMs: 0,
-        exitCode: 1,
-        stderrTail: verificationErrors.join("\n"),
-      });
-    }
-    const afterVersion = await readPackageVersion(verifiedPackageRoot);
-    const failedStep =
-      finalStep.exitCode !== 0
-        ? finalStep
-        : (steps.find((step) => step.name === "global install verify" && step.exitCode !== 0) ??
-          null);
    return {
-      status: failedStep ? "error" : "ok",
+      status: packageUpdate.failedStep ? "error" : "ok",
      mode: globalManager,
-      root: verifiedPackageRoot,
-      reason: failedStep ? failedStep.name : undefined,
+      root: packageUpdate.verifiedPackageRoot ?? pkgRoot,
+      reason: packageUpdate.failedStep ? packageUpdate.failedStep.name : undefined,
      before: { version: beforeVersion },
-      after: { version: afterVersion },
-      steps,
+      after: { version: packageUpdate.afterVersion },
+      steps: packageUpdate.steps,
      durationMs: Date.now() - startedAt,
    };
  }
--- a/src/plugins/uninstall.ts
+++ b/src/plugins/uninstall.ts
@@ -18,6 +18,60 @@ export type UninstallActions = {
  directory: boolean;
 };

+export const UNINSTALL_ACTION_LABELS = {
+  entry: "config entry",
+  install: "install record",
+  allowlist: "allowlist entry",
+  loadPath: "load path",
+  memorySlot: "memory slot",
+  contextEngineSlot: "context engine slot",
+  channelConfig: "channel config",
+  directory: "directory",
+} satisfies Record<keyof UninstallActions, string>;
+
+const UNINSTALL_ACTION_ORDER = [
+  "entry",
+  "install",
+  "allowlist",
+  "loadPath",
+  "memorySlot",
+  "contextEngineSlot",
+  "channelConfig",
+  "directory",
+] as const satisfies ReadonlyArray<keyof UninstallActions>;
+
+export function createEmptyUninstallActions(
+  overrides: Partial<UninstallActions> = {},
+): UninstallActions {
+  return {
+    entry: false,
+    install: false,
+    allowlist: false,
+    loadPath: false,
+    memorySlot: false,
+    contextEngineSlot: false,
+    channelConfig: false,
+    directory: false,
+    ...overrides,
+  };
+}
+
+export function createEmptyConfigUninstallActions(): Omit<UninstallActions, "directory"> {
+  const { directory: _directory, ...actions } = createEmptyUninstallActions();
+  return actions;
+}
+
+export function formatUninstallActionLabels(actions: UninstallActions): string[] {
+  return UNINSTALL_ACTION_ORDER.flatMap((key) =>
+    actions[key] ? [UNINSTALL_ACTION_LABELS[key]] : [],
+  );
+}
+
+export function formatUninstallSlotResetPreview(slotKey: "memory" | "contextEngine"): string {
+  const actionKey = slotKey === "memory" ? "memorySlot" : "contextEngineSlot";
+  return `${UNINSTALL_ACTION_LABELS[actionKey]} (will reset to "${defaultSlotIdForKey(slotKey)}")`;
+}
+
 export type UninstallPluginResult =
  | {
      ok: true;
@@ -150,15 +204,7 @@ export function removePluginFromConfig(
  pluginId: string,
  opts?: { channelIds?: string[] },
 ): { config: OpenClawConfig; actions: Omit<UninstallActions, "directory"> } {
-  const actions: Omit<UninstallActions, "directory"> = {
-    entry: false,
-    install: false,
-    allowlist: false,
-    loadPath: false,
-    memorySlot: false,
-    contextEngineSlot: false,
-    channelConfig: false,
-  };
+  const actions = createEmptyConfigUninstallActions();

  const pluginsConfig = cfg.plugins ?? {};