fix(onboarding): preserve plugin ownership from included config (#103372)

* fix(onboarding): preserve included plugin install records

* fix(config): validate inherited plugin install records

* fix(onboarding): preserve canonical plugin install ownership

* test(onboarding): cover completed install migrations
This commit is contained in:
Peter Steinberger
2026-07-10 06:07:41 +01:00
committed by GitHub
parent 4a40f5d7f0
commit a7c5b2c6e6
7 changed files with 199 additions and 4 deletions

View File

@@ -14,11 +14,14 @@ import { withEnvAsync } from "../test-utils/env.js";
const mocks = vi.hoisted(() => ({
loadInstalledPluginIndexInstallRecords: vi.fn(),
replaceConfigFile: vi.fn(),
transformConfigFileWithRetry: vi.fn(),
writePersistedInstalledPluginIndexInstallRecords: vi.fn(),
}));
vi.mock("../config/config.js", () => ({
replaceConfigFile: mocks.replaceConfigFile,
resolveConfigWriteAfterWrite: (value?: unknown) => value ?? { mode: "auto" },
transformConfigFileWithRetry: mocks.transformConfigFileWithRetry,
}));
vi.mock("../plugins/installed-plugin-index-records.js", async (importOriginal) => {
@@ -37,6 +40,7 @@ import {
commitConfigWriteWithPendingPluginInstalls,
commitPluginInstallRecordsWithConfig,
stripPendingPluginInstallRecords,
transformConfigWithPendingPluginInstalls,
unchangedPendingPluginInstallRecordIds,
} from "./plugins-install-record-commit.js";
@@ -119,6 +123,91 @@ describe("commitConfigWithPendingPluginInstalls", () => {
});
});
it("migrates source records below the canonical index and explicit pending records", async () => {
const sourceConfig: OpenClawConfig = {
plugins: {
installs: {
stale: { source: "npm", spec: "stale@1.0.0" },
missing: { source: "npm", spec: "missing@1.0.0" },
codex: { source: "npm", spec: "codex@1.0.0" },
},
},
};
const existingRecords: Record<string, PluginInstallRecord> = {
stale: { source: "npm", spec: "stale@2.0.0" },
codex: { source: "npm", spec: "codex@2.0.0" },
};
const nextConfig: OpenClawConfig = {
plugins: {
installs: {
...sourceConfig.plugins?.installs,
codex: { source: "npm", spec: "codex@3.0.0" },
concurrent: { source: "npm", spec: "concurrent@1.0.0" },
},
},
};
const commit = vi.fn(async () => undefined);
mocks.loadInstalledPluginIndexInstallRecords.mockResolvedValue(existingRecords);
const result = await commitConfigWriteWithPendingPluginInstalls({
nextConfig,
sourceConfig,
commit,
});
expect(mocks.writePersistedInstalledPluginIndexInstallRecords).toHaveBeenCalledWith({
stale: existingRecords.stale,
missing: sourceConfig.plugins?.installs?.missing,
codex: nextConfig.plugins?.installs?.codex,
concurrent: nextConfig.plugins?.installs?.concurrent,
});
expect(commit).toHaveBeenCalledWith({}, {
afterWrite: { mode: "restart", reason: "plugin source changed" },
unsetPaths: [["plugins", "installs"]],
});
expect(result.installRecords).toStrictEqual({
stale: existingRecords.stale,
missing: sourceConfig.plugins?.installs?.missing,
codex: nextConfig.plugins?.installs?.codex,
concurrent: nextConfig.plugins?.installs?.concurrent,
});
});
it("preserves source records omitted by a transform callback", async () => {
const sourceConfig: OpenClawConfig = {
plugins: {
installs: {
other: { source: "npm", spec: "other@1.0.0" },
},
},
};
const codexRecord: PluginInstallRecord = { source: "npm", spec: "codex@2.0.0" };
const snapshot = { sourceConfig };
mocks.transformConfigFileWithRetry.mockImplementationOnce(async (params: unknown) => {
const transformParams = params as {
transform: (
config: OpenClawConfig,
context: { snapshot: typeof snapshot },
) => { nextConfig: OpenClawConfig };
commit: (input: unknown) => Promise<unknown>;
};
const transformed = transformParams.transform(sourceConfig, { snapshot });
await transformParams.commit({ nextConfig: transformed.nextConfig, snapshot });
return {};
});
await transformConfigWithPendingPluginInstalls({
transform: () => ({
nextConfig: { plugins: { installs: { codex: codexRecord } } },
}),
});
expect(mocks.writePersistedInstalledPluginIndexInstallRecords).toHaveBeenCalledWith({
other: sourceConfig.plugins?.installs?.other,
codex: codexRecord,
});
});
it("strips only selected pending plugin install records", () => {
const config: OpenClawConfig = {
plugins: {

View File

@@ -12,6 +12,7 @@ import {
type TransformConfigFileWithRetryParams,
} from "../config/config.js";
import type { ConfigWriteOptions } from "../config/io.js";
import { extractShippedPluginInstallConfigRecords } from "../config/plugin-install-config-migration.js";
import type { OpenClawConfig } from "../config/types.openclaw.js";
import type { PluginInstallRecord } from "../config/types.plugins.js";
import { isPathInside } from "../infra/path-guards.js";
@@ -349,6 +350,8 @@ export async function commitPluginInstallRecordsWithConfig(params: {
/** Commit config while migrating any pending install records into the install index. */
export async function commitConfigWriteWithPendingPluginInstalls(params: {
nextConfig: OpenClawConfig;
/** Source snapshot whose transient records migrate below the canonical index. */
sourceConfig?: OpenClawConfig;
writeOptions?: ConfigWriteOptions;
commit: ConfigCommit;
}): Promise<{
@@ -357,7 +360,19 @@ export async function commitConfigWriteWithPendingPluginInstalls(params: {
movedInstallRecords: boolean;
persistedHash: string | null;
}> {
if (!hasPendingPluginInstallRecords(params.nextConfig)) {
const sourceInstallRecords = extractShippedPluginInstallConfigRecords(params.sourceConfig);
const nextPendingConfig = params.sourceConfig
? stripPendingPluginInstallRecords(
params.nextConfig,
unchangedPendingPluginInstallRecordIds(params.nextConfig, {
plugins: { installs: sourceInstallRecords },
}),
)
: params.nextConfig;
if (
Object.keys(sourceInstallRecords).length === 0 &&
!hasPendingPluginInstallRecords(nextPendingConfig)
) {
const committed = params.writeOptions
? await params.commit(params.nextConfig, params.writeOptions)
: await params.commit(params.nextConfig);
@@ -369,9 +384,10 @@ export async function commitConfigWriteWithPendingPluginInstalls(params: {
};
}
const pendingInstallRecords = params.nextConfig.plugins?.installs ?? {};
const pendingInstallRecords = nextPendingConfig.plugins?.installs ?? {};
const previousInstallRecords = await loadInstalledPluginIndexInstallRecords();
const nextInstallRecords = {
...sourceInstallRecords,
...previousInstallRecords,
...pendingInstallRecords,
};
@@ -423,6 +439,7 @@ export async function transformConfigWithPendingPluginInstalls<T = void>(
const requestedAfterWrite = params.afterWrite ?? params.writeOptions?.afterWrite;
const committed = await commitConfigWriteWithPendingPluginInstalls({
nextConfig,
sourceConfig: snapshot.sourceConfig,
...(writeOptions ? { writeOptions: mergeAfterWrite(writeOptions, params.afterWrite) } : {}),
commit: async (config, commitWriteOptions) => {
return await replaceConfigFile({

View File

@@ -88,7 +88,7 @@ describe("Crestodian chat channel setup", () => {
expect(reply.text).toContain("matrix is configured");
expect(mocks.writeWizardConfigFile).toHaveBeenCalledWith(
{ channels: { matrix: { enabled: true } } },
{ allowConfigSizeDrop: false },
{ allowConfigSizeDrop: false, migrationBaseConfig: {} },
);
expect(mocks.runCollectedChannelOnboardingPostWriteHooks).toHaveBeenCalledWith({
hooks: [mocks.hook],

View File

@@ -141,6 +141,7 @@ function defaultChannelSetupWizardRunner(
});
const committedConfig = await writeWizardConfigFile(nextConfig, {
allowConfigSizeDrop: false,
migrationBaseConfig: baseConfig,
});
await runCollectedChannelOnboardingPostWriteHooks({
hooks: postWriteHooks.drain(),

View File

@@ -183,7 +183,10 @@ export async function applyCrestodianSetup(
command: "onboard",
mode: "local",
});
nextConfig = await writeWizardConfigFile(nextConfig, { allowConfigSizeDrop: false });
nextConfig = await writeWizardConfigFile(nextConfig, {
allowConfigSizeDrop: false,
migrationBaseConfig: baseConfig,
});
await onboardHelpers.ensureWorkspaceAndSessions(workspace, runtime, {
skipBootstrap: Boolean(nextConfig.agents?.defaults?.skipBootstrap),

View File

@@ -0,0 +1,77 @@
import { beforeEach, describe, expect, it, vi } from "vitest";
import type { OpenClawConfig } from "../config/types.openclaw.js";
import { withoutPluginInstallRecords } from "../plugins/installed-plugin-index-records.js";
const mocks = vi.hoisted(() => ({
commitConfigWriteWithPendingPluginInstalls: vi.fn(),
}));
vi.mock("../cli/plugins-install-record-commit.js", async (importOriginal) => ({
...(await importOriginal<typeof import("../cli/plugins-install-record-commit.js")>()),
commitConfigWriteWithPendingPluginInstalls:
mocks.commitConfigWriteWithPendingPluginInstalls,
}));
import { writeWizardConfigFile } from "./setup.shared.js";
describe("writeWizardConfigFile pending install ownership", () => {
beforeEach(() => {
vi.clearAllMocks();
mocks.commitConfigWriteWithPendingPluginInstalls.mockImplementation(
async (params: { nextConfig: OpenClawConfig }) => ({
config: withoutPluginInstallRecords(params.nextConfig),
installRecords: {},
movedInstallRecords: true,
persistedHash: "test-hash",
}),
);
});
it("rejects a normal write with pending records but no migration base", async () => {
const config: OpenClawConfig = {
plugins: { installs: { demo: { source: "npm", spec: "demo@1.0.0" } } },
};
await expect(
writeWizardConfigFile(config, { allowConfigSizeDrop: false }),
).rejects.toThrow("declare migration ownership");
expect(mocks.commitConfigWriteWithPendingPluginInstalls).not.toHaveBeenCalled();
});
it("migrates the baseline as source before the final wizard write", async () => {
const baseConfig: OpenClawConfig = {
plugins: { installs: { demo: { source: "npm", spec: "demo@1.0.0" } } },
};
await writeWizardConfigFile(baseConfig, {
allowConfigSizeDrop: false,
migrationBaseConfig: baseConfig,
});
expect(mocks.commitConfigWriteWithPendingPluginInstalls).toHaveBeenNthCalledWith(
1,
expect.objectContaining({
nextConfig: baseConfig,
sourceConfig: baseConfig,
writeOptions: { allowConfigSizeDrop: true },
}),
);
expect(mocks.commitConfigWriteWithPendingPluginInstalls).toHaveBeenCalledTimes(2);
});
it("commits fresh pending records after baseline migration is complete", async () => {
const config: OpenClawConfig = {
plugins: { installs: { fresh: { source: "npm", spec: "fresh@1.0.0" } } },
};
await writeWizardConfigFile(config, {
allowConfigSizeDrop: false,
migrationBaseConfig: undefined,
});
expect(mocks.commitConfigWriteWithPendingPluginInstalls).toHaveBeenCalledOnce();
expect(mocks.commitConfigWriteWithPendingPluginInstalls).toHaveBeenCalledWith(
expect.objectContaining({ nextConfig: config }),
);
});
});

View File

@@ -63,10 +63,18 @@ export async function writeWizardConfigFile(
let config = configInput;
const allowConfigSizeDrop = opts.allowConfigSizeDrop === true;
if (!allowConfigSizeDrop && hasPendingPluginInstallRecords(config)) {
// Explicit undefined means this writer already migrated its baseline; an omitted
// key cannot distinguish fresh pending records from stale authored metadata.
if (!Object.hasOwn(opts, "migrationBaseConfig")) {
throw new Error(
"Wizard config writes with pending plugin installs must declare migration ownership.",
);
}
const migrationBaseConfig = opts.migrationBaseConfig;
if (migrationBaseConfig && hasPendingPluginInstallRecords(migrationBaseConfig)) {
await commitConfigWriteWithPendingPluginInstalls({
nextConfig: migrationBaseConfig,
sourceConfig: migrationBaseConfig,
writeOptions: { allowConfigSizeDrop: true },
commit: async (nextConfig, writeOptions) => {
return await replaceConfigFile({