refactor(skills): trim internal exports (#107353)

* refactor(skills): trim internal exports

* test(skills): derive mismatch fixture type
This commit is contained in:
Peter Steinberger
2026-07-14 02:58:34 -07:00
committed by GitHub
parent a9c70f3956
commit d8f705c0a2
18 changed files with 238 additions and 232 deletions
-18
View File
@@ -734,29 +734,11 @@ export const KNIP_UNUSED_EXPORT_BASELINE = [
"src/sessions/session-upstream-monitor.ts: runSessionUpstreamMonitorTick",
"src/sessions/session-upstream-monitor.ts: SessionUpstreamMonitor",
"src/sessions/user-turn-transcript.ts: persistUserTurnTranscript",
"src/skills/discovery/chat-commands.ts: testing",
"src/skills/discovery/filter.ts: normalizeSkillFilterForComparison",
"src/skills/discovery/skill-index.ts: isSkillPromptVisible",
"src/skills/discovery/skill-index.ts: isSkillRuntimeVisible",
"src/skills/discovery/skill-index.ts: isSkillUserInvocable",
"src/skills/lifecycle/gh-config-discovery.ts: GhConfigDirMismatch",
"src/skills/lifecycle/install.ts: testing",
"src/skills/lifecycle/upload-store.ts: createSkillUploadStore",
"src/skills/lifecycle/upload-store.ts: MAX_ACTIVE_SKILL_UPLOADS",
"src/skills/loading/plugin-skills.ts: testing",
"src/skills/runtime/refresh.ts: bumpSkillsSnapshotVersion",
"src/skills/runtime/refresh.ts: resetSkillsRefreshForTest",
"src/skills/runtime/refresh.ts: shouldIgnoreSkillsWatchPath",
"src/skills/runtime/remote-skills.ts: resetRemoteNodeSkillsForTests",
"src/skills/runtime/session-snapshot.ts: resetResolvedSkillsCacheForTests",
"src/skills/workshop/curator.ts: ARCHIVE_AFTER_MS",
"src/skills/workshop/curator.ts: CURATOR_INITIAL_DELAY_MS",
"src/skills/workshop/curator.ts: CURATOR_SWEEP_INTERVAL_MS",
"src/skills/workshop/curator.ts: DOCTOR_WEDGED_AFTER_MS",
"src/skills/workshop/curator.ts: recordSkillUsage",
"src/skills/workshop/curator.ts: registerSkillUsageTracking",
"src/skills/workshop/curator.ts: runSkillCuratorSweep",
"src/skills/workshop/curator.ts: STALE_AFTER_MS",
"src/status/status-text.ts: resolveStatusChannelFeatureLine",
"src/talk/agent-consult-runtime.ts: setRealtimeVoiceAgentConsultDepsForTest",
"src/tasks/detached-task-runtime-state.ts: DetachedTaskLifecycleRuntime",
+2 -1
View File
@@ -3,7 +3,8 @@
import { afterEach, describe, expect, it, vi } from "vitest";
import { managedWorktrees } from "../agents/worktrees/service.js";
import type { HealthSummary } from "../commands/health.js";
import { CURATOR_INITIAL_DELAY_MS, CURATOR_SWEEP_INTERVAL_MS } from "../skills/workshop/curator.js";
const CURATOR_INITIAL_DELAY_MS = 5 * 60_000;
const CURATOR_SWEEP_INTERVAL_MS = 24 * 60 * 60_000;
import type { ChatAbortControllerEntry } from "./chat-abort.js";
import { DEDUPE_MAX, DEDUPE_TTL_MS } from "./server-constants.js";
import { pendingChatSendDedupeKey } from "./server-shared.js";
+2 -42
View File
@@ -7,7 +7,6 @@ import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from "vites
let listSkillCommandsForAgents: typeof import("./chat-commands.js").listSkillCommandsForAgents;
let listSkillCommandsForWorkspace: typeof import("./chat-commands.js").listSkillCommandsForWorkspace;
let resolveSkillCommandInvocation: typeof import("./chat-commands.js").resolveSkillCommandInvocation;
let skillCommandsTesting: typeof import("./chat-commands.js").testing;
const tempDirs: string[] = [];
const resolveNodeExecEligibilityMock = vi.hoisted(() =>
@@ -162,12 +161,8 @@ vi.mock("./agent-filter.js", () => ({
}));
beforeAll(async () => {
({
listSkillCommandsForAgents,
listSkillCommandsForWorkspace,
resolveSkillCommandInvocation,
testing: skillCommandsTesting,
} = await import("./chat-commands.js"));
({ listSkillCommandsForAgents, listSkillCommandsForWorkspace, resolveSkillCommandInvocation } =
await import("./chat-commands.js"));
});
afterAll(async () => {
@@ -457,38 +452,3 @@ describe("listSkillCommandsForWorkspace", () => {
);
});
});
describe("dedupeBySkillName", () => {
it("keeps the first entry when multiple commands share a skillName", () => {
const input = [
{ name: "github", skillName: "github", description: "GitHub" },
{ name: "github_2", skillName: "github", description: "GitHub" },
{ name: "weather", skillName: "weather", description: "Weather" },
{ name: "weather_2", skillName: "weather", description: "Weather" },
];
const output = skillCommandsTesting.dedupeBySkillName(input);
expect(output.map((e) => e.name)).toEqual(["github", "weather"]);
});
it("matches skillName case-insensitively", () => {
const input = [
{ name: "ClawHub", skillName: "ClawHub", description: "ClawHub" },
{ name: "clawhub_2", skillName: "clawhub", description: "ClawHub" },
];
const output = skillCommandsTesting.dedupeBySkillName(input);
expect(output).toHaveLength(1);
expect(output[0]?.name).toBe("ClawHub");
});
it("passes through commands with an empty skillName", () => {
const input = [
{ name: "a", skillName: "", description: "A" },
{ name: "b", skillName: "", description: "B" },
];
expect(skillCommandsTesting.dedupeBySkillName(input)).toHaveLength(2);
});
it("returns an empty array for empty input", () => {
expect(skillCommandsTesting.dedupeBySkillName([])).toStrictEqual([]);
});
});
-4
View File
@@ -136,7 +136,3 @@ export function listSkillCommandsForAgents(params: {
left.skillName.localeCompare(right.skillName, "en"),
);
}
export const testing = {
dedupeBySkillName,
};
+1 -12
View File
@@ -1,10 +1,6 @@
// Skill filter tests cover allowlist and agent-scoped skill selection behavior.
import { describe, expect, it } from "vitest";
import {
matchesSkillFilter,
normalizeSkillFilter,
normalizeSkillFilterForComparison,
} from "./filter.js";
import { matchesSkillFilter, normalizeSkillFilter } from "./filter.js";
describe("skills/filter", () => {
it("normalizes configured filters with trimming", () => {
@@ -19,13 +15,6 @@ describe("skills/filter", () => {
expect(normalizeSkillFilter(undefined)).toBeUndefined();
});
it("normalizes for comparison with dedupe + ordering", () => {
expect(normalizeSkillFilterForComparison(["weather", "meme-factory", "weather"])).toEqual([
"meme-factory",
"weather",
]);
});
it("matches equivalent filters after normalization", () => {
expect(matchesSkillFilter(["weather", "meme-factory"], [" meme-factory ", "weather"])).toBe(
true,
+1 -1
View File
@@ -12,7 +12,7 @@ export function normalizeSkillFilter(skillFilter?: ReadonlyArray<unknown>): stri
return normalizeStringEntries(skillFilter);
}
export function normalizeSkillFilterForComparison(
function normalizeSkillFilterForComparison(
skillFilter?: ReadonlyArray<unknown>,
): string[] | undefined {
const normalized = normalizeSkillFilter(skillFilter);
-6
View File
@@ -5,9 +5,6 @@ import {
buildSkillIndexEntries,
filterPromptVisibleSkillEntries,
filterUserInvocableSkillEntries,
isSkillPromptVisible,
isSkillRuntimeVisible,
isSkillUserInvocable,
normalizeSkillIndexName,
} from "./skill-index.js";
@@ -77,9 +74,6 @@ describe("skill index", () => {
promptHidden,
legacyPromptHidden,
]);
expect(isSkillRuntimeVisible(runtimeHidden)).toBe(false);
expect(isSkillPromptVisible(legacyPromptHidden)).toBe(false);
expect(isSkillUserInvocable(commandHidden)).toBe(false);
});
it("records source, bundled state, skill key, and agent filter state", () => {
+3 -3
View File
@@ -34,11 +34,11 @@ export function normalizeSkillIndexName(value: string): string {
.replace(/^-+|-+$/g, "");
}
export function isSkillRuntimeVisible(entry: SkillEntry): boolean {
function isSkillRuntimeVisible(entry: SkillEntry): boolean {
return entry.exposure?.includeInRuntimeRegistry ?? true;
}
export function isSkillPromptVisible(entry: SkillEntry): boolean {
function isSkillPromptVisible(entry: SkillEntry): boolean {
if (entry.exposure) {
return entry.exposure.includeInAvailableSkillsPrompt ?? true;
}
@@ -48,7 +48,7 @@ export function isSkillPromptVisible(entry: SkillEntry): boolean {
return !entry.skill.disableModelInvocation;
}
export function isSkillUserInvocable(entry: SkillEntry): boolean {
function isSkillUserInvocable(entry: SkillEntry): boolean {
if (entry.exposure) {
return entry.exposure.userInvocable ?? true;
}
@@ -3,10 +3,14 @@ import { describe, expect, it } from "vitest";
import {
detectGhConfigDirMismatch,
formatGhConfigDirMismatchHint,
type GhConfigDirMismatch,
type GhConfigDiscoveryInput,
} from "./gh-config-discovery.js";
type GhConfigDirMismatch = Omit<
Extract<ReturnType<typeof detectGhConfigDirMismatch>, { kind: "mismatch" }>,
"kind"
>;
function makeInput(overrides: Partial<GhConfigDiscoveryInput>): GhConfigDiscoveryInput {
return {
platform: "linux",
+1 -1
View File
@@ -32,7 +32,7 @@ export type GhConfigDiscoveryInput = {
candidateOperatorHomes?: readonly string[];
};
export type GhConfigDirMismatch = {
type GhConfigDirMismatch = {
// The directory `gh` would actually consult given the current process env.
effectiveConfigDir: string;
// The directory that contains the operator's real `hosts.yml`.
+33 -24
View File
@@ -1,5 +1,5 @@
// Plugin skill loading tests cover skill discovery from plugin-provided skill bundles.
import fsSync, { type Dirent } from "node:fs";
import fsSync from "node:fs";
import fs from "node:fs/promises";
import path from "node:path";
import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
@@ -10,7 +10,6 @@ import {
import type { OpenClawConfig } from "../../config/config.js";
import type { PluginManifestRegistry } from "../../plugins/manifest-registry.js";
import { createTrackedTempDirs } from "../../test-utils/tracked-temp-dirs.js";
import { testing } from "./plugin-skills.js";
const hoisted = vi.hoisted(() => {
const loadManifestRegistry = vi.fn();
@@ -393,7 +392,38 @@ describe("resolvePluginSkillDirs", () => {
});
describe("publishPluginSkills", () => {
const { isGeneratedPluginSkillEntry, publishPluginSkills, resolvePluginSkillLinkType } = testing;
beforeAll(async () => {
({ resolvePluginSkillDirs } = await import("./plugin-skills.js"));
});
function publishPluginSkills(skillDirs: string[], opts: { pluginSkillsDir: string }): void {
const plugins = skillDirs.map((rootDir, index) => ({
id: `publish-test-${index}`,
name: `Publish Test ${index}`,
channels: [],
providers: [],
cliBackends: [],
skills: ["."],
hooks: [],
origin: "workspace" as const,
rootDir,
source: rootDir,
manifestPath: path.join(rootDir, "openclaw.plugin.json"),
}));
hoisted.loadPluginManifestRegistryForInstalledIndex.mockReturnValue({
diagnostics: [],
plugins,
});
resolvePluginSkillDirs({
workspaceDir: opts.pluginSkillsDir,
pluginSkillsDir: opts.pluginSkillsDir,
config: {
plugins: {
entries: Object.fromEntries(plugins.map((plugin) => [plugin.id, { enabled: true }])),
},
},
});
}
function withPlatform<T>(platform: NodeJS.Platform, fn: () => T): T {
const originalPlatform = process.platform;
@@ -436,12 +466,6 @@ describe("publishPluginSkills", () => {
expect(fsSync.readlinkSync(linkB)).toBe(dirB);
});
it("uses junction links for plugin skill directories on Windows", () => {
expect(resolvePluginSkillLinkType("win32")).toBe("junction");
expect(resolvePluginSkillLinkType("linux")).toBe("dir");
expect(resolvePluginSkillLinkType("darwin")).toBe("dir");
});
it("is idempotent: skips symlinks that already point to the same target", async () => {
const skillParent = await tempDirs.make("plugin-skills-");
const managedDir = await tempDirs.make("managed-skills-");
@@ -538,21 +562,6 @@ describe("publishPluginSkills", () => {
expect(fsSync.existsSync(staleDir)).toBe(false);
});
it("treats Windows directory entries as generated plugin skill entries", () => {
const directoryEntry = {
isDirectory: () => true,
isSymbolicLink: () => false,
} as Dirent;
const regularEntry = {
isDirectory: () => false,
isSymbolicLink: () => false,
} as Dirent;
expect(withPlatform("win32", () => isGeneratedPluginSkillEntry(directoryEntry))).toBe(true);
expect(withPlatform("linux", () => isGeneratedPluginSkillEntry(directoryEntry))).toBe(false);
expect(withPlatform("win32", () => isGeneratedPluginSkillEntry(regularEntry))).toBe(false);
});
it("cleans up broken symlinks (dangling)", async () => {
const skillParent = await tempDirs.make("plugin-skills-");
const managedDir = await tempDirs.make("managed-skills-");
-6
View File
@@ -297,9 +297,3 @@ function isNotFoundError(err: unknown): boolean {
const code = (err as Record<string, unknown>).code;
return code === "ENOENT" || code === "ENOTDIR";
}
export const testing = {
isGeneratedPluginSkillEntry,
publishPluginSkills,
resolvePluginSkillLinkType,
};
+33 -22
View File
@@ -4,6 +4,7 @@ import os from "node:os";
import path from "node:path";
import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
import {
bumpSkillsSnapshotVersion,
getSkillsSnapshotVersion,
shouldRefreshSnapshotForVersion,
type SkillsChangeEvent,
@@ -68,7 +69,17 @@ describe("ensureSkillsWatcher", () => {
// Each unique directory gets its own watcher (one path argument per call).
const calls = watchMock.mock.calls as unknown as Array<
[string, { depth?: number; followSymlinks?: boolean; ignored?: unknown }]
[
string,
{
depth?: number;
followSymlinks?: boolean;
ignored?: (
watchPath: string,
stats?: { isDirectory?: () => boolean; isSymbolicLink?: () => boolean },
) => boolean;
},
]
>;
expect(calls.length).toBeGreaterThan(0);
const targets = calls.map((call) => call[0]);
@@ -86,33 +97,33 @@ describe("ensureSkillsWatcher", () => {
expect(targets).toContain(posix(path.join(os.homedir(), ".agents", "skills")));
const wildcardTargets = targets.filter((target) => target.includes("*"));
expect(wildcardTargets).toStrictEqual([]);
const ignored = refreshModule.shouldIgnoreSkillsWatchPath;
const ignored = opts.ignored;
expect(ignored).toBeDefined();
// Node/JS paths
expect(ignored("/tmp/workspace/skills/node_modules/pkg/index.js")).toBe(true);
expect(ignored("/tmp/workspace/skills/dist/index.js")).toBe(true);
expect(ignored("/tmp/workspace/skills/.git/config")).toBe(true);
expect(ignored?.("/tmp/workspace/skills/node_modules/pkg/index.js")).toBe(true);
expect(ignored?.("/tmp/workspace/skills/dist/index.js")).toBe(true);
expect(ignored?.("/tmp/workspace/skills/.git/config")).toBe(true);
// Python virtual environments and caches
expect(ignored("/tmp/workspace/skills/scripts/.venv/bin/python")).toBe(true);
expect(ignored("/tmp/workspace/skills/venv/lib/python3.10/site.py")).toBe(true);
expect(ignored("/tmp/workspace/skills/__pycache__/module.pyc")).toBe(true);
expect(ignored("/tmp/workspace/skills/.mypy_cache/3.10/foo.json")).toBe(true);
expect(ignored("/tmp/workspace/skills/.pytest_cache/v/cache")).toBe(true);
expect(ignored?.("/tmp/workspace/skills/scripts/.venv/bin/python")).toBe(true);
expect(ignored?.("/tmp/workspace/skills/venv/lib/python3.10/site.py")).toBe(true);
expect(ignored?.("/tmp/workspace/skills/__pycache__/module.pyc")).toBe(true);
expect(ignored?.("/tmp/workspace/skills/.mypy_cache/3.10/foo.json")).toBe(true);
expect(ignored?.("/tmp/workspace/skills/.pytest_cache/v/cache")).toBe(true);
// Build artifacts and caches
expect(ignored("/tmp/workspace/skills/build/output.js")).toBe(true);
expect(ignored("/tmp/workspace/skills/.cache/data.json")).toBe(true);
expect(ignored?.("/tmp/workspace/skills/build/output.js")).toBe(true);
expect(ignored?.("/tmp/workspace/skills/.cache/data.json")).toBe(true);
// Paths without stats stay visible so chokidar can stat and classify them.
expect(ignored("/tmp/.hidden/skills/index.md")).toBe(false);
expect(ignored("/tmp/workspace/skills/my-skill", { isDirectory: () => true })).toBe(false);
expect(ignored("/tmp/workspace/skills/my-skill", { isSymbolicLink: () => true })).toBe(false);
expect(ignored("/tmp/workspace/skills/my-skill/README.md", {})).toBe(true);
expect(ignored("/tmp/workspace/skills/my-skill/SKILL.md", {})).toBe(true);
expect(ignored("/tmp/workspace/skills/my-skill/SKILL.md", {}, { usePolling: true })).toBe(
expect(ignored?.("/tmp/.hidden/skills/index.md")).toBe(false);
expect(ignored?.("/tmp/workspace/skills/my-skill", { isDirectory: () => true })).toBe(false);
expect(ignored?.("/tmp/workspace/skills/my-skill", { isSymbolicLink: () => true })).toBe(
false,
);
expect(ignored?.("/tmp/workspace/skills/my-skill/README.md", {})).toBe(true);
expect(ignored?.("/tmp/workspace/skills/my-skill/SKILL.md", {})).toBe(true);
} finally {
await fs.rm(workspaceDir, { recursive: true, force: true });
}
@@ -856,7 +867,7 @@ describe("ensureSkillsWatcher", () => {
config: { skills: { load: { watchDebounceMs: 10 } } },
});
const firstVersion = refreshModule.bumpSkillsSnapshotVersion({
const firstVersion = bumpSkillsSnapshotVersion({
workspaceDir,
reason: "watch",
changedPath: `${workspaceDir}/skills/demo/SKILL.md`,
@@ -870,7 +881,7 @@ describe("ensureSkillsWatcher", () => {
expect(nextVersion).toBeGreaterThan(firstVersion);
expect(shouldRefreshSnapshotForVersion(firstVersion, nextVersion)).toBe(true);
vi.setSystemTime(new Date(nextVersion));
const followupVersion = refreshModule.bumpSkillsSnapshotVersion({
const followupVersion = bumpSkillsSnapshotVersion({
workspaceDir,
reason: "watch",
});
@@ -890,7 +901,7 @@ describe("ensureSkillsWatcher", () => {
(target) => target === `${idleWorkspaceDir}/skills`,
);
expect(idleSkillsIndex).toBeGreaterThanOrEqual(0);
const firstVersion = refreshModule.bumpSkillsSnapshotVersion({
const firstVersion = bumpSkillsSnapshotVersion({
workspaceDir: idleWorkspaceDir,
reason: "watch",
});
@@ -906,7 +917,7 @@ describe("ensureSkillsWatcher", () => {
expect(evictedVersion).toBeGreaterThan(firstVersion);
expect(shouldRefreshSnapshotForVersion(firstVersion, evictedVersion)).toBe(true);
vi.setSystemTime(new Date(evictedVersion));
const followupVersion = refreshModule.bumpSkillsSnapshotVersion({
const followupVersion = bumpSkillsSnapshotVersion({
workspaceDir: idleWorkspaceDir,
});
expect(followupVersion).toBeGreaterThan(evictedVersion);
+2 -2
View File
@@ -18,7 +18,7 @@ import {
resetSkillsRefreshStateForTest,
setSkillsChangeListenerErrorHandler,
} from "./refresh-state.js";
export { bumpSkillsSnapshotVersion, registerSkillsChangeListener } from "./refresh-state.js";
export { registerSkillsChangeListener } from "./refresh-state.js";
type SkillsPathWatchState = {
watcher: FSWatcher;
@@ -374,7 +374,7 @@ function isPathInsideAnyRoot(roots: readonly string[], child: string): boolean {
return roots.some((root) => isPathInside(root, child));
}
export function shouldIgnoreSkillsWatchPath(
function shouldIgnoreSkillsWatchPath(
watchPath: string,
stats?: { isDirectory?: () => boolean; isSymbolicLink?: () => boolean },
options: { usePolling?: boolean } = {},
+4 -4
View File
@@ -47,13 +47,13 @@ vi.mock("./refresh-state.js", () => ({
shouldRefreshSnapshotForVersion: shouldRefreshSnapshotForVersionMock,
}));
const { resolveReusableWorkspaceSkillSnapshot, resetResolvedSkillsCacheForTests } =
await import("./session-snapshot.js");
let resolveReusableWorkspaceSkillSnapshot: typeof import("./session-snapshot.js").resolveReusableWorkspaceSkillSnapshot;
describe("resolveReusableWorkspaceSkillSnapshot", () => {
beforeEach(() => {
beforeEach(async () => {
vi.resetModules();
({ resolveReusableWorkspaceSkillSnapshot } = await import("./session-snapshot.js"));
vi.clearAllMocks();
resetResolvedSkillsCacheForTests();
buildWorkspaceSkillSnapshotMock.mockReturnValue({ prompt: "", skills: [], resolvedSkills: [] });
ensureSkillsWatcherMock.mockImplementation(() => undefined);
getSkillsSnapshotVersionMock.mockReturnValue(1);
-4
View File
@@ -33,10 +33,6 @@ type ReusableSkillSnapshotResult = {
snapshotVersion: number;
};
export function resetResolvedSkillsCacheForTests(): void {
resolvedSkillsCache.clear();
}
function fingerprintSkillSnapshotConfig(config: OpenClawConfig): string {
return crypto
.createHash("sha256")
+143 -73
View File
@@ -33,23 +33,93 @@ vi.mock("./store.js", () => ({
}));
import {
ARCHIVE_AFTER_MS,
DOCTOR_WEDGED_AFTER_MS,
STALE_AFTER_MS,
getSkillCuratorDoctorWarning,
getSkillCuratorStatus,
pinCuratedSkill,
recordSkillUsage,
registerSkillUsageTracking,
restoreCuratedSkill,
runSkillCuratorSweep,
startSkillCuratorMaintenance,
unpinCuratedSkill,
} from "./curator.js";
const STALE_AFTER_MS = 30 * 24 * 60 * 60_000;
const ARCHIVE_AFTER_MS = 90 * 24 * 60 * 60_000;
const CURATOR_INITIAL_DELAY_MS = 5 * 60_000;
const DOCTOR_WEDGED_AFTER_MS = 7 * 24 * 60 * 60_000;
let rootDir = "";
let stateDir = "";
let originalStateDir: string | undefined;
function registerSkillUsageTracking(): () => void {
return startSkillCuratorMaintenance({
onError: (error) => {
throw error;
},
runSweep: async () => undefined,
});
}
async function recordSkillUsage(event: {
skillFile: string;
skillName: string;
skillSource: "bundled" | "unknown" | "workspace";
agentId?: string;
ts: number;
}): Promise<void> {
const cleanup = registerSkillUsageTracking();
const now = vi.spyOn(Date, "now").mockReturnValue(event.ts);
try {
emitTrustedSkillUsedDiagnosticEvent(
{
type: "skill.used",
skillName: event.skillName,
skillSource: event.skillSource,
activation: "read",
agentId: event.agentId,
},
{ skillUsage: { skillFile: event.skillFile } },
);
} finally {
now.mockRestore();
}
await waitForDiagnosticEventsDrained();
cleanup();
}
async function runSkillCuratorSweep(options: {
env?: NodeJS.ProcessEnv;
nowMs?: number;
}): Promise<void> {
const nowMs = options.nowMs ?? Date.now();
vi.useFakeTimers();
vi.setSystemTime(nowMs - CURATOR_INITIAL_DELAY_MS);
let failure: unknown;
const cleanup = startSkillCuratorMaintenance({
onError: (error) => {
failure = error;
},
registerUsageTracking: () => () => undefined,
});
try {
await vi.advanceTimersByTimeAsync(CURATOR_INITIAL_DELAY_MS);
for (let attempt = 0; attempt < 50; attempt += 1) {
const status = getSkillCuratorStatus({ env: process.env });
if (failure || status.lastSuccessAtMs === nowMs || status.lastError) {
break;
}
await Promise.resolve();
}
} finally {
cleanup();
vi.useRealTimers();
}
if (failure) {
throw failure instanceof Error
? failure
: new Error("skill curator sweep failed", { cause: failure });
}
}
function addAppliedSkill(params: {
name: string;
appliedAtMs: number;
@@ -152,7 +222,7 @@ describe("skill curator usage", () => {
const nowMs = Date.now();
const skillFile = path.join(rootDir, "agent", "skills", "daily-brief", "SKILL.md");
addAppliedSkill({ name: "Daily Brief", appliedAtMs: nowMs });
const unregister = registerSkillUsageTracking({ env: process.env });
const unregister = registerSkillUsageTracking();
emitTrustedSkillUsedDiagnosticEvent(
{
type: "skill.used",
@@ -187,7 +257,7 @@ describe("skill curator usage", () => {
it("skips usage events without a canonical skill file", async () => {
const nowMs = Date.now();
addAppliedSkill({ name: "Nameless Usage", appliedAtMs: nowMs });
const unregister = registerSkillUsageTracking({ env: process.env });
const unregister = registerSkillUsageTracking();
emitTrustedSkillUsedDiagnosticEvent({
type: "skill.used",
skillName: "Nameless Usage",
@@ -207,26 +277,20 @@ describe("skill curator usage", () => {
it("keeps last-used time monotonic when events arrive out of order", async () => {
const skillFile = path.join(rootDir, "agent", "skills", "ordered", "SKILL.md");
addAppliedSkill({ name: "Ordered", appliedAtMs: 0 });
recordSkillUsage(
{
skillFile,
skillName: "Ordered",
skillSource: "workspace",
agentId: "newer",
ts: 200,
},
{ env: process.env },
);
recordSkillUsage(
{
skillFile,
skillName: "Ordered",
skillSource: "workspace",
agentId: "older",
ts: 100,
},
{ env: process.env },
);
await recordSkillUsage({
skillFile,
skillName: "Ordered",
skillSource: "workspace",
agentId: "newer",
ts: 200,
});
await recordSkillUsage({
skillFile,
skillName: "Ordered",
skillSource: "workspace",
agentId: "older",
ts: 100,
});
await runSkillCuratorSweep({ env: process.env, nowMs: 201 });
expect(getSkillCuratorStatus({ env: process.env }).skills[0]).toMatchObject({
@@ -236,7 +300,7 @@ describe("skill curator usage", () => {
});
it("contains subscriber failures without throwing into the emitter", async () => {
const unregister = registerSkillUsageTracking({ env: process.env });
const unregister = registerSkillUsageTracking();
expect(() =>
emitTrustedSkillUsedDiagnosticEvent(
{
@@ -288,26 +352,20 @@ describe("skill curator lifecycle", () => {
addAppliedSkill({ name: "Unused Archive", appliedAtMs: nowMs - ARCHIVE_AFTER_MS - 1 });
await runSkillCuratorSweep({ env: process.env, nowMs });
recordSkillUsage(
{
skillFile: path.join(rootDir, "agent", "skills", "dormant", "SKILL.md"),
skillName: "Dormant",
skillSource: "workspace",
agentId: "main",
ts: nowMs + 1,
},
{ env: process.env },
);
recordSkillUsage(
{
skillFile: path.join(rootDir, "agent", "skills", "deep-archive", "SKILL.md"),
skillName: "Deep Archive",
skillSource: "workspace",
agentId: "main",
ts: nowMs + 1,
},
{ env: process.env },
);
await recordSkillUsage({
skillFile: path.join(rootDir, "agent", "skills", "dormant", "SKILL.md"),
skillName: "Dormant",
skillSource: "workspace",
agentId: "main",
ts: nowMs + 1,
});
await recordSkillUsage({
skillFile: path.join(rootDir, "agent", "skills", "deep-archive", "SKILL.md"),
skillName: "Deep Archive",
skillSource: "workspace",
agentId: "main",
ts: nowMs + 1,
});
await runSkillCuratorSweep({ env: process.env, nowMs: nowMs + 2 });
const byKey = new Map(
@@ -352,16 +410,13 @@ describe("skill curator lifecycle", () => {
proposalId: "shared-name-b",
agentDirName: "agent-b",
});
recordSkillUsage(
{
skillFile: firstSkillFile,
skillName: "Shared Name",
skillSource: "workspace",
agentId: "agent-a",
ts: nowMs,
},
{ env: process.env },
);
await recordSkillUsage({
skillFile: firstSkillFile,
skillName: "Shared Name",
skillSource: "workspace",
agentId: "agent-a",
ts: nowMs,
});
await runSkillCuratorSweep({ env: process.env, nowMs });
const status = getSkillCuratorStatus({ env: process.env });
@@ -498,21 +553,36 @@ describe("skill curator lifecycle", () => {
resolveManifest = resolve;
}),
);
const sweep = runSkillCuratorSweep({ env: process.env, nowMs });
expect(
getSkillCuratorDoctorWarning({
env: process.env,
nowMs: nowMs + DOCTOR_WEDGED_AFTER_MS + 1,
}),
).toContain("skill curator has not completed a sweep");
resolveManifest?.({
schema: "openclaw.skill-workshop.proposals-manifest.v1",
updatedAt: new Date(nowMs).toISOString(),
proposals: [],
vi.useFakeTimers();
vi.setSystemTime(nowMs - CURATOR_INITIAL_DELAY_MS);
let failure: unknown;
const cleanup = startSkillCuratorMaintenance({
onError: (error) => {
failure = error;
},
registerUsageTracking: () => () => undefined,
});
await sweep;
try {
vi.advanceTimersByTime(CURATOR_INITIAL_DELAY_MS);
await Promise.resolve();
expect(
getSkillCuratorDoctorWarning({
env: process.env,
nowMs: nowMs + DOCTOR_WEDGED_AFTER_MS + 1,
}),
).toContain("skill curator has not completed a sweep");
resolveManifest?.({
schema: "openclaw.skill-workshop.proposals-manifest.v1",
updatedAt: new Date(nowMs).toISOString(),
proposals: [],
});
await vi.advanceTimersByTimeAsync(0);
expect(failure).toBeUndefined();
} finally {
cleanup();
vi.useRealTimers();
}
});
it("filters archived skills from snapshots while retaining stale skills", async () => {
+8 -8
View File
@@ -20,11 +20,11 @@ import { readSkillProposalManifest, readSkillProposalRecord } from "./store.js";
import type { SkillProposalRecord } from "./types.js";
// Fixed policy keeps lifecycle behavior predictable and avoids another config surface.
export const STALE_AFTER_MS = 30 * 24 * 60 * 60_000;
export const ARCHIVE_AFTER_MS = 90 * 24 * 60 * 60_000;
export const CURATOR_SWEEP_INTERVAL_MS = 24 * 60 * 60_000;
export const CURATOR_INITIAL_DELAY_MS = 5 * 60_000;
export const DOCTOR_WEDGED_AFTER_MS = 7 * 24 * 60 * 60_000;
const STALE_AFTER_MS = 30 * 24 * 60 * 60_000;
const ARCHIVE_AFTER_MS = 90 * 24 * 60 * 60_000;
const CURATOR_SWEEP_INTERVAL_MS = 24 * 60 * 60_000;
const CURATOR_INITIAL_DELAY_MS = 5 * 60_000;
const DOCTOR_WEDGED_AFTER_MS = 7 * 24 * 60 * 60_000;
const log = createSubsystemLogger("skills/curator");
const CURATOR_STATE_ID = 1;
@@ -102,7 +102,7 @@ function canonicalSkillKey(name: string): string {
return key;
}
export function recordSkillUsage(
function recordSkillUsage(
event: Pick<DiagnosticSkillUsedEvent, "agentId" | "skillName" | "skillSource" | "ts"> & {
skillFile?: string;
},
@@ -154,7 +154,7 @@ export function recordSkillUsage(
}
/** Register once per Gateway lifetime; listener failures never reach tool execution. */
export function registerSkillUsageTracking(options: OpenClawStateDatabaseOptions = {}): () => void {
function registerSkillUsageTracking(options: OpenClawStateDatabaseOptions = {}): () => void {
return onTrustedInternalDiagnosticEvent((event, metadata, privateData) => {
if (!metadata.trusted || event.type !== "skill.used") {
return;
@@ -357,7 +357,7 @@ function writeSweepFailure(
}, options);
}
export async function runSkillCuratorSweep(
async function runSkillCuratorSweep(
options: CuratorOptions = {},
): Promise<SkillCuratorSweepResult> {
const nowMs = options.nowMs ?? Date.now();