From 140ac291727b2271201e26bcbaceaf813d992c83 Mon Sep 17 00:00:00 2001 From: Shakker Date: Sun, 26 Apr 2026 01:34:08 +0100 Subject: [PATCH] fix: defer onboarding plugin install records --- CHANGELOG.md | 1 + .../channel-setup/plugin-install.test.ts | 8 ++- src/commands/channels.add.test.ts | 67 +++++++++++++++++++ .../onboarding-plugin-install.test.ts | 26 ++++++- src/commands/onboarding-plugin-install.ts | 29 +------- 5 files changed, 100 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6acc81624cc..e3e2af493e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -99,6 +99,7 @@ Docs: https://docs.openclaw.ai Thanks @Kobe9312 and @vincentkoc. - Plugins/install: restore the previous plugin index records if a concurrent config write conflict interrupts install, update, or uninstall metadata commits. Thanks @shakkernerd. - Plugins/update: restore previous plugin index records if core update or channel setup hits a concurrent config write conflict after plugin metadata changes. Thanks @shakkernerd. +- Plugins/onboarding: defer channel/provider plugin install records until the owning config write commits, keeping setup failures from advancing the plugin index ahead of `openclaw.json`. Thanks @shakkernerd. - Sessions: keep embedded runtime context out of the visible user prompt by sending it as a hidden next-turn custom message, and teach doctor to repair affected 2026.4.24 transcripts with duplicated prompt-rewrite branches. diff --git a/src/commands/channel-setup/plugin-install.test.ts b/src/commands/channel-setup/plugin-install.test.ts index 949cb286b15..2c8869cdbad 100644 --- a/src/commands/channel-setup/plugin-install.test.ts +++ b/src/commands/channel-setup/plugin-install.test.ts @@ -314,7 +314,13 @@ describe("ensureChannelSetupPluginInstalled", () => { expect(result.installed).toBe(true); expect(result.cfg.plugins?.entries?.["bundled-chat"]?.enabled).toBe(true); expect(result.cfg.plugins?.allow).toContain("bundled-chat"); - expect(result.cfg.plugins?.installs).toBeUndefined(); + expect(result.cfg.plugins?.installs).toEqual({ + "bundled-chat": expect.objectContaining({ + source: "npm", + spec: bundledChatNpmSpec, + installPath: "/tmp/bundled-chat", + }), + }); expect(installPluginFromNpmSpec).toHaveBeenCalledWith( expect.objectContaining({ expectedIntegrity: bundledChatIntegrity, diff --git a/src/commands/channels.add.test.ts b/src/commands/channels.add.test.ts index 05455e4862f..644da5a9aab 100644 --- a/src/commands/channels.add.test.ts +++ b/src/commands/channels.add.test.ts @@ -1,6 +1,7 @@ import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; import type { ChannelPluginCatalogEntry } from "../channels/plugins/catalog.js"; import type { ChannelPlugin } from "../channels/plugins/types.js"; +import type { PluginInstallRecord } from "../config/types.plugins.js"; import { resetPluginRuntimeStateForTest, setActivePluginRegistry } from "../plugins/runtime.js"; import { DEFAULT_ACCOUNT_ID } from "../routing/session-key.js"; import { createChannelTestPluginBase, createTestRegistry } from "../test-utils/channel-plugins.js"; @@ -34,6 +35,10 @@ const registryRefreshMocks = vi.hoisted(() => ({ refreshPluginRegistryAfterConfigMutation: vi.fn(async () => undefined), })); +const pluginInstallRecordCommitMocks = vi.hoisted(() => ({ + commitPluginInstallRecordsWithConfig: vi.fn(), +})); + vi.mock("../channels/plugins/catalog.js", () => ({ listChannelPluginCatalogEntries: catalogMocks.listChannelPluginCatalogEntries, })); @@ -56,6 +61,8 @@ vi.mock("./channel-setup/plugin-install.js", () => pluginInstallMocks); vi.mock("../cli/plugins-registry-refresh.js", () => registryRefreshMocks); +vi.mock("../cli/plugins-install-record-commit.js", () => pluginInstallRecordCommitMocks); + const runtime = createTestRuntime(); function listConfiguredAccountIds( @@ -256,6 +263,12 @@ describe("channelsAddCommand", () => { .mockImplementation(async (params: { nextConfig: unknown }) => { await configMocks.writeConfigFile(params.nextConfig); }); + pluginInstallRecordCommitMocks.commitPluginInstallRecordsWithConfig.mockReset(); + pluginInstallRecordCommitMocks.commitPluginInstallRecordsWithConfig.mockImplementation( + async (params: { nextConfig: unknown }) => { + await configMocks.writeConfigFile(params.nextConfig); + }, + ); lifecycleMocks.onAccountConfigChanged.mockClear(); runtime.log.mockClear(); runtime.error.mockClear(); @@ -525,6 +538,60 @@ describe("channelsAddCommand", () => { expectExternalChatEnabledConfigWrite(); }); + it("commits channel setup plugin install records with the guarded config write", async () => { + configMocks.readConfigFileSnapshot.mockResolvedValue({ + ...baseConfigSnapshot, + hash: "config-1", + }); + setActivePluginRegistry(createTestRegistry()); + const catalogEntry = createExternalChatCatalogEntry(); + catalogMocks.listChannelPluginCatalogEntries.mockReturnValue([catalogEntry]); + registerExternalChatSetupPlugin("external-chat"); + const installRecords: Record = { + "@vendor/external-chat-plugin": { + source: "npm", + spec: "@vendor/external-chat@1.2.3", + }, + }; + vi.mocked(ensureChannelSetupPluginInstalled).mockImplementation(async ({ cfg }) => ({ + cfg: { + ...cfg, + plugins: { + ...cfg.plugins, + installs: installRecords, + }, + }, + installed: true, + pluginId: "@vendor/external-chat-plugin", + status: "installed", + })); + + await channelsAddCommand( + { + channel: "external-chat", + account: "default", + token: "tenant-scoped", + }, + runtime, + { hasFlags: true }, + ); + + expect( + pluginInstallRecordCommitMocks.commitPluginInstallRecordsWithConfig, + ).toHaveBeenCalledWith({ + nextInstallRecords: installRecords, + nextConfig: expect.not.objectContaining({ + plugins: expect.objectContaining({ installs: expect.anything() }), + }), + baseHash: "config-1", + }); + expect(registryRefreshMocks.refreshPluginRegistryAfterConfigMutation).toHaveBeenCalledWith( + expect.objectContaining({ + installRecords, + }), + ); + }); + it("uses the installed plugin id when channel and plugin ids differ", async () => { configMocks.readConfigFileSnapshot.mockResolvedValue({ ...baseConfigSnapshot }); setActivePluginRegistry(createTestRegistry()); diff --git a/src/commands/onboarding-plugin-install.test.ts b/src/commands/onboarding-plugin-install.test.ts index 87bf700f203..4105c870e0c 100644 --- a/src/commands/onboarding-plugin-install.test.ts +++ b/src/commands/onboarding-plugin-install.test.ts @@ -140,7 +140,13 @@ describe("ensureOnboardingPluginInstalled", () => { ); expect(result.installed).toBe(true); expect(result.status).toBe("installed"); - expect(result.cfg.plugins?.installs).toBeUndefined(); + expect(result.cfg.plugins?.installs).toEqual({ + "demo-plugin": expect.objectContaining({ + pluginId: "demo-plugin", + source: "npm", + spec: "@wecom/wecom-openclaw-plugin@1.2.3", + }), + }); expect(refreshPluginRegistryAfterConfigMutation).toHaveBeenCalledWith( expect.objectContaining({ config: result.cfg, @@ -467,7 +473,14 @@ describe("ensureOnboardingPluginInstalled", () => { ); expect(result.installed).toBe(true); expect(result.status).toBe("installed"); - expect(result.cfg.plugins?.installs).toBeUndefined(); + expect(result.cfg.plugins?.installs).toEqual({ + "demo-plugin": { + pluginId: "demo-plugin", + source: "path", + sourcePath: "./plugins/demo", + spec: "@demo/plugin@1.2.3", + }, + }); }); }); @@ -527,7 +540,14 @@ describe("ensureOnboardingPluginInstalled", () => { ); expect(result.installed).toBe(true); expect(result.status).toBe("installed"); - expect(result.cfg.plugins?.installs).toBeUndefined(); + expect(result.cfg.plugins?.installs).toEqual({ + "demo-plugin": { + pluginId: "demo-plugin", + source: "path", + sourcePath: "./plugins/demo", + spec: "@demo/plugin@1.2.3", + }, + }); }, ); }); diff --git a/src/commands/onboarding-plugin-install.ts b/src/commands/onboarding-plugin-install.ts index 699b231848b..82f90bbd967 100644 --- a/src/commands/onboarding-plugin-install.ts +++ b/src/commands/onboarding-plugin-install.ts @@ -10,12 +10,6 @@ import { } from "../plugins/bundled-sources.js"; import { enablePluginInConfig, type PluginEnableResult } from "../plugins/enable.js"; import { installPluginFromNpmSpec } from "../plugins/install.js"; -import { - loadInstalledPluginIndexInstallRecords, - recordPluginInstallInRecords, - withoutPluginInstallRecords, - writePersistedInstalledPluginIndexInstallRecords, -} from "../plugins/installed-plugin-index-records.js"; import { buildNpmResolutionInstallFields, recordPluginInstall } from "../plugins/installs.js"; import type { PluginPackageInstall } from "../plugins/manifest.js"; import type { RuntimeEnv } from "../runtime.js"; @@ -142,17 +136,6 @@ function formatPortableLocalPath(localPath: string, workspaceDir?: string): stri return undefined; } -async function persistOnboardingPluginInstallRecord(params: { - cfg: OpenClawConfig; - install: Parameters[1]; -}) { - const records = await loadInstalledPluginIndexInstallRecords(); - await writePersistedInstalledPluginIndexInstallRecords( - recordPluginInstallInRecords(records, params.install), - { config: params.cfg }, - ); -} - async function refreshRegistryAfterOnboardingPluginInstall(params: { cfg: OpenClawConfig; refreshRegistry?: boolean; @@ -184,11 +167,7 @@ async function recordLocalPluginInstall(params: { ...(sourcePath ? { sourcePath } : {}), ...(params.npmSpec ? { spec: params.npmSpec } : {}), } as const; - await persistOnboardingPluginInstallRecord({ - cfg: params.cfg, - install, - }); - return withoutPluginInstallRecords(recordPluginInstall(params.cfg, install)); + return recordPluginInstall(params.cfg, install); } function resolveLocalPath(params: { @@ -599,11 +578,7 @@ export async function ensureOnboardingPluginInstalled(params: { version: result.version, ...buildNpmResolutionInstallFields(result.npmResolution), } as const; - await persistOnboardingPluginInstallRecord({ - cfg: next, - install, - }); - next = withoutPluginInstallRecords(recordPluginInstall(next, install)); + next = recordPluginInstall(next, install); await refreshRegistryAfterOnboardingPluginInstall({ cfg: next, refreshRegistry: params.refreshRegistry,