fix #77296: [Bug]: Plugin manifest skills field not published to agent skill discovery paths (#77328)

Summary:
- The PR publishes enabled plugin-declared skill directories into a generated `~/.openclaw/plugin-skills` syml ... plugin-skill precedence, cleans stale generated links, adds regression coverage, and updates the changelog.
- Reproducibility: yes. source-based. Current main resolves plugin-declared skill directories for prompt loadi ... ble generated discovery path, and the linked issue provides a concrete ENOENT path for a plugin `SKILL.md`.

Automerge notes:
- Ran the ClawSweeper repair loop before final review.
- Included post-review commit in the final squash: fix: resolve issue #77296
- Included post-review commit in the final squash: fix: publish plugin manifest skills for agent discovery
- Included post-review commit in the final squash: fix(clawsweeper): address review for automerge-openclaw-openclaw-7732…

Validation:
- ClawSweeper review passed for head 0f52865ee3.
- Required merge gates passed before the squash merge.

Prepared head SHA: 0f52865ee3
Review: https://github.com/openclaw/openclaw/pull/77328#issuecomment-4371415857

Co-authored-by: zhang-guiping <zhang.guiping@xydigit.com>
Co-authored-by: clawsweeper <274271284+clawsweeper[bot]@users.noreply.github.com>
This commit is contained in:
zhang-guiping
2026-05-04 23:31:53 +08:00
committed by GitHub
parent cb38535875
commit 1df2ac442a
5 changed files with 512 additions and 7 deletions

View File

