mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-26 20:35:39 -06:00
fix(plugins): reuse published metadata on hot paths (#127355)
This commit is contained in:
committed by
GitHub
parent
ccbfa6c3a3
commit
cddc1acdf7
@@ -1,11 +1,15 @@
|
||||
// Read-only command default tests cover command defaulting for read-only channel plugins.
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { STATE_DIR } from "../../config/paths.js";
|
||||
|
||||
const loadPluginMetadataSnapshot = vi.hoisted(() => vi.fn());
|
||||
const { loadPluginMetadataSnapshot, resolvePluginMetadataSnapshot } = vi.hoisted(() => ({
|
||||
loadPluginMetadataSnapshot: vi.fn(),
|
||||
resolvePluginMetadataSnapshot: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("../../plugins/plugin-metadata-snapshot.js", () => ({
|
||||
loadPluginMetadataSnapshot,
|
||||
resolvePluginMetadataSnapshot: loadPluginMetadataSnapshot,
|
||||
resolvePluginMetadataSnapshot,
|
||||
}));
|
||||
|
||||
import { resolveReadOnlyChannelCommandDefaults } from "./read-only-command-defaults.js";
|
||||
@@ -13,10 +17,31 @@ import { resolveReadOnlyChannelCommandDefaults } from "./read-only-command-defau
|
||||
describe("resolveReadOnlyChannelCommandDefaults", () => {
|
||||
beforeEach(() => {
|
||||
loadPluginMetadataSnapshot.mockReset();
|
||||
resolvePluginMetadataSnapshot.mockReset();
|
||||
loadPluginMetadataSnapshot.mockReturnValue({
|
||||
index: { plugins: [] },
|
||||
plugins: [],
|
||||
});
|
||||
resolvePluginMetadataSnapshot.mockImplementation(loadPluginMetadataSnapshot);
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ name: "process", env: process.env, stateDir: STATE_DIR },
|
||||
{
|
||||
name: "custom",
|
||||
env: { HOME: "/home/demo", OPENCLAW_STATE_DIR: "/state" },
|
||||
stateDir: "/state",
|
||||
},
|
||||
])("reuses published metadata for the $name state root without scanning", ({ env, stateDir }) => {
|
||||
const currentSnapshot = { index: { plugins: [] }, plugins: [] };
|
||||
resolvePluginMetadataSnapshot.mockImplementation((params) =>
|
||||
params.stateDir === undefined ? currentSnapshot : loadPluginMetadataSnapshot(params),
|
||||
);
|
||||
|
||||
expect(resolveReadOnlyChannelCommandDefaults("demo", { config: {}, env, stateDir })).toBe(
|
||||
undefined,
|
||||
);
|
||||
expect(loadPluginMetadataSnapshot).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("resolves command defaults from the shared metadata snapshot", () => {
|
||||
|
||||
@@ -4,6 +4,7 @@
|
||||
* Reads native command/skill defaults from installed plugin manifests without loading plugins.
|
||||
*/
|
||||
import { normalizeOptionalString } from "@openclaw/normalization-core/string-coerce";
|
||||
import { resolveStateDir, STATE_DIR } from "../../config/paths.js";
|
||||
import type { OpenClawConfig } from "../../config/types.openclaw.js";
|
||||
import { isBlockedObjectKey } from "../../infra/prototype-keys.js";
|
||||
import { isInstalledPluginEnabled } from "../../plugins/installed-plugin-index.js";
|
||||
@@ -87,7 +88,11 @@ export function resolveReadOnlyChannelCommandDefaults(
|
||||
const env = options.env ?? process.env;
|
||||
const resolvedSnapshot = resolvePluginMetadataSnapshot({
|
||||
config: options.config,
|
||||
stateDir: options.stateDir,
|
||||
stateDir:
|
||||
options.stateDir !== undefined &&
|
||||
options.stateDir === (env === process.env ? STATE_DIR : resolveStateDir(env))
|
||||
? undefined
|
||||
: options.stateDir,
|
||||
workspaceDir: options.workspaceDir,
|
||||
env,
|
||||
allowWorkspaceScopedCurrent: true,
|
||||
|
||||
@@ -3,8 +3,10 @@ import fs from "node:fs";
|
||||
import fsp from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it } from "vitest";
|
||||
import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import type { OpenClawConfig } from "../config/config.js";
|
||||
import { setCurrentPluginMetadataSnapshot } from "../plugins/current-plugin-metadata-snapshot.js";
|
||||
import { loadPluginMetadataSnapshot } from "../plugins/plugin-metadata-snapshot.js";
|
||||
import {
|
||||
clearInternalHooks,
|
||||
createInternalHookEvent,
|
||||
@@ -12,6 +14,7 @@ import {
|
||||
triggerInternalHook,
|
||||
} from "./internal-hooks.js";
|
||||
import { loadInternalHooks } from "./loader.js";
|
||||
import { resolvePluginHookDirs } from "./plugin-hooks.js";
|
||||
import { loadWorkspaceHookEntries } from "./workspace.js";
|
||||
|
||||
describe("bundle plugin hooks", () => {
|
||||
@@ -34,6 +37,8 @@ describe("bundle plugin hooks", () => {
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
setCurrentPluginMetadataSnapshot(undefined);
|
||||
vi.restoreAllMocks();
|
||||
clearInternalHooks();
|
||||
setInternalHooksEnabled(true);
|
||||
if (previousBundledHooksDir === undefined) {
|
||||
@@ -125,6 +130,24 @@ describe("bundle plugin hooks", () => {
|
||||
expect(entry.metadata?.events).toEqual(["command:new"]);
|
||||
});
|
||||
|
||||
it("reuses published plugin metadata without rescanning manifests", async () => {
|
||||
const bundleRoot = await writeBundleHookFixture();
|
||||
const config = createConfig(true);
|
||||
const snapshot = loadPluginMetadataSnapshot({ config, workspaceDir, env: process.env });
|
||||
setCurrentPluginMetadataSnapshot(snapshot, { config, workspaceDir });
|
||||
const hookDir = fs.realpathSync.native(path.join(bundleRoot, "hooks"));
|
||||
const manifestRegistry = await import("../plugins/manifest-registry-installed.js");
|
||||
const scanManifests = vi.spyOn(manifestRegistry, "loadPluginManifestRegistryForInstalledIndex");
|
||||
|
||||
for (let iteration = 0; iteration < 2; iteration += 1) {
|
||||
expect(resolvePluginHookDirs({ workspaceDir, config })).toEqual([
|
||||
{ dir: hookDir, pluginId: "sample-bundle" },
|
||||
]);
|
||||
}
|
||||
|
||||
expect(scanManifests).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("loads and executes enabled bundle hooks through the internal hook loader", async () => {
|
||||
await writeBundleHookFixture();
|
||||
|
||||
|
||||
@@ -8,7 +8,7 @@ import {
|
||||
resolvePolicyPluginActivationState,
|
||||
} from "../plugins/config-policy.js";
|
||||
import { resolveMemorySlotDecision } from "../plugins/config-state.js";
|
||||
import { loadPluginMetadataSnapshot } from "../plugins/plugin-metadata-snapshot.js";
|
||||
import { resolvePluginMetadataSnapshot } from "../plugins/plugin-metadata-snapshot.js";
|
||||
import { hasKind } from "../plugins/slots.js";
|
||||
import { isPathInsideWithRealpath } from "../security/scan-paths.js";
|
||||
|
||||
@@ -28,9 +28,9 @@ export function resolvePluginHookDirs(params: {
|
||||
if (!workspaceDir) {
|
||||
return [];
|
||||
}
|
||||
const metadataSnapshot = loadPluginMetadataSnapshot({
|
||||
const metadataSnapshot = resolvePluginMetadataSnapshot({
|
||||
workspaceDir,
|
||||
config: params.config ?? {},
|
||||
config: params.config,
|
||||
env: process.env,
|
||||
});
|
||||
const registry = metadataSnapshot.manifestRegistry;
|
||||
|
||||
@@ -2269,6 +2269,47 @@ describe("resolvePluginTools optional tools", () => {
|
||||
expect(factory).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("executes cached plugin tools from the published generation without rescanning manifests", async () => {
|
||||
const context = createContext();
|
||||
const runtimeConfig: OpenClawConfig = {
|
||||
...context.config,
|
||||
channels: { telegram: { enabled: false } },
|
||||
};
|
||||
const getRuntimeConfig = vi.fn(() => runtimeConfig);
|
||||
const toolContext = { ...context, getRuntimeConfig };
|
||||
const factory = vi.fn(() => makeTool("cached_generation_tool"));
|
||||
setRegistry(
|
||||
[
|
||||
createNamedToolEntry("cache-generation", "cached_generation_tool", {
|
||||
factory,
|
||||
}),
|
||||
],
|
||||
context.config,
|
||||
);
|
||||
|
||||
resolvePluginTools(createResolveToolsParams({ context: toolContext }));
|
||||
const [cachedTool] = resolvePluginTools(createResolveToolsParams({ context: toolContext }));
|
||||
expect(cachedTool?.name).toBe("cached_generation_tool");
|
||||
expect(getRuntimeConfig).toHaveBeenCalledTimes(2);
|
||||
|
||||
const manifestRegistry = await import("./manifest-registry-installed.js");
|
||||
const manifestScan = vi
|
||||
.spyOn(manifestRegistry, "loadPluginManifestRegistryForInstalledIndex")
|
||||
.mockImplementation(() => {
|
||||
throw new Error("cached plugin execution rescanned manifests");
|
||||
});
|
||||
try {
|
||||
await expect(cachedTool?.execute("call", {}, undefined)).resolves.toEqual({
|
||||
content: [{ type: "text", text: "ok" }],
|
||||
});
|
||||
expect(manifestScan).not.toHaveBeenCalled();
|
||||
expect(getRuntimeConfig).toHaveBeenCalledTimes(2);
|
||||
expect(factory).toHaveBeenCalledTimes(2);
|
||||
} finally {
|
||||
manifestScan.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
it("keeps cached ordinary plugin tools free of network provenance", async () => {
|
||||
const factory = vi.fn(() => makeTool("cached_ordinary_tool"));
|
||||
setRegistry([
|
||||
|
||||
+6
-25
@@ -31,6 +31,7 @@ import {
|
||||
} from "./manifest-contract-eligibility.js";
|
||||
import type { PluginManifestRecord } from "./manifest-registry.js";
|
||||
import { hasManifestToolAvailability } from "./manifest-tool-availability.js";
|
||||
import { resolvePluginMetadataSnapshot } from "./plugin-metadata-snapshot.js";
|
||||
import type { PluginMetadataManifestView } from "./plugin-metadata-snapshot.types.js";
|
||||
import type { PluginRegistry, PluginToolRegistration } from "./registry-types.js";
|
||||
import {
|
||||
@@ -292,26 +293,6 @@ function blocksHostRestrictedConversationReadRegistration(params: {
|
||||
);
|
||||
}
|
||||
|
||||
function resolveCurrentManifestPlugin(params: {
|
||||
pluginId: string;
|
||||
ctx: OpenClawPluginToolContext;
|
||||
loadContext: ReturnType<typeof resolvePluginRuntimeLoadContext>;
|
||||
}): PluginManifestRecord | undefined {
|
||||
let config = params.ctx.runtimeConfig ?? params.ctx.config ?? params.loadContext.config;
|
||||
if (params.ctx.getRuntimeConfig) {
|
||||
try {
|
||||
config = params.ctx.getRuntimeConfig() ?? config;
|
||||
} catch {
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
return loadManifestContractSnapshot({
|
||||
config,
|
||||
workspaceDir: params.loadContext.workspaceDir,
|
||||
env: params.loadContext.env,
|
||||
}).plugins.find((plugin) => plugin.id === params.pluginId);
|
||||
}
|
||||
|
||||
/**
|
||||
* Builds a collision-proof key for plugin-owned tool metadata lookups.
|
||||
*/
|
||||
@@ -876,11 +857,11 @@ function createCachedDescriptorPluginTool(params: {
|
||||
const resolveCandidateTool = (
|
||||
candidate: PluginToolRegistration,
|
||||
): AnyAgentTool | undefined => {
|
||||
const manifestPlugin = resolveCurrentManifestPlugin({
|
||||
pluginId,
|
||||
ctx: params.ctx,
|
||||
loadContext: params.loadContext,
|
||||
});
|
||||
const manifestPlugin = resolvePluginMetadataSnapshot({
|
||||
config: params.loadContext.config,
|
||||
workspaceDir: params.loadContext.workspaceDir,
|
||||
env: params.loadContext.env,
|
||||
}).byPluginId.get(pluginId);
|
||||
if (
|
||||
blocksHostRestrictedConversationReadRegistration({
|
||||
entry: candidate,
|
||||
|
||||
@@ -18,14 +18,16 @@ const pluginRegistryMocks = vi.hoisted(() => {
|
||||
plugins: [],
|
||||
diagnostics: [],
|
||||
}));
|
||||
const loadPluginMetadataSnapshot = vi.fn((_params?: unknown) => ({
|
||||
plugins: loadManifestRegistry().plugins,
|
||||
manifestRegistry: loadManifestRegistry(),
|
||||
}));
|
||||
return {
|
||||
loadPluginManifestRegistryForInstalledIndex: loadManifestRegistry,
|
||||
loadPluginManifestRegistryForPluginRegistry: loadManifestRegistry,
|
||||
loadPluginRegistrySnapshot: vi.fn(() => ({ plugins: [] })),
|
||||
loadPluginMetadataSnapshot: vi.fn(() => ({
|
||||
plugins: loadManifestRegistry().plugins,
|
||||
manifestRegistry: loadManifestRegistry(),
|
||||
})),
|
||||
loadPluginMetadataSnapshot,
|
||||
resolvePluginMetadataSnapshot: vi.fn((params: unknown) => loadPluginMetadataSnapshot(params)),
|
||||
};
|
||||
});
|
||||
|
||||
@@ -42,6 +44,7 @@ vi.mock("../plugins/plugin-registry.js", () => ({
|
||||
|
||||
vi.mock("../plugins/plugin-metadata-snapshot.js", () => ({
|
||||
loadPluginMetadataSnapshot: pluginRegistryMocks.loadPluginMetadataSnapshot,
|
||||
resolvePluginMetadataSnapshot: pluginRegistryMocks.resolvePluginMetadataSnapshot,
|
||||
}));
|
||||
|
||||
describe("channel env vars dynamic package metadata", () => {
|
||||
@@ -55,6 +58,10 @@ describe("channel env vars dynamic package metadata", () => {
|
||||
pluginRegistryMocks.loadPluginRegistrySnapshot.mockReset();
|
||||
pluginRegistryMocks.loadPluginRegistrySnapshot.mockReturnValue({ plugins: [] });
|
||||
pluginRegistryMocks.loadPluginMetadataSnapshot.mockClear();
|
||||
pluginRegistryMocks.resolvePluginMetadataSnapshot.mockReset();
|
||||
pluginRegistryMocks.resolvePluginMetadataSnapshot.mockImplementation((params) =>
|
||||
pluginRegistryMocks.loadPluginMetadataSnapshot(params),
|
||||
);
|
||||
});
|
||||
|
||||
it("includes later-installed plugin env vars without a bundled generated map", async () => {
|
||||
@@ -81,4 +88,31 @@ describe("channel env vars dynamic package metadata", () => {
|
||||
expect(knownNames).toContain("MATTERMOST_BOT_TOKEN");
|
||||
expect(knownNames).toContain("MATTERMOST_URL");
|
||||
});
|
||||
|
||||
it("reuses published channel metadata without rescanning manifests", async () => {
|
||||
pluginRegistryMocks.resolvePluginMetadataSnapshot.mockReturnValue({
|
||||
plugins: [
|
||||
{
|
||||
id: "external-mattermost",
|
||||
origin: "global",
|
||||
packageChannel: {
|
||||
id: "mattermost",
|
||||
configuredState: { env: { anyOf: ["MATTERMOST_BOT_TOKEN"] } },
|
||||
},
|
||||
},
|
||||
],
|
||||
manifestRegistry: { plugins: [], diagnostics: [] },
|
||||
});
|
||||
const mod = await import("./channel-env-vars.js");
|
||||
|
||||
expect(mod.getChannelEnvVars("mattermost")).toEqual(["MATTERMOST_BOT_TOKEN"]);
|
||||
expect(mod.listKnownChannelEnvVarNames()).toEqual(["MATTERMOST_BOT_TOKEN"]);
|
||||
expect(pluginRegistryMocks.loadPluginMetadataSnapshot).not.toHaveBeenCalled();
|
||||
expect(pluginRegistryMocks.resolvePluginMetadataSnapshot).toHaveBeenCalledTimes(2);
|
||||
expect(pluginRegistryMocks.resolvePluginMetadataSnapshot).toHaveBeenCalledWith({
|
||||
config: undefined,
|
||||
workspaceDir: undefined,
|
||||
env: process.env,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
/** Discovers plugin-declared environment variable names for channel credential setup. */
|
||||
import { uniqueStrings } from "@openclaw/normalization-core/string-normalization";
|
||||
import type { OpenClawConfig } from "../config/types.openclaw.js";
|
||||
import { loadPluginMetadataSnapshot } from "../plugins/plugin-metadata-snapshot.js";
|
||||
import { resolvePluginMetadataSnapshot } from "../plugins/plugin-metadata-snapshot.js";
|
||||
import { appendUniqueEnvVarCandidates } from "../shared/env-var-candidates.js";
|
||||
|
||||
type ChannelEnvVarLookupParams = {
|
||||
@@ -20,8 +20,8 @@ type ChannelEnvVarLookupParams = {
|
||||
function resolveChannelEnvVars(
|
||||
params?: ChannelEnvVarLookupParams,
|
||||
): Record<string, readonly string[]> {
|
||||
const snapshot = loadPluginMetadataSnapshot({
|
||||
config: params?.config ?? {},
|
||||
const snapshot = resolvePluginMetadataSnapshot({
|
||||
config: params?.config,
|
||||
workspaceDir: params?.workspaceDir,
|
||||
env: params?.env ?? process.env,
|
||||
});
|
||||
|
||||
@@ -302,6 +302,24 @@ describe("resolvePluginSkillDirs", () => {
|
||||
expect(hoisted.loadPluginMetadataSnapshot).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("preserves absent config when resolving current lifecycle metadata", async () => {
|
||||
const workspaceDir = await tempDirs.make("openclaw-");
|
||||
const manifestRegistry: PluginManifestRegistry = { diagnostics: [], plugins: [] };
|
||||
const metadataSnapshot = {
|
||||
manifestRegistry,
|
||||
plugins: manifestRegistry.plugins,
|
||||
normalizePluginId: (pluginId: string) => pluginId,
|
||||
};
|
||||
hoisted.resolvePluginMetadataSnapshot.mockImplementationOnce((params: unknown) =>
|
||||
(params as { config?: OpenClawConfig }).config === undefined
|
||||
? metadataSnapshot
|
||||
: hoisted.loadPluginMetadataSnapshot(params),
|
||||
);
|
||||
|
||||
expect(resolvePluginSkillDirs({ workspaceDir })).toEqual([]);
|
||||
expect(hoisted.loadPluginMetadataSnapshot).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it.each([
|
||||
{
|
||||
name: "unavailable to available",
|
||||
|
||||
@@ -50,10 +50,9 @@ export function resolvePluginSkillDirs(params: {
|
||||
});
|
||||
return [];
|
||||
}
|
||||
const config = params.config ?? {};
|
||||
const metadataSnapshot = resolvePluginMetadataSnapshot({
|
||||
workspaceDir,
|
||||
config,
|
||||
config: params.config,
|
||||
env: process.env,
|
||||
});
|
||||
return resolvePluginSkillDirsFromMetadata({ ...params, metadataSnapshot });
|
||||
|
||||
Reference in New Issue
Block a user