diff --git a/docs/cli/plugins.md b/docs/cli/plugins.md index a90748859d30..a58fdf61b47a 100644 --- a/docs/cli/plugins.md +++ b/docs/cli/plugins.md @@ -182,7 +182,9 @@ non-npm sources are not rewritten. If your `plugins` section is backed by a single-file `$include`, `plugins install/update/enable/disable/uninstall` write through to that included file and leave `openclaw.json` untouched. Root includes, include arrays, and includes with sibling overrides fail closed instead of flattening. See [Config includes](/gateway/configuration) for the supported shapes. - If config is invalid during install, `plugins install` normally fails closed and tells you to run `openclaw doctor --fix` first. During Gateway startup and hot reload, invalid plugin config fails closed like any other invalid config; `openclaw doctor --fix` can quarantine the invalid plugin entry. The only documented install-time exception is a narrow bundled-plugin recovery path for plugins that explicitly opt into `openclaw.install.allowInvalidConfigRecovery`. + If config is invalid before install, `plugins install` normally fails closed and tells you to run `openclaw doctor --fix` first. During Gateway startup and hot reload, invalid plugin config fails closed like any other invalid config; `openclaw doctor --fix` can quarantine the invalid plugin entry. The only pre-existing-config exception is a narrow bundled-plugin recovery path for plugins that explicitly opt into `openclaw.install.allowInvalidConfigRecovery`. + + When the existing host config is valid but the newly installed plugin's own config is absent, OpenClaw records the install disabled instead of writing an invalid enabled entry. Configure `plugins.entries..config`, then run `openclaw plugins enable `. If an existing plugin config entry is present but invalid, install fails without rewriting it. diff --git a/docs/plugins/manage-plugins.md b/docs/plugins/manage-plugins.md index 6509c3944646..7437665167a9 100644 --- a/docs/plugins/manage-plugins.md +++ b/docs/plugins/manage-plugins.md @@ -138,6 +138,11 @@ ClawHub, or hook-pack install, use `openclaw plugins update` instead. With `--link`, `--force` only confirms the source; the linked directory is not copied or overwritten. +If a newly installed plugin requires configuration that is not present yet, +OpenClaw records the install but leaves the plugin disabled. Configure +`plugins.entries..config`, then run `openclaw plugins enable `. If an +existing config entry is present but invalid, install fails without rewriting it. + ## Restart and inspect A running managed Gateway with config reload enabled restarts automatically diff --git a/src/auto-reply/reply/commands-plugins.ts b/src/auto-reply/reply/commands-plugins.ts index 59076431c6da..12dfc9bf9be4 100644 --- a/src/auto-reply/reply/commands-plugins.ts +++ b/src/auto-reply/reply/commands-plugins.ts @@ -11,6 +11,7 @@ import { type ConfigSnapshotForInstallPersist, } from "../../plugins/install-persistence.js"; import { loadInstalledPluginIndexInstallRecords } from "../../plugins/installed-plugin-index-records.js"; +import { withPluginLifecycleLease } from "../../plugins/plugin-lifecycle-lease.js"; import { refreshPluginRegistryAfterConfigMutation } from "../../plugins/registry-refresh.js"; import type { PluginRecord } from "../../plugins/registry.js"; import { @@ -274,134 +275,142 @@ export const handlePluginsCommand: CommandHandler = async (params, allowTextComm } if (pluginsCommand.action === "install") { - const loadedConfig = await loadPluginCommandConfig(); - if (!loadedConfig.ok) { + return await withPluginLifecycleLease({}, async () => { + const loadedConfig = await loadPluginCommandConfig(); + if (!loadedConfig.ok) { + return { + shouldContinue: false, + reply: { text: `⚠️ ${loadedConfig.error}` }, + }; + } + const installed = await installPluginFromPluginsCommand({ + raw: pluginsCommand.spec, + force: pluginsCommand.force, + config: loadedConfig.snapshot.config, + snapshot: loadedConfig.snapshot, + }); + if (!installed.ok) { + return { + shouldContinue: false, + reply: { text: `⚠️ ${installed.error}` }, + }; + } return { shouldContinue: false, - reply: { text: `⚠️ ${loadedConfig.error}` }, + reply: { + text: [ + `🔌 Installed plugin "${installed.pluginId}". Gateway restart will load the new plugin source.`, + ...(installed.warnings ?? []).map((warning) => `⚠️ ${warning}`), + ].join("\n"), + }, }; - } - const installed = await installPluginFromPluginsCommand({ - raw: pluginsCommand.spec, - force: pluginsCommand.force, - config: loadedConfig.snapshot.config, - snapshot: loadedConfig.snapshot, }); - if (!installed.ok) { + } + + const handleLoadedCommand = async () => { + const loaded = await loadPluginCommandState(params.workspaceDir, { + loadModules: pluginsCommand.action === "inspect", + }); + if (!loaded.ok) { return { shouldContinue: false, - reply: { text: `⚠️ ${installed.error}` }, + reply: { text: `⚠️ ${loaded.error}` }, }; } - return { - shouldContinue: false, - reply: { - text: [ - `🔌 Installed plugin "${installed.pluginId}". Gateway restart will load the new plugin source.`, - ...(installed.warnings ?? []).map((warning) => `⚠️ ${warning}`), - ].join("\n"), - }, - }; - } - const loaded = await loadPluginCommandState(params.workspaceDir, { - loadModules: pluginsCommand.action === "inspect", - }); - if (!loaded.ok) { - return { - shouldContinue: false, - reply: { text: `⚠️ ${loaded.error}` }, - }; - } - - if (pluginsCommand.action === "list") { - return { - shouldContinue: false, - reply: { text: formatPluginsList(loaded.report) }, - }; - } - - if (pluginsCommand.action === "inspect") { - const installRecords = await loadInstalledPluginIndexInstallRecords(); - if (!pluginsCommand.name) { + if (pluginsCommand.action === "list") { return { shouldContinue: false, reply: { text: formatPluginsList(loaded.report) }, }; } - if (normalizeOptionalLowercaseString(pluginsCommand.name) === "all") { + + if (pluginsCommand.action === "inspect") { + const installRecords = await loadInstalledPluginIndexInstallRecords(); + if (!pluginsCommand.name) { + return { + shouldContinue: false, + reply: { text: formatPluginsList(loaded.report) }, + }; + } + if (normalizeOptionalLowercaseString(pluginsCommand.name) === "all") { + return { + shouldContinue: false, + reply: { + text: renderJsonBlock( + "🔌 Plugins", + buildAllPluginInspectJson({ ...loaded, installRecords }), + ), + }, + }; + } + const payload = buildPluginInspectJson({ + id: pluginsCommand.name, + config: loaded.config, + installRecords, + report: loaded.report, + }); + if (!payload) { + return { + shouldContinue: false, + reply: { text: `🔌 No plugin named "${pluginsCommand.name}" found.` }, + }; + } return { shouldContinue: false, reply: { - text: renderJsonBlock( - "🔌 Plugins", - buildAllPluginInspectJson({ ...loaded, installRecords }), - ), + text: renderJsonBlock(`🔌 Plugin "${payload.inspect.plugin.id}"`, { + ...payload.inspect, + compatibilityWarnings: payload.compatibilityWarnings, + install: payload.install, + }), }, }; } - const payload = buildPluginInspectJson({ - id: pluginsCommand.name, - config: loaded.config, - installRecords, - report: loaded.report, - }); - if (!payload) { + + const plugin = findPlugin(loaded.report, pluginsCommand.name); + if (!plugin) { return { shouldContinue: false, reply: { text: `🔌 No plugin named "${pluginsCommand.name}" found.` }, }; } + + let registryWarning: string | undefined; + try { + const committedConfig = await setPluginEnabledFromCommand({ + pluginId: plugin.id, + enabled: pluginsCommand.action === "enable", + action: pluginsCommand.action, + }); + await refreshPluginRegistryAfterConfigMutation({ + config: committedConfig, + reason: "policy-changed", + logger: { + warn: (message) => { + registryWarning = message; + }, + }, + }); + } catch (error) { + if (error instanceof AutoReplyConfigMutationError) { + return { shouldContinue: false, reply: { text: `⚠️ ${error.message}` } }; + } + throw error; + } + return { shouldContinue: false, reply: { - text: renderJsonBlock(`🔌 Plugin "${payload.inspect.plugin.id}"`, { - ...payload.inspect, - compatibilityWarnings: payload.compatibilityWarnings, - install: payload.install, - }), + text: + `🔌 Plugin "${plugin.id}" ${pluginsCommand.action}d in ${loaded.path}. Gateway reload will apply it to new agent turns.` + + (registryWarning ? `\n${registryWarning}` : ""), }, }; - } - - const plugin = findPlugin(loaded.report, pluginsCommand.name); - if (!plugin) { - return { - shouldContinue: false, - reply: { text: `🔌 No plugin named "${pluginsCommand.name}" found.` }, - }; - } - - let committedConfig: OpenClawConfig; - try { - committedConfig = await setPluginEnabledFromCommand({ - pluginId: plugin.id, - enabled: pluginsCommand.action === "enable", - action: pluginsCommand.action, - }); - } catch (error) { - if (error instanceof AutoReplyConfigMutationError) { - return { shouldContinue: false, reply: { text: `⚠️ ${error.message}` } }; - } - throw error; - } - let registryWarning: string | undefined; - await refreshPluginRegistryAfterConfigMutation({ - config: committedConfig, - reason: "policy-changed", - logger: { - warn: (message) => { - registryWarning = message; - }, - }, - }); - - return { - shouldContinue: false, - reply: { - text: - `🔌 Plugin "${plugin.id}" ${pluginsCommand.action}d in ${loaded.path}. Gateway reload will apply it to new agent turns.` + - (registryWarning ? `\n${registryWarning}` : ""), - }, }; + + if (pluginsCommand.action === "enable" || pluginsCommand.action === "disable") { + return await withPluginLifecycleLease({}, handleLoadedCommand); + } + return await handleLoadedCommand(); }; diff --git a/src/claws/package-remove.ts b/src/claws/package-remove.ts index 2b388187d536..5006a23600d4 100644 --- a/src/claws/package-remove.ts +++ b/src/claws/package-remove.ts @@ -1,6 +1,7 @@ import { runPluginUninstallCommand } from "../cli/plugins-uninstall-command.js"; import { normalizeClawHubSha256Integrity } from "../infra/clawhub.js"; import { resolveInstalledClawHubPlugin } from "../plugins/plugin-install-preflight.js"; +import { withPluginLifecycleLease } from "../plugins/plugin-lifecycle-lease.js"; import { applyClawHubSkillUninstall, planClawHubSkillUninstall, @@ -360,9 +361,26 @@ export async function planClawPackageRemovals( return decisions; } +type ApplyClawPackageRemovalOptions = OpenClawStateDatabaseOptions & { + deps?: PackageRemovalDeps; +}; + export async function applyClawPackageRemovals( decisions: ClawPackageRemovalDecision[], - options: OpenClawStateDatabaseOptions & { deps?: PackageRemovalDeps } = {}, + options: ApplyClawPackageRemovalOptions = {}, +): Promise { + if (!decisions.some((decision) => decision.packageRef.kind === "plugin")) { + return await applyClawPackageRemovalsUnlocked(decisions, options); + } + return await withPluginLifecycleLease( + {}, + async () => await applyClawPackageRemovalsUnlocked(decisions, options), + ); +} + +async function applyClawPackageRemovalsUnlocked( + decisions: ClawPackageRemovalDecision[], + options: ApplyClawPackageRemovalOptions, ): Promise { const deps = options.deps ?? {}; const results: ClawPackageRemovalResult[] = []; diff --git a/src/claws/packages.ts b/src/claws/packages.ts index 03ad5081a2ea..bc05ace0d6c5 100644 --- a/src/claws/packages.ts +++ b/src/claws/packages.ts @@ -6,6 +6,7 @@ import { preflightPluginInstall, resolveInstalledClawHubPlugin, } from "../plugins/plugin-install-preflight.js"; +import { withPluginLifecycleLease } from "../plugins/plugin-lifecycle-lease.js"; import { defaultRuntime, type RuntimeEnv } from "../runtime.js"; import { installSkillFromClawHub, preflightSkillFromClawHub } from "../skills/lifecycle/clawhub.js"; import { @@ -207,14 +208,32 @@ export async function preflightClawPackage( }; } +type InstallClawPackagesOptions = OpenClawStateDatabaseOptions & { + deps?: PackageInstallerDeps; + runtime?: RuntimeEnv; + nowMs?: number; + onExternalMutation?: (pkg: ClawPackage) => void; +}; + export async function installClawPackages( plan: ClawAddPlan, - options: OpenClawStateDatabaseOptions & { - deps?: PackageInstallerDeps; - runtime?: RuntimeEnv; - nowMs?: number; - onExternalMutation?: (pkg: ClawPackage) => void; - } = {}, + options: InstallClawPackagesOptions = {}, +): Promise { + const includesPlugin = plan.actions.some( + (action) => action.kind === "package" && action.details?.kind === "plugin", + ); + if (!includesPlugin) { + return await installClawPackagesUnlocked(plan, options); + } + return await withPluginLifecycleLease( + {}, + async () => await installClawPackagesUnlocked(plan, options), + ); +} + +async function installClawPackagesUnlocked( + plan: ClawAddPlan, + options: InstallClawPackagesOptions, ): Promise { const deps = options.deps ?? {}; const installPlugin = deps.installPlugin ?? runPluginInstallCommand; diff --git a/src/cli/plugins-cli.install.test.ts b/src/cli/plugins-cli.install.test.ts index 51be4f0c696a..9fb05a217e15 100644 --- a/src/cli/plugins-cli.install.test.ts +++ b/src/cli/plugins-cli.install.test.ts @@ -1766,13 +1766,13 @@ describe("plugins cli install", () => { expect(clawHubInstallCall().spec).toBe("clawhub:demo"); }); - it("does not persist incomplete config entries for config-gated bundled installs", async () => { + it("preserves non-config policy for unconfigured bundled installs", async () => { const pluginId = "config-required-plugin"; const cfg = { plugins: { entries: { [pluginId]: { - config: {}, + hooks: { timeoutMs: 5_000 }, }, }, load: { @@ -1801,7 +1801,10 @@ describe("plugins cli install", () => { const writtenConfig = writeConfigFile.mock.calls[ writeConfigFile.mock.calls.length - 1 ]?.[0] as OpenClawConfig; - expect(writtenConfig.plugins?.entries?.[pluginId]).toBeUndefined(); + expect(writtenConfig.plugins?.entries?.[pluginId]).toEqual({ + enabled: false, + hooks: { timeoutMs: 5_000 }, + }); expect(writtenConfig.plugins?.load?.paths).toEqual(["/existing/plugin"]); const record = persistedInstallRecord(pluginId); expect(record.source).toBe("path"); @@ -1812,6 +1815,38 @@ describe("plugins cli install", () => { expect(runtimeLogsContain("requires configuration first")).toBe(true); }); + it("rejects invalid authored config for config-gated bundled installs", async () => { + const pluginId = "config-required-plugin"; + const cfg = { + plugins: { + entries: { + [pluginId]: { + config: {}, + hooks: { timeoutMs: 5_000 }, + }, + }, + }, + } as OpenClawConfig; + loadConfig.mockReturnValue(cfg); + findBundledPluginSourceMock.mockReturnValue({ + pluginId, + localPath: `/app/dist/extensions/${pluginId}`, + configSchema: { + type: "object", + required: ["token"], + properties: { token: { type: "string" } }, + }, + requiresConfig: true, + }); + + await expect(runPluginsCommand(["plugins", "install", pluginId])).rejects.toThrow( + "has invalid configured settings", + ); + + expect(writeConfigFile).not.toHaveBeenCalled(); + expect(enablePluginInConfig).not.toHaveBeenCalled(); + }); + it("enables config-gated bundled installs when provider-backed config is explicit", async () => { const pluginId = "config-required-plugin"; const cfg = { diff --git a/src/cli/plugins-cli.runtime.ts b/src/cli/plugins-cli.runtime.ts index 5db9d6f6ad89..df526ac87656 100644 --- a/src/cli/plugins-cli.runtime.ts +++ b/src/cli/plugins-cli.runtime.ts @@ -14,6 +14,7 @@ import { } from "../config/config.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; import { emitDiagnosticsTimelineEvent } from "../infra/diagnostics-timeline.js"; +import { withPluginLifecycleLease } from "../plugins/plugin-lifecycle-lease.js"; import { tracePluginLifecyclePhaseAsync } from "../plugins/plugin-lifecycle-trace.js"; import { defaultRuntime } from "../runtime.js"; import { shortenHomeInString } from "../utils.js"; @@ -188,6 +189,14 @@ function collectConfiguredRuntimePluginWarnings(params: { /** Enable a plugin in config and refresh the registry snapshot for the changed policy. */ export async function runPluginsEnableCommand(idInput: string): Promise { + assertConfigWriteAllowedInCurrentMode(); + return await withPluginLifecycleLease( + {}, + async () => await runPluginsEnableCommandUnlocked(idInput), + ); +} + +async function runPluginsEnableCommandUnlocked(idInput: string): Promise { let id = idInput; assertConfigWriteAllowedInCurrentMode(); @@ -235,6 +244,14 @@ export async function runPluginsEnableCommand(idInput: string): Promise { /** Disable a plugin in config and refresh the registry snapshot for the changed policy. */ export async function runPluginsDisableCommand(idInput: string): Promise { + assertConfigWriteAllowedInCurrentMode(); + return await withPluginLifecycleLease( + {}, + async () => await runPluginsDisableCommandUnlocked(idInput), + ); +} + +async function runPluginsDisableCommandUnlocked(idInput: string): Promise { let id = idInput; assertConfigWriteAllowedInCurrentMode(); diff --git a/src/cli/plugins-cli.uninstall.test.ts b/src/cli/plugins-cli.uninstall.test.ts index f924dc003223..74aa8341e35e 100644 --- a/src/cli/plugins-cli.uninstall.test.ts +++ b/src/cli/plugins-cli.uninstall.test.ts @@ -86,7 +86,8 @@ describe("plugins cli uninstall", () => { expect(writeConfigFile).not.toHaveBeenCalled(); }); - it("shows uninstall dry-run preview without mutating config", async () => { + it("shows uninstall dry-run preview without mutating config or acquiring write mode", async () => { + process.env.OPENCLAW_NIX_MODE = "1"; loadConfig.mockReturnValue({ plugins: { entries: { @@ -198,12 +199,12 @@ describe("plugins cli uninstall", () => { entries: {}, }, }, - writeOptions: { + writeOptions: expect.objectContaining({ allowConfigSizeDrop: true, auditOrigin: "plugin-install", afterWrite: { mode: "restart", reason: "plugin source changed" }, unsetPaths: [["plugins", "installs"]], - }, + }), }); expect(refreshPluginRegistry).toHaveBeenCalledWith({ config: { @@ -450,18 +451,80 @@ describe("plugins cli uninstall", () => { const configWriteOrder = writeConfigFile.mock.invocationCallOrder[0] ?? 0; const deleteOrder = applyPluginUninstallDirectoryRemoval.mock.invocationCallOrder[0] ?? Number.MAX_SAFE_INTEGER; + const finalConfigWriteOrder = + writeConfigFile.mock.invocationCallOrder[1] ?? Number.MAX_SAFE_INTEGER; const refreshOrder = refreshPluginRegistry.mock.invocationCallOrder[0] ?? Number.MAX_SAFE_INTEGER; - expect(writeConfigFile).toHaveBeenCalledTimes(1); + expect(writeConfigFile).toHaveBeenCalledTimes(2); expect(applyPluginUninstallDirectoryRemoval).toHaveBeenCalledTimes(1); expect(refreshPluginRegistry).toHaveBeenCalledTimes(1); expect(deleteOrder).toBeGreaterThan(configWriteOrder); - expect(refreshOrder).toBeGreaterThan(deleteOrder); + expect(finalConfigWriteOrder).toBeGreaterThan(deleteOrder); + expect(refreshOrder).toBeGreaterThan(finalConfigWriteOrder); expect(applyPluginUninstallDirectoryRemoval).toHaveBeenCalledWith({ target: ALPHA_INSTALL_PATH, }); }); + it("keeps the install tracked and disabled when directory removal fails", async () => { + const installPath = tempDirs.make("openclaw-plugin-uninstall-failure-"); + const installRecords = { + alpha: { + source: "npm", + spec: "alpha@1.0.0", + installPath, + }, + } as const; + const baseConfig = { + plugins: { + entries: { + alpha: { enabled: true }, + }, + installs: installRecords, + }, + } as OpenClawConfig; + loadConfig.mockReturnValue(baseConfig); + setInstalledPluginIndexInstallRecords(installRecords); + buildPluginSnapshotReport.mockReturnValue({ + plugins: [{ id: "alpha", name: "alpha" }], + diagnostics: [], + }); + planPluginUninstall.mockReturnValue({ + ok: true, + config: { plugins: { entries: {}, installs: {} } } as OpenClawConfig, + actions: { + entry: true, + install: true, + allowlist: false, + denylist: false, + loadPath: false, + memorySlot: false, + contextEngineSlot: false, + directory: false, + }, + directoryRemoval: { target: installPath }, + }); + applyPluginUninstallDirectoryRemoval.mockResolvedValue({ + directoryRemoved: false, + warnings: ["simulated removal failure"], + }); + + await expect(runPluginsCommand(["plugins", "uninstall", "alpha", "--force"])).rejects.toThrow( + "remains disabled and tracked", + ); + + expect(writeConfigFile).toHaveBeenCalledWith({ + plugins: { + entries: { + alpha: { enabled: false }, + }, + installs: installRecords, + }, + }); + expect(writePersistedInstalledPluginIndexInstallRecords).not.toHaveBeenCalled(); + expect(refreshPluginRegistry).not.toHaveBeenCalled(); + }); + it("cleans stale policy refs even when plugin is absent from the current registry", async () => { const baseConfig = { plugins: { diff --git a/src/cli/plugins-install-command.ts b/src/cli/plugins-install-command.ts index 3f0d96f047c5..a87ae1d90a95 100644 --- a/src/cli/plugins-install-command.ts +++ b/src/cli/plugins-install-command.ts @@ -43,6 +43,7 @@ import { resolveMarketplaceInstallShortcut, } from "../plugins/marketplace.js"; import { resolveCatalogOfficialExternalInstallPlan } from "../plugins/official-external-install-trust.js"; +import { withPluginLifecycleLease } from "../plugins/plugin-lifecycle-lease.js"; import { tracePluginLifecyclePhaseAsync } from "../plugins/plugin-lifecycle-trace.js"; import { defaultRuntime, type RuntimeEnv } from "../runtime.js"; import { markClawPackageIndependentlyOwned } from "../state/claw-package-adoption.js"; @@ -757,7 +758,7 @@ if (process.env.VITEST || process.env.NODE_ENV === "test") { ] = { loadConfigForInstall }; } -export async function runPluginInstallCommand(params: { +type RunPluginInstallCommandParams = { raw: string; opts: InstallSafetyOverrides & { acknowledgeClawHubRisk?: boolean; @@ -771,7 +772,17 @@ export async function runPluginInstallCommand(params: { invalidateRuntimeCache?: boolean; clawManaged?: boolean; runtime?: RuntimeEnv; -}) { +}; + +export async function runPluginInstallCommand(params: RunPluginInstallCommandParams) { + assertConfigWriteAllowedInCurrentMode(); + return await withPluginLifecycleLease( + {}, + async () => await runPluginInstallCommandUnlocked(params), + ); +} + +async function runPluginInstallCommandUnlocked(params: RunPluginInstallCommandParams) { assertConfigWriteAllowedInCurrentMode(); const runtime = params.runtime ?? defaultRuntime; diff --git a/src/cli/plugins-uninstall-command.ts b/src/cli/plugins-uninstall-command.ts index 94a7aab377d3..518d1d61a9b4 100644 --- a/src/cli/plugins-uninstall-command.ts +++ b/src/cli/plugins-uninstall-command.ts @@ -2,10 +2,15 @@ import os from "node:os"; import path from "node:path"; import { theme } from "../../packages/terminal-core/src/theme.js"; -import { assertConfigWriteAllowedInCurrentMode, readConfigFileSnapshot } from "../config/config.js"; +import { + assertConfigWriteAllowedInCurrentMode, + readConfigFileSnapshotForWrite, + replaceConfigFile, +} from "../config/config.js"; import { resolveStateDir } from "../config/paths.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; import { parseClawHubPluginSpec } from "../infra/clawhub.js"; +import { withPluginLifecycleLease } from "../plugins/plugin-lifecycle-lease.js"; import { tracePluginLifecyclePhase, tracePluginLifecyclePhaseAsync, @@ -37,8 +42,29 @@ export async function runPluginUninstallCommand( opts: PluginUninstallOptions = {}, runtime: RuntimeEnv = defaultRuntime, ): Promise { - // Uninstall mutates config/install records and optionally managed files, so guard write mode first. + if (opts.dryRun) { + return await runPluginUninstallCommandUnlocked(id, opts, runtime); + } assertConfigWriteAllowedInCurrentMode(); + if (!opts.force) { + return await runPluginUninstallCommandUnlocked(id, opts, runtime); + } + return await withPluginLifecycleLease( + {}, + async () => await runPluginUninstallCommandUnlocked(id, opts, runtime), + ); +} + +async function runPluginUninstallCommandUnlocked( + id: string, + opts: PluginUninstallOptions, + runtime: RuntimeEnv, + skipPreview = false, +): Promise { + // Dry-run only reads state; real uninstalls fail before any lifecycle lease or mutation. + if (!opts.dryRun) { + assertConfigWriteAllowedInCurrentMode(); + } const { loadInstalledPluginIndexInstallRecords, @@ -52,20 +78,25 @@ export async function runPluginUninstallCommand( formatUninstallActionLabels, formatUninstallSlotResetPreview, planPluginUninstall, + pluginUninstallTargetExists, + prepareConfigForPendingPluginDirectoryRemoval, resolveUninstallChannelConfigKeys, UNINSTALL_ACTION_LABELS, } = await import("../plugins/uninstall.js"); const { commitPluginInstallRecordsWithConfig } = await import("../plugins/install-record-commit.js"); + const { selectInstallMutationWriteOptions } = await import("../plugins/install-persistence.js"); const { refreshPluginRegistryAfterConfigMutation } = await import("../plugins/registry-refresh.js"); const { resolvePluginUninstallId } = await import("./plugins-uninstall-selection.js"); const { PromptInputClosedError, promptYesNo } = await import("./prompt.js"); - const snapshot = await tracePluginLifecyclePhaseAsync( + const prepared = await tracePluginLifecyclePhaseAsync( "config read", - () => readConfigFileSnapshot(), + () => readConfigFileSnapshotForWrite(), { command: "uninstall" }, ); + const { snapshot } = prepared; + const mutationWriteOptions = selectInstallMutationWriteOptions(prepared.writeOptions); const sourceConfig = (snapshot.sourceConfig ?? snapshot.config) as OpenClawConfig; const installRecords = await tracePluginLifecyclePhaseAsync( "install records load", @@ -91,24 +122,25 @@ export async function runPluginUninstallCommand( plugins: report.plugins, }); const channelIds = plugin?.status === "loaded" ? plugin.channelIds : undefined; - const plan = planPluginUninstall({ + const initialPlan = planPluginUninstall({ config: cfg, pluginId, channelIds, deleteFiles: !keepFiles, extensionsDir, }); - if (!plan.ok) { + if (!initialPlan.ok) { if (plugin) { runtime.error( `Plugin "${pluginId}" is not managed by plugins config/install records and cannot be uninstalled.`, ); } else { - runtime.error(plan.error); + runtime.error(initialPlan.error); } runtime.exit(1); return; } + let plan = initialPlan; const hasInstall = Object.hasOwn(cfg.plugins?.installs ?? {}, pluginId); const preview: string[] = []; @@ -145,22 +177,24 @@ export async function runPluginUninstallCommand( preview.push(`directory: ${shortenHomePath(plan.directoryRemoval.target)}`); } - const pluginName = plugin?.name || pluginId; - runtime.log( - `Plugin: ${theme.command(pluginName)}${pluginName !== pluginId ? theme.muted(` (${pluginId})`) : ""}`, - ); - runtime.log(`Will remove: ${preview.length > 0 ? preview.join(", ") : "(nothing)"}`); + if (!skipPreview) { + const pluginName = plugin?.name || pluginId; + runtime.log( + `Plugin: ${theme.command(pluginName)}${pluginName !== pluginId ? theme.muted(` (${pluginId})`) : ""}`, + ); + runtime.log(`Will remove: ${preview.length > 0 ? preview.join(", ") : "(nothing)"}`); - const { collectClawPluginUninstallWarnings } = - await import("../plugins/uninstall-claw-references.js"); - for (const warning of collectClawPluginUninstallWarnings({ - pluginId, - installRecord: cfg.plugins?.installs?.[pluginId], - })) { - runtime.log(theme.warn(warning)); + const { collectClawPluginUninstallWarnings } = + await import("../plugins/uninstall-claw-references.js"); + for (const warning of collectClawPluginUninstallWarnings({ + pluginId, + installRecord: cfg.plugins?.installs?.[pluginId], + })) { + runtime.log(theme.warn(warning)); + } } - const nextConfig = withoutPluginInstallRecords(plan.config); + let nextConfig = withoutPluginInstallRecords(plan.config); if (opts.dryRun) { runtime.log(theme.muted("Dry run, no changes made.")); @@ -185,9 +219,66 @@ export async function runPluginUninstallCommand( runtime.log("Cancelled."); return; } + return await withPluginLifecycleLease( + {}, + async () => + await runPluginUninstallCommandUnlocked(id, { ...opts, force: true }, runtime, true), + ); } const uninstall = async () => { + let finalBaseHash = snapshot.hash; + let finalWriteOptions = mutationWriteOptions; + let directoryResult = { directoryRemoved: false, warnings: [] as string[] }; + if (plan.directoryRemoval) { + const disabledConfig = prepareConfigForPendingPluginDirectoryRemoval(sourceConfig, pluginId); + const disabledCommit = await tracePluginLifecyclePhaseAsync( + "config disable", + () => + replaceConfigFile({ + nextConfig: disabledConfig, + ...(snapshot.hash !== undefined ? { baseHash: snapshot.hash } : {}), + writeOptions: { + ...mutationWriteOptions, + afterWrite: { mode: "auto" }, + }, + }), + { command: "uninstall" }, + ); + finalBaseHash = disabledCommit?.persistedHash ?? snapshot.hash; + directoryResult = await applyPluginUninstallDirectoryRemoval(plan.directoryRemoval); + for (const warning of directoryResult.warnings) { + runtime.log(theme.warn(warning)); + } + if (pluginUninstallTargetExists(plan.directoryRemoval.target)) { + throw new Error( + `Failed to remove plugin directory ${shortenHomePath(plan.directoryRemoval.target)}; the plugin remains disabled and tracked so uninstall can be retried.`, + ); + } + const refreshedPrepared = await tracePluginLifecyclePhaseAsync( + "config reread", + () => readConfigFileSnapshotForWrite(), + { command: "uninstall" }, + ); + const refreshedSnapshot = refreshedPrepared.snapshot; + const refreshedSourceConfig = (refreshedSnapshot.sourceConfig ?? + refreshedSnapshot.config) as OpenClawConfig; + const refreshedPlan = planPluginUninstall({ + config: withPluginInstallRecords(refreshedSourceConfig, installRecords), + pluginId, + channelIds, + deleteFiles: true, + extensionsDir, + }); + if (!refreshedPlan.ok) { + throw new Error(refreshedPlan.error); + } + plan = refreshedPlan; + nextConfig = withoutPluginInstallRecords(plan.config); + finalBaseHash = refreshedSnapshot.hash; + finalWriteOptions = selectInstallMutationWriteOptions(refreshedPrepared.writeOptions); + } + const nextInstallRecords = removePluginInstallRecordFromRecords(installRecords, pluginId); await tracePluginLifecyclePhaseAsync( "config mutation", @@ -196,18 +287,17 @@ export async function runPluginUninstallCommand( previousInstallRecords: installRecords, nextInstallRecords, nextConfig, - ...(snapshot.hash !== undefined ? { baseHash: snapshot.hash } : {}), + ...(finalBaseHash !== undefined ? { baseHash: finalBaseHash } : {}), writeOptions: { + ...finalWriteOptions, allowConfigSizeDrop: true, - auditOrigin: "plugin-install", afterWrite: { mode: "restart", reason: "plugin source changed" }, }, }), { command: "uninstall" }, ); - const directoryResult = await applyPluginUninstallDirectoryRemoval(plan.directoryRemoval); - for (const warning of directoryResult.warnings) { - runtime.log(theme.warn(warning)); + if (!plan.directoryRemoval) { + directoryResult = await applyPluginUninstallDirectoryRemoval(null); } await refreshPluginRegistryAfterConfigMutation({ config: nextConfig, diff --git a/src/cli/plugins-update-command.ts b/src/cli/plugins-update-command.ts index d514d2a9162e..aa381c67f179 100644 --- a/src/cli/plugins-update-command.ts +++ b/src/cli/plugins-update-command.ts @@ -35,6 +35,7 @@ import { withoutPluginInstallRecords, withPluginInstallRecords, } from "../plugins/installed-plugin-index-records.js"; +import { withPluginLifecycleLease } from "../plugins/plugin-lifecycle-lease.js"; import { refreshPluginRegistryAfterConfigMutation } from "../plugins/registry-refresh.js"; import { isPluginInstallRecordUpdateSource, @@ -170,8 +171,7 @@ async function assertRecordsOnlyUpdateConfigFresh(params: { } } -/** Run plugin/hook-pack updates, persist changed install records, and refresh runtime registry. */ -export async function runPluginUpdateCommand(params: { +type RunPluginUpdateCommandParams = { id?: string; opts: { all?: boolean; @@ -179,7 +179,21 @@ export async function runPluginUpdateCommand(params: { dryRun?: boolean; dangerouslyForceUnsafeInstall?: boolean; }; -}) { +}; + +/** Run plugin/hook-pack updates, persist changed install records, and refresh runtime registry. */ +export async function runPluginUpdateCommand(params: RunPluginUpdateCommandParams) { + assertConfigWriteAllowedInCurrentMode(); + if (params.opts.dryRun) { + return await runPluginUpdateCommandUnlocked(params); + } + return await withPluginLifecycleLease( + {}, + async () => await runPluginUpdateCommandUnlocked(params), + ); +} + +async function runPluginUpdateCommandUnlocked(params: RunPluginUpdateCommandParams) { assertConfigWriteAllowedInCurrentMode(); const sourceSnapshotPromise = readConfigFileSnapshotForWrite() diff --git a/src/cli/update-cli/update-command-post-core.ts b/src/cli/update-cli/update-command-post-core.ts index a62a0c4d9dfe..a7f63dd74654 100644 --- a/src/cli/update-cli/update-command-post-core.ts +++ b/src/cli/update-cli/update-command-post-core.ts @@ -44,6 +44,7 @@ import { loadInstalledPluginIndexInstallRecords, writePersistedInstalledPluginIndexInstallRecords, } from "../../plugins/installed-plugin-index-records.js"; +import { withPluginLifecycleLease } from "../../plugins/plugin-lifecycle-lease.js"; import { runExec } from "../../process/exec.js"; import { defaultRuntime } from "../../runtime.js"; import { VERSION } from "../../version.js"; @@ -87,7 +88,7 @@ export const POST_CORE_UPDATE_REQUESTED_CHANNEL_ENV = "OPENCLAW_UPDATE_POST_CORE export const POST_CORE_UPDATE_RESULT_PATH_ENV = "OPENCLAW_UPDATE_POST_CORE_RESULT_PATH"; export const POST_CORE_UPDATE_INSTALL_RECORDS_PATH_ENV = "OPENCLAW_UPDATE_POST_CORE_INSTALL_RECORDS_PATH"; -const POST_CORE_UPDATE_STARTED_AT_ENV = "OPENCLAW_UPDATE_POST_CORE_STARTED_AT_MS"; +export const POST_CORE_UPDATE_STARTED_AT_ENV = "OPENCLAW_UPDATE_POST_CORE_STARTED_AT_MS"; const POST_CORE_UPDATE_RESULT_POLL_MS = 100; export async function reportPreMutationUpdateFailure(params: { @@ -195,56 +196,58 @@ export async function updateFinalizeCommand(opts: UpdateFinalizeOptions): Promis }); } - const initialPluginUpdate = await withUpdateFinalizationEnv(async () => { - await createUpdateConfigSnapshot(); - await doctorCommand(defaultRuntime, { - nonInteractive: true, - repair: true, - yes: opts.yes === true, - }); - configSnapshot = await readConfigFileSnapshot({ skipPluginValidation: true }); - if (requestedChannel) { - configSnapshot = await persistRequestedUpdateChannel({ - configSnapshot, - requestedChannel, + const completedPluginUpdate = await withPluginLifecycleLease({}, async () => { + const initialPluginUpdate = await withUpdateFinalizationEnv(async () => { + await createUpdateConfigSnapshot(); + await doctorCommand(defaultRuntime, { + nonInteractive: true, + repair: true, + yes: opts.yes === true, + }); + configSnapshot = await readConfigFileSnapshot({ skipPluginValidation: true }); + if (requestedChannel) { + configSnapshot = await persistRequestedUpdateChannel({ + configSnapshot, + requestedChannel, + }); + } + const restoredConfig = restoreDroppedPreUpdateChannels(configSnapshot, preFinalizeConfig); + configSnapshot = restoredConfig.snapshot; + const postDoctorStoredChannel = configSnapshot.valid + ? normalizeUpdateChannel(configSnapshot.config.update?.channel) + : null; + const postDoctorChannel = + requestedChannel ?? + postDoctorStoredChannel ?? + storedChannel ?? + effectiveChannel ?? + DEFAULT_PACKAGE_CHANNEL; + const pluginInstallRecords = await loadInstalledPluginIndexInstallRecords(); + return await updatePluginsAfterCoreUpdate({ + root, + channel: postDoctorChannel, + configSnapshot, + configChanged: restoredConfig.changed, + restoredAuthoredChannels: restoredConfig.authoredChannels, + opts: { + json: opts.json, + timeout: opts.timeout, + yes: opts.yes, + restart: false, + acknowledgeClawHubRisk: opts.acknowledgeClawHubRisk, + }, + timeoutMs: timeoutMs ?? DEFAULT_UPDATE_STEP_TIMEOUT_MS, + pluginInstallRecords, }); - } - const restoredConfig = restoreDroppedPreUpdateChannels(configSnapshot, preFinalizeConfig); - configSnapshot = restoredConfig.snapshot; - const postDoctorStoredChannel = configSnapshot.valid - ? normalizeUpdateChannel(configSnapshot.config.update?.channel) - : null; - const postDoctorChannel = - requestedChannel ?? - postDoctorStoredChannel ?? - storedChannel ?? - effectiveChannel ?? - DEFAULT_PACKAGE_CHANNEL; - const pluginInstallRecords = await loadInstalledPluginIndexInstallRecords(); - return await updatePluginsAfterCoreUpdate({ - root, - channel: postDoctorChannel, - configSnapshot, - configChanged: restoredConfig.changed, - restoredAuthoredChannels: restoredConfig.authoredChannels, - opts: { - json: opts.json, - timeout: opts.timeout, - yes: opts.yes, - restart: false, - acknowledgeClawHubRisk: opts.acknowledgeClawHubRisk, - }, - timeoutMs: timeoutMs ?? DEFAULT_UPDATE_STEP_TIMEOUT_MS, - pluginInstallRecords, }); - }); - const completedPluginUpdate = await completePostCorePluginUpdate({ - root, - pluginUpdate: initialPluginUpdate, - freshDoctorRequired: initialPluginUpdate.changed, - yes: opts.yes === true, - json: opts.json === true, - timeoutMs: timeoutMs ?? DEFAULT_UPDATE_STEP_TIMEOUT_MS, + return await completePostCorePluginUpdate({ + root, + pluginUpdate: initialPluginUpdate, + freshDoctorRequired: initialPluginUpdate.changed, + yes: opts.yes === true, + json: opts.json === true, + timeoutMs: timeoutMs ?? DEFAULT_UPDATE_STEP_TIMEOUT_MS, + }); }); const pluginUpdate = completedPluginUpdate.pluginUpdate; configSnapshot = completedPluginUpdate.configSnapshot; diff --git a/src/cli/update-cli/update-command-post-update.ts b/src/cli/update-cli/update-command-post-update.ts index 436bf07e02e3..733a58d0e16c 100644 --- a/src/cli/update-cli/update-command-post-update.ts +++ b/src/cli/update-cli/update-command-post-update.ts @@ -11,6 +11,7 @@ import { } from "../../infra/update-control-plane-sentinel.js"; import type { UpdateRunResult } from "../../infra/update-runner.js"; import { loadInstalledPluginIndexInstallRecords } from "../../plugins/installed-plugin-index-records.js"; +import { withPluginLifecycleLease } from "../../plugins/plugin-lifecycle-lease.js"; import { defaultRuntime } from "../../runtime.js"; import { VERSION } from "../../version.js"; import { replaceCliName, resolveCliName } from "../cli-name.js"; @@ -162,16 +163,7 @@ export async function finishUpdate(params: { downgradeRisk: params.downgradeRisk, }); - let postUpdateConfigSnapshot = await readConfigFileSnapshot({ - skipPluginValidation: true, - suppressFutureVersionWarning: shouldResumePostCoreInFreshProcess, - }); - if (!shouldResumePostCoreInFreshProcess) { - postUpdateConfigSnapshot = await persistRequestedUpdateChannel({ - configSnapshot: postUpdateConfigSnapshot, - requestedChannel: params.requestedChannel, - }); - } + let postUpdateConfigSnapshot: Awaited> | undefined; if ( params.requestedChannel && params.configSnapshot.valid && @@ -224,74 +216,78 @@ export async function finishUpdate(params: { } if (!pluginsUpdatedInFreshProcess) { - if (shouldResumePostCoreInFreshProcess) { + await withPluginLifecycleLease({}, async () => { + postUpdateConfigSnapshot = await readConfigFileSnapshot({ + skipPluginValidation: true, + suppressFutureVersionWarning: shouldResumePostCoreInFreshProcess, + }); postUpdateConfigSnapshot = await persistRequestedUpdateChannel({ configSnapshot: postUpdateConfigSnapshot, requestedChannel: params.requestedChannel, }); - } - const restoredConfig = restoreDroppedPreUpdateChannels( - postUpdateConfigSnapshot, - params.configSnapshot.valid - ? { - sourceConfig: params.configSnapshot.sourceConfig, - authoredConfig: isRecord(params.configSnapshot.parsed) - ? (params.configSnapshot.parsed as OpenClawConfig) - : params.configSnapshot.sourceConfig, - } - : undefined, - ); - postUpdateConfigSnapshot = restoredConfig.snapshot; - // Current-process post-core convergence still reports the pre-update - // VERSION. During downgrades, pin compatibility checks to the installed - // target so incompatible newer plugins are disabled before restart. - const postUpdateInstalledVersion = await readPackageVersion(postUpdateRoot); - const versionComparison = - postUpdateInstalledVersion && VERSION - ? compareSemverStrings(VERSION, postUpdateInstalledVersion) - : null; - const compatibilityDowngradeTarget = - versionComparison != null && versionComparison > 0 ? postUpdateInstalledVersion : null; - const previousCompatibilityHostVersion = process.env.OPENCLAW_COMPATIBILITY_HOST_VERSION; - if (compatibilityDowngradeTarget) { - process.env.OPENCLAW_COMPATIBILITY_HOST_VERSION = compatibilityDowngradeTarget; - } - try { - const initialPluginUpdate = await updatePluginsAfterCoreUpdate({ - root: postUpdateRoot, - channel: params.channel, - configSnapshot: postUpdateConfigSnapshot, - configChanged: restoredConfig.changed, - restoredAuthoredChannels: restoredConfig.authoredChannels, - opts: params.opts, - timeoutMs: params.updateStepTimeoutMs, - pluginInstallRecords: params.preUpdatePluginInstallRecords, - }); - const completedPluginUpdate = await completePostCorePluginUpdate({ - root: postUpdateRoot, - pluginUpdate: initialPluginUpdate, - // A plugin-only update can replace its migration owner without replacing core. - // Downgrades and resume fallbacks can also leave an updated core on disk in this process. - freshDoctorRequired: - didCoreUpdateChangeInstall(params.result) || - initialPluginUpdate.sync.changed || - initialPluginUpdate.npm.changed, - yes: params.opts.yes === true, - json: params.opts.json === true, - timeoutMs: params.updateStepTimeoutMs, - ...(params.packageUpdateNodeRunner ? { nodeRunner: params.packageUpdateNodeRunner } : {}), - }); - postCorePluginUpdate = completedPluginUpdate.pluginUpdate; - postUpdateConfigSnapshot = completedPluginUpdate.configSnapshot; - } finally { + const restoredConfig = restoreDroppedPreUpdateChannels( + postUpdateConfigSnapshot, + params.configSnapshot.valid + ? { + sourceConfig: params.configSnapshot.sourceConfig, + authoredConfig: isRecord(params.configSnapshot.parsed) + ? (params.configSnapshot.parsed as OpenClawConfig) + : params.configSnapshot.sourceConfig, + } + : undefined, + ); + postUpdateConfigSnapshot = restoredConfig.snapshot; + // Current-process post-core convergence still reports the pre-update + // VERSION. During downgrades, pin compatibility checks to the installed + // target so incompatible newer plugins are disabled before restart. + const postUpdateInstalledVersion = await readPackageVersion(postUpdateRoot); + const versionComparison = + postUpdateInstalledVersion && VERSION + ? compareSemverStrings(VERSION, postUpdateInstalledVersion) + : null; + const compatibilityDowngradeTarget = + versionComparison != null && versionComparison > 0 ? postUpdateInstalledVersion : null; + const previousCompatibilityHostVersion = process.env.OPENCLAW_COMPATIBILITY_HOST_VERSION; if (compatibilityDowngradeTarget) { - if (previousCompatibilityHostVersion === undefined) { - delete process.env.OPENCLAW_COMPATIBILITY_HOST_VERSION; - } else { - process.env.OPENCLAW_COMPATIBILITY_HOST_VERSION = previousCompatibilityHostVersion; + process.env.OPENCLAW_COMPATIBILITY_HOST_VERSION = compatibilityDowngradeTarget; + } + try { + const initialPluginUpdate = await updatePluginsAfterCoreUpdate({ + root: postUpdateRoot, + channel: params.channel, + configSnapshot: postUpdateConfigSnapshot, + configChanged: restoredConfig.changed, + restoredAuthoredChannels: restoredConfig.authoredChannels, + opts: params.opts, + timeoutMs: params.updateStepTimeoutMs, + pluginInstallRecords: params.preUpdatePluginInstallRecords, + }); + const completedPluginUpdate = await completePostCorePluginUpdate({ + root: postUpdateRoot, + pluginUpdate: initialPluginUpdate, + // A plugin-only update can replace its migration owner without replacing core. + // Downgrades and resume fallbacks can also leave an updated core on disk in this process. + freshDoctorRequired: + didCoreUpdateChangeInstall(params.result) || + initialPluginUpdate.sync.changed || + initialPluginUpdate.npm.changed, + yes: params.opts.yes === true, + json: params.opts.json === true, + timeoutMs: params.updateStepTimeoutMs, + ...(params.packageUpdateNodeRunner ? { nodeRunner: params.packageUpdateNodeRunner } : {}), + }); + postCorePluginUpdate = completedPluginUpdate.pluginUpdate; + postUpdateConfigSnapshot = completedPluginUpdate.configSnapshot; + } finally { + if (compatibilityDowngradeTarget) { + if (previousCompatibilityHostVersion === undefined) { + delete process.env.OPENCLAW_COMPATIBILITY_HOST_VERSION; + } else { + process.env.OPENCLAW_COMPATIBILITY_HOST_VERSION = previousCompatibilityHostVersion; + } } } - } + }); } const resultWithPostUpdate: UpdateRunResult = postCorePluginUpdate @@ -332,12 +328,18 @@ export async function finishUpdate(params: { return; } + const restartConfigSnapshot = + postUpdateConfigSnapshot ?? + (await readConfigFileSnapshot({ + skipPluginValidation: true, + suppressFutureVersionWarning: shouldResumePostCoreInFreshProcess, + })); let restartScriptPath: string | null = null; let refreshGatewayServiceEnv = false; let gatewayServiceEnv: NodeJS.ProcessEnv | undefined; let skipLegacyServiceRestart = false; let gatewayPort = resolveUpdatedGatewayRestartPort({ - config: postUpdateConfigSnapshot.valid ? postUpdateConfigSnapshot.config : undefined, + config: restartConfigSnapshot.valid ? restartConfigSnapshot.config : undefined, processEnv: process.env, }); if (params.shouldRestart) { @@ -382,7 +384,7 @@ export async function finishUpdate(params: { ) { gatewayServiceEnv = serviceState.env; gatewayPort = resolveUpdatedGatewayRestartPort({ - config: postUpdateConfigSnapshot.valid ? postUpdateConfigSnapshot.config : undefined, + config: restartConfigSnapshot.valid ? restartConfigSnapshot.config : undefined, processEnv: process.env, serviceEnv: gatewayServiceEnv, }); diff --git a/src/cli/update-cli/update-command-resume.ts b/src/cli/update-cli/update-command-resume.ts index 9b306158affd..f91cb0055876 100644 --- a/src/cli/update-cli/update-command-resume.ts +++ b/src/cli/update-cli/update-command-resume.ts @@ -3,6 +3,8 @@ import { normalizeUpdateChannel } from "../../infra/update-channels.js"; import { POST_CORE_UPDATE_SOURCE_CONFIG_PATH_ENV } from "../../infra/update-post-core-context.js"; import type { UpdateRunResult } from "../../infra/update-runner.js"; import { loadInstalledPluginIndexInstallRecords } from "../../plugins/installed-plugin-index-records.js"; +import { readPersistedInstalledPluginIndex } from "../../plugins/installed-plugin-index-store.js"; +import { withPluginLifecycleLease } from "../../plugins/plugin-lifecycle-lease.js"; import { defaultRuntime } from "../../runtime.js"; import { VERSION } from "../../version.js"; import { readPackageVersion, type UpdateCommandOptions } from "./shared.js"; @@ -17,17 +19,24 @@ import { POST_CORE_UPDATE_INSTALL_RECORDS_PATH_ENV, POST_CORE_UPDATE_REQUESTED_CHANNEL_ENV, POST_CORE_UPDATE_RESULT_PATH_ENV, + POST_CORE_UPDATE_STARTED_AT_ENV, readPostCorePluginInstallRecordsFile, resolvePostCoreUpdateStartedAtMs, writePostCorePluginUpdateResultFile, } from "./update-command-post-core.js"; -export async function resumePostCoreUpdate(params: { +type ResumePostCoreUpdateParams = { root: string; channel: string | undefined; opts: UpdateCommandOptions; timeoutMs: number; -}): Promise { +}; + +export async function resumePostCoreUpdate(params: ResumePostCoreUpdateParams): Promise { + return await withPluginLifecycleLease({}, async () => await resumePostCoreUpdateUnlocked(params)); +} + +async function resumePostCoreUpdateUnlocked(params: ResumePostCoreUpdateParams): Promise { if ( params.channel !== "stable" && params.channel !== "extended-stable" && @@ -56,10 +65,11 @@ export async function resumePostCoreUpdate(params: { skipPluginValidation: true, suppressFutureVersionWarning: true, }); + const updateStartedAtMs = await resolvePostCoreUpdateStartedAtMs(process.env); const preUpdateSourceConfig = await readPostCorePreUpdateSourceConfig({ sourceConfigPath: process.env[POST_CORE_UPDATE_SOURCE_CONFIG_PATH_ENV], currentSnapshot: configSnapshot, - updateStartedAtMs: await resolvePostCoreUpdateStartedAtMs(process.env), + updateStartedAtMs, }); configSnapshot = await persistRequestedUpdateChannel({ configSnapshot, @@ -69,12 +79,21 @@ export async function resumePostCoreUpdate(params: { const parentPluginInstallRecords = await readPostCorePluginInstallRecordsFile( process.env[POST_CORE_UPDATE_INSTALL_RECORDS_PATH_ENV], ); - // The updated doctor may have repaired plugin installs before this fresh process resumed. + // The updated doctor may have repaired or removed plugin installs before this process resumed. const currentPluginInstallRecords = await loadInstalledPluginIndexInstallRecords(); - const pluginInstallRecords = - Object.keys(currentPluginInstallRecords).length > 0 - ? currentPluginInstallRecords - : parentPluginInstallRecords; + const persistedPluginIndex = await readPersistedInstalledPluginIndex(); + const hasForwardedUpdateStart = Boolean(process.env[POST_CORE_UPDATE_STARTED_AT_ENV]?.trim()); + const currentIndexIsAuthoritative = + Object.keys(currentPluginInstallRecords).length > 0 || + Boolean( + persistedPluginIndex && + hasForwardedUpdateStart && + updateStartedAtMs !== undefined && + persistedPluginIndex.generatedAtMs >= updateStartedAtMs, + ); + const pluginInstallRecords = currentIndexIsAuthoritative + ? currentPluginInstallRecords + : parentPluginInstallRecords; const initialPluginUpdate = await updatePluginsAfterCoreUpdate({ root: params.root, diff --git a/src/plugins/bundled-install.ts b/src/plugins/bundled-install.ts index 2fe2c4718520..15781aa5eaad 100644 --- a/src/plugins/bundled-install.ts +++ b/src/plugins/bundled-install.ts @@ -8,42 +8,54 @@ import { } from "./install-persistence.js"; import { validateJsonSchemaValue } from "./schema-validator.js"; -function hasValidBundledPluginConfig(params: { +type BundledPluginConfigEnablement = + | { mode: "ready" } + | { mode: "missing" } + | { mode: "invalid"; error: string }; + +function resolveBundledPluginConfigEnablement(params: { bundledSource: BundledPluginSource; existingEntry: unknown; -}): boolean { +}): BundledPluginConfigEnablement { if (!params.bundledSource.requiresConfig) { - return true; + return { mode: "ready" }; } - if (!isRecord(params.existingEntry)) { - return false; - } - const config = params.existingEntry.config; - if (!isRecord(config)) { - return false; + const entry = isRecord(params.existingEntry) ? params.existingEntry : undefined; + if (!entry || !Object.hasOwn(entry, "config")) { + return { mode: "missing" }; } + const config = entry.config; if (!params.bundledSource.configSchema) { - return Object.keys(config).length > 0; + return isRecord(config) && Object.keys(config).length > 0 + ? { mode: "ready" } + : { mode: "invalid", error: "config must be a non-empty object" }; } - return validateJsonSchemaValue({ + const result = validateJsonSchemaValue({ schema: params.bundledSource.configSchema, cacheKey: `bundled-install:${params.bundledSource.pluginId}`, value: config, applyDefaults: true, - }).ok; + }); + return result.ok + ? { mode: "ready" } + : { mode: "invalid", error: result.errors[0]?.text ?? "invalid plugin config" }; } function prepareConfigForDisabledBundledInstall( config: OpenClawConfig, pluginId: string, ): OpenClawConfig { - const entries = config.plugins?.entries ?? {}; - const { [pluginId]: _removedEntry, ...nextEntries } = entries; + const entry = config.plugins?.entries?.[pluginId]; + const policy = isRecord(entry) ? { ...entry } : {}; + delete policy.config; return { ...config, plugins: { ...config.plugins, - entries: nextEntries, + entries: { + ...config.plugins?.entries, + [pluginId]: { ...policy, enabled: false }, + }, }, }; } @@ -58,10 +70,16 @@ export async function installBundledPluginSource(params: { }): Promise<{ pluginId: string; warnings: string[] }> { // Bundled plugins with required config are recorded but not enabled until config validates. const existingEntry = params.snapshot.config.plugins?.entries?.[params.bundledSource.pluginId]; - const shouldEnable = hasValidBundledPluginConfig({ + const configEnablement = resolveBundledPluginConfigEnablement({ bundledSource: params.bundledSource, existingEntry, }); + if (configEnablement.mode === "invalid") { + throw new Error( + `Plugin "${params.bundledSource.pluginId}" has invalid configured settings: ${configEnablement.error}. Fix plugins.entries.${params.bundledSource.pluginId}.config, then rerun the install.`, + ); + } + const shouldEnable = configEnablement.mode === "ready"; const configBase = shouldEnable ? params.snapshot.config : prepareConfigForDisabledBundledInstall(params.snapshot.config, params.bundledSource.pluginId); diff --git a/src/plugins/install-persistence.test.ts b/src/plugins/install-persistence.test.ts index d57f9508ccfd..fe3f34075ddd 100644 --- a/src/plugins/install-persistence.test.ts +++ b/src/plugins/install-persistence.test.ts @@ -698,7 +698,7 @@ describe("persistPluginInstall", () => { onlyPluginIds: ["legacy-memory"], }); expect( - requireMockCallArg(loadPluginManifestRegistry, "loadPluginManifestRegistry").config, + requireMockCallArg(loadPluginManifestRegistry, "loadPluginManifestRegistry", 1).config, ).toBe(enabledConfig); expect(next.plugins?.entries?.["legacy-memory-a"]?.enabled).toBe(true); expect(next.plugins?.slots?.memory).toBe("legacy-memory"); @@ -767,7 +767,7 @@ describe("persistPluginInstall", () => { expect(buildPluginDiagnosticsReport).not.toHaveBeenCalled(); expect( - requireMockCallArg(loadPluginManifestRegistry, "loadPluginManifestRegistry").config, + requireMockCallArg(loadPluginManifestRegistry, "loadPluginManifestRegistry", 1).config, ).toBe(enabledConfig); expect(next.plugins?.entries?.["legacy-memory-a"]?.enabled).toBe(true); expect(next.plugins?.slots?.memory).toBe("memory-b"); @@ -822,11 +822,125 @@ describe("persistPluginInstall", () => { onlyPluginIds: ["plain"], }); expect( - requireMockCallArg(loadPluginManifestRegistry, "loadPluginManifestRegistry").config, + requireMockCallArg(loadPluginManifestRegistry, "loadPluginManifestRegistry", 1).config, ).toBe(enabledConfig); expect(next).toEqual(enabledConfig); }); + it("installs a plugin disabled when its required configuration is missing", async () => { + const { persistPluginInstall } = await import("./install-persistence.js"); + const baseConfig = { + plugins: { + allow: ["memory-core"], + deny: ["needs-config"], + entries: { + "needs-config": { hooks: { timeoutMs: 5_000 } }, + }, + }, + } as OpenClawConfig; + loadPluginManifestRegistry.mockReturnValue({ + plugins: [ + { + id: "needs-config", + manifestPath: "/tmp/needs-config/openclaw.plugin.json", + configSchema: { + type: "object", + required: ["token"], + properties: { token: { type: "string" } }, + }, + }, + ], + diagnostics: [], + }); + + const next = await persistPluginInstall({ + snapshot: { + config: baseConfig, + baseHash: "config-1", + writeOptions: installWriteOptions, + }, + pluginId: "needs-config", + install: { + source: "npm", + spec: "needs-config@1.0.0", + installPath: "/tmp/needs-config", + }, + }); + + expect(next).toEqual({ + plugins: { + allow: ["memory-core", "needs-config"], + entries: { + "needs-config": { enabled: false, hooks: { timeoutMs: 5_000 } }, + }, + }, + }); + expect(enablePluginInConfig).not.toHaveBeenCalled(); + expect(applyExclusiveSlotSelection).not.toHaveBeenCalled(); + expectRuntimeLogIncludes( + 'Installed plugin "needs-config" without enabling it because it requires configuration first.', + ); + const persistedRecords = requireMockCallArg( + writePersistedInstalledPluginIndexInstallRecords, + "writePersistedInstalledPluginIndexInstallRecords", + ); + expect(persistedRecords["needs-config"]).toMatchObject({ + source: "npm", + spec: "needs-config@1.0.0", + installPath: "/tmp/needs-config", + }); + }); + + it("rejects invalid authored plugin config even for a disabled install", async () => { + const { persistPluginInstall } = await import("./install-persistence.js"); + const baseConfig = { + plugins: { + entries: { + "needs-config": { + enabled: false, + config: null as never, + hooks: { timeoutMs: 5_000 }, + }, + }, + }, + } as OpenClawConfig; + loadPluginManifestRegistry.mockReturnValue({ + plugins: [ + { + id: "needs-config", + manifestPath: "/tmp/needs-config/openclaw.plugin.json", + configSchema: { + type: "object", + required: ["token"], + properties: { token: { type: "string" } }, + }, + }, + ], + diagnostics: [], + }); + + await expect( + persistPluginInstall({ + snapshot: { + config: baseConfig, + baseHash: "config-1", + writeOptions: installWriteOptions, + }, + pluginId: "needs-config", + enable: false, + install: { + source: "npm", + spec: "needs-config@1.0.0", + installPath: "/tmp/needs-config", + }, + }), + ).rejects.toThrow("has invalid configured settings"); + + expect(enablePluginInConfig).not.toHaveBeenCalled(); + expect(writePersistedInstalledPluginIndexInstallRecords).not.toHaveBeenCalled(); + expect(writeConfigFile).not.toHaveBeenCalled(); + }); + it("can persist an install record without enabling a plugin that needs config first", async () => { const { persistPluginInstall } = await import("./install-persistence.js"); const baseConfig = { diff --git a/src/plugins/install-persistence.ts b/src/plugins/install-persistence.ts index 6322da5585e9..6aa12f55c4f7 100644 --- a/src/plugins/install-persistence.ts +++ b/src/plugins/install-persistence.ts @@ -23,8 +23,10 @@ import { withoutPluginInstallRecords, } from "./installed-plugin-index-records.js"; import type { PluginInstallUpdate } from "./installs.js"; +import { loadPluginManifestRegistry } from "./manifest-registry.js"; import { tracePluginLifecyclePhaseAsync } from "./plugin-lifecycle-trace.js"; import { refreshPluginRegistryAfterConfigMutation } from "./registry-refresh.js"; +import { validateJsonSchemaValue } from "./schema-validator.js"; import { applySlotSelectionForPlugin } from "./slot-selection.js"; import { buildPluginSnapshotReport } from "./status.js"; import { @@ -427,6 +429,56 @@ function resolveReplacedManagedInstallRemoval(params: { return plan.directoryRemoval; } +function prepareConfigForDisabledInstall(config: OpenClawConfig, pluginId: string): OpenClawConfig { + const entry = config.plugins?.entries?.[pluginId]; + const policy = isRecord(entry) ? { ...entry } : {}; + delete policy.config; + return { + ...config, + plugins: { + ...config.plugins, + entries: { + ...config.plugins?.entries, + [pluginId]: { ...policy, enabled: false }, + }, + }, + }; +} + +type PluginConfigEnablement = + | { mode: "ready" } + | { mode: "missing" } + | { mode: "invalid"; error: string }; + +function resolvePluginConfigEnablement(params: { + config: OpenClawConfig; + pluginId: string; + installRecords: Record; +}): PluginConfigEnablement { + const manifest = loadPluginManifestRegistry({ + config: params.config, + installRecords: params.installRecords, + }).plugins.find((plugin) => plugin.id === params.pluginId); + if (!manifest?.configSchema) { + return { mode: "ready" }; + } + const entry = params.config.plugins?.entries?.[params.pluginId]; + const hasConfig = isRecord(entry) && Object.hasOwn(entry, "config"); + const result = validateJsonSchemaValue({ + schema: manifest.configSchema, + cacheKey: manifest.schemaCacheKey ?? manifest.manifestPath, + value: hasConfig ? entry.config : {}, + applyDefaults: true, + }); + if (result.ok) { + return { mode: "ready" }; + } + if (!hasConfig) { + return { mode: "missing" }; + } + return { mode: "invalid", error: result.errors[0]?.text ?? "invalid plugin config" }; +} + export async function persistPluginInstall(params: { snapshot: ConfigSnapshotForInstallPersist; pluginId: string; @@ -438,19 +490,6 @@ export async function persistPluginInstall(params: { runtime?: RuntimeEnv; }): Promise { const runtime = params.runtime ?? defaultRuntime; - const installConfig = - params.enable === false - ? params.snapshot.config - : removeInstalledPluginFromDenylist( - addInstalledPluginToAllowlist(params.snapshot.config, params.pluginId), - params.pluginId, - ); - let next = - params.enable === false - ? installConfig - : enablePluginInConfig(installConfig, params.pluginId, { - updateChannelConfig: false, - }).config; const installRecords = await tracePluginLifecyclePhaseAsync( "install records load", () => loadInstalledPluginIndexInstallRecords(), @@ -466,14 +505,40 @@ export async function persistPluginInstall(params: { pluginId: params.pluginId, ...params.install, }); - const slotResult = + const configEnablement = resolvePluginConfigEnablement({ + config: params.snapshot.config, + pluginId: params.pluginId, + installRecords: nextInstallRecords, + }); + if (configEnablement.mode === "invalid") { + throw new Error( + `Plugin "${params.pluginId}" has invalid configured settings: ${configEnablement.error}. Fix plugins.entries.${params.pluginId}.config, then rerun the install.`, + ); + } + const shouldEnable = params.enable !== false && configEnablement.mode === "ready"; + const configBase = + params.enable === false || configEnablement.mode === "ready" + ? params.snapshot.config + : prepareConfigForDisabledInstall(params.snapshot.config, params.pluginId); + const installConfig = params.enable === false - ? { config: next, warnings: [] } - : await tracePluginLifecyclePhaseAsync( - "slot selection", - async () => applySlotSelectionForPlugin(next, params.pluginId), - { command: "install", pluginId: params.pluginId }, + ? configBase + : removeInstalledPluginFromDenylist( + addInstalledPluginToAllowlist(configBase, params.pluginId), + params.pluginId, ); + let next = shouldEnable + ? enablePluginInConfig(installConfig, params.pluginId, { + updateChannelConfig: false, + }).config + : installConfig; + const slotResult = shouldEnable + ? await tracePluginLifecyclePhaseAsync( + "slot selection", + async () => applySlotSelectionForPlugin(next, params.pluginId), + { command: "install", pluginId: params.pluginId }, + ) + : { config: next, warnings: [] }; next = withoutPluginInstallRecords(slotResult.config); await tracePluginLifecyclePhaseAsync( "config mutation", @@ -518,8 +583,13 @@ export async function persistPluginInstall(params: { }, }); logSlotWarnings(slotResult.warnings, runtime); - if (params.warningMessage) { - runtime.log(theme.warn(params.warningMessage)); + const configWarning = + params.enable !== false && configEnablement.mode === "missing" + ? `Installed plugin "${params.pluginId}" without enabling it because it requires configuration first. Configure it, then run \`openclaw plugins enable ${params.pluginId}\`.` + : undefined; + const warningMessage = [params.warningMessage, configWarning].filter(Boolean).join("\n"); + if (warningMessage) { + runtime.log(theme.warn(warningMessage)); } runtime.log(params.successMessage ?? `Installed plugin: ${params.pluginId}`); logShadowedNpmInstallWarning({ diff --git a/src/plugins/management-service.ts b/src/plugins/management-service.ts index 8e07a810bcc8..64a5da79943a 100644 --- a/src/plugins/management-service.ts +++ b/src/plugins/management-service.ts @@ -14,7 +14,6 @@ import type { OpenClawConfig } from "../config/types.openclaw.js"; import type { PluginInstallRecord } from "../config/types.plugins.js"; import { parseClawHubPluginSpec } from "../infra/clawhub-spec.js"; import { formatErrorMessage } from "../infra/errors.js"; -import { createAsyncLock } from "../infra/json-files.js"; import { parseRegistryNpmSpec } from "../infra/npm-registry-spec.js"; import type { RuntimeEnv } from "../runtime.js"; import { CLAWHUB_INSTALL_ERROR_CODE } from "./clawhub-error-codes.js"; @@ -54,6 +53,7 @@ import { type HostedOfficialExternalPluginCatalogLoadResult, type OfficialExternalPluginCatalogEntry, } from "./official-external-plugin-catalog.js"; +import { withPluginLifecycleLease } from "./plugin-lifecycle-lease.js"; import { loadPluginMetadataSnapshot } from "./plugin-metadata-snapshot.js"; import { resolveManifestProviderAuthChoices } from "./provider-auth-choices.js"; import { listRecommendedToolInstalls } from "./recommended-tool-installs.js"; @@ -65,6 +65,8 @@ import { applyPluginUninstallDirectoryRemoval, formatUninstallActionLabels, planPluginUninstall, + pluginUninstallTargetExists, + prepareConfigForPendingPluginDirectoryRemoval, } from "./uninstall.js"; type ManagedPluginCatalogEntry = { @@ -758,8 +760,6 @@ export async function listManagedPlugins(params: { }; } -const withManagedPluginMutationLock = createAsyncLock(); - function assertValidConfigSnapshot( prepared: Awaited>, ): ConfigSnapshotForInstallPersist { @@ -1132,8 +1132,8 @@ export async function installManagedPlugin(params: { request: ManagedPluginInstallRequest; env?: NodeJS.ProcessEnv; }): Promise<{ plugin: ManagedPluginCatalogEntry; warnings?: string[] }> { - return await withManagedPluginMutationLock(async () => { - const env = params.env ?? process.env; + const env = params.env ?? process.env; + return await withPluginLifecycleLease({ env }, async () => { const snapshot = await readPluginMutationSnapshot(env); const officialCatalog = await loadOfficialCatalog(); const warnings: string[] = []; @@ -1181,8 +1181,8 @@ export async function setManagedPluginEnabled(params: { changedPaths: string[]; warnings?: string[]; }> { - return await withManagedPluginMutationLock(async () => { - const env = params.env ?? process.env; + const env = params.env ?? process.env; + return await withPluginLifecycleLease({ env }, async () => { const snapshot = await readPluginMutationSnapshot(env); const metadata = loadPluginMetadataSnapshot({ config: snapshot.config, env }); const pluginId = metadata.normalizePluginId(params.pluginId.trim()); @@ -1247,8 +1247,8 @@ export async function uninstallManagedPlugin(params: { pluginId: string; env?: NodeJS.ProcessEnv; }): Promise<{ pluginId: string; removed: string[]; warnings?: string[] }> { - return await withManagedPluginMutationLock(async () => { - const env = params.env ?? process.env; + const env = params.env ?? process.env; + return await withPluginLifecycleLease({ env }, async () => { const snapshot = await readPluginMutationSnapshot(env); const installRecords = await loadInstalledPluginIndexInstallRecords(); // Mirror the CLI uninstall flow: plan against config carrying install records @@ -1267,15 +1267,55 @@ export async function uninstallManagedPlugin(params: { // planPluginUninstall keeps its plugin-id fallback for channel config keys. const channelIds = manifest && manifest.channels.length > 0 ? manifest.channels : undefined; const extensionsDir = resolveDefaultPluginExtensionsDir(env); - const plan = planPluginUninstall({ + const initialPlan = planPluginUninstall({ config: configWithRecords, pluginId, ...(channelIds ? { channelIds } : {}), deleteFiles: true, extensionsDir, }); - if (!plan.ok) { - throw new ManagedPluginLifecycleError(plan.error); + if (!initialPlan.ok) { + throw new ManagedPluginLifecycleError(initialPlan.error); + } + let plan = initialPlan; + let finalSnapshot = snapshot; + let directoryResult = { directoryRemoved: false, warnings: [] as string[] }; + if (plan.directoryRemoval) { + const disabledConfig = prepareConfigForPendingPluginDirectoryRemoval( + snapshot.config, + pluginId, + ); + await replaceConfigFile({ + nextConfig: disabledConfig, + baseHash: snapshot.baseHash, + writeOptions: { + ...snapshot.writeOptions, + afterWrite: { mode: "auto" }, + }, + }); + directoryResult = await applyPluginUninstallDirectoryRemoval(plan.directoryRemoval); + if (pluginUninstallTargetExists(plan.directoryRemoval.target)) { + throw new ManagedPluginLifecycleError( + `Failed to remove plugin directory ${plan.directoryRemoval.target}; the plugin remains disabled and tracked so uninstall can be retried.`, + { kind: "unavailable" }, + ); + } + finalSnapshot = await readPluginMutationSnapshot(env); + const refreshedConfigWithRecords = withPluginInstallRecords( + finalSnapshot.config, + installRecords, + ); + const refreshedPlan = planPluginUninstall({ + config: refreshedConfigWithRecords, + pluginId, + ...(channelIds ? { channelIds } : {}), + deleteFiles: true, + extensionsDir, + }); + if (!refreshedPlan.ok) { + throw new ManagedPluginLifecycleError(refreshedPlan.error); + } + plan = refreshedPlan; } const nextConfig = withoutPluginInstallRecords(plan.config); const nextInstallRecords = removePluginInstallRecordFromRecords(installRecords, pluginId); @@ -1283,10 +1323,9 @@ export async function uninstallManagedPlugin(params: { previousInstallRecords: installRecords, nextInstallRecords, nextConfig, - baseHash: snapshot.baseHash, - writeOptions: snapshot.writeOptions, + baseHash: finalSnapshot.baseHash, + writeOptions: finalSnapshot.writeOptions, }); - const directoryResult = await applyPluginUninstallDirectoryRemoval(plan.directoryRemoval); const warnings = [ ...collectClawPluginUninstallWarnings({ pluginId, diff --git a/src/plugins/plugin-lifecycle-lease.test.ts b/src/plugins/plugin-lifecycle-lease.test.ts new file mode 100644 index 000000000000..b6f176fb5184 --- /dev/null +++ b/src/plugins/plugin-lifecycle-lease.test.ts @@ -0,0 +1,180 @@ +import { spawn } from "node:child_process"; +import fs from "node:fs/promises"; +import path from "node:path"; +import { pathToFileURL } from "node:url"; +import { afterEach, describe, expect, it } from "vitest"; +import { closeOpenClawStateDatabaseForTest } from "../state/openclaw-state-db.js"; +import { withOpenClawTestState } from "../test-utils/openclaw-test-state.js"; +import { withPluginLifecycleLease } from "./plugin-lifecycle-lease.js"; + +afterEach(() => { + closeOpenClawStateDatabaseForTest(); +}); + +async function waitForPath(filePath: string): Promise { + for (let attempt = 0; attempt < 100; attempt += 1) { + try { + await fs.access(filePath); + return; + } catch { + await new Promise((resolve) => { + setTimeout(resolve, 25); + }); + } + } + throw new Error(`timed out waiting for ${filePath}`); +} + +function deferred(): { promise: Promise; resolve: () => void } { + let resolve!: () => void; + const promise = new Promise((resolvePromise) => { + resolve = resolvePromise; + }); + return { promise, resolve }; +} + +function runLeaseChild(scriptPath: string, args: string[]): Promise { + return new Promise((resolve, reject) => { + const child = spawn(process.execPath, ["--import", "tsx", scriptPath, ...args], { + stdio: ["ignore", "pipe", "pipe"], + }); + let output = ""; + child.stdout.on("data", (chunk) => (output += chunk)); + child.stderr.on("data", (chunk) => (output += chunk)); + child.on("close", (code) => { + if (code === 0) { + resolve(); + } else { + reject(new Error(`lease child exited ${code}: ${output}`)); + } + }); + }); +} + +describe("plugin lifecycle lease", () => { + it("serializes lifecycle work sharing one state directory", async () => { + await withOpenClawTestState({ label: "plugin-lifecycle-lease" }, async (state) => { + const firstEntered = deferred(); + const releaseFirst = deferred(); + const events: string[] = []; + + const first = withPluginLifecycleLease( + { env: state.env, leaseMs: 1_000, waitMs: 3_000 }, + async () => { + events.push("first-enter"); + firstEntered.resolve(); + await releaseFirst.promise; + events.push("first-exit"); + }, + ); + await firstEntered.promise; + + const second = withPluginLifecycleLease( + { env: state.env, leaseMs: 1_000, waitMs: 3_000 }, + async () => { + events.push("second-enter"); + }, + ); + await new Promise((resolve) => { + setTimeout(resolve, 100); + }); + expect(events).toEqual(["first-enter"]); + + releaseFirst.resolve(); + await Promise.all([first, second]); + expect(events).toEqual(["first-enter", "first-exit", "second-enter"]); + }); + }); + + it("serializes lifecycle work across processes", async () => { + await withOpenClawTestState({ label: "plugin-lifecycle-processes" }, async (state) => { + const firstMarker = state.path("first-entered"); + const releaseMarker = state.path("release-first"); + const secondMarker = state.path("second-entered"); + const secondReady = state.path("second-ready"); + const secondResult = state.path("second-result"); + const leaseModuleUrl = pathToFileURL( + path.resolve("src/plugins/plugin-lifecycle-lease.ts"), + ).href; + const childScript = await state.writeText( + "lease-child.mts", + ` + import fs from "node:fs/promises"; + import { withPluginLifecycleLease } from ${JSON.stringify(leaseModuleUrl)}; + const [role, stateDir, firstMarker, releaseMarker, secondMarker, secondReady, secondResult] = process.argv.slice(2); + const env = { ...process.env, OPENCLAW_STATE_DIR: stateDir }; + if (role === "second") { + await fs.writeFile(secondReady, "ready"); + try { + await withPluginLifecycleLease({ env, leaseMs: 1_000, waitMs: 0 }, async () => { + await fs.writeFile(secondMarker, "entered"); + }); + await fs.writeFile(secondResult, "acquired"); + } catch (error) { + await fs.writeFile(secondResult, error?.code ?? String(error)); + } + } else { + await withPluginLifecycleLease({ env, leaseMs: 1_000, waitMs: 5_000 }, async () => { + await fs.writeFile(firstMarker, "entered"); + while (true) { + try { + await fs.access(releaseMarker); + break; + } catch { + await new Promise((resolve) => { + setTimeout(resolve, 25); + }); + } + } + }); + } + `, + ); + + const childArgs = [ + state.stateDir, + firstMarker, + releaseMarker, + secondMarker, + secondReady, + secondResult, + ]; + const first = runLeaseChild(childScript, ["first", ...childArgs]); + await waitForPath(firstMarker); + const second = runLeaseChild(childScript, ["second", ...childArgs]); + await waitForPath(secondReady); + await waitForPath(secondResult); + + let assertionError: unknown; + try { + await expect(fs.readFile(secondResult, "utf8")).resolves.toBe( + "OPENCLAW_STATE_LEASE_TIMEOUT", + ); + await expect(fs.access(secondMarker)).rejects.toMatchObject({ code: "ENOENT" }); + } catch (error) { + assertionError = error; + } finally { + await fs.writeFile(releaseMarker, "release"); + } + await Promise.all([first, second]); + if (assertionError) { + throw assertionError instanceof Error + ? assertionError + : new Error("cross-process lease assertion failed", { cause: assertionError }); + } + }); + }); + + it("reuses the active lease for nested lifecycle work", async () => { + await withOpenClawTestState({ label: "plugin-lifecycle-reentrant" }, async (state) => { + const events: string[] = []; + await withPluginLifecycleLease({ env: state.env, leaseMs: 1_000, waitMs: 0 }, async () => { + events.push("outer"); + await withPluginLifecycleLease({ env: state.env, leaseMs: 1_000, waitMs: 0 }, async () => { + events.push("inner"); + }); + }); + expect(events).toEqual(["outer", "inner"]); + }); + }); +}); diff --git a/src/plugins/plugin-lifecycle-lease.ts b/src/plugins/plugin-lifecycle-lease.ts new file mode 100644 index 000000000000..4ac3afabc1f0 --- /dev/null +++ b/src/plugins/plugin-lifecycle-lease.ts @@ -0,0 +1,76 @@ +import { AsyncLocalStorage } from "node:async_hooks"; +import path from "node:path"; +import { resolveOpenClawStateSqlitePath } from "../state/openclaw-state-db.paths.js"; +import { + OpenClawStateLeaseError, + withOpenClawStateLease, + type OpenClawStateLeaseContext, +} from "../state/openclaw-state-lease.js"; + +const PLUGIN_LIFECYCLE_LEASE_SCOPE = "core:plugin-lifecycle"; +const PLUGIN_LIFECYCLE_LEASE_KEY = "global"; +const DEFAULT_PLUGIN_LIFECYCLE_LEASE_MS = 5 * 60_000; +const DEFAULT_PLUGIN_LIFECYCLE_WAIT_MS = 10 * 60_000; + +type ActivePluginLifecycleLease = { + databasePath: string; + lease: OpenClawStateLeaseContext; +}; + +type PluginLifecycleLeaseOptions = { + env?: NodeJS.ProcessEnv; + signal?: AbortSignal; + leaseMs?: number; + waitMs?: number; +}; + +const activePluginLifecycleLease = new AsyncLocalStorage(); + +function resolveLifecycleLeaseEnv(env: NodeJS.ProcessEnv | undefined): NodeJS.ProcessEnv { + const requested = env ?? process.env; + if (!process.env.VITEST || requested.VITEST || requested.OPENCLAW_STATE_DIR) { + return requested; + } + return { + ...requested, + VITEST: process.env.VITEST, + VITEST_WORKER_ID: process.env.VITEST_WORKER_ID, + VITEST_POOL_ID: process.env.VITEST_POOL_ID, + }; +} + +/** Serialize plugin artifact, install-index, and config mutations across processes. */ +export async function withPluginLifecycleLease( + options: PluginLifecycleLeaseOptions, + run: (lease: OpenClawStateLeaseContext) => Promise, +): Promise { + const env = resolveLifecycleLeaseEnv(options.env); + const databasePath = path.resolve(resolveOpenClawStateSqlitePath(env)); + const active = activePluginLifecycleLease.getStore(); + if (active) { + if (active.databasePath !== databasePath) { + throw new OpenClawStateLeaseError( + "nested plugin lifecycle lease cannot switch the shared state database", + { code: "OPENCLAW_STATE_LEASE_INVALID_INPUT" }, + ); + } + options.signal?.throwIfAborted(); + active.lease.assertOwned(); + return await run(active.lease); + } + + return await withOpenClawStateLease( + { + scope: PLUGIN_LIFECYCLE_LEASE_SCOPE, + key: PLUGIN_LIFECYCLE_LEASE_KEY, + database: { scope: "shared", options: { env } }, + leaseMs: options.leaseMs ?? DEFAULT_PLUGIN_LIFECYCLE_LEASE_MS, + waitMs: options.waitMs ?? DEFAULT_PLUGIN_LIFECYCLE_WAIT_MS, + ...(options.signal ? { signal: options.signal } : {}), + leaseLabel: "plugin lifecycle lease", + operationLabel: "plugins.lifecycle.lease", + }, + async (lease) => + await activePluginLifecycleLease.run({ databasePath, lease }, async () => await run(lease)), + ); +} diff --git a/src/plugins/uninstall-directory-removal.test.ts b/src/plugins/uninstall-directory-removal.test.ts new file mode 100644 index 000000000000..d55d09250a6c --- /dev/null +++ b/src/plugins/uninstall-directory-removal.test.ts @@ -0,0 +1,22 @@ +import fs from "node:fs/promises"; +import path from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { useAutoCleanupTempDirTracker } from "../../test/helpers/temp-dir.js"; +import { applyPluginUninstallDirectoryRemoval } from "./uninstall.js"; + +const tempDirs = useAutoCleanupTempDirTracker(afterEach); + +describe("plugin uninstall directory removal", () => { + it("removes a dangling managed-target symlink", async () => { + const root = tempDirs.make("openclaw-plugin-uninstall-"); + const target = path.join(root, "plugin"); + await fs.symlink(path.join(root, "missing-target"), target, "dir"); + + await expect(fs.lstat(target)).resolves.toBeDefined(); + await expect(applyPluginUninstallDirectoryRemoval({ target })).resolves.toEqual({ + directoryRemoved: true, + warnings: [], + }); + await expect(fs.lstat(target)).rejects.toMatchObject({ code: "ENOENT" }); + }); +}); diff --git a/src/plugins/uninstall.ts b/src/plugins/uninstall.ts index 86b98ff2ace4..cb10a5465cda 100644 --- a/src/plugins/uninstall.ts +++ b/src/plugins/uninstall.ts @@ -1,5 +1,5 @@ // Removes installed plugins and updates plugin index records. -import { realpathSync } from "node:fs"; +import { lstatSync, realpathSync } from "node:fs"; import fs from "node:fs/promises"; import path from "node:path"; import type { OpenClawConfig } from "../config/types.openclaw.js"; @@ -82,6 +82,26 @@ export function formatUninstallActionLabels(actions: UninstallActions): string[] ); } +/** Keep a staged plugin disabled until its managed directory is removed. */ +export function prepareConfigForPendingPluginDirectoryRemoval( + config: OpenClawConfig, + pluginId: string, +): OpenClawConfig { + return { + ...config, + plugins: { + ...config.plugins, + entries: { + ...config.plugins?.entries, + [pluginId]: { + ...config.plugins?.entries?.[pluginId], + enabled: false, + }, + }, + }, + }; +} + function hasUninstallAction(actions: Omit): boolean { return Object.values(actions).some(Boolean); } @@ -595,6 +615,15 @@ export function planPluginUninstall(params: UninstallPluginParams): PluginUninst }; } +export function pluginUninstallTargetExists(target: string): boolean { + try { + lstatSync(target); + return true; + } catch (error) { + return (error as NodeJS.ErrnoException).code !== "ENOENT"; + } +} + export async function applyPluginUninstallDirectoryRemoval( removal: PluginUninstallDirectoryRemoval | null, ): Promise<{ directoryRemoved: boolean; warnings: string[] }> { @@ -602,11 +631,7 @@ export async function applyPluginUninstallDirectoryRemoval( return { directoryRemoved: false, warnings: [] }; } - const existed = - (await fs - .access(removal.target) - .then(() => true) - .catch(() => false)) ?? false; + const existed = pluginUninstallTargetExists(removal.target); const warnings: string[] = []; if (!existed && removal.cleanup?.kind !== "npm") { return { directoryRemoved: false, warnings };