diff --git a/src/cli/channels-list-catalog-row-discovery.test.ts b/src/cli/channels-list-catalog-row-discovery.test.ts new file mode 100644 index 000000000000..6c08828f3a9c --- /dev/null +++ b/src/cli/channels-list-catalog-row-discovery.test.ts @@ -0,0 +1,126 @@ +// The real channels-list route must resolve catalog-row repair hints from prepared +// manifest facts instead of rebuilding the manifest registry once per catalog row. +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { afterAll, beforeEach, expect, it, vi } from "vitest"; +import type { ChannelPluginCatalogEntry } from "../channels/plugins/catalog.js"; +import type { OpenClawConfig } from "../config/types.openclaw.js"; + +const testState = vi.hoisted(() => ({ + config: {} as OpenClawConfig, + json: [] as unknown[], + catalogEntries: [] as ChannelPluginCatalogEntry[], + manifestRegistryRebuilds: 0, +})); + +vi.mock("../plugins/plugin-registry-contributions.js", async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, + loadPluginManifestRegistryForPluginRegistry: ( + ...args: Parameters + ) => { + testState.manifestRegistryRebuilds += 1; + return actual.loadPluginManifestRegistryForPluginRegistry(...args); + }, + }; +}); + +vi.mock("./command-execution-startup.js", () => ({ + applyCliExecutionStartupPresentation: vi.fn(async () => {}), + ensureCliExecutionBootstrap: vi.fn(async () => {}), + resolveCliExecutionStartupContext: vi.fn(() => ({ + startupPolicy: { loadPlugins: false, suppressDoctorStdout: true }, + })), +})); + +vi.mock("../commands/channels/shared.js", () => ({ + formatChannelAccountLabel: vi.fn(), + requireValidConfig: vi.fn(async () => testState.config), +})); + +vi.mock("../commands/channel-setup/trusted-catalog.js", () => ({ + listTrustedChannelPluginCatalogEntries: vi.fn(() => testState.catalogEntries), +})); + +vi.mock("../agents/agent-scope.js", () => ({ + resolveAgentWorkspaceDir: vi.fn(() => undefined), + resolveDefaultAgentId: vi.fn(() => "main"), +})); + +vi.mock("../runtime.js", () => ({ + defaultRuntime: { + error: vi.fn(), + exit: vi.fn(), + log: vi.fn(), + writeJson: (value: unknown) => testState.json.push(value), + writeStdout: vi.fn(), + }, + writeRuntimeJson: vi.fn((runtime: { writeJson: (value: unknown) => void }, value: unknown) => + runtime.writeJson(value), + ), +})); + +import { tryRouteCli } from "./route.js"; + +const tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-channels-list-catalog-rows-")); + +// Official external channels whose owner plugin is not installed. Each one is +// configured, so `channels list` renders it as a catalog-only row and resolves a +// repair hint for it. +const OWNERLESS_CHANNEL_IDS = ["feishu", "googlechat", "matrix", "twitch"] as const; + +function officialExternalCatalogEntry(channelId: string): ChannelPluginCatalogEntry { + return { + id: channelId, + meta: { label: channelId }, + install: { npmSpec: `@openclaw/${channelId}` }, + } as ChannelPluginCatalogEntry; +} + +async function runChannelsListJson(channelIds: readonly string[]): Promise<{ + rebuilds: number; + chat: unknown; +}> { + testState.catalogEntries = channelIds.map(officialExternalCatalogEntry); + testState.config = { + channels: Object.fromEntries(channelIds.map((channelId) => [channelId, { enabled: true }])), + } as OpenClawConfig; + testState.json = []; + testState.manifestRegistryRebuilds = 0; + await expect(tryRouteCli(["node", "openclaw", "channels", "list", "--json"])).resolves.toBe(true); + return { + rebuilds: testState.manifestRegistryRebuilds, + chat: (testState.json[0] as { chat: unknown }).chat, + }; +} + +beforeEach(() => { + vi.stubEnv("OPENCLAW_DISABLE_BUNDLED_PLUGINS", "1"); + vi.stubEnv("OPENCLAW_HOME", path.join(tempRoot, "home")); + vi.stubEnv("OPENCLAW_STATE_DIR", path.join(tempRoot, "state")); +}); + +afterAll(() => { + fs.rmSync(tempRoot, { recursive: true, force: true }); + vi.unstubAllEnvs(); +}); + +it("resolves catalog-row repair hints without rebuilding the manifest registry", async () => { + const empty = await runChannelsListJson([]); + const rows = await runChannelsListJson(OWNERLESS_CHANNEL_IDS); + + // Every row still resolves its repair hint, so the presence policy really ran. + expect(rows.chat).toEqual( + Object.fromEntries( + OWNERLESS_CHANNEL_IDS.map((channelId) => [ + channelId, + { accounts: [], installed: false, origin: "configured" }, + ]), + ), + ); + // Discovery is prepared once per invocation, so row count cannot move this number. + expect({ noRows: empty.rebuilds, fourRows: rows.rebuilds }).toEqual({ noRows: 0, fourRows: 0 }); +}); diff --git a/src/cli/command-catalog.ts b/src/cli/command-catalog.ts index 78d1356f93f3..7dbeee451640 100644 --- a/src/cli/command-catalog.ts +++ b/src/cli/command-catalog.ts @@ -439,6 +439,11 @@ export const cliCommandCatalog: readonly CliCommandCatalogEntry[] = [ }, }, { commandPath: ["nodes"], policy: { networkProxy: "bypass" } }, + // Both bodies are pure gateway RPC reads, so they skip the config guard like + // `channels status`. Bare `openclaw nodes` keeps it because it still resolves + // plugin-provided node subcommands from validated config. + { commandPath: ["nodes", "status"], exact: true, policy: { configGuard: "skip" } }, + { commandPath: ["nodes", "list"], exact: true, policy: { configGuard: "skip" } }, { commandPath: ["pairing"], policy: { networkProxy: "bypass" } }, { commandPath: ["proxy"], policy: { networkProxy: "bypass" } }, { commandPath: ["qr"], policy: { networkProxy: "bypass" } }, diff --git a/src/cli/command-path-policy.test.ts b/src/cli/command-path-policy.test.ts index 2dbfa6ef4912..dc3402060dd4 100644 --- a/src/cli/command-path-policy.test.ts +++ b/src/cli/command-path-policy.test.ts @@ -74,6 +74,20 @@ describe("command-path-policy", () => { }); }); + it("keeps RPC-only nodes reads off the config guard", () => { + expectResolvedPolicy(["nodes", "status"], { + configGuard: "skip", + networkProxy: "bypass", + }); + expectResolvedPolicy(["nodes", "list"], { + configGuard: "skip", + networkProxy: "bypass", + }); + // Bare `openclaw nodes` still resolves plugin subcommands from validated config. + expectResolvedPolicy(["nodes"], { networkProxy: "bypass" }); + expectResolvedPolicy(["nodes", "pair"], { networkProxy: "bypass" }); + }); + it("applies exact overrides after broader channel plugin rules", () => { expectResolvedPolicy(["channels", "send"], { loadPlugins: "always", diff --git a/src/commands/channels.list.test.ts b/src/commands/channels.list.test.ts index 931b1a66b1f4..201e069f514d 100644 --- a/src/commands/channels.list.test.ts +++ b/src/commands/channels.list.test.ts @@ -411,6 +411,8 @@ describe("channels list", () => { }, channelId: "discord", workspaceDir: "/tmp/workspace", + // Prepared once for the invocation; the row loop must not rediscover. + manifestRecords: expect.any(Array), }); const output = stripAnsi(loggedText(runtime)); expect(output).toContain("Discord"); diff --git a/src/commands/channels/list.ts b/src/commands/channels/list.ts index 1e1a43c7cba3..415e97e7b544 100644 --- a/src/commands/channels/list.ts +++ b/src/commands/channels/list.ts @@ -281,6 +281,7 @@ export async function channelsListCommand( config: cfg, channelId: entry.id, ...(workspaceDir ? { workspaceDir } : {}), + manifestRecords: metadataSnapshot.plugins, }); return { entry, diff --git a/src/plugins/channel-presence-policy.ts b/src/plugins/channel-presence-policy.ts index 2517d96760fa..f6279ec451d2 100644 --- a/src/plugins/channel-presence-policy.ts +++ b/src/plugins/channel-presence-policy.ts @@ -599,6 +599,7 @@ export function resolveConfiguredChannelPluginIds(params: { activationSourceConfig?: OpenClawConfig; workspaceDir?: string; env: NodeJS.ProcessEnv; + manifestRecords?: readonly PluginManifestRecord[]; }): string[] { const configuredChannelIds = normalizeChannelIds([ ...listConfiguredChannelIdsForReadOnlyScope({ @@ -606,6 +607,7 @@ export function resolveConfiguredChannelPluginIds(params: { activationSourceConfig: params.activationSourceConfig, workspaceDir: params.workspaceDir, env: params.env, + manifestRecords: params.manifestRecords, }), ...listExplicitConfiguredChannelIdsForConfig(params.activationSourceConfig ?? params.config), ]); diff --git a/src/plugins/effective-plugin-ids.ts b/src/plugins/effective-plugin-ids.ts index 9e72ec492c2b..874a8fc9f744 100644 --- a/src/plugins/effective-plugin-ids.ts +++ b/src/plugins/effective-plugin-ids.ts @@ -16,6 +16,8 @@ import { import { normalizePluginsConfig } from "./config-state.js"; import { loadManifestMetadataSnapshot } from "./manifest-contract-eligibility.js"; import { passesManifestOwnerBasePolicy } from "./manifest-owner-policy.js"; +import type { PluginManifestRecord } from "./manifest-registry.js"; +import type { PluginMetadataSnapshot } from "./plugin-metadata-snapshot.types.js"; import { defaultSlotIdForKey } from "./slots.js"; function collectConfiguredChannelIds( @@ -48,6 +50,7 @@ function collectBundledChannelOwnerPluginIds(params: { env: NodeJS.ProcessEnv; workspaceDir?: string; bundledPluginsDir?: string; + manifestRecords?: readonly PluginManifestRecord[]; }): string[] { const plugins = normalizePluginsConfig(params.config.plugins); const channelIds = new Set( @@ -67,13 +70,15 @@ function collectBundledChannelOwnerPluginIds(params: { : {}), } : params.env; - const snapshot = loadManifestMetadataSnapshot({ - config: params.config, - env, - workspaceDir: params.workspaceDir, - }); + const records = + params.manifestRecords ?? + loadManifestMetadataSnapshot({ + config: params.config, + env, + workspaceDir: params.workspaceDir, + }).plugins; const pluginIds = new Set(); - for (const plugin of snapshot.plugins) { + for (const plugin of records) { if (plugin.origin !== "bundled") { continue; } @@ -148,10 +153,22 @@ export function resolveEffectivePluginIds(params: { env: NodeJS.ProcessEnv; workspaceDir?: string; bundledPluginsDir?: string; + /** Prepared metadata for this invocation. Without it every lookup below rebuilds + * the registry, so callers that already hold a snapshot must pass it. */ + metadataSnapshot?: PluginMetadataSnapshot; }): string[] { + // Effective ids are a whole-config question. A plugin-scoped snapshot only carries + // its own manifests, and a bundled-plugins-dir override rewrites the discovery env, + // so neither can answer it — those callers keep re-deriving. + const prepared = + params.bundledPluginsDir || params.metadataSnapshot?.pluginIds + ? undefined + : params.metadataSnapshot; const autoEnabled = applyPluginAutoEnable({ config: params.config, env: params.env, + ...(prepared ? { manifestRegistry: prepared.manifestRegistry } : {}), + ...(prepared?.discovery ? { discovery: prepared.discovery } : {}), }); const effectiveConfig = autoEnabled.config; const ids = new Set(collectExplicitEffectivePluginIds(effectiveConfig)); @@ -168,6 +185,7 @@ export function resolveEffectivePluginIds(params: { activationSourceConfig: params.config, workspaceDir: params.workspaceDir, env: params.env, + manifestRecords: prepared?.plugins, })) { ids.add(pluginId); } @@ -176,6 +194,7 @@ export function resolveEffectivePluginIds(params: { channelIds: configuredChannelIds, env: params.env, workspaceDir: params.workspaceDir, + manifestRecords: prepared?.plugins, ...(params.bundledPluginsDir ? { bundledPluginsDir: params.bundledPluginsDir } : {}), })) { ids.add(pluginId); @@ -185,6 +204,7 @@ export function resolveEffectivePluginIds(params: { activationSourceConfig: params.config, workspaceDir: params.workspaceDir, env: params.env, + ...(prepared ? { metadataSnapshot: prepared } : {}), }).pluginIds) { ids.add(pluginId); } diff --git a/src/plugins/official-external-plugin-repair-hints.ts b/src/plugins/official-external-plugin-repair-hints.ts index 5d331d70f8fb..63c5c509f6a6 100644 --- a/src/plugins/official-external-plugin-repair-hints.ts +++ b/src/plugins/official-external-plugin-repair-hints.ts @@ -1,6 +1,7 @@ /** Builds doctor/install repair hints for missing official external plugin owners. */ import type { OpenClawConfig } from "../config/types.openclaw.js"; import { resolveConfiguredChannelPresencePolicy } from "./channel-plugin-ids.js"; +import type { PluginManifestRecord } from "./manifest-registry.js"; import { getOfficialExternalPluginCatalogEntry, getOfficialExternalPluginCatalogManifest, @@ -60,6 +61,9 @@ export function resolveMissingOfficialExternalChannelPluginRepairHint(params: { channelId: string; workspaceDir?: string; env?: NodeJS.ProcessEnv; + /** Prepared manifest facts. Callers resolving many channels must pass these, or + * presence policy rebuilds the whole manifest registry once per channel. */ + manifestRecords?: readonly PluginManifestRecord[]; }): OfficialExternalPluginRepairHint | null { const hint = resolveOfficialExternalPluginRepairHint(params.channelId); if (!hint?.channelId || hint.channelId !== params.channelId) { @@ -71,6 +75,7 @@ export function resolveMissingOfficialExternalChannelPluginRepairHint(params: { workspaceDir: params.workspaceDir, env: params.env, includePersistedAuthState: false, + manifestRecords: params.manifestRecords, }).find((entry) => entry.channelId === hint.channelId); if (!policy || policy.effective) { return null; diff --git a/src/plugins/status-effective-plugin-discovery.test.ts b/src/plugins/status-effective-plugin-discovery.test.ts new file mode 100644 index 000000000000..73f475a4f9cf --- /dev/null +++ b/src/plugins/status-effective-plugin-discovery.test.ts @@ -0,0 +1,135 @@ +// `plugins doctor` reports effective-only plugins from the metadata snapshot it already +// built, so asking for effective ids must not re-derive plugin discovery. +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { afterAll, beforeEach, expect, it, vi } from "vitest"; +import type { OpenClawConfig } from "../config/types.openclaw.js"; +import type { PluginMetadataSnapshot } from "./plugin-metadata-snapshot.types.js"; +import { createColdPluginFixture } from "./test-helpers/cold-plugin-fixtures.js"; + +const counters = vi.hoisted(() => ({ manifestRegistryRebuilds: 0, discoveryScans: 0 })); + +vi.mock("./plugin-registry-contributions.js", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + loadPluginManifestRegistryForPluginRegistry: ( + ...args: Parameters + ) => { + counters.manifestRegistryRebuilds += 1; + return actual.loadPluginManifestRegistryForPluginRegistry(...args); + }, + }; +}); + +vi.mock("./discovery.js", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + discoverOpenClawPlugins: (...args: Parameters) => { + counters.discoveryScans += 1; + return actual.discoverOpenClawPlugins(...args); + }, + }; +}); + +const { buildPluginDiagnosticsReport } = await import("./status.js"); +const { resolveEffectivePluginIds } = await import("./effective-plugin-ids.js"); +const { loadPluginMetadataSnapshot } = await import("./plugin-metadata-snapshot.js"); + +const tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-effective-plugin-ids-")); + +function coldPluginRoot(pluginId: string, channelId: string): string { + const rootDir = path.join(tempRoot, pluginId); + fs.mkdirSync(rootDir, { recursive: true }); + createColdPluginFixture({ + rootDir, + pluginId, + channelId, + providerId: `${pluginId}-provider`, + packageName: `@example/${pluginId}`, + }); + return rootDir; +} + +const channelOwnerRoot = coldPluginRoot("cold-plugin", "cold-channel"); +const otherRoot = coldPluginRoot("other-plugin", "other-channel"); + +const config: OpenClawConfig = { + channels: { "cold-channel": { enabled: true } }, + plugins: { + load: { paths: [channelOwnerRoot, otherRoot] }, + entries: { "cold-plugin": { enabled: true }, "other-plugin": { enabled: true } }, + }, +} as OpenClawConfig; + +function countReport(params: { effectiveOnly: boolean; onlyPluginIds?: readonly string[] }): { + rebuilds: number; + scans: number; + ids: string[]; +} { + counters.manifestRegistryRebuilds = 0; + counters.discoveryScans = 0; + const report = buildPluginDiagnosticsReport({ config, env: process.env, ...params }); + return { + rebuilds: counters.manifestRegistryRebuilds, + scans: counters.discoveryScans, + ids: report.plugins.map((plugin) => plugin.id).toSorted(), + }; +} + +function countResolve(metadataSnapshot: PluginMetadataSnapshot): { + rebuilds: number; + ids: string[]; +} { + counters.manifestRegistryRebuilds = 0; + const ids = resolveEffectivePluginIds({ config, env: process.env, metadataSnapshot }); + return { rebuilds: counters.manifestRegistryRebuilds, ids }; +} + +beforeEach(() => { + vi.stubEnv("OPENCLAW_DISABLE_BUNDLED_PLUGINS", "1"); + vi.stubEnv("OPENCLAW_HOME", path.join(tempRoot, "home")); + vi.stubEnv("OPENCLAW_STATE_DIR", path.join(tempRoot, "state")); +}); + +afterAll(() => { + fs.rmSync(tempRoot, { recursive: true, force: true }); + vi.unstubAllEnvs(); +}); + +it("does not re-derive discovery when reporting effective-only plugins", () => { + const all = countReport({ effectiveOnly: false }); + const effective = countReport({ effectiveOnly: true }); + + // The effective-only filter still selects the configured channel owner. + expect(effective.ids).toEqual(["cold-plugin", "other-plugin"]); + // ...and it reuses the snapshot the report already built instead of rediscovering. + expect({ + rebuilds: effective.rebuilds, + scansMoreThanFullReport: effective.scans > all.scans, + }).toEqual({ rebuilds: 0, scansMoreThanFullReport: false }); +}); + +// A supplied snapshot is an optimization, never an input to the answer. +it("only reuses a snapshot that answers for the whole config", () => { + const env = process.env; + const withoutSnapshot = resolveEffectivePluginIds({ config, env }); + const full = countResolve(loadPluginMetadataSnapshot({ config, env })); + // `recordPluginInstallSource` asks for one plugin's effective state, which scopes the + // snapshot to that plugin and truncates its manifest set to that plugin alone. + const scoped = countResolve( + loadPluginMetadataSnapshot({ config, env, pluginIds: ["other-plugin"] }), + ); + + expect({ full: full.ids, scoped: scoped.ids }).toEqual({ + full: withoutSnapshot, + scoped: withoutSnapshot, + }); + // A whole-config snapshot is reused; a plugin-scoped one cannot stand in for it. + expect({ fullReused: full.rebuilds === 0, scopedReused: scoped.rebuilds === 0 }).toEqual({ + fullReused: true, + scopedReused: false, + }); +}); diff --git a/src/plugins/status.ts b/src/plugins/status.ts index 385cfc15d9a2..b7cb467086cd 100644 --- a/src/plugins/status.ts +++ b/src/plugins/status.ts @@ -268,6 +268,7 @@ function buildPluginReport( config: rawConfig, workspaceDir, env: params?.env ?? process.env, + metadataSnapshot, }) : params?.onlyPluginIds === undefined ? undefined