From e06bca9e6cd08df3477e99468b9bd7e3060014de Mon Sep 17 00:00:00 2001 From: zhang-guiping Date: Mon, 4 May 2026 21:50:28 +0800 Subject: [PATCH] fix: resolve issue #77296 --- src/agents/skills/plugin-skills.test.ts | 31 +++++++++-- src/agents/skills/plugin-skills.ts | 69 ++++++++++++++++++------- 2 files changed, 78 insertions(+), 22 deletions(-) diff --git a/src/agents/skills/plugin-skills.test.ts b/src/agents/skills/plugin-skills.test.ts index 8eaa9b1a54b..966f3c0b7f3 100644 --- a/src/agents/skills/plugin-skills.test.ts +++ b/src/agents/skills/plugin-skills.test.ts @@ -391,7 +391,7 @@ describe("publishPluginSkillsToManagedSkillsDir", () => { expect(fsSync.readlinkSync(path.join(managedDir, "my-skill"))).toBe(dir); }); - it("replaces a symlink that points to a different target", async () => { + it("preserves existing managed skill symlinks when a plugin skill has the same name", async () => { const skillParent = await tempDirs.make("plugin-skills-"); const managedDir = await tempDirs.make("managed-skills-"); @@ -401,10 +401,11 @@ describe("publishPluginSkillsToManagedSkillsDir", () => { // Manually create a symlink to dir1 under the same name as dir2's basename. fsSync.symlinkSync(dir1, path.join(managedDir, "my-skill"), "dir"); - // Now publish dir2 (basename "my-skill"); should replace the symlink. + // Now publish dir2 (basename "my-skill"); must NOT replace existing symlink. publishPluginSkillsToManagedSkillsDir([dir2], { managedSkillsDir: managedDir }); - expect(fsSync.readlinkSync(path.join(managedDir, "my-skill"))).toBe(dir2); + // Existing managed symlink is preserved. + expect(fsSync.readlinkSync(path.join(managedDir, "my-skill"))).toBe(dir1); }); it("cleans up stale symlinks whose targets no longer exist", async () => { @@ -451,7 +452,7 @@ describe("publishPluginSkillsToManagedSkillsDir", () => { expect(fsSync.existsSync(managedDir)).toBe(false); }); - it("skips directories that do not contain a SKILL.md", async () => { + 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-"); @@ -466,6 +467,28 @@ describe("publishPluginSkillsToManagedSkillsDir", () => { 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"); + + publishPluginSkillsToManagedSkillsDir([parentDir], { + managedSkillsDir: 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-"); publishPluginSkillsToManagedSkillsDir([], { managedSkillsDir: managedDir }); diff --git a/src/agents/skills/plugin-skills.ts b/src/agents/skills/plugin-skills.ts index 2aa2a662667..6b8efefa6dd 100644 --- a/src/agents/skills/plugin-skills.ts +++ b/src/agents/skills/plugin-skills.ts @@ -107,6 +107,49 @@ function resolveDefaultManagedSkillsDir(): string { return path.join(CONFIG_DIR, "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): void { + if (fs.existsSync(path.join(dir, "SKILL.md"))) { + 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 to managed skills`, + ); + 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 (!fs.existsSync(path.join(childPath, "SKILL.md"))) 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 to managed skills`, + ); + continue; + } + targets.set(basename, childPath); + } +} + /** * Creates symlinks from each resolved plugin skill directory into the * managed skills directory (~/.openclaw/skills/) so the agent SDK can @@ -120,25 +163,15 @@ function publishPluginSkillsToManagedSkillsDir( const managedTargets = new Map(); // Collect basename → target mappings, reporting collisions. - // Only publish directories that contain a SKILL.md (actual skill dirs, - // not parent containers like ./skills/ that hold multiple skills). + // 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) { - if (!fs.existsSync(path.join(dir, "SKILL.md"))) { - continue; - } - const basename = path.basename(dir); - const existing = managedTargets.get(basename); - if (existing) { - log.warn( - `plugin skill name collision: "${basename}" resolves to both ${existing} and ${dir}; ` + - `only the first will be published to managed skills`, - ); - continue; - } - managedTargets.set(basename, dir); + collectSkillTargets(dir, managedTargets); } - // Create or update symlinks. + // Create symlinks — but never replace an existing managed entry. + // Managed skills outrank plugin extra dirs. for (const [name, target] of managedTargets) { const linkPath = path.join(managedSkillsDir, name); try { @@ -152,9 +185,9 @@ function publishPluginSkillsToManagedSkillsDir( continue; } log.warn( - `managed skill symlink "${linkPath}" points to ${existingTarget}, replacing with ${target}`, + `managed skill symlink "${linkPath}" already exists, skipping plugin skill "${target}"`, ); - fs.unlinkSync(linkPath); + continue; } catch (err) { if (!isNotFoundError(err)) { log.warn(`failed to inspect managed skill symlink "${linkPath}": ${String(err)}`);