diff --git a/src/cli/channel-auth.test.ts b/src/cli/channel-auth.test.ts index 84a7e31085b..1acb60e875c 100644 --- 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 } }; + 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); diff --git a/src/cli/channel-auth.ts b/src/cli/channel-auth.ts index edb1825ffc5..558c18b6cb4 100644 --- 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) { diff --git a/src/cli/directory-cli.ts b/src/cli/directory-cli.ts index ad48d4d7008..03d60c5e479 100644 --- 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, diff --git a/src/cli/plugins-install-record-commit.test.ts b/src/cli/plugins-install-record-commit.test.ts index 0d21c66900d..23f382cc8c3 100644 --- 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, + }); + }); }); diff --git a/src/cli/plugins-install-record-commit.ts b/src/cli/plugins-install-record-commit.ts index 3146cbe8df2..93fc4dd9cb9 100644 --- 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; + +async function commitPluginInstallRecordsWithWriter(params: { previousInstallRecords?: Record; nextInstallRecords: Record; nextConfig: OpenClawConfig; - baseHash?: string; writeOptions?: ConfigWriteOptions; + commit: ConfigCommit; }): Promise { 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; + nextInstallRecords: Record; nextConfig: OpenClawConfig; baseHash?: string; writeOptions?: ConfigWriteOptions; +}): Promise { + 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; @@ -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; + 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 } : {}), + }); + }, + }); +} diff --git a/src/wizard/setup.ts b/src/wizard/setup.ts index 5608091feed..8058a539de9 100644 --- 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 { return modelPickerModulePromise; } +async function writeWizardConfigFile(config: OpenClawConfig): Promise { + 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({