perf(skills): reuse plugin metadata for watcher refresh (#127088)

Amp-Thread-ID: https://ampcode.com/threads/T-01a021f5-984a-7628-a30c-491c166ff247

Co-authored-by: Amp <amp@ampcode.com>
This commit is contained in:
Peter Steinberger
2026-08-21 01:06:23 -07:00
committed by GitHub
parent fd3a919063
commit dfce5f6958
6 changed files with 88 additions and 8 deletions
+31 -2
View File
@@ -13,7 +13,7 @@ import { createTrackedTempDirs } from "../../test-utils/tracked-temp-dirs.js";
const hoisted = vi.hoisted(() => {
const loadManifestRegistry = vi.fn();
const loadPluginMetadataSnapshot = vi.fn(() => {
const loadPluginMetadataSnapshot = vi.fn((_params?: unknown) => {
const manifestRegistry = loadManifestRegistry();
return {
manifestRegistry,
@@ -24,10 +24,14 @@ const hoisted = vi.hoisted(() => {
)?.id ?? pluginId,
};
});
const resolvePluginMetadataSnapshot = vi.fn((params: unknown) =>
loadPluginMetadataSnapshot(params),
);
return {
loadPluginManifestRegistryForInstalledIndex: loadManifestRegistry,
loadPluginManifestRegistryForPluginRegistry: loadManifestRegistry,
loadPluginMetadataSnapshot,
resolvePluginMetadataSnapshot,
loadPluginRegistrySnapshot: vi.fn(() => ({ plugins: [] })),
};
});
@@ -43,7 +47,7 @@ vi.mock("../../plugins/plugin-registry.js", () => ({
vi.mock("../../plugins/plugin-metadata-snapshot.js", () => ({
loadPluginMetadataSnapshot: hoisted.loadPluginMetadataSnapshot,
resolvePluginMetadataSnapshot: hoisted.loadPluginMetadataSnapshot,
resolvePluginMetadataSnapshot: hoisted.resolvePluginMetadataSnapshot,
}));
let resolvePluginSkillDirs: typeof import("./plugin-skills.js").resolvePluginSkillDirs;
@@ -182,6 +186,7 @@ function registerHealthyAcpBackend() {
afterEach(async () => {
hoisted.loadPluginManifestRegistryForInstalledIndex.mockReset();
hoisted.loadPluginMetadataSnapshot.mockClear();
hoisted.resolvePluginMetadataSnapshot.mockClear();
hoisted.loadPluginRegistrySnapshot.mockReset();
acpRuntimeTesting.resetAcpRuntimeBackendsForTests();
await tempDirs.cleanup();
@@ -221,6 +226,7 @@ describe("resolvePluginSkillDirs", () => {
plugins: [],
});
hoisted.loadPluginMetadataSnapshot.mockClear();
hoisted.resolvePluginMetadataSnapshot.mockClear();
hoisted.loadPluginRegistrySnapshot.mockReset();
hoisted.loadPluginRegistrySnapshot.mockReturnValue({ plugins: [] });
});
@@ -273,6 +279,29 @@ describe("resolvePluginSkillDirs", () => {
expect(dirs).toEqual(expectedDirs({ acpxRoot, helperRoot }));
});
it("reuses current lifecycle metadata before falling back to a cold load", async () => {
const { workspaceDir, acpxRoot, helperRoot } = await setupAcpxAndHelperRegistry();
registerHealthyAcpBackend();
const manifestRegistry = buildRegistry({ acpxRoot, helperRoot });
hoisted.resolvePluginMetadataSnapshot.mockReturnValueOnce({
manifestRegistry,
plugins: manifestRegistry.plugins,
normalizePluginId: (pluginId: string) => pluginId,
});
const dirs = resolvePluginSkillDirs({
workspaceDir,
config: {
acp: { enabled: true },
plugins: { entries: { acpx: { enabled: true }, helper: { enabled: true } } },
} as OpenClawConfig,
});
expect(dirs).toEqual([path.resolve(acpxRoot, "skills"), path.resolve(helperRoot, "skills")]);
expect(hoisted.resolvePluginMetadataSnapshot).toHaveBeenCalledOnce();
expect(hoisted.loadPluginMetadataSnapshot).not.toHaveBeenCalled();
});
it.each([
{
name: "unavailable to available",
+2 -2
View File
@@ -12,7 +12,7 @@ import {
} from "../../plugins/config-policy.js";
import { resolveMemorySlotDecision } from "../../plugins/config-state.js";
import { registerPluginMetadataProcessMemoLifecycleClear } from "../../plugins/plugin-metadata-lifecycle.js";
import { loadPluginMetadataSnapshot } from "../../plugins/plugin-metadata-snapshot.js";
import { resolvePluginMetadataSnapshot } from "../../plugins/plugin-metadata-snapshot.js";
import type { PluginMetadataSnapshot } from "../../plugins/plugin-metadata-snapshot.types.js";
import { hasKind } from "../../plugins/slots.js";
import { isPathInsideWithRealpath } from "../../security/scan-paths.js";
@@ -51,7 +51,7 @@ export function resolvePluginSkillDirs(params: {
return [];
}
const config = params.config ?? {};
const metadataSnapshot = loadPluginMetadataSnapshot({
const metadataSnapshot = resolvePluginMetadataSnapshot({
workspaceDir,
config,
env: process.env,
+33 -1
View File
@@ -3,6 +3,7 @@ import fs from "node:fs/promises";
import os from "node:os";
import path from "node:path";
import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
import type { PluginMetadataSnapshot } from "../../plugins/plugin-metadata-snapshot.types.js";
import { withEnvAsync } from "../../test-utils/env.js";
import {
bumpSkillsSnapshotVersion,
@@ -38,6 +39,10 @@ const watchMock = vi.fn(() => {
createdWatchers.push(watcher);
return watcher;
});
const pluginSkillsMocks = vi.hoisted(() => ({
resolvePluginSkillDirs: vi.fn((): string[] => []),
resolvePluginSkillDirsFromMetadata: vi.fn((): string[] => []),
}));
let refreshModule: typeof import("./refresh.js");
let refreshTestSupport: typeof import("./refresh.test-support.js");
@@ -47,7 +52,8 @@ vi.mock("chokidar", () => ({
}));
vi.mock("../loading/plugin-skills.js", () => ({
resolvePluginSkillDirs: vi.fn(() => []),
resolvePluginSkillDirs: pluginSkillsMocks.resolvePluginSkillDirs,
resolvePluginSkillDirsFromMetadata: pluginSkillsMocks.resolvePluginSkillDirsFromMetadata,
}));
describe("ensureSkillsWatcher", () => {
@@ -59,6 +65,8 @@ describe("ensureSkillsWatcher", () => {
beforeEach(() => {
watchMock.mockClear();
createdWatchers.length = 0;
pluginSkillsMocks.resolvePluginSkillDirs.mockClear();
pluginSkillsMocks.resolvePluginSkillDirsFromMetadata.mockClear();
});
afterEach(async () => {
@@ -534,6 +542,30 @@ describe("ensureSkillsWatcher", () => {
}
});
it("reuses prepared plugin metadata when reconciling watch targets", () => {
const config = { skills: { load: {} } };
const pluginMetadataSnapshot = { policyHash: "prepared" } as PluginMetadataSnapshot;
refreshModule.ensureSkillsWatcher({
workspaceDir: "/tmp/workspace",
config,
pluginMetadataSnapshot,
});
refreshModule.ensureSkillsWatcher({
workspaceDir: "/tmp/workspace",
config,
pluginMetadataSnapshot,
});
expect(pluginSkillsMocks.resolvePluginSkillDirs).not.toHaveBeenCalled();
expect(pluginSkillsMocks.resolvePluginSkillDirsFromMetadata).toHaveBeenCalledTimes(2);
expect(pluginSkillsMocks.resolvePluginSkillDirsFromMetadata).toHaveBeenLastCalledWith({
workspaceDir: "/tmp/workspace",
config,
metadataSnapshot: pluginMetadataSnapshot,
});
});
it("watches extra-dir roots and companion skills folders without resolving them", async () => {
const repoDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-skills-watch-pair-"));
try {
+15 -2
View File
@@ -8,8 +8,12 @@ import { isDefaultStateDir } from "../../config/paths.js";
import type { OpenClawConfig } from "../../config/types.openclaw.js";
import { isPathInside } from "../../infra/path-guards.js";
import { createSubsystemLogger } from "../../logging/subsystem.js";
import type { PluginMetadataSnapshot } from "../../plugins/plugin-metadata-snapshot.types.js";
import { CONFIG_DIR, resolveUserPath } from "../../utils.js";
import { resolvePluginSkillDirs } from "../loading/plugin-skills.js";
import {
resolvePluginSkillDirs,
resolvePluginSkillDirsFromMetadata,
} from "../loading/plugin-skills.js";
import {
resolveAllowedSkillSymlinkTargetRealPaths,
tryRealpath,
@@ -99,6 +103,7 @@ function resolveWatchTargets(
config: OpenClawConfig | undefined,
executionSkillsDir: string | undefined,
watcherKey: string,
pluginMetadataSnapshot: PluginMetadataSnapshot | undefined,
): WatchTarget[] {
const baseRoots: Array<{ path: string; source: string }> = [];
if (workspaceDir.trim()) {
@@ -123,7 +128,13 @@ function resolveWatchTargets(
.map((d) => normalizeOptionalString(d) ?? "")
.filter(Boolean)
.map((dir) => resolveUserPath(dir));
const pluginSkillDirs = resolvePluginSkillDirs({ workspaceDir, config });
const pluginSkillDirs = pluginMetadataSnapshot
? resolvePluginSkillDirsFromMetadata({
workspaceDir,
config,
metadataSnapshot: pluginMetadataSnapshot,
})
: resolvePluginSkillDirs({ workspaceDir, config });
const allowedSymlinkTargetRealPaths = resolveAllowedSkillSymlinkTargetRealPaths(config);
const signature = JSON.stringify({
basePaths: baseRoots.map((root) => toWatchRoot(root.path)),
@@ -668,6 +679,7 @@ export function ensureSkillsWatcher(params: {
workspaceDir: string;
executionSkillsDir?: string;
config?: OpenClawConfig;
pluginMetadataSnapshot?: PluginMetadataSnapshot;
}) {
const workspaceDir = params.workspaceDir.trim();
if (!workspaceDir) {
@@ -694,6 +706,7 @@ export function ensureSkillsWatcher(params: {
params.config,
params.executionSkillsDir,
watcherKey,
params.pluginMetadataSnapshot,
);
const targetsUnchanged = sameWatchTargets(previousTargets, watchTargets);
const watcherDepthsCoverTargets = watchTargets.every(
+4 -1
View File
@@ -75,7 +75,7 @@ describe("resolveReusableWorkspaceSkillSnapshot", () => {
);
});
it("reuses prepared plugin metadata when loading execution-workspace skills", () => {
it("reuses prepared plugin metadata for watcher reconciliation and skill loading", () => {
const pluginMetadataSnapshot = { policyHash: "prepared" } as PluginMetadataSnapshot;
resolveReusableWorkspaceSkillSnapshot({
@@ -89,6 +89,9 @@ describe("resolveReusableWorkspaceSkillSnapshot", () => {
expect(loadMergedWorkspaceSkillsMock.mock.calls[0]?.[0].pluginMetadataSnapshot).toBe(
pluginMetadataSnapshot,
);
expect(ensureSkillsWatcherMock).toHaveBeenCalledWith(
expect.objectContaining({ pluginMetadataSnapshot }),
);
});
it("reuses cached resolvedSkills across calls with the same workspace, version, and filter", () => {
+3
View File
@@ -68,6 +68,9 @@ export function resolveReusableWorkspaceSkillSnapshot(
workspaceDir: watcherWorkspaceDir,
...(skillRoots ? { executionSkillsDir: skillRoots.executionSkillsDir } : {}),
config: params.config,
...(params.pluginMetadataSnapshot
? { pluginMetadataSnapshot: params.pluginMetadataSnapshot }
: {}),
});
}
const snapshotVersion = params.snapshotVersion ?? getSkillsSnapshotVersion(watcherWorkspaceDir);