@@ -77,6 +77,7 @@ function loadTestWorkspaceSkillEntries(
return loadWorkspaceSkillEntries(workspaceDir, {
managedSkillsDir: path.join(workspaceDir, ".managed"),
bundledSkillsDir: "",
pluginSkillsDir: path.join(workspaceDir, ".plugin-skills"),
...opts,
});
}
@@ -195,7 +196,17 @@ describe("loadWorkspaceSkillEntries", () => {
managedSkillsDir: managedDir,
});
expect(enabledEntries.map((entry) => entry.skill.name)).toContain("browser-automation");
const browserEntry = enabledEntries.find((entry) => entry.skill.name === "browser-automation");
const browserSkillDir = path.join(pluginRoot, "skills", "browser-automation");
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);
const blockedEntries = loadTestWorkspaceSkillEntries(workspaceDir, {
config: {
@@ -207,6 +218,9 @@ describe("loadWorkspaceSkillEntries", () => {
});
expect(blockedEntries.map((entry) => entry.skill.name)).not.toContain("browser-automation");
await expect(
fs.lstat(path.join(workspaceDir, ".plugin-skills", "browser-automation")),
).rejects.toMatchObject({ code: "ENOENT" });
});
it("loads frontmatter edge cases in one workspace", async () => {

View File

@@ -1,3 +1,4 @@
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";
@@ -8,6 +9,7 @@ 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();
@@ -279,6 +281,31 @@ describe("resolvePluginSkillDirs", () => {
expect(dirs).toEqual([]);
});
it("cleans up generated plugin skill links when the plugin registry is empty", async () => {
const workspaceDir = await tempDirs.make("openclaw-");
const pluginSkillsDir = await tempDirs.make("managed-plugin-skills-");
const staleRoot = await tempDirs.make("stale-plugin-skills-");
const staleSkill = path.join(staleRoot, "stale-skill");
await fs.mkdir(staleSkill, { recursive: true });
fsSync.symlinkSync(staleSkill, path.join(pluginSkillsDir, "stale-skill"), "dir");
hoisted.loadPluginManifestRegistryForInstalledIndex.mockReturnValue({
diagnostics: [],
plugins: [],
});
const dirs = resolvePluginSkillDirs({
workspaceDir,
config: {} as OpenClawConfig,
pluginSkillsDir,
});
expect(dirs).toEqual([]);
await expect(fs.lstat(path.join(pluginSkillsDir, "stale-skill"))).rejects.toMatchObject({
code: "ENOENT",
});
});
it("resolves Claude bundle command roots through the normal plugin skill path", async () => {
const workspaceDir = await tempDirs.make("openclaw-");
const pluginRoot = await tempDirs.make("openclaw-claude-bundle-");
@@ -337,3 +364,191 @@ describe("resolvePluginSkillDirs", () => {
expect(dirs).toEqual([path.resolve(pluginRoot, "skills")]);
});
});
describe("publishPluginSkills", () => {
const { publishPluginSkills } = __testing;
async function writeSkillDir(
parentDir: string,
name: string,
description = `${name} description`,
) {
const dir = path.join(parentDir, name);
await fs.mkdir(dir, { recursive: true });
await fs.writeFile(
path.join(dir, "SKILL.md"),
`---\nname: ${name}\ndescription: ${description}\n---\n\n# ${name}\n`,
);
return dir;
}
it("creates symlinks for each plugin skill dir", async () => {
const skillParent = await tempDirs.make("plugin-skills-");
const managedDir = await tempDirs.make("managed-skills-");
const dirA = await writeSkillDir(skillParent, "skill-a");
const dirB = await writeSkillDir(skillParent, "skill-b");
publishPluginSkills([dirA, dirB], {
pluginSkillsDir: managedDir,
});
const linkA = path.join(managedDir, "skill-a");
const linkB = path.join(managedDir, "skill-b");
expect(fsSync.readlinkSync(linkA)).toBe(dirA);
expect(fsSync.readlinkSync(linkB)).toBe(dirB);
});
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-");
const dir = await writeSkillDir(skillParent, "my-skill");
publishPluginSkills([dir], { pluginSkillsDir: managedDir });
const mtimeAfterFirst = (await fs.lstat(path.join(managedDir, "my-skill"))).mtimeMs;
// Second call with same input should preserve the existing symlink.
publishPluginSkills([dir], { pluginSkillsDir: managedDir });
const mtimeAfterSecond = (await fs.lstat(path.join(managedDir, "my-skill"))).mtimeMs;
expect(mtimeAfterSecond).toBe(mtimeAfterFirst);
expect(fsSync.readlinkSync(path.join(managedDir, "my-skill"))).toBe(dir);
});
it("replaces owned generated symlinks when a plugin skill target moves", async () => {
const skillParent1 = await tempDirs.make("plugin-skills-1-");
const skillParent2 = await tempDirs.make("plugin-skills-2-");
const managedDir = await tempDirs.make("managed-skills-");
const dir1 = await writeSkillDir(skillParent1, "my-skill", "old");
const dir2 = await writeSkillDir(skillParent2, "my-skill", "new");
fsSync.symlinkSync(dir1, path.join(managedDir, "my-skill"), "dir");
publishPluginSkills([dir2], { pluginSkillsDir: managedDir });
expect(fsSync.readlinkSync(path.join(managedDir, "my-skill"))).toBe(dir2);
});
it("cleans up stale symlinks whose targets still exist", async () => {
const skillParent = await tempDirs.make("plugin-skills-");
const managedDir = await tempDirs.make("managed-skills-");
const dir = await writeSkillDir(skillParent, "current-skill");
const staleDir = await writeSkillDir(skillParent, "stale-skill");
fsSync.symlinkSync(staleDir, path.join(managedDir, "stale-skill"), "dir");
publishPluginSkills([dir], { pluginSkillsDir: managedDir });
expect(fsSync.existsSync(path.join(managedDir, "current-skill"))).toBe(true);
expect(fsSync.existsSync(path.join(managedDir, "stale-skill"))).toBe(false);
});
it("cleans up broken symlinks (dangling)", async () => {
const skillParent = await tempDirs.make("plugin-skills-");
const managedDir = await tempDirs.make("managed-skills-");
const dir = await writeSkillDir(skillParent, "current-skill");
const nonexistentDir = path.join(skillParent, "nonexistent");
// Create a symlink to a nonexistent directory.
fsSync.symlinkSync(nonexistentDir, path.join(managedDir, "broken-skill"), "dir");
publishPluginSkills([dir], { pluginSkillsDir: managedDir });
expect(fsSync.existsSync(path.join(managedDir, "current-skill"))).toBe(true);
// Broken symlink pointing to nonexistent target should be removed.
expect(fsSync.existsSync(path.join(managedDir, "broken-skill"))).toBe(false);
});
it.runIf(process.platform !== "win32")(
"skips child skill directories whose SKILL.md symlinks outside the declared root",
async () => {
const skillParent = await tempDirs.make("plugin-skills-");
const managedDir = await tempDirs.make("managed-skills-");
const outsideDir = await tempDirs.make("outside-skill-file-");
const parentDir = path.join(skillParent, "skills");
const leakDir = path.join(parentDir, "leak");
await fs.mkdir(leakDir, { recursive: true });
await fs.writeFile(
path.join(outsideDir, "SKILL.md"),
"---\nname: leak\ndescription: Outside\n---\n",
);
await fs.symlink(path.join(outsideDir, "SKILL.md"), path.join(leakDir, "SKILL.md"));
const validDir = await writeSkillDir(parentDir, "valid");
publishPluginSkills([parentDir], { pluginSkillsDir: managedDir });
expect(fsSync.existsSync(path.join(managedDir, "leak"))).toBe(false);
expect(fsSync.readlinkSync(path.join(managedDir, "valid"))).toBe(validDir);
},
);
it("does not create managed skills dir when skill dirs list is empty", async () => {
const parent = await tempDirs.make("parent-");
const managedDir = path.join(parent, "does-not-exist");
publishPluginSkills([], { pluginSkillsDir: managedDir });
expect(fsSync.existsSync(managedDir)).toBe(false);
});
it("skips directories that do not contain a SKILL.md and have no skill children", async () => {
const skillParent = await tempDirs.make("plugin-skills-");
const managedDir = await tempDirs.make("managed-skills-");
// Create a dir without SKILL.md should be skipped.
const emptyDir = path.join(skillParent, "empty-dir");
await fs.mkdir(emptyDir, { recursive: true });
publishPluginSkills([emptyDir], {
pluginSkillsDir: managedDir,
});
expect(fsSync.existsSync(path.join(managedDir, "empty-dir"))).toBe(false);
});
it("expands parent skill containers to child directories that contain SKILL.md", async () => {
const skillParent = await tempDirs.make("plugin-skills-");
const managedDir = await tempDirs.make("managed-skills-");
// Create a parent skills dir with child skill dirs (the layout used by
// bundled plugins like browser and memory-wiki).
const parentDir = path.join(skillParent, "skills");
const childA = await writeSkillDir(parentDir, "browser");
const childB = await writeSkillDir(parentDir, "memory");
publishPluginSkills([parentDir], {
pluginSkillsDir: managedDir,
});
// Child skill dirs should be published under their basenames.
expect(fsSync.readlinkSync(path.join(managedDir, "browser"))).toBe(childA);
expect(fsSync.readlinkSync(path.join(managedDir, "memory"))).toBe(childB);
// The parent dir itself should NOT be published (no SKILL.md there).
expect(fsSync.existsSync(path.join(managedDir, "skills"))).toBe(false);
});
it("handles empty skill dirs list without error", async () => {
const managedDir = await tempDirs.make("managed-skills-");
publishPluginSkills([], { pluginSkillsDir: managedDir });
// No error expected. The managed dir may or may not be created.
});
it("handles collision: same basename from different plugins uses first one", async () => {
const skillParent1 = await tempDirs.make("plugin-skills-1-");
const skillParent2 = await tempDirs.make("plugin-skills-2-");
const managedDir = await tempDirs.make("managed-skills-");
const dir1 = await writeSkillDir(skillParent1, "shared-name", "first");
const dir2 = await writeSkillDir(skillParent2, "shared-name", "second");
publishPluginSkills([dir1, dir2], {
pluginSkillsDir: managedDir,
});
// First one wins.
expect(fsSync.readlinkSync(path.join(managedDir, "shared-name"))).toBe(dir1);
});
});

View File

@@ -11,12 +11,15 @@ import {
import { loadPluginMetadataSnapshot } from "../../plugins/plugin-metadata-snapshot.js";
import { hasKind } from "../../plugins/slots.js";
import { isPathInsideWithRealpath } from "../../security/scan-paths.js";
import { CONFIG_DIR } from "../../utils.js";
const log = createSubsystemLogger("skills");
export function resolvePluginSkillDirs(params: {
workspaceDir: string | undefined;
config?: OpenClawConfig;
/** Override the plugin skills directory for testing. */
pluginSkillsDir?: string;
}): string[] {
const workspaceDir = (params.workspaceDir ?? "").trim();
if (!workspaceDir) {
@@ -29,6 +32,9 @@ export function resolvePluginSkillDirs(params: {
});
const registry = metadataSnapshot.manifestRegistry;
if (registry.plugins.length === 0) {
publishPluginSkills([], {
pluginSkillsDir: params.pluginSkillsDir,
});
return [];
}
const normalizedPlugins = normalizePluginsConfigWithResolver(
@@ -93,5 +99,160 @@ export function resolvePluginSkillDirs(params: {
}
}
publishPluginSkills(resolved, {
pluginSkillsDir: params.pluginSkillsDir,
});
return resolved;
}
function resolveDefaultPluginSkillsDir(): string {
return path.join(CONFIG_DIR, "plugin-skills");
}
/**
* Collect skill dir targets from a resolved directory.
* If the directory contains a direct SKILL.md it is published as-is.
* Otherwise child subdirectories that contain SKILL.md are expanded.
*/
function collectSkillTargets(dir: string, targets: Map<string, string>): void {
if (hasPublishableSkillFile({ skillDir: dir, rootDir: dir })) {
const basename = path.basename(dir);
const existing = targets.get(basename);
if (existing) {
log.warn(
`plugin skill name collision: "${basename}" resolves to both ${existing} and ${dir}; ` +
`only the first will be published`,
);
return;
}
targets.set(basename, dir);
return;
}
let entries: fs.Dirent[];
try {
entries = fs.readdirSync(dir, { withFileTypes: true });
} catch {
return;
}
for (const entry of entries) {
if (!entry.isDirectory()) continue;
const childPath = path.join(dir, entry.name);
if (!hasPublishableSkillFile({ skillDir: childPath, rootDir: dir })) continue;
const basename = entry.name;
const existing = targets.get(basename);
if (existing) {
log.warn(
`plugin skill name collision: "${basename}" resolves to both ${existing} and ${childPath}; ` +
`only the first will be published`,
);
continue;
}
targets.set(basename, childPath);
}
}
function hasPublishableSkillFile(params: { skillDir: string; rootDir: string }): boolean {
const skillMd = path.join(params.skillDir, "SKILL.md");
let skillMdStat: fs.Stats;
try {
skillMdStat = fs.lstatSync(skillMd);
} catch {
return false;
}
if (!skillMdStat.isFile() || skillMdStat.isSymbolicLink()) {
log.warn(`plugin skill SKILL.md is not a regular file: ${skillMd}`);
return false;
}
if (!isPathInsideWithRealpath(params.rootDir, skillMd, { requireRealpath: true })) {
log.warn(`plugin skill SKILL.md escapes declared skill root: ${skillMd}`);
return false;
}
return true;
}
/**
* Creates symlinks from each resolved plugin skill directory into the
* plugin skills directory (~/.openclaw/plugin-skills/) so the agent SDK can
* discover them at the conventional file-system path.
*
* The plugin-skills directory is fully owned by OpenClaw — every entry is
* a generated symlink. Cleanup of stale links is therefore safe.
*/
function publishPluginSkills(skillDirs: string[], opts?: { pluginSkillsDir?: string }): void {
const pluginSkillsDir = opts?.pluginSkillsDir ?? resolveDefaultPluginSkillsDir();
const managedTargets = new Map<string, string>();
// Collect basename → target mappings, reporting collisions.
// Directories that contain SKILL.md are published as-is.
// Parent containers (e.g. ./skills/) are expanded to their child
// directories that each contain a SKILL.md.
for (const dir of skillDirs) {
collectSkillTargets(dir, managedTargets);
}
// Plugin skill symlinks are owned by OpenClaw and publish at extra-dir
// precedence, so they never shadow managed or bundled skills.
for (const [name, target] of managedTargets) {
const linkPath = path.join(pluginSkillsDir, name);
try {
fs.mkdirSync(pluginSkillsDir, { recursive: true });
} catch {
// best-effort; symlink will fail below if dir is truly unusable
}
try {
const existingTarget = fs.readlinkSync(linkPath);
if (existingTarget === target) {
continue;
}
fs.unlinkSync(linkPath);
} catch (err) {
if (!isNotFoundError(err)) {
log.warn(`failed to inspect plugin skill symlink "${linkPath}": ${String(err)}`);
continue;
}
}
try {
fs.symlinkSync(target, linkPath, "dir");
} catch (err) {
log.warn(`failed to create plugin skill symlink "${linkPath}" → "${target}": ${String(err)}`);
}
}
// Clean up stale symlinks for plugin skills that are no longer active.
// The plugin-skills directory is fully owned by OpenClaw: every entry is a
// generated symlink, so stale-link removal is safe without extra proof.
let existingEntries: fs.Dirent[];
try {
existingEntries = fs.readdirSync(pluginSkillsDir, { withFileTypes: true });
} catch {
return;
}
for (const entry of existingEntries) {
if (!entry.isSymbolicLink()) {
continue;
}
if (managedTargets.has(entry.name)) {
continue;
}
const linkPath = path.join(pluginSkillsDir, entry.name);
try {
fs.unlinkSync(linkPath);
} catch {
// best-effort cleanup
}
}
}
function isNotFoundError(err: unknown): boolean {
if (!err || typeof err !== "object") {
return false;
}
const code = (err as Record<string, unknown>).code;
return code === "ENOENT" || code === "ENOTDIR";
}
export const __testing = {
publishPluginSkills,
};

View File

@@ -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?: {
@@ -412,6 +514,7 @@ function loadSkillEntries(
agentId?: string;
managedSkillsDir?: string;
bundledSkillsDir?: string;
pluginSkillsDir?: string;
},
): SkillEntry[] {
const limits = resolveSkillsLimits(opts?.config, opts?.agentId);
@@ -631,11 +734,13 @@ function loadSkillEntries(
const managedSkillsDir = opts?.managedSkillsDir ?? path.join(CONFIG_DIR, "skills");
const workspaceSkillsDir = path.resolve(workspaceDir, "skills");
const bundledSkillsDir = opts?.bundledSkillsDir ?? resolveBundledSkillsDir();
const pluginSkillsDir = opts?.pluginSkillsDir ?? path.join(CONFIG_DIR, "plugin-skills");
const extraDirsRaw = opts?.config?.skills?.load?.extraDirs ?? [];
const extraDirs = extraDirsRaw.map((d) => normalizeOptionalString(d) ?? "").filter(Boolean);
const pluginSkillDirs = resolvePluginSkillDirs({
workspaceDir,
config: opts?.config,
pluginSkillsDir,
});
const mergedExtraDirs = [...extraDirs, ...pluginSkillDirs];
@@ -645,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",
@@ -937,6 +1050,7 @@ export function loadWorkspaceSkillEntries(
config?: OpenClawConfig;
managedSkillsDir?: string;
bundledSkillsDir?: string;
pluginSkillsDir?: string;
skillFilter?: string[];
agentId?: string;
eligibility?: SkillEligibilityContext;