mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-27 21:07:01 -06:00
perf(cli): carry prepared plugin metadata through cold read paths (#118460)
* perf(cli): carry prepared plugin metadata through cold read paths * perf(plugins): thread prepared metadata into auto-enable detection --------- Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
@@ -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<typeof import("../plugins/plugin-registry-contributions.js")>();
|
||||
return {
|
||||
...actual,
|
||||
loadPluginManifestRegistryForPluginRegistry: (
|
||||
...args: Parameters<typeof actual.loadPluginManifestRegistryForPluginRegistry>
|
||||
) => {
|
||||
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 });
|
||||
});
|
||||
@@ -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" } },
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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");
|
||||
|
||||
@@ -281,6 +281,7 @@ export async function channelsListCommand(
|
||||
config: cfg,
|
||||
channelId: entry.id,
|
||||
...(workspaceDir ? { workspaceDir } : {}),
|
||||
manifestRecords: metadataSnapshot.plugins,
|
||||
});
|
||||
return {
|
||||
entry,
|
||||
|
||||
@@ -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),
|
||||
]);
|
||||
|
||||
@@ -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<string>();
|
||||
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);
|
||||
}
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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<typeof import("./plugin-registry-contributions.js")>();
|
||||
return {
|
||||
...actual,
|
||||
loadPluginManifestRegistryForPluginRegistry: (
|
||||
...args: Parameters<typeof actual.loadPluginManifestRegistryForPluginRegistry>
|
||||
) => {
|
||||
counters.manifestRegistryRebuilds += 1;
|
||||
return actual.loadPluginManifestRegistryForPluginRegistry(...args);
|
||||
},
|
||||
};
|
||||
});
|
||||
|
||||
vi.mock("./discovery.js", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import("./discovery.js")>();
|
||||
return {
|
||||
...actual,
|
||||
discoverOpenClawPlugins: (...args: Parameters<typeof actual.discoverOpenClawPlugins>) => {
|
||||
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,
|
||||
});
|
||||
});
|
||||
@@ -268,6 +268,7 @@ function buildPluginReport(
|
||||
config: rawConfig,
|
||||
workspaceDir,
|
||||
env: params?.env ?? process.env,
|
||||
metadataSnapshot,
|
||||
})
|
||||
: params?.onlyPluginIds === undefined
|
||||
? undefined
|
||||
|
||||
Reference in New Issue
Block a user