From 0f52865ee3a9b268bf33ab012a787b2b32fdd050 Mon Sep 17 00:00:00 2001 From: clawsweeper <274271284+clawsweeper[bot]@users.noreply.github.com> Date: Mon, 4 May 2026 15:20:54 +0000 Subject: [PATCH] fix(clawsweeper): address review for automerge-openclaw-openclaw-77328 (1) --- .../skills.loadworkspaceskillentries.test.ts | 7 +- src/agents/skills/workspace.ts | 124 +++++++++++++++++- 2 files changed, 123 insertions(+), 8 deletions(-) diff --git a/src/agents/skills.loadworkspaceskillentries.test.ts b/src/agents/skills.loadworkspaceskillentries.test.ts index ca8e2d59cb7..5f0ba4a3347 100644 --- a/src/agents/skills.loadworkspaceskillentries.test.ts +++ b/src/agents/skills.loadworkspaceskillentries.test.ts @@ -198,7 +198,12 @@ describe("loadWorkspaceSkillEntries", () => { const browserEntry = enabledEntries.find((entry) => entry.skill.name === "browser-automation"); const browserSkillDir = path.join(pluginRoot, "skills", "browser-automation"); - expect(browserEntry?.skill.baseDir).toBe(browserSkillDir); + expect(browserEntry?.skill.baseDir).toBe( + path.join(workspaceDir, ".plugin-skills", "browser-automation"), + ); + expect(browserEntry?.skill.filePath).toBe( + path.join(workspaceDir, ".plugin-skills", "browser-automation", "SKILL.md"), + ); await expect( fs.readlink(path.join(workspaceDir, ".plugin-skills", "browser-automation")), ).resolves.toBe(browserSkillDir); diff --git a/src/agents/skills/workspace.ts b/src/agents/skills/workspace.ts index 554de3a682d..0b4b569779a 100644 --- a/src/agents/skills/workspace.ts +++ b/src/agents/skills/workspace.ts @@ -405,6 +405,108 @@ function loadContainedSkillRecords(params: { ); } +function isPathInsideAnyRoot(rootRealPaths: readonly string[], candidateRealPath: string): boolean { + return rootRealPaths.some((rootRealPath) => isPathInside(rootRealPath, candidateRealPath)); +} + +function resolvePluginSkillRootRealPaths(pluginSkillDirs: readonly string[]): string[] { + return pluginSkillDirs + .map((dir) => tryRealpath(dir)) + .filter((dir): dir is string => Boolean(dir)) + .filter((dir, index, all) => all.indexOf(dir) === index); +} + +function loadGeneratedPluginSkillRecords(params: { + pluginSkillsDir: string; + pluginSkillDirs: readonly string[]; + source: string; + limits: ResolvedSkillsLimits; +}): LoadedSkillRecord[] { + const allowedRootRealPaths = resolvePluginSkillRootRealPaths(params.pluginSkillDirs); + if (allowedRootRealPaths.length === 0) { + return []; + } + + const rootDir = path.resolve(params.pluginSkillsDir); + if (!fs.existsSync(rootDir)) { + return []; + } + const rootRealPath = tryRealpath(rootDir) ?? rootDir; + const maxCandidatesPerRoot = Math.max(0, params.limits.maxCandidatesPerRoot); + const maxSkillsLoadedPerSource = Math.max(0, params.limits.maxSkillsLoadedPerSource); + const childDirScan = listChildDirectories(rootDir, { + maxCandidateDirs: maxCandidatesPerRoot, + }); + const childDirs = + maxSkillsLoadedPerSource === 0 + ? [] + : childDirScan.dirs.toSorted().slice(0, maxCandidatesPerRoot); + const loadedSkills: LoadedSkillRecord[] = []; + + for (const name of childDirs) { + const skillDir = path.join(rootDir, name); + if (!isSymlinkPath(skillDir)) { + continue; + } + const skillDirRealPath = tryRealpath(skillDir); + if (!skillDirRealPath || !isPathInsideAnyRoot(allowedRootRealPaths, skillDirRealPath)) { + if (skillDirRealPath) { + warnEscapedSkillPath({ + source: params.source, + rootDir, + rootRealPath, + candidatePath: path.resolve(skillDir), + candidateRealPath: skillDirRealPath, + }); + } + continue; + } + + const skillMd = path.join(skillDir, "SKILL.md"); + let skillMdStat: fs.Stats; + try { + skillMdStat = fs.lstatSync(skillMd); + } catch { + continue; + } + if (!skillMdStat.isFile() || skillMdStat.isSymbolicLink()) { + continue; + } + const skillMdRealPath = tryRealpath(skillMd); + if (!skillMdRealPath || !isPathInside(skillDirRealPath, skillMdRealPath)) { + continue; + } + if (skillMdStat.size > params.limits.maxSkillFileBytes) { + skillsLogger.warn("Skipping skill due to oversized SKILL.md.", { + skill: name, + filePath: skillMd, + size: skillMdStat.size, + maxSkillFileBytes: params.limits.maxSkillFileBytes, + }); + continue; + } + + loadedSkills.push( + ...loadContainedSkillRecords({ + skillDir, + source: params.source, + maxSkillFileBytes: params.limits.maxSkillFileBytes, + }), + ); + if (loadedSkills.length >= maxSkillsLoadedPerSource) { + break; + } + } + + if (loadedSkills.length > maxSkillsLoadedPerSource) { + return loadedSkills + .slice() + .sort((a, b) => a.skill.name.localeCompare(b.skill.name, "en")) + .slice(0, maxSkillsLoadedPerSource); + } + return loadedSkills; +} + function loadSkillEntries( workspaceDir: string, opts?: { @@ -640,7 +742,7 @@ function loadSkillEntries( config: opts?.config, pluginSkillsDir, }); - const mergedExtraDirs = [...extraDirs, pluginSkillsDir, ...pluginSkillDirs]; + const mergedExtraDirs = [...extraDirs, ...pluginSkillDirs]; const bundledSkills = bundledSkillsDir ? loadSkills({ @@ -648,13 +750,21 @@ function loadSkillEntries( source: "openclaw-bundled", }) : []; - const extraSkills = mergedExtraDirs.flatMap((dir) => { - const resolved = resolveUserPath(dir); - return loadSkills({ - dir: resolved, + const extraSkills = [ + ...mergedExtraDirs.flatMap((dir) => { + const resolved = resolveUserPath(dir); + return loadSkills({ + dir: resolved, + source: "openclaw-extra", + }); + }), + ...loadGeneratedPluginSkillRecords({ + pluginSkillsDir, + pluginSkillDirs, source: "openclaw-extra", - }); - }); + limits, + }), + ]; const managedSkills = loadSkills({ dir: managedSkillsDir, source: "openclaw-managed",