fix(plugins): preserve install index state across failures (#119228)

* fix(plugins): preserve install index policy config

* fix(plugins): restore complete install index state

* fix(plugins): fence install index rollback

* fix(plugins): fence generic install index rollback

* fix(plugins): keep lifecycle lease context private

* test(cli): align plugin index rollback mocks

* test(plugins): enforce index rollback receipts

* fix(plugins): serialize install rollback with config commit

* test(plugins): keep legacy index writer mock private
This commit is contained in:
Vincent Koc
2026-08-05 01:04:17 +08:00
committed by GitHub
parent bfba83956f
commit ef11eae39b
17 changed files with 1314 additions and 242 deletions
+66 -36
View File
@@ -8,13 +8,16 @@ import { resolveRegistryUpdateChannel } from "../infra/update-channels.js";
import { CLAWHUB_INSTALL_ERROR_CODE } from "../plugins/clawhub-error-codes.js";
import { VERSION } from "../version.js";
import {
createTestInstalledPluginIndex,
loadConfig,
notifyGatewayPluginMetadataChanged,
readConfigFileSnapshotForWrite,
readPersistedInstalledPluginIndex,
refreshPluginRegistry,
registerPluginsCli,
replaceConfigFile,
resetPluginsCliTestState,
restorePersistedInstalledPluginIndexIfCurrent,
runPluginsCommand,
runtimeErrors,
runtimeLogs,
@@ -23,7 +26,7 @@ import {
updateNpmInstalledHookPacks,
updateNpmInstalledPlugins,
writeConfigFile,
writePersistedInstalledPluginIndexInstallRecords,
writePersistedInstalledPluginIndexInstallRecordsWithLease,
} from "./plugins-cli-test-helpers.js";
const ORIGINAL_OPENCLAW_NIX_MODE = process.env.OPENCLAW_NIX_MODE;
@@ -81,6 +84,17 @@ function expectRestartNoticeLogged() {
).toBe(true);
}
function expectInstallRecordsWrittenWithLease(records: unknown, config: unknown) {
expect(writePersistedInstalledPluginIndexInstallRecordsWithLease).toHaveBeenCalledWith(
records,
expect.objectContaining({
config,
filePath: expect.any(String),
lease: expect.anything(),
}),
);
}
function expectSingleCallParams(mockFn: ReturnType<typeof vi.fn>) {
expect(mockFn).toHaveBeenCalledTimes(1);
const params = mockFn.mock.calls[0]?.[0] as Record<string, unknown> | undefined;
@@ -218,7 +232,7 @@ async function expectSkippedClawHubPluginUpdate(params: {
await expect(runPluginsCommand(["plugins", "update", "demo"])).rejects.toThrow("__exit__:1");
expect(writePersistedInstalledPluginIndexInstallRecords).not.toHaveBeenCalled();
expect(writePersistedInstalledPluginIndexInstallRecordsWithLease).not.toHaveBeenCalled();
expect(runtimeLogs.at(-1)).toContain(params.expectedLog);
}
@@ -301,7 +315,7 @@ describe("plugins cli update", () => {
expect(acquireLease).not.toHaveBeenCalled();
expect(writeConfigFile).not.toHaveBeenCalled();
expect(replaceConfigFile).not.toHaveBeenCalled();
expect(writePersistedInstalledPluginIndexInstallRecords).not.toHaveBeenCalled();
expect(writePersistedInstalledPluginIndexInstallRecordsWithLease).not.toHaveBeenCalled();
expect(refreshPluginRegistry).not.toHaveBeenCalled();
expect(runtimeLogs).toContain("Would update alpha: 1.0.0 -> 1.1.0.");
} finally {
@@ -591,9 +605,7 @@ describe("plugins cli update", () => {
expect(runtimeErrors).toEqual([]);
expect(updateNpmInstalledPlugins).toHaveBeenCalledOnce();
expect(updateNpmInstalledHookPacks).not.toHaveBeenCalled();
expect(writePersistedInstalledPluginIndexInstallRecords).toHaveBeenCalledWith(
nextConfig.plugins?.installs,
);
expectInstallRecordsWrittenWithLease(nextConfig.plugins?.installs, cfg);
expect(writeConfigFile).not.toHaveBeenCalled();
});
@@ -632,7 +644,7 @@ describe("plugins cli update", () => {
expect(runtimeErrors).toEqual([]);
expect(updateNpmInstalledPlugins).toHaveBeenCalledOnce();
expect(writePersistedInstalledPluginIndexInstallRecords).toHaveBeenCalledWith(pluginRecords);
expectInstallRecordsWrittenWithLease(pluginRecords, cfg);
expect(writeConfigFile).not.toHaveBeenCalled();
});
@@ -671,7 +683,7 @@ describe("plugins cli update", () => {
await runPluginsCommand(["plugins", "update", "brave"]);
expect(runtimeErrors).toEqual([]);
expect(writePersistedInstalledPluginIndexInstallRecords).toHaveBeenCalledWith(nextRecords);
expectInstallRecordsWrittenWithLease(nextRecords, sourceCfg);
expect(writeConfigFile).not.toHaveBeenCalled();
expect(replaceConfigFile).not.toHaveBeenCalled();
expect(refreshPluginRegistry).toHaveBeenCalledWith({
@@ -722,7 +734,11 @@ describe("plugins cli update", () => {
await runPluginsCommand(["plugins", "update", "brave"]);
expect(writePersistedInstalledPluginIndexInstallRecords).toHaveBeenCalledWith(nextRecords);
expectInstallRecordsWrittenWithLease(nextRecords, {
plugins: {
load: { paths: [nextInstallPath, customPath] },
},
});
expect(replaceConfigFile).toHaveBeenCalledWith({
nextConfig: {
plugins: {
@@ -783,18 +799,24 @@ describe("plugins cli update", () => {
.mockResolvedValueOnce(initialSnapshot)
.mockResolvedValueOnce(changedSnapshot);
const { previousRecords, nextRecords } = primeBravePluginRecordUpdate(cfg);
const previousPersistedIndex = createTestInstalledPluginIndex({
policyHash: "previous-policy",
installRecords: previousRecords,
});
readPersistedInstalledPluginIndex.mockResolvedValue(previousPersistedIndex);
await expect(runPluginsCommand(["plugins", "update", "brave"])).rejects.toThrow(
"config changed since last load",
);
expect(writePersistedInstalledPluginIndexInstallRecords).toHaveBeenNthCalledWith(
1,
nextRecords,
);
expect(writePersistedInstalledPluginIndexInstallRecords).toHaveBeenNthCalledWith(
2,
previousRecords,
expectInstallRecordsWrittenWithLease(nextRecords, cfg);
expect(restorePersistedInstalledPluginIndexIfCurrent).toHaveBeenCalledWith(
previousPersistedIndex,
expect.any(Number),
expect.objectContaining({
filePath: expect.any(String),
lease: expect.anything(),
}),
);
expect(writeConfigFile).not.toHaveBeenCalled();
expect(replaceConfigFile).not.toHaveBeenCalled();
@@ -839,18 +861,24 @@ describe("plugins cli update", () => {
.mockResolvedValueOnce(initialSnapshot)
.mockResolvedValueOnce(changedSnapshot);
const { previousRecords, nextRecords } = primeBravePluginRecordUpdate(cfg);
const previousPersistedIndex = createTestInstalledPluginIndex({
policyHash: "previous-policy",
installRecords: previousRecords,
});
readPersistedInstalledPluginIndex.mockResolvedValue(previousPersistedIndex);
await expect(runPluginsCommand(["plugins", "update", "brave"])).rejects.toThrow(
"included config changed since last load",
);
expect(writePersistedInstalledPluginIndexInstallRecords).toHaveBeenNthCalledWith(
1,
nextRecords,
);
expect(writePersistedInstalledPluginIndexInstallRecords).toHaveBeenNthCalledWith(
2,
previousRecords,
expectInstallRecordsWrittenWithLease(nextRecords, cfg);
expect(restorePersistedInstalledPluginIndexIfCurrent).toHaveBeenCalledWith(
previousPersistedIndex,
expect.any(Number),
expect.objectContaining({
filePath: expect.any(String),
lease: expect.anything(),
}),
);
expect(writeConfigFile).not.toHaveBeenCalled();
expect(replaceConfigFile).not.toHaveBeenCalled();
@@ -888,18 +916,24 @@ describe("plugins cli update", () => {
.mockResolvedValueOnce(initialSnapshot)
.mockResolvedValueOnce(invalidSnapshot);
const { previousRecords, nextRecords } = primeBravePluginRecordUpdate(cfg);
const previousPersistedIndex = createTestInstalledPluginIndex({
policyHash: "previous-policy",
installRecords: previousRecords,
});
readPersistedInstalledPluginIndex.mockResolvedValue(previousPersistedIndex);
await expect(runPluginsCommand(["plugins", "update", "brave"])).rejects.toThrow(
"invalid config for plugin brave",
);
expect(writePersistedInstalledPluginIndexInstallRecords).toHaveBeenNthCalledWith(
1,
nextRecords,
);
expect(writePersistedInstalledPluginIndexInstallRecords).toHaveBeenNthCalledWith(
2,
previousRecords,
expectInstallRecordsWrittenWithLease(nextRecords, cfg);
expect(restorePersistedInstalledPluginIndexIfCurrent).toHaveBeenCalledWith(
previousPersistedIndex,
expect.any(Number),
expect.objectContaining({
filePath: expect.any(String),
lease: expect.anything(),
}),
);
expect(writeConfigFile).not.toHaveBeenCalled();
expect(replaceConfigFile).not.toHaveBeenCalled();
@@ -1420,9 +1454,7 @@ describe("plugins cli update", () => {
expect(updateParams.config).toEqual(runtimeConfig);
expect(updateParams.pluginIds).toEqual(["alpha"]);
expect(updateParams.dryRun).toBe(false);
expect(writePersistedInstalledPluginIndexInstallRecords).toHaveBeenCalledWith(
nextConfig.plugins?.installs,
);
expectInstallRecordsWrittenWithLease(nextConfig.plugins?.installs, {});
expect(updateNpmInstalledHookPacks).not.toHaveBeenCalled();
expect(writeConfigFile).toHaveBeenCalledWith({});
expect(replaceConfigFile).toHaveBeenCalledWith({
@@ -1489,9 +1521,7 @@ describe("plugins cli update", () => {
await expect(runPluginsCommand(["plugins", "update", "--all"])).rejects.toThrow("__exit__:1");
expect(writePersistedInstalledPluginIndexInstallRecords).toHaveBeenCalledWith(
nextConfig.plugins?.installs,
);
expectInstallRecordsWrittenWithLease(nextConfig.plugins?.installs, {});
expect(refreshPluginRegistry).toHaveBeenCalledWith({
config: {},
installRecords: nextConfig.plugins?.installs,