From af711f9e9f61477c0c85c571a4d7ad95eff103e4 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 19 Apr 2026 01:57:24 +0100 Subject: [PATCH] perf: speed up subagent and skill tests --- src/agents/skills/local-loader.ts | 54 ++++++++----- src/agents/skills/workspace.ts | 76 +++++++++++-------- .../subagent-registry.steer-restart.test.ts | 15 +++- 3 files changed, 97 insertions(+), 48 deletions(-) diff --git a/src/agents/skills/local-loader.ts b/src/agents/skills/local-loader.ts index 0785bac169c..c7f367e62b8 100644 --- a/src/agents/skills/local-loader.ts +++ b/src/agents/skills/local-loader.ts @@ -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; } { 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(); + 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: { diff --git a/src/agents/skills/workspace.ts b/src/agents/skills/workspace.ts index 7f06b99026a..496131452bc 100644 --- a/src/agents/skills/workspace.ts +++ b/src/agents/skills/workspace.ts @@ -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) + : 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(); + const merged = new Map(); // 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, diff --git a/src/agents/subagent-registry.steer-restart.test.ts b/src/agents/subagent-registry.steer-restart.test.ts index d8e212e43ba..1c68cc72202 100644 --- a/src/agents/subagent-registry.steer-restart.test.ts +++ b/src/agents/subagent-registry.steer-restart.test.ts @@ -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();