From a5abb25ef04b0b40a60a178cd86f3bf3c6cab72c Mon Sep 17 00:00:00 2001 From: zhang-guiping Date: Mon, 4 May 2026 22:35:27 +0800 Subject: [PATCH] fix: resolve issue #77296 --- .../skills.loadworkspaceskillentries.test.ts | 1 + src/agents/skills/plugin-skills.test.ts | 34 +++++------ src/agents/skills/plugin-skills.ts | 56 +++++++++---------- src/agents/skills/workspace.ts | 7 ++- 4 files changed, 51 insertions(+), 47 deletions(-) diff --git a/src/agents/skills.loadworkspaceskillentries.test.ts b/src/agents/skills.loadworkspaceskillentries.test.ts index 3f3c0a49c2b..eabaf41eeef 100644 --- a/src/agents/skills.loadworkspaceskillentries.test.ts +++ b/src/agents/skills.loadworkspaceskillentries.test.ts @@ -77,6 +77,7 @@ function loadTestWorkspaceSkillEntries( return loadWorkspaceSkillEntries(workspaceDir, { managedSkillsDir: path.join(workspaceDir, ".managed"), bundledSkillsDir: "", + pluginSkillsDir: path.join(workspaceDir, ".plugin-skills"), ...opts, }); } diff --git a/src/agents/skills/plugin-skills.test.ts b/src/agents/skills/plugin-skills.test.ts index 966f3c0b7f3..a690d3a1dd8 100644 --- a/src/agents/skills/plugin-skills.test.ts +++ b/src/agents/skills/plugin-skills.test.ts @@ -340,8 +340,8 @@ describe("resolvePluginSkillDirs", () => { }); }); -describe("publishPluginSkillsToManagedSkillsDir", () => { - const { publishPluginSkillsToManagedSkillsDir } = __testing; +describe("publishPluginSkills", () => { + const { publishPluginSkills } = __testing; async function writeSkillDir( parentDir: string, @@ -364,8 +364,8 @@ describe("publishPluginSkillsToManagedSkillsDir", () => { const dirA = await writeSkillDir(skillParent, "skill-a"); const dirB = await writeSkillDir(skillParent, "skill-b"); - publishPluginSkillsToManagedSkillsDir([dirA, dirB], { - managedSkillsDir: managedDir, + publishPluginSkills([dirA, dirB], { + pluginSkillsDir: managedDir, }); const linkA = path.join(managedDir, "skill-a"); @@ -380,11 +380,11 @@ describe("publishPluginSkillsToManagedSkillsDir", () => { const dir = await writeSkillDir(skillParent, "my-skill"); - publishPluginSkillsToManagedSkillsDir([dir], { managedSkillsDir: managedDir }); + 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. - publishPluginSkillsToManagedSkillsDir([dir], { managedSkillsDir: managedDir }); + publishPluginSkills([dir], { pluginSkillsDir: managedDir }); const mtimeAfterSecond = (await fs.lstat(path.join(managedDir, "my-skill"))).mtimeMs; expect(mtimeAfterSecond).toBe(mtimeAfterFirst); @@ -402,7 +402,7 @@ describe("publishPluginSkillsToManagedSkillsDir", () => { fsSync.symlinkSync(dir1, path.join(managedDir, "my-skill"), "dir"); // Now publish dir2 (basename "my-skill"); must NOT replace existing symlink. - publishPluginSkillsToManagedSkillsDir([dir2], { managedSkillsDir: managedDir }); + publishPluginSkills([dir2], { pluginSkillsDir: managedDir }); // Existing managed symlink is preserved. expect(fsSync.readlinkSync(path.join(managedDir, "my-skill"))).toBe(dir1); @@ -421,7 +421,7 @@ describe("publishPluginSkillsToManagedSkillsDir", () => { await fs.rm(staleDir, { recursive: true, force: true }); // Publish only the current skill; stale should be cleaned up. - publishPluginSkillsToManagedSkillsDir([dir], { managedSkillsDir: managedDir }); + publishPluginSkills([dir], { pluginSkillsDir: managedDir }); expect(fsSync.existsSync(path.join(managedDir, "current-skill"))).toBe(true); // Stale symlink pointing to nonexistent target should be removed. @@ -438,7 +438,7 @@ describe("publishPluginSkillsToManagedSkillsDir", () => { // Create a symlink to a nonexistent directory. fsSync.symlinkSync(nonexistentDir, path.join(managedDir, "broken-skill"), "dir"); - publishPluginSkillsToManagedSkillsDir([dir], { managedSkillsDir: managedDir }); + publishPluginSkills([dir], { pluginSkillsDir: managedDir }); expect(fsSync.existsSync(path.join(managedDir, "current-skill"))).toBe(true); // Broken symlink pointing to nonexistent target should be removed. @@ -448,7 +448,7 @@ describe("publishPluginSkillsToManagedSkillsDir", () => { 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"); - publishPluginSkillsToManagedSkillsDir([], { managedSkillsDir: managedDir }); + publishPluginSkills([], { pluginSkillsDir: managedDir }); expect(fsSync.existsSync(managedDir)).toBe(false); }); @@ -460,8 +460,8 @@ describe("publishPluginSkillsToManagedSkillsDir", () => { const emptyDir = path.join(skillParent, "empty-dir"); await fs.mkdir(emptyDir, { recursive: true }); - publishPluginSkillsToManagedSkillsDir([emptyDir], { - managedSkillsDir: managedDir, + publishPluginSkills([emptyDir], { + pluginSkillsDir: managedDir, }); expect(fsSync.existsSync(path.join(managedDir, "empty-dir"))).toBe(false); @@ -477,8 +477,8 @@ describe("publishPluginSkillsToManagedSkillsDir", () => { const childA = await writeSkillDir(parentDir, "browser"); const childB = await writeSkillDir(parentDir, "memory"); - publishPluginSkillsToManagedSkillsDir([parentDir], { - managedSkillsDir: managedDir, + publishPluginSkills([parentDir], { + pluginSkillsDir: managedDir, }); // Child skill dirs should be published under their basenames. @@ -491,7 +491,7 @@ describe("publishPluginSkillsToManagedSkillsDir", () => { it("handles empty skill dirs list without error", async () => { const managedDir = await tempDirs.make("managed-skills-"); - publishPluginSkillsToManagedSkillsDir([], { managedSkillsDir: managedDir }); + publishPluginSkills([], { pluginSkillsDir: managedDir }); // No error expected. The managed dir may or may not be created. }); @@ -503,8 +503,8 @@ describe("publishPluginSkillsToManagedSkillsDir", () => { const dir1 = await writeSkillDir(skillParent1, "shared-name", "first"); const dir2 = await writeSkillDir(skillParent2, "shared-name", "second"); - publishPluginSkillsToManagedSkillsDir([dir1, dir2], { - managedSkillsDir: managedDir, + publishPluginSkills([dir1, dir2], { + pluginSkillsDir: managedDir, }); // First one wins. diff --git a/src/agents/skills/plugin-skills.ts b/src/agents/skills/plugin-skills.ts index 6b8efefa6dd..7d65cbc50f8 100644 --- a/src/agents/skills/plugin-skills.ts +++ b/src/agents/skills/plugin-skills.ts @@ -18,8 +18,8 @@ const log = createSubsystemLogger("skills"); export function resolvePluginSkillDirs(params: { workspaceDir: string | undefined; config?: OpenClawConfig; - /** Override the managed skills directory for testing. */ - managedSkillsDir?: string; + /** Override the plugin skills directory for testing. */ + pluginSkillsDir?: string; }): string[] { const workspaceDir = (params.workspaceDir ?? "").trim(); if (!workspaceDir) { @@ -96,15 +96,15 @@ export function resolvePluginSkillDirs(params: { } } - publishPluginSkillsToManagedSkillsDir(resolved, { - managedSkillsDir: params.managedSkillsDir, + publishPluginSkills(resolved, { + pluginSkillsDir: params.pluginSkillsDir, }); return resolved; } -function resolveDefaultManagedSkillsDir(): string { - return path.join(CONFIG_DIR, "skills"); +function resolveDefaultPluginSkillsDir(): string { + return path.join(CONFIG_DIR, "plugin-skills"); } /** @@ -119,7 +119,7 @@ function collectSkillTargets(dir: string, targets: Map): void { if (existing) { log.warn( `plugin skill name collision: "${basename}" resolves to both ${existing} and ${dir}; ` + - `only the first will be published to managed skills`, + `only the first will be published`, ); return; } @@ -142,7 +142,7 @@ function collectSkillTargets(dir: string, targets: Map): void { if (existing) { log.warn( `plugin skill name collision: "${basename}" resolves to both ${existing} and ${childPath}; ` + - `only the first will be published to managed skills`, + `only the first will be published`, ); continue; } @@ -152,14 +152,14 @@ function collectSkillTargets(dir: string, targets: Map): void { /** * Creates symlinks from each resolved plugin skill directory into the - * managed skills directory (~/.openclaw/skills/) so the agent SDK can + * 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 publishPluginSkillsToManagedSkillsDir( - skillDirs: string[], - opts?: { managedSkillsDir?: string }, -): void { - const managedSkillsDir = opts?.managedSkillsDir ?? resolveDefaultManagedSkillsDir(); +function publishPluginSkills(skillDirs: string[], opts?: { pluginSkillsDir?: string }): void { + const pluginSkillsDir = opts?.pluginSkillsDir ?? resolveDefaultPluginSkillsDir(); const managedTargets = new Map(); // Collect basename → target mappings, reporting collisions. @@ -170,12 +170,12 @@ function publishPluginSkillsToManagedSkillsDir( collectSkillTargets(dir, managedTargets); } - // Create symlinks — but never replace an existing managed entry. - // Managed skills outrank plugin extra dirs. + // 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(managedSkillsDir, name); + const linkPath = path.join(pluginSkillsDir, name); try { - fs.mkdirSync(managedSkillsDir, { recursive: true }); + fs.mkdirSync(pluginSkillsDir, { recursive: true }); } catch { // best-effort; symlink will fail below if dir is truly unusable } @@ -185,39 +185,39 @@ function publishPluginSkillsToManagedSkillsDir( continue; } log.warn( - `managed skill symlink "${linkPath}" already exists, skipping plugin skill "${target}"`, + `plugin skill symlink "${linkPath}" already exists, skipping plugin skill "${target}"`, ); continue; } catch (err) { if (!isNotFoundError(err)) { - log.warn(`failed to inspect managed skill symlink "${linkPath}": ${String(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 managed skill symlink "${linkPath}" → "${target}": ${String(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. - let managedEntries: fs.Dirent[]; + // 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 { - managedEntries = fs.readdirSync(managedSkillsDir, { withFileTypes: true }); + existingEntries = fs.readdirSync(pluginSkillsDir, { withFileTypes: true }); } catch { return; } - for (const entry of managedEntries) { + for (const entry of existingEntries) { if (!entry.isSymbolicLink()) { continue; } if (managedTargets.has(entry.name)) { continue; } - const linkPath = path.join(managedSkillsDir, entry.name); + const linkPath = path.join(pluginSkillsDir, entry.name); try { const target = fs.readlinkSync(linkPath); // Only remove symlinks that point to directories that no longer exist. @@ -244,5 +244,5 @@ function isNotFoundError(err: unknown): boolean { } export const __testing = { - publishPluginSkillsToManagedSkillsDir, + publishPluginSkills, }; diff --git a/src/agents/skills/workspace.ts b/src/agents/skills/workspace.ts index dd3562b238a..554de3a682d 100644 --- a/src/agents/skills/workspace.ts +++ b/src/agents/skills/workspace.ts @@ -412,6 +412,7 @@ function loadSkillEntries( agentId?: string; managedSkillsDir?: string; bundledSkillsDir?: string; + pluginSkillsDir?: string; }, ): SkillEntry[] { const limits = resolveSkillsLimits(opts?.config, opts?.agentId); @@ -631,14 +632,15 @@ 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, - managedSkillsDir, + pluginSkillsDir, }); - const mergedExtraDirs = [...extraDirs, ...pluginSkillDirs]; + const mergedExtraDirs = [...extraDirs, pluginSkillsDir, ...pluginSkillDirs]; const bundledSkills = bundledSkillsDir ? loadSkills({ @@ -938,6 +940,7 @@ export function loadWorkspaceSkillEntries( config?: OpenClawConfig; managedSkillsDir?: string; bundledSkillsDir?: string; + pluginSkillsDir?: string; skillFilter?: string[]; agentId?: string; eligibility?: SkillEligibilityContext;