fix: persist pending onboarding plugin installs

2026-05-06 13:10:43 +00:00 · 2026-04-26 02:33:07 +01:00
parent 2495585a32
commit 29463b9c47
6 changed files with 224 additions and 44 deletions
--- a/src/cli/channel-auth.test.ts
+++ b/src/cli/channel-auth.test.ts
@@ -14,6 +14,7 @@ const mocks = vi.hoisted(() => ({
  readConfigFileSnapshot: vi.fn(),
  applyPluginAutoEnable: vi.fn(),
  replaceConfigFile: vi.fn(),
+  commitConfigWithPendingPluginInstalls: vi.fn(),
  setVerbose: vi.fn(),
  callGateway: vi.fn(),
  createClackPrompter: vi.fn(),
@@ -62,6 +63,10 @@ vi.mock("../gateway/call.js", () => ({
  callGateway: mocks.callGateway,
 }));

+vi.mock("./plugins-install-record-commit.js", () => ({
+  commitConfigWithPendingPluginInstalls: mocks.commitConfigWithPendingPluginInstalls,
+}));
+
 vi.mock("../wizard/clack-prompter.js", () => ({
  createClackPrompter: mocks.createClackPrompter,
 }));
@@ -94,6 +99,45 @@ describe("channel-auth", () => {
    mocks.readConfigFileSnapshot.mockResolvedValue({ hash: "config-1" });
    mocks.applyPluginAutoEnable.mockImplementation(({ config }) => ({ config, changes: [] }));
    mocks.replaceConfigFile.mockResolvedValue(undefined);
+    mocks.commitConfigWithPendingPluginInstalls.mockImplementation(
+      async ({
+        nextConfig,
+        baseHash,
+      }: {
+        nextConfig: { plugins?: { installs?: Record<string, unknown> } };
+        baseHash?: string;
+      }) => {
+        if (
+          !nextConfig.plugins?.installs ||
+          Object.keys(nextConfig.plugins.installs).length === 0
+        ) {
+          await mocks.replaceConfigFile({
+            nextConfig,
+            ...(baseHash !== undefined ? { baseHash } : {}),
+          });
+          return {
+            config: nextConfig,
+            installRecords: {},
+            movedInstallRecords: false,
+          };
+        }
+        const { installs: _installs, ...plugins } = nextConfig.plugins;
+        const strippedConfig =
+          Object.keys(plugins).length > 0
+            ? { ...nextConfig, plugins }
+            : Object.fromEntries(Object.entries(nextConfig).filter(([key]) => key !== "plugins"));
+        await mocks.replaceConfigFile({
+          nextConfig: strippedConfig,
+          ...(baseHash !== undefined ? { baseHash } : {}),
+          writeOptions: { unsetPaths: [["plugins", "installs"]] },
+        });
+        return {
+          config: strippedConfig,
+          installRecords: nextConfig.plugins.installs,
+          movedInstallRecords: true,
+        };
+      },
+    );
    mocks.callGateway.mockResolvedValue({ ok: true });
    mocks.listChannelPlugins.mockReturnValue([plugin]);
    mocks.resolveDefaultAgentId.mockReturnValue("main");
@@ -361,6 +405,73 @@ describe("channel-auth", () => {
    expect(mocks.login).toHaveBeenCalled();
  });

+  it("strips pending install records before persisting install-on-demand login config", async () => {
+    const catalogEntry = {
+      id: "whatsapp",
+      pluginId: "@openclaw/whatsapp",
+      meta: {
+        id: "whatsapp",
+        label: "WhatsApp",
+        selectionLabel: "WhatsApp",
+        docsPath: "/channels/whatsapp",
+        blurb: "wa",
+      },
+      install: {
+        npmSpec: "@openclaw/whatsapp",
+      },
+    };
+    mocks.getChannelPlugin.mockReturnValueOnce(undefined);
+    mocks.listChannelPluginCatalogEntries.mockReturnValueOnce([catalogEntry]);
+    mocks.ensureChannelSetupPluginInstalled.mockResolvedValueOnce({
+      cfg: {
+        channels: { whatsapp: {} },
+        plugins: {
+          entries: { whatsapp: { enabled: true } },
+          installs: {
+            whatsapp: {
+              source: "npm",
+              spec: "@openclaw/whatsapp",
+            },
+          },
+        },
+      },
+      installed: true,
+      pluginId: "whatsapp",
+    });
+    mocks.loadChannelSetupPluginRegistrySnapshotForChannel
+      .mockReturnValueOnce({
+        channels: [],
+        channelSetups: [],
+      })
+      .mockReturnValueOnce({
+        channels: [{ plugin }],
+        channelSetups: [],
+      });
+
+    await runChannelLogin({ channel: "whatsapp" }, runtime);
+
+    expect(mocks.replaceConfigFile).toHaveBeenCalledWith({
+      nextConfig: {
+        channels: { whatsapp: {} },
+        plugins: {
+          entries: { whatsapp: { enabled: true } },
+        },
+      },
+      baseHash: "config-1",
+      writeOptions: { unsetPaths: [["plugins", "installs"]] },
+    });
+    expect(mocks.login).toHaveBeenCalledWith(
+      expect.objectContaining({
+        cfg: {
+          channels: { whatsapp: {} },
+          plugins: {
+            entries: { whatsapp: { enabled: true } },
+          },
+        },
+      }),
+    );
+  });
+
  it("resolves explicit channel login through the catalog when registry normalize misses", async () => {
    mocks.normalizeChannelId.mockReturnValueOnce(undefined).mockReturnValue("whatsapp");
    mocks.getChannelPlugin.mockReturnValueOnce(undefined);
--- a/src/cli/channel-auth.ts
+++ b/src/cli/channel-auth.ts
@@ -5,12 +5,7 @@ import {
  normalizeChannelId,
 } from "../channels/plugins/index.js";
 import { resolveInstallableChannelPlugin } from "../commands/channel-setup/channel-plugin-resolution.js";
-import {
-  loadConfig,
-  readConfigFileSnapshot,
-  replaceConfigFile,
-  type OpenClawConfig,
-} from "../config/config.js";
+import { loadConfig, readConfigFileSnapshot, type OpenClawConfig } from "../config/config.js";
 import { applyPluginAutoEnable } from "../config/plugin-auto-enable.js";
 import { callGateway } from "../gateway/call.js";
 import { setVerbose } from "../globals.js";
@@ -20,6 +15,7 @@ import { defaultRuntime, type RuntimeEnv } from "../runtime.js";
 import { normalizeOptionalString } from "../shared/string-coerce.js";
 import { sanitizeForLog } from "../terminal/ansi.js";
 import { GATEWAY_CLIENT_MODES, GATEWAY_CLIENT_NAMES } from "../utils/message-channel.js";
+import { commitConfigWithPendingPluginInstalls } from "./plugins-install-record-commit.js";

 type ChannelAuthOptions = {
  channel?: string;
@@ -182,17 +178,15 @@ export async function runChannelLogin(
    env: process.env,
  });
  const loadedCfg = autoEnabled.config;
-  const { cfg, configChanged, channelInput, plugin } = await resolveChannelPluginForMode(
-    opts,
-    "login",
-    loadedCfg,
-    runtime,
-  );
+  const resolvedChannel = await resolveChannelPluginForMode(opts, "login", loadedCfg, runtime);
+  let cfg = resolvedChannel.cfg;
+  const { configChanged, channelInput, plugin } = resolvedChannel;
  if (autoEnabled.changes.length > 0 || configChanged) {
-    await replaceConfigFile({
+    const committed = await commitConfigWithPendingPluginInstalls({
      nextConfig: cfg,
      baseHash: (await sourceSnapshotPromise)?.hash,
    });
+    cfg = committed.config;
  }
  const login = plugin.auth?.login;
  if (!login) {
@@ -227,17 +221,15 @@ export async function runChannelLogout(
    env: process.env,
  });
  const loadedCfg = autoEnabled.config;
-  const { cfg, configChanged, channelInput, plugin } = await resolveChannelPluginForMode(
-    opts,
-    "logout",
-    loadedCfg,
-    runtime,
-  );
+  const resolvedChannel = await resolveChannelPluginForMode(opts, "logout", loadedCfg, runtime);
+  let cfg = resolvedChannel.cfg;
+  const { configChanged, channelInput, plugin } = resolvedChannel;
  if (autoEnabled.changes.length > 0 || configChanged) {
-    await replaceConfigFile({
+    const committed = await commitConfigWithPendingPluginInstalls({
      nextConfig: cfg,
      baseHash: (await sourceSnapshotPromise)?.hash,
    });
+    cfg = committed.config;
  }
  const logoutAccount = plugin.gateway?.logoutAccount;
  if (!logoutAccount) {
--- a/src/cli/directory-cli.ts
+++ b/src/cli/directory-cli.ts
@@ -15,6 +15,7 @@ import { formatDocsLink } from "../terminal/links.js";
 import { getTerminalTableWidth, renderTable } from "../terminal/table.js";
 import { theme } from "../terminal/theme.js";
 import { formatHelpExamples } from "./help-format.js";
+import { commitConfigWithPendingPluginInstalls } from "./plugins-install-record-commit.js";

 function parseLimit(value: unknown): number | null {
  if (typeof value === "number" && Number.isFinite(value)) {
@@ -120,10 +121,11 @@ export function registerDirectoryCli(program: Command) {
      : null;
    if (resolvedExplicit?.configChanged) {
      cfg = resolvedExplicit.cfg;
-      await replaceConfigFile({
+      const committed = await commitConfigWithPendingPluginInstalls({
        nextConfig: cfg,
        baseHash: (await sourceSnapshotPromise)?.hash,
      });
+      cfg = committed.config;
    } else if (autoEnabled.changes.length > 0) {
      await replaceConfigFile({
        nextConfig: cfg,
--- a/src/cli/plugins-install-record-commit.test.ts
+++ b/src/cli/plugins-install-record-commit.test.ts
@@ -23,7 +23,10 @@ vi.mock("../plugins/installed-plugin-index-records.js", async (importOriginal) =
  };
 });

-import { commitConfigWithPendingPluginInstalls } from "./plugins-install-record-commit.js";
+import {
+  commitConfigWithPendingPluginInstalls,
+  commitConfigWriteWithPendingPluginInstalls,
+} from "./plugins-install-record-commit.js";

 describe("commitConfigWithPendingPluginInstalls", () => {
  beforeEach(() => {
@@ -155,4 +158,25 @@ describe("commitConfigWithPendingPluginInstalls", () => {
      movedInstallRecords: false,
    });
  });
+
+  it("supports non-replace config writers without adding an undefined write options argument", async () => {
+    const writeConfigFile = vi.fn(async () => undefined);
+    const nextConfig: OpenClawConfig = {
+      gateway: {
+        mode: "local",
+      },
+    };
+
+    const result = await commitConfigWriteWithPendingPluginInstalls({
+      nextConfig,
+      commit: writeConfigFile,
+    });
+
+    expect(writeConfigFile).toHaveBeenCalledWith(nextConfig);
+    expect(result).toEqual({
+      config: nextConfig,
+      installRecords: {},
+      movedInstallRecords: false,
+    });
+  });
 });
--- a/src/cli/plugins-install-record-commit.ts
+++ b/src/cli/plugins-install-record-commit.ts
@@ -17,26 +17,24 @@ function mergeUnsetPaths(
  return merged.length > 0 ? merged : undefined;
 }

-export async function commitPluginInstallRecordsWithConfig(params: {
+type ConfigCommit = (config: OpenClawConfig, writeOptions?: ConfigWriteOptions) => Promise<void>;
+
+async function commitPluginInstallRecordsWithWriter(params: {
  previousInstallRecords?: Record<string, PluginInstallRecord>;
  nextInstallRecords: Record<string, PluginInstallRecord>;
  nextConfig: OpenClawConfig;
-  baseHash?: string;
  writeOptions?: ConfigWriteOptions;
+  commit: ConfigCommit;
 }): Promise<void> {
  const previousInstallRecords =
    params.previousInstallRecords ?? (await loadInstalledPluginIndexInstallRecords());
  await writePersistedInstalledPluginIndexInstallRecords(params.nextInstallRecords);
  try {
-    await replaceConfigFile({
-      nextConfig: params.nextConfig,
-      ...(params.baseHash !== undefined ? { baseHash: params.baseHash } : {}),
-      writeOptions: {
-        ...params.writeOptions,
-        unsetPaths: mergeUnsetPaths(params.writeOptions?.unsetPaths, [
-          Array.from(PLUGIN_INSTALLS_CONFIG_PATH),
-        ]),
-      },
+    await params.commit(params.nextConfig, {
+      ...params.writeOptions,
+      unsetPaths: mergeUnsetPaths(params.writeOptions?.unsetPaths, [
+        Array.from(PLUGIN_INSTALLS_CONFIG_PATH),
+      ]),
    });
  } catch (error) {
    try {
@@ -51,10 +49,29 @@ export async function commitPluginInstallRecordsWithConfig(params: {
  }
 }

-export async function commitConfigWithPendingPluginInstalls(params: {
+export async function commitPluginInstallRecordsWithConfig(params: {
+  previousInstallRecords?: Record<string, PluginInstallRecord>;
+  nextInstallRecords: Record<string, PluginInstallRecord>;
  nextConfig: OpenClawConfig;
  baseHash?: string;
  writeOptions?: ConfigWriteOptions;
+}): Promise<void> {
+  await commitPluginInstallRecordsWithWriter({
+    ...params,
+    commit: async (nextConfig, writeOptions) => {
+      await replaceConfigFile({
+        nextConfig,
+        ...(params.baseHash !== undefined ? { baseHash: params.baseHash } : {}),
+        ...(writeOptions ? { writeOptions } : {}),
+      });
+    },
+  });
+}
+
+export async function commitConfigWriteWithPendingPluginInstalls(params: {
+  nextConfig: OpenClawConfig;
+  writeOptions?: ConfigWriteOptions;
+  commit: ConfigCommit;
 }): Promise<{
  config: OpenClawConfig;
  installRecords: Record<string, PluginInstallRecord>;
@@ -62,11 +79,11 @@ export async function commitConfigWithPendingPluginInstalls(params: {
 }> {
  const pendingInstallRecords = params.nextConfig.plugins?.installs ?? {};
  if (Object.keys(pendingInstallRecords).length === 0) {
-    await replaceConfigFile({
-      nextConfig: params.nextConfig,
-      ...(params.baseHash !== undefined ? { baseHash: params.baseHash } : {}),
-      ...(params.writeOptions ? { writeOptions: params.writeOptions } : {}),
-    });
+    if (params.writeOptions) {
+      await params.commit(params.nextConfig, params.writeOptions);
+    } else {
+      await params.commit(params.nextConfig);
+    }
    return {
      config: params.nextConfig,
      installRecords: {},
@@ -80,12 +97,12 @@ export async function commitConfigWithPendingPluginInstalls(params: {
    ...pendingInstallRecords,
  };
  const strippedConfig = withoutPluginInstallRecords(params.nextConfig);
-  await commitPluginInstallRecordsWithConfig({
+  await commitPluginInstallRecordsWithWriter({
    previousInstallRecords,
    nextInstallRecords,
    nextConfig: strippedConfig,
-    ...(params.baseHash !== undefined ? { baseHash: params.baseHash } : {}),
    ...(params.writeOptions ? { writeOptions: params.writeOptions } : {}),
+    commit: params.commit,
  });
  return {
    config: strippedConfig,
@@ -93,3 +110,25 @@ export async function commitConfigWithPendingPluginInstalls(params: {
    movedInstallRecords: true,
  };
 }
+
+export async function commitConfigWithPendingPluginInstalls(params: {
+  nextConfig: OpenClawConfig;
+  baseHash?: string;
+  writeOptions?: ConfigWriteOptions;
+}): Promise<{
+  config: OpenClawConfig;
+  installRecords: Record<string, PluginInstallRecord>;
+  movedInstallRecords: boolean;
+}> {
+  return await commitConfigWriteWithPendingPluginInstalls({
+    nextConfig: params.nextConfig,
+    ...(params.writeOptions ? { writeOptions: params.writeOptions } : {}),
+    commit: async (nextConfig, writeOptions) => {
+      await replaceConfigFile({
+        nextConfig,
+        ...(params.baseHash !== undefined ? { baseHash: params.baseHash } : {}),
+        ...(writeOptions ? { writeOptions } : {}),
+      });
+    },
+  });
+}
--- a/src/wizard/setup.ts
+++ b/src/wizard/setup.ts
@@ -1,4 +1,5 @@
 import { formatCliCommand } from "../cli/command-format.js";
+import { commitConfigWriteWithPendingPluginInstalls } from "../cli/plugins-install-record-commit.js";
 import type {
  AuthChoice,
  GatewayAuthChoice,
@@ -49,6 +50,17 @@ function loadModelPickerModule(): Promise<ModelPickerModule> {
  return modelPickerModulePromise;
 }

+async function writeWizardConfigFile(config: OpenClawConfig): Promise<OpenClawConfig> {
+  const committed = await commitConfigWriteWithPendingPluginInstalls({
+    nextConfig: config,
+    commit: async (nextConfig, writeOptions) =>
+      writeOptions
+        ? await writeConfigFile(nextConfig, writeOptions)
+        : await writeConfigFile(nextConfig),
+  });
+  return committed.config;
+}
+
 async function resolveAuthChoiceModelSelectionPolicy(params: {
  authChoice: string;
  config: OpenClawConfig;
@@ -471,7 +483,7 @@ export async function runSetupWizard(
      nextConfig = applySkipBootstrapConfig(nextConfig);
    }
    nextConfig = onboardHelpers.applyWizardMetadata(nextConfig, { command: "onboard", mode });
-    await writeConfigFile(nextConfig);
+    nextConfig = await writeWizardConfigFile(nextConfig);
    logConfigUpdated(runtime);
    await prompter.outro("Remote gateway configured.");
    return;
@@ -655,7 +667,7 @@ export async function runSetupWizard(
    });
  }

-  await writeConfigFile(nextConfig);
+  nextConfig = await writeWizardConfigFile(nextConfig);
  const { logConfigUpdated } = await loadConfigLoggingModule();
  logConfigUpdated(runtime);
  await onboardHelpers.ensureWorkspaceAndSessions(workspaceDir, runtime, {
@@ -694,7 +706,7 @@ export async function runSetupWizard(
  nextConfig = await setupInternalHooks(nextConfig, runtime, prompter);

  nextConfig = onboardHelpers.applyWizardMetadata(nextConfig, { command: "onboard", mode });
-  await writeConfigFile(nextConfig);
+  nextConfig = await writeWizardConfigFile(nextConfig);

  const { finalizeSetupWizard } = await import("./setup.finalize.js");
  const { launchedTui } = await finalizeSetupWizard({