From f911bbc35329596cc5c836b60fbf1f09e4bef86f Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Fri, 3 Apr 2026 03:22:37 +0900 Subject: [PATCH] refactor(plugins): separate activation from enablement (#59844) * refactor(plugins): separate activation from enablement * fix(cli): sanitize verbose plugin activation reasons --- src/agents/pi-project-settings.ts | 9 ++-- src/agents/skills/plugin-skills.ts | 6 +-- src/cli/plugins-cli.list.test.ts | 22 ++++++++++ src/cli/plugins-cli.ts | 7 ++++ .../channel-setup/plugin-install.test.ts | 21 +++++++++- src/commands/channel-setup/plugin-install.ts | 13 ++++-- .../doctor/shared/channel-plugin-blockers.ts | 13 +++--- src/config/validation.ts | 8 ++-- src/gateway/server-methods/send.test.ts | 12 +++++- src/hooks/plugin-hooks.ts | 6 +-- src/plugins/bundle-commands.ts | 6 +-- src/plugins/bundle-config-shared.ts | 6 +-- src/plugins/providers.ts | 14 +++---- src/plugins/status.test.ts | 41 +++++++++++++++++++ src/plugins/web-search-providers.ts | 6 +-- 15 files changed, 148 insertions(+), 42 deletions(-) diff --git a/src/agents/pi-project-settings.ts b/src/agents/pi-project-settings.ts index 9732e8088a9..f8f237bc14c 100644 --- a/src/agents/pi-project-settings.ts +++ b/src/agents/pi-project-settings.ts @@ -6,7 +6,10 @@ import { applyMergePatch } from "../config/merge-patch.js"; import { openBoundaryFileSync } from "../infra/boundary-file-read.js"; import { createSubsystemLogger } from "../logging/subsystem.js"; import type { BundleMcpServerConfig } from "../plugins/bundle-mcp.js"; -import { normalizePluginsConfig, resolveEffectiveEnableState } from "../plugins/config-state.js"; +import { + normalizePluginsConfig, + resolveEffectivePluginActivationState, +} from "../plugins/config-state.js"; import { loadPluginManifestRegistry } from "../plugins/manifest-registry.js"; import { isRecord } from "../utils.js"; import { loadEmbeddedPiMcpConfig } from "./embedded-pi-mcp.js"; @@ -90,13 +93,13 @@ export function loadEnabledBundlePiSettingsSnapshot(params: { if (record.format !== "bundle" || settingsFiles.length === 0) { continue; } - const enableState = resolveEffectiveEnableState({ + const activationState = resolveEffectivePluginActivationState({ id: record.id, origin: record.origin, config: normalizedPlugins, rootConfig: params.cfg, }); - if (!enableState.enabled) { + if (!activationState.activated) { continue; } for (const relativePath of settingsFiles) { diff --git a/src/agents/skills/plugin-skills.ts b/src/agents/skills/plugin-skills.ts index c7a371ad689..e3604345c20 100644 --- a/src/agents/skills/plugin-skills.ts +++ b/src/agents/skills/plugin-skills.ts @@ -4,7 +4,7 @@ import type { OpenClawConfig } from "../../config/config.js"; import { createSubsystemLogger } from "../../logging/subsystem.js"; import { normalizePluginsConfig, - resolveEffectiveEnableState, + resolveEffectivePluginActivationState, resolveMemorySlotDecision, } from "../../plugins/config-state.js"; import { loadPluginManifestRegistry } from "../../plugins/manifest-registry.js"; @@ -39,13 +39,13 @@ export function resolvePluginSkillDirs(params: { if (!record.skills || record.skills.length === 0) { continue; } - const enableState = resolveEffectiveEnableState({ + const activationState = resolveEffectivePluginActivationState({ id: record.id, origin: record.origin, config: normalizedPlugins, rootConfig: params.config, }); - if (!enableState.enabled) { + if (!activationState.activated) { continue; } // ACP router skills should not be attached when ACP is explicitly disabled. diff --git a/src/cli/plugins-cli.list.test.ts b/src/cli/plugins-cli.list.test.ts index c77d38be4f1..11b9b01bdec 100644 --- a/src/cli/plugins-cli.list.test.ts +++ b/src/cli/plugins-cli.list.test.ts @@ -69,6 +69,28 @@ describe("plugins cli list", () => { expect(output).toContain("explicitly enabled: no"); }); + it("sanitizes activation reasons in verbose output", async () => { + buildPluginSnapshotReport.mockReturnValue({ + plugins: [ + createPluginRecord({ + id: "demo", + name: "Demo Plugin", + activated: true, + activationSource: "auto", + activationReason: "\u001B[31mconfigured\nnext\tstep", + }), + ], + diagnostics: [], + }); + + await runPluginsCommand(["plugins", "list", "--verbose"]); + + const output = runtimeLogs.join("\n"); + expect(output).toContain("activation reason: configured\\nnext\\tstep"); + expect(output).not.toContain("\u001B[31m"); + expect(output.match(/activation reason:/g)).toHaveLength(1); + }); + it("keeps doctor on a module-loading snapshot", async () => { buildPluginDiagnosticsReport.mockReturnValue({ plugins: [], diff --git a/src/cli/plugins-cli.ts b/src/cli/plugins-cli.ts index 8de55764aa0..4aad99be57d 100644 --- a/src/cli/plugins-cli.ts +++ b/src/cli/plugins-cli.ts @@ -156,6 +156,13 @@ function formatPluginLine(plugin: PluginRecord, verbose = false): string { if (plugin.providerIds.length > 0) { parts.push(` providers: ${plugin.providerIds.join(", ")}`); } + if (plugin.activated !== undefined || plugin.activationSource || plugin.activationReason) { + const activationSummary = + plugin.activated === false + ? "inactive" + : (plugin.activationSource ?? (plugin.activated ? "active" : "inactive")); + parts.push(` activation: ${activationSummary}`); + } if (plugin.error) { parts.push(theme.error(` error: ${plugin.error}`)); } diff --git a/src/commands/channel-setup/plugin-install.test.ts b/src/commands/channel-setup/plugin-install.test.ts index d134099d782..63bfcb36894 100644 --- a/src/commands/channel-setup/plugin-install.test.ts +++ b/src/commands/channel-setup/plugin-install.test.ts @@ -119,6 +119,7 @@ beforeEach(() => { applyPluginAutoEnable.mockImplementation((params: { config: unknown }) => ({ config: params.config, changes: [], + autoEnabledReasons: {}, })); resolveBundledPluginSources.mockReturnValue(new Map()); getChannelPluginCatalogEntry.mockReturnValue(undefined); @@ -378,6 +379,8 @@ describe("ensureChannelSetupPluginInstalled", () => { expect(loadOpenClawPlugins).toHaveBeenCalledWith( expect.objectContaining({ config: cfg, + activationSourceConfig: cfg, + autoEnabledReasons: {}, workspaceDir: "/tmp/openclaw-workspace", cache: false, includeSetupOnlyChannelPlugins: true, @@ -402,7 +405,11 @@ describe("ensureChannelSetupPluginInstalled", () => { }, }, } as OpenClawConfig; - applyPluginAutoEnable.mockReturnValue({ config: autoEnabledConfig, changes: [] }); + applyPluginAutoEnable.mockReturnValue({ + config: autoEnabledConfig, + changes: [], + autoEnabledReasons: {}, + }); reloadChannelSetupPluginRegistry({ cfg, @@ -417,6 +424,8 @@ describe("ensureChannelSetupPluginInstalled", () => { expect(loadOpenClawPlugins).toHaveBeenCalledWith( expect.objectContaining({ config: autoEnabledConfig, + activationSourceConfig: cfg, + autoEnabledReasons: {}, }), ); }); @@ -436,6 +445,8 @@ describe("ensureChannelSetupPluginInstalled", () => { expect(loadOpenClawPlugins).toHaveBeenCalledWith( expect.objectContaining({ config: cfg, + activationSourceConfig: cfg, + autoEnabledReasons: {}, workspaceDir: "/tmp/openclaw-workspace", cache: false, onlyPluginIds: ["@openclaw/telegram-plugin"], @@ -503,6 +514,8 @@ describe("ensureChannelSetupPluginInstalled", () => { expect(loadOpenClawPlugins).toHaveBeenCalledWith( expect.objectContaining({ + activationSourceConfig: cfg, + autoEnabledReasons: {}, onlyPluginIds: ["@openclaw/telegram-plugin"], }), ); @@ -523,6 +536,8 @@ describe("ensureChannelSetupPluginInstalled", () => { expect(loadOpenClawPlugins).toHaveBeenCalledWith( expect.objectContaining({ config: cfg, + activationSourceConfig: cfg, + autoEnabledReasons: {}, workspaceDir: "/tmp/openclaw-workspace", cache: false, onlyPluginIds: ["@openclaw/telegram-plugin"], @@ -568,6 +583,8 @@ describe("ensureChannelSetupPluginInstalled", () => { expect(loadOpenClawPlugins).toHaveBeenCalledWith( expect.objectContaining({ config: cfg, + activationSourceConfig: cfg, + autoEnabledReasons: {}, workspaceDir: "/tmp/openclaw-workspace", cache: false, onlyPluginIds: ["custom-telegram-plugin"], @@ -592,6 +609,8 @@ describe("ensureChannelSetupPluginInstalled", () => { expect(loadOpenClawPlugins).toHaveBeenCalledWith( expect.objectContaining({ config: cfg, + activationSourceConfig: cfg, + autoEnabledReasons: {}, workspaceDir: "/tmp/openclaw-workspace", cache: false, onlyPluginIds: ["@openclaw/msteams-plugin"], diff --git a/src/commands/channel-setup/plugin-install.ts b/src/commands/channel-setup/plugin-install.ts index d4321c22ec4..0c64ec2f539 100644 --- a/src/commands/channel-setup/plugin-install.ts +++ b/src/commands/channel-setup/plugin-install.ts @@ -244,13 +244,16 @@ function loadChannelSetupPluginRegistry(params: { activate?: boolean; }): PluginRegistry { clearPluginDiscoveryCache(); - const resolvedConfig = applyPluginAutoEnable({ config: params.cfg, env: process.env }).config; + const autoEnabled = applyPluginAutoEnable({ config: params.cfg, env: process.env }); + const resolvedConfig = autoEnabled.config; const workspaceDir = params.workspaceDir ?? resolveAgentWorkspaceDir(resolvedConfig, resolveDefaultAgentId(resolvedConfig)); const log = createSubsystemLogger("plugins"); return loadOpenClawPlugins({ config: resolvedConfig, + activationSourceConfig: params.cfg, + autoEnabledReasons: autoEnabled.autoEnabledReasons, workspaceDir, cache: false, logger: createPluginLoaderLogger(log), @@ -270,9 +273,11 @@ function resolveScopedChannelPluginId(params: { if (explicitPluginId) { return explicitPluginId; } - return getChannelPluginCatalogEntry(params.channel, { - workspaceDir: params.workspaceDir, - })?.pluginId ?? resolveUniqueManifestScopedChannelPluginId(params); + return ( + getChannelPluginCatalogEntry(params.channel, { + workspaceDir: params.workspaceDir, + })?.pluginId ?? resolveUniqueManifestScopedChannelPluginId(params) + ); } function resolveUniqueManifestScopedChannelPluginId(params: { diff --git a/src/commands/doctor/shared/channel-plugin-blockers.ts b/src/commands/doctor/shared/channel-plugin-blockers.ts index a33c673a0b6..334213e73d9 100644 --- a/src/commands/doctor/shared/channel-plugin-blockers.ts +++ b/src/commands/doctor/shared/channel-plugin-blockers.ts @@ -2,7 +2,7 @@ import { listPotentialConfiguredChannelIds } from "../../../channels/config-pres import type { OpenClawConfig } from "../../../config/config.js"; import { normalizePluginsConfig, - resolveEffectiveEnableState, + resolveEffectivePluginActivationState, } from "../../../plugins/config-state.js"; import { loadPluginManifestRegistry } from "../../../plugins/manifest-registry.js"; import { sanitizeForLog } from "../../../terminal/ansi.js"; @@ -36,7 +36,7 @@ export function scanConfiguredChannelPluginBlockers( continue; } - const enableState = resolveEffectiveEnableState({ + const activationState = resolveEffectivePluginActivationState({ id: plugin.id, origin: plugin.origin, config: pluginsConfig, @@ -44,9 +44,10 @@ export function scanConfiguredChannelPluginBlockers( enabledByDefault: plugin.enabledByDefault, }); if ( - enableState.enabled || - !enableState.reason || - (enableState.reason !== "disabled in config" && enableState.reason !== "plugins disabled") + activationState.activated || + !activationState.reason || + (activationState.reason !== "disabled in config" && + activationState.reason !== "plugins disabled") ) { continue; } @@ -58,7 +59,7 @@ export function scanConfiguredChannelPluginBlockers( hits.push({ channelId, pluginId: plugin.id, - reason: enableState.reason, + reason: activationState.reason, }); } } diff --git a/src/config/validation.ts b/src/config/validation.ts index 1e912158444..c9fa024bcea 100644 --- a/src/config/validation.ts +++ b/src/config/validation.ts @@ -5,7 +5,7 @@ import { withBundledPluginAllowlistCompat } from "../plugins/bundled-compat.js"; import { listBundledWebSearchPluginIds } from "../plugins/bundled-web-search-ids.js"; import { normalizePluginsConfig, - resolveEffectiveEnableState, + resolveEffectivePluginActivationState, resolveMemorySlotDecision, } from "../plugins/config-state.js"; import { loadPluginManifestRegistry } from "../plugins/manifest-registry.js"; @@ -914,14 +914,14 @@ function validateConfigObjectWithPluginsBase( const entry = normalizedPlugins.entries[pluginId]; const entryHasConfig = Boolean(entry?.config); - const enableState = resolveEffectiveEnableState({ + const activationState = resolveEffectivePluginActivationState({ id: pluginId, origin: record.origin, config: normalizedPlugins, rootConfig: config, }); - let enabled = enableState.enabled; - let reason = enableState.reason; + let enabled = activationState.activated; + let reason = activationState.reason; if (enabled) { const memoryDecision = resolveMemorySlotDecision({ diff --git a/src/gateway/server-methods/send.test.ts b/src/gateway/server-methods/send.test.ts index 248d3689de7..cc1e6c6eaeb 100644 --- a/src/gateway/server-methods/send.test.ts +++ b/src/gateway/server-methods/send.test.ts @@ -171,7 +171,11 @@ describe("gateway send mirroring", () => { vi.clearAllMocks(); registrySeq += 1; setActivePluginRegistry(createTestRegistry([]), `send-test-${registrySeq}`); - mocks.applyPluginAutoEnable.mockImplementation(({ config }) => ({ config, changes: [] })); + mocks.applyPluginAutoEnable.mockImplementation(({ config }) => ({ + config, + changes: [], + autoEnabledReasons: {}, + })); mocks.resolveOutboundTarget.mockReturnValue({ ok: true, to: "resolved" }); mocks.resolveOutboundSessionRoute.mockImplementation( async ({ agentId, channel }: { agentId?: string; channel?: string }) => ({ @@ -319,7 +323,11 @@ describe("gateway send mirroring", () => { it("auto-picks the single configured channel from the auto-enabled config snapshot for send", async () => { const autoEnabledConfig = { channels: { slack: {} }, plugins: { allow: ["slack"] } }; - mocks.applyPluginAutoEnable.mockReturnValue({ config: autoEnabledConfig, changes: [] }); + mocks.applyPluginAutoEnable.mockReturnValue({ + config: autoEnabledConfig, + changes: [], + autoEnabledReasons: {}, + }); mockDeliverySuccess("m-single-send-auto"); const { respond } = await runSend({ diff --git a/src/hooks/plugin-hooks.ts b/src/hooks/plugin-hooks.ts index 0f53fb61bca..4f6a9793dfc 100644 --- a/src/hooks/plugin-hooks.ts +++ b/src/hooks/plugin-hooks.ts @@ -4,7 +4,7 @@ import type { OpenClawConfig } from "../config/config.js"; import { createSubsystemLogger } from "../logging/subsystem.js"; import { normalizePluginsConfig, - resolveEffectiveEnableState, + resolveEffectivePluginActivationState, resolveMemorySlotDecision, } from "../plugins/config-state.js"; import { loadPluginManifestRegistry } from "../plugins/manifest-registry.js"; @@ -46,13 +46,13 @@ export function resolvePluginHookDirs(params: { if (!record.hooks || record.hooks.length === 0) { continue; } - const enableState = resolveEffectiveEnableState({ + const activationState = resolveEffectivePluginActivationState({ id: record.id, origin: record.origin, config: normalizedPlugins, rootConfig: params.config, }); - if (!enableState.enabled) { + if (!activationState.activated) { continue; } diff --git a/src/plugins/bundle-commands.ts b/src/plugins/bundle-commands.ts index 6797626f128..75f15124f70 100644 --- a/src/plugins/bundle-commands.ts +++ b/src/plugins/bundle-commands.ts @@ -9,7 +9,7 @@ import { mergeBundlePathLists, normalizeBundlePathList, } from "./bundle-manifest.js"; -import { normalizePluginsConfig, resolveEffectiveEnableState } from "./config-state.js"; +import { normalizePluginsConfig, resolveEffectivePluginActivationState } from "./config-state.js"; import { loadPluginManifestRegistry } from "./manifest-registry.js"; export type ClaudeBundleCommandSpec = { @@ -179,13 +179,13 @@ export function loadEnabledClaudeBundleCommands(params: { ) { continue; } - const enableState = resolveEffectiveEnableState({ + const activationState = resolveEffectivePluginActivationState({ id: record.id, origin: record.origin, config: normalizedPlugins, rootConfig: params.cfg, }); - if (!enableState.enabled) { + if (!activationState.activated) { continue; } for (const relativeRoot of resolveClaudeCommandRootDirs(record.rootDir)) { diff --git a/src/plugins/bundle-config-shared.ts b/src/plugins/bundle-config-shared.ts index 7980f72a6fa..b2cacd0a4ba 100644 --- a/src/plugins/bundle-config-shared.ts +++ b/src/plugins/bundle-config-shared.ts @@ -4,7 +4,7 @@ import type { OpenClawConfig } from "../config/config.js"; import { applyMergePatch } from "../config/merge-patch.js"; import { matchBoundaryFileOpenFailure, openBoundaryFileSync } from "../infra/boundary-file-read.js"; import { isRecord } from "../utils.js"; -import { normalizePluginsConfig, resolveEffectiveEnableState } from "./config-state.js"; +import { normalizePluginsConfig, resolveEffectivePluginActivationState } from "./config-state.js"; import { loadPluginManifestRegistry } from "./manifest-registry.js"; import type { PluginBundleFormat } from "./types.js"; @@ -114,13 +114,13 @@ export function loadEnabledBundleConfig(params: { if (record.format !== "bundle" || !record.bundleFormat) { continue; } - const enableState = resolveEffectiveEnableState({ + const activationState = resolveEffectivePluginActivationState({ id: record.id, origin: record.origin, config: normalizedPlugins, rootConfig: params.cfg, }); - if (!enableState.enabled) { + if (!activationState.activated) { continue; } diff --git a/src/plugins/providers.ts b/src/plugins/providers.ts index 10442937bfe..06832b1679b 100644 --- a/src/plugins/providers.ts +++ b/src/plugins/providers.ts @@ -1,6 +1,6 @@ import { normalizeProviderId } from "../agents/provider-id.js"; import { withBundledPluginVitestCompat } from "./bundled-compat.js"; -import { normalizePluginsConfig, resolveEffectiveEnableState } from "./config-state.js"; +import { normalizePluginsConfig, resolveEffectivePluginActivationState } from "./config-state.js"; import type { PluginLoadOptions } from "./loader.js"; import { loadPluginManifestRegistry } from "./manifest-registry.js"; @@ -53,12 +53,12 @@ export function resolveEnabledProviderPluginIds(params: { (plugin) => plugin.providers.length > 0 && (!onlyPluginIdSet || onlyPluginIdSet.has(plugin.id)) && - resolveEffectiveEnableState({ + resolveEffectivePluginActivationState({ id: plugin.id, origin: plugin.origin, config: normalizedConfig, rootConfig: params.config, - }).enabled, + }).activated, ) .map((plugin) => plugin.id) .toSorted((left, right) => left.localeCompare(right)); @@ -111,12 +111,12 @@ export function resolveNonBundledProviderPluginIds(params: { (plugin) => plugin.origin !== "bundled" && plugin.providers.length > 0 && - resolveEffectiveEnableState({ + resolveEffectivePluginActivationState({ id: plugin.id, origin: plugin.origin, config: normalizedConfig, rootConfig: params.config, - }).enabled, + }).activated, ) .map((plugin) => plugin.id) .toSorted((left, right) => left.localeCompare(right)); @@ -137,12 +137,12 @@ export function resolveCatalogHookProviderPluginIds(params: { .filter( (plugin) => plugin.providers.length > 0 && - resolveEffectiveEnableState({ + resolveEffectivePluginActivationState({ id: plugin.id, origin: plugin.origin, config: normalizedConfig, rootConfig: params.config, - }).enabled, + }).activated, ) .map((plugin) => plugin.id); const bundledCompatPluginIds = resolveBundledProviderCompatPluginIds(params); diff --git a/src/plugins/status.test.ts b/src/plugins/status.test.ts index d987e6c0085..239ef11b460 100644 --- a/src/plugins/status.test.ts +++ b/src/plugins/status.test.ts @@ -440,6 +440,47 @@ describe("plugin status reports", () => { }); }); + it("preserves raw config activation context for compatibility-derived reports", () => { + const { rawConfig, autoEnabledConfig } = createAutoEnabledStatusConfig( + { + demo: { enabled: true }, + }, + { channels: { demo: { enabled: true } } }, + ); + applyPluginAutoEnableMock.mockReturnValue({ + config: autoEnabledConfig, + changes: [], + autoEnabledReasons: { + demo: ["demo configured"], + }, + }); + setSinglePluginLoadResult( + createPluginRecord({ + id: "demo", + name: "Demo", + description: "Auto-enabled plugin", + origin: "bundled", + hookCount: 1, + }), + { + typedHooks: [createTypedHook({ pluginId: "demo", hookName: "before_agent_start" })], + }, + ); + + expect(buildPluginCompatibilityNotices({ config: rawConfig })).toEqual([ + createCompatibilityNotice({ pluginId: "demo", code: "legacy-before-agent-start" }), + createCompatibilityNotice({ pluginId: "demo", code: "hook-only" }), + ]); + + expectAutoEnabledStatusLoad({ + rawConfig, + autoEnabledConfig, + autoEnabledReasons: { + demo: ["demo configured"], + }, + }); + }); + it("normalizes bundled plugin versions to the core base release", () => { setSinglePluginLoadResult( createPluginRecord({ diff --git a/src/plugins/web-search-providers.ts b/src/plugins/web-search-providers.ts index f61cdbd5362..5ee3df67c34 100644 --- a/src/plugins/web-search-providers.ts +++ b/src/plugins/web-search-providers.ts @@ -1,5 +1,5 @@ import { listBundledWebSearchProviders as listBundledWebSearchProviderEntries } from "./bundled-web-search.js"; -import { resolveEffectiveEnableState } from "./config-state.js"; +import { resolveEffectivePluginActivationState } from "./config-state.js"; import type { PluginLoadOptions } from "./loader.js"; import type { PluginWebSearchProviderEntry } from "./types.js"; import { @@ -26,11 +26,11 @@ export function resolveBundledPluginWebSearchProviders(params: { if (onlyPluginIdSet && !onlyPluginIdSet.has(provider.pluginId)) { return false; } - return resolveEffectiveEnableState({ + return resolveEffectivePluginActivationState({ id: provider.pluginId, origin: "bundled", config: normalized, rootConfig: config, - }).enabled; + }).activated; }); }