From e2be054172d4e6e62da76dc3f8a0314490e97033 Mon Sep 17 00:00:00 2001 From: zhang-guiping Date: Mon, 4 May 2026 21:06:03 +0800 Subject: [PATCH] fix: resolve issue #77296 --- CHANGELOG.md | 1 + src/agents/skills/plugin-skills.test.ts | 151 ++++++++++++++++++++++++ src/agents/skills/plugin-skills.ts | 118 ++++++++++++++++++ src/agents/skills/workspace.ts | 1 + 4 files changed, 271 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e9378424dec..645c9734eff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,7 @@ Docs: https://docs.openclaw.ai - Cron: surface failed isolated-run diagnostics in `cron show`, status, and run history when requested tools are unavailable, so blocked cron runs report the actual tool-policy failure instead of a misleading green result. Fixes #75763. Thanks @RyanSandoval. - TUI/escape abort: track the in-flight runId after `chat.send` resolves so pressing Esc during the gap before the first gateway event aborts the run instead of repeatedly printing `no active run`. Fixes #1296. Thanks @Lukavyi and @romneyda. - TUI/render: stop the long-token sanitizer from injecting literal spaces inside inline code spans, fenced code blocks, table borders, and bare hyphenated/dotted identifiers, so copied package names, entity IDs, and shell line-continuations stay byte-for-byte intact while narrow-terminal protection still chunks unidentifiable long prose tokens. Fixes #48432, #39505. Thanks @DocOellerson, @xeusoc, @CCcassiusdjs, @akramcodez, @brokemac79, @romneyda. +- Plugin skills: publish plugin-declared skills into the managed skills directory (`~/.openclaw/skills/`) via symlinks at resolution time, so the agent SDK file-based discovery paths find plugin skill SKILL.md files and stop logging ENOENT when the agent tries to read them. Fixes #77296. - Gateway/status: label Linux managed gateway services as `systemd user`, making status output explicit about the user-service scope instead of implying a system-level unit. Thanks @vincentkoc. - Plugins/install: remove the previous managed plugin directory when a reinstall switches sources, so stale ClawHub and npm copies no longer keep duplicate plugin ids in discovery after the new install wins. Thanks @vincentkoc. - Plugins/install: let official plugin reinstall recovery repair source-only installed runtime shadows, so `openclaw plugins install npm:@openclaw/discord --force` can replace the bad package instead of stopping at stale config validation. Thanks @vincentkoc. diff --git a/src/agents/skills/plugin-skills.test.ts b/src/agents/skills/plugin-skills.test.ts index 3e1a34c54fa..8eaa9b1a54b 100644 --- a/src/agents/skills/plugin-skills.test.ts +++ b/src/agents/skills/plugin-skills.test.ts @@ -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(); @@ -337,3 +339,152 @@ describe("resolvePluginSkillDirs", () => { expect(dirs).toEqual([path.resolve(pluginRoot, "skills")]); }); }); + +describe("publishPluginSkillsToManagedSkillsDir", () => { + const { publishPluginSkillsToManagedSkillsDir } = __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"); + + publishPluginSkillsToManagedSkillsDir([dirA, dirB], { + managedSkillsDir: 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"); + + publishPluginSkillsToManagedSkillsDir([dir], { managedSkillsDir: 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 }); + 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 a symlink that points to a different target", async () => { + const skillParent = await tempDirs.make("plugin-skills-"); + const managedDir = await tempDirs.make("managed-skills-"); + + const dir1 = await writeSkillDir(skillParent, "skill-v1", "old"); + const dir2 = await writeSkillDir(skillParent, "my-skill", "new"); + + // 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. + publishPluginSkillsToManagedSkillsDir([dir2], { managedSkillsDir: managedDir }); + + expect(fsSync.readlinkSync(path.join(managedDir, "my-skill"))).toBe(dir2); + }); + + it("cleans up stale symlinks whose targets no longer 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 = path.join(skillParent, "stale-skill"); + await fs.mkdir(staleDir, { recursive: true }); + + // Create a stale symlink pointing to a directory we'll delete. + fsSync.symlinkSync(staleDir, path.join(managedDir, "stale-skill"), "dir"); + await fs.rm(staleDir, { recursive: true, force: true }); + + // Publish only the current skill; stale should be cleaned up. + publishPluginSkillsToManagedSkillsDir([dir], { managedSkillsDir: managedDir }); + + expect(fsSync.existsSync(path.join(managedDir, "current-skill"))).toBe(true); + // Stale symlink pointing to nonexistent target should be removed. + 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"); + + publishPluginSkillsToManagedSkillsDir([dir], { managedSkillsDir: 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("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 }); + expect(fsSync.existsSync(managedDir)).toBe(false); + }); + + it("skips directories that do not contain a SKILL.md", 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 }); + + publishPluginSkillsToManagedSkillsDir([emptyDir], { + managedSkillsDir: managedDir, + }); + + expect(fsSync.existsSync(path.join(managedDir, "empty-dir"))).toBe(false); + }); + + it("handles empty skill dirs list without error", async () => { + const managedDir = await tempDirs.make("managed-skills-"); + publishPluginSkillsToManagedSkillsDir([], { managedSkillsDir: 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"); + + publishPluginSkillsToManagedSkillsDir([dir1, dir2], { + managedSkillsDir: managedDir, + }); + + // First one wins. + expect(fsSync.readlinkSync(path.join(managedDir, "shared-name"))).toBe(dir1); + }); +}); diff --git a/src/agents/skills/plugin-skills.ts b/src/agents/skills/plugin-skills.ts index df4b24826ab..2aa2a662667 100644 --- a/src/agents/skills/plugin-skills.ts +++ b/src/agents/skills/plugin-skills.ts @@ -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 managed skills directory for testing. */ + managedSkillsDir?: string; }): string[] { const workspaceDir = (params.workspaceDir ?? "").trim(); if (!workspaceDir) { @@ -93,5 +96,120 @@ export function resolvePluginSkillDirs(params: { } } + publishPluginSkillsToManagedSkillsDir(resolved, { + managedSkillsDir: params.managedSkillsDir, + }); + return resolved; } + +function resolveDefaultManagedSkillsDir(): string { + return path.join(CONFIG_DIR, "skills"); +} + +/** + * Creates symlinks from each resolved plugin skill directory into the + * managed skills directory (~/.openclaw/skills/) so the agent SDK can + * discover them at the conventional file-system path. + */ +function publishPluginSkillsToManagedSkillsDir( + skillDirs: string[], + opts?: { managedSkillsDir?: string }, +): void { + const managedSkillsDir = opts?.managedSkillsDir ?? resolveDefaultManagedSkillsDir(); + 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). + 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); + } + + // Create or update symlinks. + for (const [name, target] of managedTargets) { + const linkPath = path.join(managedSkillsDir, name); + try { + fs.mkdirSync(managedSkillsDir, { recursive: true }); + } catch { + // best-effort; symlink will fail below if dir is truly unusable + } + try { + const existingTarget = fs.readlinkSync(linkPath); + if (existingTarget === target) { + continue; + } + log.warn( + `managed skill symlink "${linkPath}" points to ${existingTarget}, replacing with ${target}`, + ); + fs.unlinkSync(linkPath); + } catch (err) { + if (!isNotFoundError(err)) { + log.warn(`failed to inspect managed 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)}`, + ); + } + } + + // Clean up stale symlinks for plugin skills that are no longer active. + let managedEntries: fs.Dirent[]; + try { + managedEntries = fs.readdirSync(managedSkillsDir, { withFileTypes: true }); + } catch { + return; + } + for (const entry of managedEntries) { + if (!entry.isSymbolicLink()) { + continue; + } + if (managedTargets.has(entry.name)) { + continue; + } + const linkPath = path.join(managedSkillsDir, entry.name); + try { + const target = fs.readlinkSync(linkPath); + // Only remove symlinks that point to directories that no longer exist. + if (!fs.existsSync(target)) { + fs.unlinkSync(linkPath); + } + } catch { + // Broken symlink or other issue — best-effort cleanup. + try { + fs.unlinkSync(linkPath); + } catch { + // ignore + } + } + } +} + +function isNotFoundError(err: unknown): boolean { + if (!err || typeof err !== "object") { + return false; + } + const code = (err as Record).code; + return code === "ENOENT" || code === "ENOTDIR"; +} + +export const __testing = { + publishPluginSkillsToManagedSkillsDir, +}; diff --git a/src/agents/skills/workspace.ts b/src/agents/skills/workspace.ts index bced0bb130f..dd3562b238a 100644 --- a/src/agents/skills/workspace.ts +++ b/src/agents/skills/workspace.ts @@ -636,6 +636,7 @@ function loadSkillEntries( const pluginSkillDirs = resolvePluginSkillDirs({ workspaceDir, config: opts?.config, + managedSkillsDir, }); const mergedExtraDirs = [...extraDirs, ...pluginSkillDirs];