mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 09:00:42 +00:00
perf: speed up subagent and skill tests
This commit is contained in:
@@ -3,6 +3,12 @@ import path from "node:path";
|
||||
import { openVerifiedFileSync } from "../../infra/safe-open-sync.js";
|
||||
import { parseFrontmatter, resolveSkillInvocationPolicy } from "./frontmatter.js";
|
||||
import { createSyntheticSourceInfo, type Skill } from "./skill-contract.js";
|
||||
import type { ParsedSkillFrontmatter } from "./types.js";
|
||||
|
||||
type LoadedLocalSkill = {
|
||||
skill: Skill;
|
||||
frontmatter: ParsedSkillFrontmatter;
|
||||
};
|
||||
|
||||
function isPathWithinRoot(rootRealPath: string, candidatePath: string): boolean {
|
||||
const relative = path.relative(rootRealPath, candidatePath);
|
||||
@@ -40,7 +46,7 @@ function loadSingleSkillDirectory(params: {
|
||||
source: string;
|
||||
rootRealPath: string;
|
||||
maxBytes?: number;
|
||||
}): Skill | null {
|
||||
}): LoadedLocalSkill | null {
|
||||
const skillFilePath = path.join(params.skillDir, "SKILL.md");
|
||||
const raw = readSkillFileSync({
|
||||
rootRealPath: params.rootRealPath,
|
||||
@@ -69,18 +75,21 @@ function loadSingleSkillDirectory(params: {
|
||||
const baseDir = path.resolve(params.skillDir);
|
||||
|
||||
return {
|
||||
name,
|
||||
description,
|
||||
filePath,
|
||||
baseDir,
|
||||
source: params.source,
|
||||
sourceInfo: createSyntheticSourceInfo(filePath, {
|
||||
source: params.source,
|
||||
skill: {
|
||||
name,
|
||||
description,
|
||||
filePath,
|
||||
baseDir,
|
||||
scope: "project",
|
||||
origin: "top-level",
|
||||
}),
|
||||
disableModelInvocation: invocation.disableModelInvocation,
|
||||
source: params.source,
|
||||
sourceInfo: createSyntheticSourceInfo(filePath, {
|
||||
source: params.source,
|
||||
baseDir,
|
||||
scope: "project",
|
||||
origin: "top-level",
|
||||
}),
|
||||
disableModelInvocation: invocation.disableModelInvocation,
|
||||
},
|
||||
frontmatter,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -101,13 +110,14 @@ function listCandidateSkillDirs(dir: string): string[] {
|
||||
|
||||
export function loadSkillsFromDirSafe(params: { dir: string; source: string; maxBytes?: number }): {
|
||||
skills: Skill[];
|
||||
frontmatterByFilePath: ReadonlyMap<string, ParsedSkillFrontmatter>;
|
||||
} {
|
||||
const rootDir = path.resolve(params.dir);
|
||||
let rootRealPath: string;
|
||||
try {
|
||||
rootRealPath = fs.realpathSync(rootDir);
|
||||
} catch {
|
||||
return { skills: [] };
|
||||
return { skills: [], frontmatterByFilePath: new Map() };
|
||||
}
|
||||
|
||||
const rootSkill = loadSingleSkillDirectory({
|
||||
@@ -117,10 +127,13 @@ export function loadSkillsFromDirSafe(params: { dir: string; source: string; max
|
||||
maxBytes: params.maxBytes,
|
||||
});
|
||||
if (rootSkill) {
|
||||
return { skills: [rootSkill] };
|
||||
return {
|
||||
skills: [rootSkill.skill],
|
||||
frontmatterByFilePath: new Map([[rootSkill.skill.filePath, rootSkill.frontmatter]]),
|
||||
};
|
||||
}
|
||||
|
||||
const skills = listCandidateSkillDirs(rootDir)
|
||||
const loadedSkills = listCandidateSkillDirs(rootDir)
|
||||
.map((skillDir) =>
|
||||
loadSingleSkillDirectory({
|
||||
skillDir,
|
||||
@@ -129,9 +142,16 @@ export function loadSkillsFromDirSafe(params: { dir: string; source: string; max
|
||||
maxBytes: params.maxBytes,
|
||||
}),
|
||||
)
|
||||
.filter((skill): skill is Skill => skill !== null);
|
||||
.filter((skill): skill is LoadedLocalSkill => skill !== null);
|
||||
const frontmatterByFilePath = new Map<string, ParsedSkillFrontmatter>();
|
||||
for (const loaded of loadedSkills) {
|
||||
frontmatterByFilePath.set(loaded.skill.filePath, loaded.frontmatter);
|
||||
}
|
||||
|
||||
return { skills };
|
||||
return {
|
||||
skills: loadedSkills.map((loaded) => loaded.skill),
|
||||
frontmatterByFilePath,
|
||||
};
|
||||
}
|
||||
|
||||
export function readSkillFrontmatterSafe(params: {
|
||||
|
||||
@@ -135,6 +135,11 @@ type ResolvedSkillsLimits = {
|
||||
maxSkillFileBytes: number;
|
||||
};
|
||||
|
||||
type LoadedSkillRecord = {
|
||||
skill: Skill;
|
||||
frontmatter?: ParsedSkillFrontmatter;
|
||||
};
|
||||
|
||||
function resolveSkillsLimits(config?: OpenClawConfig, agentId?: string): ResolvedSkillsLimits {
|
||||
const limits = config?.skills?.limits;
|
||||
const agentSkillsLimits = resolveEffectiveAgentSkillsLimits(config, agentId);
|
||||
@@ -283,13 +288,13 @@ function resolveContainedSkillPath(params: {
|
||||
return null;
|
||||
}
|
||||
|
||||
function filterLoadedSkillsInsideRoot(params: {
|
||||
skills: Skill[];
|
||||
function filterLoadedSkillRecordsInsideRoot(params: {
|
||||
records: LoadedSkillRecord[];
|
||||
source: string;
|
||||
rootDir: string;
|
||||
rootRealPath: string;
|
||||
}): Skill[] {
|
||||
return params.skills.filter((skill) => {
|
||||
}): LoadedSkillRecord[] {
|
||||
return params.records.filter(({ skill }) => {
|
||||
const baseDirRealPath = resolveContainedSkillPath({
|
||||
source: params.source,
|
||||
rootDir: params.rootDir,
|
||||
@@ -339,14 +344,22 @@ function resolveNestedSkillsRoot(
|
||||
return { baseDir: dir };
|
||||
}
|
||||
|
||||
function unwrapLoadedSkills(loaded: unknown): Skill[] {
|
||||
function unwrapLoadedSkillRecords(loaded: unknown): LoadedSkillRecord[] {
|
||||
if (Array.isArray(loaded)) {
|
||||
return loaded as Skill[];
|
||||
return (loaded as Skill[]).map((skill) => ({ skill }));
|
||||
}
|
||||
if (loaded && typeof loaded === "object" && "skills" in loaded) {
|
||||
const skills = (loaded as { skills?: unknown }).skills;
|
||||
if (Array.isArray(skills)) {
|
||||
return skills as Skill[];
|
||||
const loadedResult = loaded as { frontmatterByFilePath?: unknown };
|
||||
const frontmatterByFilePath =
|
||||
loadedResult.frontmatterByFilePath instanceof Map
|
||||
? (loadedResult.frontmatterByFilePath as ReadonlyMap<string, ParsedSkillFrontmatter>)
|
||||
: undefined;
|
||||
return (skills as Skill[]).map((skill) => ({
|
||||
skill,
|
||||
frontmatter: frontmatterByFilePath?.get(skill.filePath),
|
||||
}));
|
||||
}
|
||||
}
|
||||
return [];
|
||||
@@ -363,7 +376,7 @@ function loadSkillEntries(
|
||||
): SkillEntry[] {
|
||||
const limits = resolveSkillsLimits(opts?.config, opts?.agentId);
|
||||
|
||||
const loadSkills = (params: { dir: string; source: string }): Skill[] => {
|
||||
const loadSkills = (params: { dir: string; source: string }): LoadedSkillRecord[] => {
|
||||
const rootDir = path.resolve(params.dir);
|
||||
if (!fs.existsSync(rootDir)) {
|
||||
return [];
|
||||
@@ -415,8 +428,8 @@ function loadSkillEntries(
|
||||
source: params.source,
|
||||
maxBytes: limits.maxSkillFileBytes,
|
||||
});
|
||||
return filterLoadedSkillsInsideRoot({
|
||||
skills: unwrapLoadedSkills(loaded),
|
||||
return filterLoadedSkillRecordsInsideRoot({
|
||||
records: unwrapLoadedSkillRecords(loaded),
|
||||
source: params.source,
|
||||
rootDir,
|
||||
rootRealPath: baseDirRealPath,
|
||||
@@ -446,7 +459,7 @@ function loadSkillEntries(
|
||||
});
|
||||
}
|
||||
|
||||
const loadedSkills: Skill[] = [];
|
||||
const loadedSkills: LoadedSkillRecord[] = [];
|
||||
|
||||
// Only consider immediate subfolders that look like skills (have SKILL.md) and are under size cap.
|
||||
for (const name of limitedChildren) {
|
||||
@@ -494,8 +507,8 @@ function loadSkillEntries(
|
||||
maxBytes: limits.maxSkillFileBytes,
|
||||
});
|
||||
loadedSkills.push(
|
||||
...filterLoadedSkillsInsideRoot({
|
||||
skills: unwrapLoadedSkills(loaded),
|
||||
...filterLoadedSkillRecordsInsideRoot({
|
||||
records: unwrapLoadedSkillRecords(loaded),
|
||||
source: params.source,
|
||||
rootDir,
|
||||
rootRealPath: baseDirRealPath,
|
||||
@@ -510,7 +523,7 @@ function loadSkillEntries(
|
||||
if (loadedSkills.length > limits.maxSkillsLoadedPerSource) {
|
||||
return loadedSkills
|
||||
.slice()
|
||||
.sort((a, b) => a.name.localeCompare(b.name, "en"))
|
||||
.sort((a, b) => a.skill.name.localeCompare(b.skill.name, "en"))
|
||||
.slice(0, limits.maxSkillsLoadedPerSource);
|
||||
}
|
||||
|
||||
@@ -563,36 +576,39 @@ function loadSkillEntries(
|
||||
source: "openclaw-workspace",
|
||||
});
|
||||
|
||||
const merged = new Map<string, Skill>();
|
||||
const merged = new Map<string, LoadedSkillRecord>();
|
||||
// Precedence: extra < bundled < managed < agents-skills-personal < agents-skills-project < workspace
|
||||
for (const skill of extraSkills) {
|
||||
merged.set(skill.name, skill);
|
||||
for (const record of extraSkills) {
|
||||
merged.set(record.skill.name, record);
|
||||
}
|
||||
for (const skill of bundledSkills) {
|
||||
merged.set(skill.name, skill);
|
||||
for (const record of bundledSkills) {
|
||||
merged.set(record.skill.name, record);
|
||||
}
|
||||
for (const skill of managedSkills) {
|
||||
merged.set(skill.name, skill);
|
||||
for (const record of managedSkills) {
|
||||
merged.set(record.skill.name, record);
|
||||
}
|
||||
for (const skill of personalAgentsSkills) {
|
||||
merged.set(skill.name, skill);
|
||||
for (const record of personalAgentsSkills) {
|
||||
merged.set(record.skill.name, record);
|
||||
}
|
||||
for (const skill of projectAgentsSkills) {
|
||||
merged.set(skill.name, skill);
|
||||
for (const record of projectAgentsSkills) {
|
||||
merged.set(record.skill.name, record);
|
||||
}
|
||||
for (const skill of workspaceSkills) {
|
||||
merged.set(skill.name, skill);
|
||||
for (const record of workspaceSkills) {
|
||||
merged.set(record.skill.name, record);
|
||||
}
|
||||
|
||||
const skillEntries: SkillEntry[] = Array.from(merged.values())
|
||||
.sort((a, b) => a.name.localeCompare(b.name, "en"))
|
||||
.map((skill) => {
|
||||
.sort((a, b) => a.skill.name.localeCompare(b.skill.name, "en"))
|
||||
.map((record) => {
|
||||
const skill = record.skill;
|
||||
const frontmatter =
|
||||
record.frontmatter ??
|
||||
readSkillFrontmatterSafe({
|
||||
rootDir: skill.baseDir,
|
||||
filePath: skill.filePath,
|
||||
maxBytes: limits.maxSkillFileBytes,
|
||||
}) ?? ({} as ParsedSkillFrontmatter);
|
||||
}) ??
|
||||
({} as ParsedSkillFrontmatter);
|
||||
const invocation = resolveSkillInvocationPolicy(frontmatter);
|
||||
return {
|
||||
skill,
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import type { ContextEngine } from "../context-engine/types.js";
|
||||
|
||||
const noop = () => {};
|
||||
let lifecycleHandler:
|
||||
@@ -66,6 +67,12 @@ vi.mock("../config/sessions.js", () => {
|
||||
const announceSpy = vi.fn(async (_params: unknown) => true);
|
||||
const runSubagentEndedHookMock = vi.fn(async (_event?: unknown, _ctx?: unknown) => {});
|
||||
const emitSessionLifecycleEventMock = vi.fn();
|
||||
const noopContextEngine = {
|
||||
info: { id: "test-context-engine", name: "Test context engine" },
|
||||
ingest: async () => ({ ingested: false }),
|
||||
assemble: async () => ({ messages: [], estimatedTokens: 0 }),
|
||||
compact: async () => ({ ok: true, compacted: false }),
|
||||
} satisfies ContextEngine;
|
||||
vi.mock("./subagent-announce.js", () => ({
|
||||
captureSubagentCompletionReply: vi.fn(async () => undefined),
|
||||
runSubagentAnnounceFlow: announceSpy,
|
||||
@@ -108,6 +115,11 @@ describe("subagent registry steer restarts", () => {
|
||||
beforeEach(() => {
|
||||
vi.useRealTimers();
|
||||
lifecycleHandler = undefined;
|
||||
mod.__testing.setDepsForTest({
|
||||
ensureContextEnginesInitialized: () => {},
|
||||
ensureRuntimePluginsLoaded: () => {},
|
||||
resolveContextEngine: async () => noopContextEngine,
|
||||
});
|
||||
announceSpy.mockReset();
|
||||
announceSpy.mockResolvedValue(true);
|
||||
runSubagentEndedHookMock.mockReset();
|
||||
@@ -221,6 +233,7 @@ describe("subagent registry steer restarts", () => {
|
||||
|
||||
afterEach(async () => {
|
||||
vi.useRealTimers();
|
||||
mod.__testing.setDepsForTest();
|
||||
announceSpy.mockReset();
|
||||
announceSpy.mockResolvedValue(true);
|
||||
runSubagentEndedHookMock.mockReset();
|
||||
@@ -294,7 +307,7 @@ describe("subagent registry steer restarts", () => {
|
||||
|
||||
emitLifecycleEnd("run-completion-delayed");
|
||||
|
||||
await vi.waitFor(() => {
|
||||
await waitForRegistrySideEffect(() => {
|
||||
expect(announceSpy).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
expect(runSubagentEndedHookMock).not.toHaveBeenCalled();
|
||||
|
||||
Reference in New Issue
Block a user