mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 06:10:44 +00:00
fix(update): isolate plugin sync failures
Disable and skip plugins that fail package-update plugin sync so broken plugin packages do not fail an otherwise successful OpenClaw update.
This commit is contained in:
@@ -52,6 +52,7 @@ Docs: https://docs.openclaw.ai
|
||||
|
||||
### Fixes
|
||||
|
||||
- CLI/update: disable and skip plugins that fail package-update plugin sync, so a broken npm/ClawHub/git/marketplace plugin cannot turn a successful OpenClaw package update into a failed update result. Thanks @vincentkoc.
|
||||
- Diagnostics: grant the internal diagnostics event bus to official installed diagnostics exporter plugins, so npm-installed `@openclaw/diagnostics-prometheus` can emit metrics without broadening the capability to arbitrary global plugins. Fixes #76628. Thanks @RayWoo.
|
||||
- Browser: enforce strict SSRF current-URL checks before existing-session screenshots, matching existing-session snapshot handling. Thanks @vincentkoc.
|
||||
- Active Memory: give timeout partial transcript recovery enough abort-settle headroom so temporary recall summaries are returned before cleanup. Thanks @vincentkoc.
|
||||
|
||||
@@ -222,6 +222,37 @@ describe("collectMissingPluginInstallPayloads", () => {
|
||||
await fs.rm(tmpDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("skips disabled tracked records when requested", async () => {
|
||||
const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-update-plugin-payload-"));
|
||||
const missingDir = path.join(tmpDir, "state", "npm", "node_modules", "@openclaw", "missing");
|
||||
try {
|
||||
await expect(
|
||||
collectMissingPluginInstallPayloads({
|
||||
env: { HOME: tmpDir } as NodeJS.ProcessEnv,
|
||||
skipDisabledPlugins: true,
|
||||
config: {
|
||||
plugins: {
|
||||
entries: {
|
||||
missing: {
|
||||
enabled: false,
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
records: {
|
||||
missing: {
|
||||
source: "npm",
|
||||
spec: "@openclaw/missing@beta",
|
||||
installPath: missingDir,
|
||||
},
|
||||
},
|
||||
}),
|
||||
).resolves.toEqual([]);
|
||||
} finally {
|
||||
await fs.rm(tmpDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("shouldUseLegacyProcessRestartAfterUpdate", () => {
|
||||
|
||||
@@ -51,6 +51,7 @@ import {
|
||||
resolveGlobalInstallSpec,
|
||||
} from "../../infra/update-global.js";
|
||||
import { runGatewayUpdate, type UpdateRunResult } from "../../infra/update-runner.js";
|
||||
import { normalizePluginsConfig, resolveEffectiveEnableState } from "../../plugins/config-state.js";
|
||||
import {
|
||||
loadInstalledPluginIndexInstallRecords,
|
||||
withoutPluginInstallRecords,
|
||||
@@ -184,9 +185,15 @@ async function pathExists(filePath: string): Promise<boolean> {
|
||||
|
||||
export async function collectMissingPluginInstallPayloads(params: {
|
||||
records: Record<string, PluginInstallRecord>;
|
||||
config?: OpenClawConfig;
|
||||
skipDisabledPlugins?: boolean;
|
||||
env?: NodeJS.ProcessEnv;
|
||||
}): Promise<MissingPluginInstallPayload[]> {
|
||||
const env = params.env ?? process.env;
|
||||
const normalizedPluginConfig =
|
||||
params.skipDisabledPlugins && params.config
|
||||
? normalizePluginsConfig(params.config.plugins)
|
||||
: undefined;
|
||||
const missing: MissingPluginInstallPayload[] = [];
|
||||
for (const [pluginId, record] of Object.entries(params.records).toSorted(([left], [right]) =>
|
||||
left.localeCompare(right),
|
||||
@@ -194,6 +201,17 @@ export async function collectMissingPluginInstallPayloads(params: {
|
||||
if (!isTrackedPackageInstallRecord(record)) {
|
||||
continue;
|
||||
}
|
||||
if (normalizedPluginConfig && params.config) {
|
||||
const enableState = resolveEffectiveEnableState({
|
||||
id: pluginId,
|
||||
origin: "global",
|
||||
config: normalizedPluginConfig,
|
||||
rootConfig: params.config,
|
||||
});
|
||||
if (!enableState.enabled) {
|
||||
continue;
|
||||
}
|
||||
}
|
||||
const rawInstallPath = normalizeOptionalString(record.installPath);
|
||||
if (!rawInstallPath) {
|
||||
missing.push({ pluginId, reason: "missing-install-path" });
|
||||
@@ -1091,7 +1109,11 @@ async function updatePluginsAfterCoreUpdate(params: {
|
||||
const repairMissingPayloads = async (
|
||||
records: Record<string, PluginInstallRecord>,
|
||||
): Promise<readonly string[]> => {
|
||||
const missing = await collectMissingPluginInstallPayloads({ records });
|
||||
const missing = await collectMissingPluginInstallPayloads({
|
||||
records,
|
||||
config: pluginConfig,
|
||||
skipDisabledPlugins: true,
|
||||
});
|
||||
if (missing.length === 0) {
|
||||
return [];
|
||||
}
|
||||
@@ -1110,6 +1132,8 @@ async function updatePluginsAfterCoreUpdate(params: {
|
||||
pluginIds: missingIds,
|
||||
timeoutMs: params.timeoutMs,
|
||||
updateChannel: params.channel,
|
||||
skipDisabledPlugins: true,
|
||||
disableOnFailure: true,
|
||||
logger: pluginLogger,
|
||||
onIntegrityDrift: onPluginIntegrityDrift,
|
||||
});
|
||||
@@ -1130,6 +1154,7 @@ async function updatePluginsAfterCoreUpdate(params: {
|
||||
updateChannel: params.channel,
|
||||
skipIds: new Set([...syncResult.summary.switchedToNpm, ...repairedMissingPayloadIds]),
|
||||
skipDisabledPlugins: true,
|
||||
disableOnFailure: true,
|
||||
logger: pluginLogger,
|
||||
onIntegrityDrift: onPluginIntegrityDrift,
|
||||
});
|
||||
@@ -1140,6 +1165,8 @@ async function updatePluginsAfterCoreUpdate(params: {
|
||||
|
||||
const remainingMissingPayloads = await collectMissingPluginInstallPayloads({
|
||||
records: pluginConfig.plugins?.installs ?? {},
|
||||
config: pluginConfig,
|
||||
skipDisabledPlugins: true,
|
||||
});
|
||||
pluginUpdateOutcomes.push(
|
||||
...remainingMissingPayloads.map(
|
||||
|
||||
@@ -1032,6 +1032,61 @@ describe("updateNpmInstalledPlugins", () => {
|
||||
]);
|
||||
});
|
||||
|
||||
it("disables enabled tracked plugin update failures when requested", async () => {
|
||||
const warn = vi.fn();
|
||||
installPluginFromNpmSpecMock.mockResolvedValue({
|
||||
ok: false,
|
||||
error: "registry timeout",
|
||||
});
|
||||
const config = {
|
||||
plugins: {
|
||||
entries: {
|
||||
demo: {
|
||||
enabled: true,
|
||||
config: { preserved: true },
|
||||
},
|
||||
},
|
||||
installs: {
|
||||
demo: {
|
||||
source: "npm" as const,
|
||||
spec: "@acme/demo",
|
||||
installPath: "/tmp/demo",
|
||||
},
|
||||
},
|
||||
},
|
||||
} satisfies OpenClawConfig;
|
||||
|
||||
const result = await updateNpmInstalledPlugins({
|
||||
config,
|
||||
skipDisabledPlugins: true,
|
||||
disableOnFailure: true,
|
||||
logger: { warn },
|
||||
});
|
||||
|
||||
expect(installPluginFromNpmSpecMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
spec: "@acme/demo",
|
||||
expectedPluginId: "demo",
|
||||
}),
|
||||
);
|
||||
const message =
|
||||
'Disabled "demo" after plugin update failure; OpenClaw will continue without it. Failed to update demo: registry timeout';
|
||||
expect(warn).toHaveBeenCalledWith(message);
|
||||
expect(result.changed).toBe(true);
|
||||
expect(result.config.plugins?.entries?.demo).toEqual({
|
||||
enabled: false,
|
||||
config: { preserved: true },
|
||||
});
|
||||
expect(result.config.plugins?.installs?.demo).toEqual(config.plugins.installs.demo);
|
||||
expect(result.outcomes).toEqual([
|
||||
{
|
||||
pluginId: "demo",
|
||||
status: "skipped",
|
||||
message,
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it("aborts exact pinned npm plugin updates on integrity drift by default", async () => {
|
||||
const warn = vi.fn();
|
||||
installPluginFromNpmSpecMock.mockImplementation(
|
||||
|
||||
@@ -747,12 +747,30 @@ function createPluginUpdateIntegrityDriftHandler(params: {
|
||||
};
|
||||
}
|
||||
|
||||
function disablePluginConfigEntry(config: OpenClawConfig, pluginId: string): OpenClawConfig {
|
||||
const existingEntry = config.plugins?.entries?.[pluginId];
|
||||
return {
|
||||
...config,
|
||||
plugins: {
|
||||
...config.plugins,
|
||||
entries: {
|
||||
...config.plugins?.entries,
|
||||
[pluginId]: {
|
||||
...existingEntry,
|
||||
enabled: false,
|
||||
},
|
||||
},
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
export async function updateNpmInstalledPlugins(params: {
|
||||
config: OpenClawConfig;
|
||||
logger?: PluginUpdateLogger;
|
||||
pluginIds?: string[];
|
||||
skipIds?: Set<string>;
|
||||
skipDisabledPlugins?: boolean;
|
||||
disableOnFailure?: boolean;
|
||||
timeoutMs?: number;
|
||||
dryRun?: boolean;
|
||||
updateChannel?: UpdateChannel;
|
||||
@@ -771,6 +789,28 @@ export async function updateNpmInstalledPlugins(params: {
|
||||
let next = params.config;
|
||||
let changed = false;
|
||||
|
||||
const recordFailure = (pluginId: string, message: string) => {
|
||||
if (params.disableOnFailure && !params.dryRun) {
|
||||
const disabledMessage =
|
||||
`Disabled "${pluginId}" after plugin update failure; OpenClaw will continue without it. ` +
|
||||
message;
|
||||
logger.warn?.(disabledMessage);
|
||||
next = disablePluginConfigEntry(next, pluginId);
|
||||
changed = true;
|
||||
outcomes.push({
|
||||
pluginId,
|
||||
status: "skipped",
|
||||
message: disabledMessage,
|
||||
});
|
||||
return;
|
||||
}
|
||||
outcomes.push({
|
||||
pluginId,
|
||||
status: "error",
|
||||
message,
|
||||
});
|
||||
};
|
||||
|
||||
for (const pluginId of targets) {
|
||||
if (params.skipIds?.has(pluginId)) {
|
||||
outcomes.push({
|
||||
@@ -928,11 +968,7 @@ export async function updateNpmInstalledPlugins(params: {
|
||||
record.installPath?.trim() || resolvePluginInstallDir(pluginId),
|
||||
);
|
||||
} catch (err) {
|
||||
outcomes.push({
|
||||
pluginId,
|
||||
status: "error",
|
||||
message: `Invalid install path for "${pluginId}": ${String(err)}`,
|
||||
});
|
||||
recordFailure(pluginId, `Invalid install path for "${pluginId}": ${String(err)}`);
|
||||
continue;
|
||||
}
|
||||
const currentVersion = await readInstalledPackageVersion(installPath);
|
||||
@@ -1037,11 +1073,7 @@ export async function updateNpmInstalledPlugins(params: {
|
||||
logger,
|
||||
});
|
||||
} catch (err) {
|
||||
outcomes.push({
|
||||
pluginId,
|
||||
status: "error",
|
||||
message: `Failed to check ${pluginId}: ${String(err)}`,
|
||||
});
|
||||
recordFailure(pluginId, `Failed to check ${pluginId}: ${String(err)}`);
|
||||
continue;
|
||||
}
|
||||
let usedNpmFallback = false;
|
||||
@@ -1096,43 +1128,41 @@ export async function updateNpmInstalledPlugins(params: {
|
||||
});
|
||||
}
|
||||
if (!probe.ok) {
|
||||
outcomes.push({
|
||||
recordFailure(
|
||||
pluginId,
|
||||
status: "error",
|
||||
message:
|
||||
record.source === "npm"
|
||||
? formatNpmInstallFailure({
|
||||
record.source === "npm"
|
||||
? formatNpmInstallFailure({
|
||||
pluginId,
|
||||
spec: npmUpdateFailureSpec({
|
||||
effectiveSpec,
|
||||
fallbackSpec: npmSpecs?.fallbackSpec,
|
||||
usedFallback: usedNpmFallback,
|
||||
}),
|
||||
phase: "check",
|
||||
result: probe,
|
||||
})
|
||||
: record.source === "clawhub"
|
||||
? formatClawHubInstallFailure({
|
||||
pluginId,
|
||||
spec: npmUpdateFailureSpec({
|
||||
effectiveSpec,
|
||||
fallbackSpec: npmSpecs?.fallbackSpec,
|
||||
usedFallback: usedNpmFallback,
|
||||
}),
|
||||
spec: effectiveSpec ?? `clawhub:${record.clawhubPackage!}`,
|
||||
phase: "check",
|
||||
result: probe,
|
||||
error: probe.error,
|
||||
})
|
||||
: record.source === "clawhub"
|
||||
? formatClawHubInstallFailure({
|
||||
: record.source === "git"
|
||||
? formatGitInstallFailure({
|
||||
pluginId,
|
||||
spec: effectiveSpec ?? `clawhub:${record.clawhubPackage!}`,
|
||||
spec: effectiveSpec!,
|
||||
phase: "check",
|
||||
error: probe.error,
|
||||
})
|
||||
: record.source === "git"
|
||||
? formatGitInstallFailure({
|
||||
pluginId,
|
||||
spec: effectiveSpec!,
|
||||
phase: "check",
|
||||
error: probe.error,
|
||||
})
|
||||
: formatMarketplaceInstallFailure({
|
||||
pluginId,
|
||||
marketplaceSource: record.marketplaceSource!,
|
||||
marketplacePlugin: record.marketplacePlugin!,
|
||||
phase: "check",
|
||||
error: probe.error,
|
||||
}),
|
||||
});
|
||||
: formatMarketplaceInstallFailure({
|
||||
pluginId,
|
||||
marketplaceSource: record.marketplaceSource!,
|
||||
marketplacePlugin: record.marketplacePlugin!,
|
||||
phase: "check",
|
||||
error: probe.error,
|
||||
}),
|
||||
);
|
||||
continue;
|
||||
}
|
||||
|
||||
@@ -1224,11 +1254,7 @@ export async function updateNpmInstalledPlugins(params: {
|
||||
logger,
|
||||
});
|
||||
} catch (err) {
|
||||
outcomes.push({
|
||||
pluginId,
|
||||
status: "error",
|
||||
message: `Failed to update ${pluginId}: ${String(err)}`,
|
||||
});
|
||||
recordFailure(pluginId, `Failed to update ${pluginId}: ${String(err)}`);
|
||||
continue;
|
||||
}
|
||||
let usedNpmFallback = false;
|
||||
@@ -1281,43 +1307,41 @@ export async function updateNpmInstalledPlugins(params: {
|
||||
});
|
||||
}
|
||||
if (!result.ok) {
|
||||
outcomes.push({
|
||||
recordFailure(
|
||||
pluginId,
|
||||
status: "error",
|
||||
message:
|
||||
record.source === "npm"
|
||||
? formatNpmInstallFailure({
|
||||
record.source === "npm"
|
||||
? formatNpmInstallFailure({
|
||||
pluginId,
|
||||
spec: npmUpdateFailureSpec({
|
||||
effectiveSpec,
|
||||
fallbackSpec: npmSpecs?.fallbackSpec,
|
||||
usedFallback: usedNpmFallback,
|
||||
}),
|
||||
phase: "update",
|
||||
result: result,
|
||||
})
|
||||
: record.source === "clawhub"
|
||||
? formatClawHubInstallFailure({
|
||||
pluginId,
|
||||
spec: npmUpdateFailureSpec({
|
||||
effectiveSpec,
|
||||
fallbackSpec: npmSpecs?.fallbackSpec,
|
||||
usedFallback: usedNpmFallback,
|
||||
}),
|
||||
spec: effectiveSpec ?? `clawhub:${record.clawhubPackage!}`,
|
||||
phase: "update",
|
||||
result: result,
|
||||
error: result.error,
|
||||
})
|
||||
: record.source === "clawhub"
|
||||
? formatClawHubInstallFailure({
|
||||
: record.source === "git"
|
||||
? formatGitInstallFailure({
|
||||
pluginId,
|
||||
spec: effectiveSpec ?? `clawhub:${record.clawhubPackage!}`,
|
||||
spec: effectiveSpec!,
|
||||
phase: "update",
|
||||
error: result.error,
|
||||
})
|
||||
: record.source === "git"
|
||||
? formatGitInstallFailure({
|
||||
pluginId,
|
||||
spec: effectiveSpec!,
|
||||
phase: "update",
|
||||
error: result.error,
|
||||
})
|
||||
: formatMarketplaceInstallFailure({
|
||||
pluginId,
|
||||
marketplaceSource: record.marketplaceSource!,
|
||||
marketplacePlugin: record.marketplacePlugin!,
|
||||
phase: "update",
|
||||
error: result.error,
|
||||
}),
|
||||
});
|
||||
: formatMarketplaceInstallFailure({
|
||||
pluginId,
|
||||
marketplaceSource: record.marketplaceSource!,
|
||||
marketplacePlugin: record.marketplacePlugin!,
|
||||
phase: "update",
|
||||
error: result.error,
|
||||
}),
|
||||
);
|
||||
continue;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user