lzh/picoclaw
1
0
Fork 0
mirror of https://github.com/sipeed/picoclaw.git synced 2026-06-12 18:08:54 +00:00
Code Issues Packages Projects Releases Wiki Activity

perf: refactoring collecting skills (#688)

Browse Source 
* perf: refactoring collecting skills

* Fix order to store dir.Name()

* Add tests
This commit is contained in:
mattn
2026-02-24 08:07:09 +09:00
committed by GitHub
parent 09b1992dd7
commit 6fe3920a4d
2 changed files with 170 additions and 101 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/skills/loader.go
+39 -101
View File
@@ -71,112 +71,50 @@ func NewSkillsLoader(workspace string, globalSkills string, builtinSkills string
func (sl *SkillsLoader) ListSkills() []SkillInfo {
skills := make([]SkillInfo, 0)
seen := make(map[string]bool)
if sl.workspaceSkills != "" {
if dirs, err := os.ReadDir(sl.workspaceSkills); err == nil {
for _, dir := range dirs {
if dir.IsDir() {
skillFile := filepath.Join(sl.workspaceSkills, dir.Name(), "SKILL.md")
if _, err := os.Stat(skillFile); err == nil {
info := SkillInfo{
Name: dir.Name(),
Path: skillFile,
Source: "workspace",
}
metadata := sl.getSkillMetadata(skillFile)
if metadata != nil {
info.Description = metadata.Description
info.Name = metadata.Name
}
if err := info.validate(); err != nil {
slog.Warn("invalid skill from workspace", "name", info.Name, "error", err)
continue
}
skills = append(skills, info)
}
}
addSkills := func(dir, source string) {
if dir == "" {
return
}
dirs, err := os.ReadDir(dir)
if err != nil {
return
}
for _, d := range dirs {
if !d.IsDir() {
continue
}
skillFile := filepath.Join(dir, d.Name(), "SKILL.md")
if _, err := os.Stat(skillFile); err != nil {
continue
}
info := SkillInfo{
Name: d.Name(),
Path: skillFile,
Source: source,
}
metadata := sl.getSkillMetadata(skillFile)
if metadata != nil {
info.Description = metadata.Description
info.Name = metadata.Name
}
if err := info.validate(); err != nil {
slog.Warn("invalid skill from "+source, "name", info.Name, "error", err)
continue
}
if seen[info.Name] {
continue
}
seen[info.Name] = true
skills = append(skills, info)
}
}
// 全局 skills (~/.picoclaw/skills) - 被 workspace skills 覆盖
if sl.globalSkills != "" {
if dirs, err := os.ReadDir(sl.globalSkills); err == nil {
for _, dir := range dirs {
if dir.IsDir() {
skillFile := filepath.Join(sl.globalSkills, dir.Name(), "SKILL.md")
if _, err := os.Stat(skillFile); err == nil {
// 检查是否已被 workspace skills 覆盖
exists := false
for _, s := range skills {
if s.Name == dir.Name() && s.Source == "workspace" {
exists = true
break
}
}
if exists {
continue
}
info := SkillInfo{
Name: dir.Name(),
Path: skillFile,
Source: "global",
}
metadata := sl.getSkillMetadata(skillFile)
if metadata != nil {
info.Description = metadata.Description
info.Name = metadata.Name
}
if err := info.validate(); err != nil {
slog.Warn("invalid skill from global", "name", info.Name, "error", err)
continue
}
skills = append(skills, info)
}
}
}
}
}
if sl.builtinSkills != "" {
if dirs, err := os.ReadDir(sl.builtinSkills); err == nil {
for _, dir := range dirs {
if dir.IsDir() {
skillFile := filepath.Join(sl.builtinSkills, dir.Name(), "SKILL.md")
if _, err := os.Stat(skillFile); err == nil {
// 检查是否已被 workspace 或 global skills 覆盖
exists := false
for _, s := range skills {
if s.Name == dir.Name() && (s.Source == "workspace" || s.Source == "global") {
exists = true
break
}
}
if exists {
continue
}
info := SkillInfo{
Name: dir.Name(),
Path: skillFile,
Source: "builtin",
}
metadata := sl.getSkillMetadata(skillFile)
if metadata != nil {
info.Description = metadata.Description
info.Name = metadata.Name
}
if err := info.validate(); err != nil {
slog.Warn("invalid skill from builtin", "name", info.Name, "error", err)
continue
}
skills = append(skills, info)
}
}
}
}
}
// Priority: workspace > global > builtin
addSkills(sl.workspaceSkills, "workspace")
addSkills(sl.globalSkills, "global")
addSkills(sl.builtinSkills, "builtin")
return skills
}
pkg/skills/loader_test.go
+131
View File
@@ -1,9 +1,12 @@
package skills
import (
"os"
"path/filepath"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestSkillsInfoValidate(t *testing.T) {
@@ -135,6 +138,134 @@ func TestExtractFrontmatter(t *testing.T) {
}
}
// createSkillDir creates a skill directory with a SKILL.md file containing the given frontmatter.
func createSkillDir(t *testing.T, base, dirName, name, description string) {
t.Helper()
dir := filepath.Join(base, dirName)
require.NoError(t, os.MkdirAll(dir, 0o755))
content := "---\nname: " + name + "\ndescription: " + description + "\n---\n\n# " + name
require.NoError(t, os.WriteFile(filepath.Join(dir, "SKILL.md"), []byte(content), 0o644))
}
func TestListSkillsWorkspaceOverridesGlobal(t *testing.T) {
tmp := t.TempDir()
ws := filepath.Join(tmp, "workspace")
global := filepath.Join(tmp, "global")
createSkillDir(t, filepath.Join(ws, "skills"), "my-skill", "my-skill", "workspace version")
createSkillDir(t, global, "my-skill", "my-skill", "global version")
sl := NewSkillsLoader(ws, global, "")
skills := sl.ListSkills()
assert.Len(t, skills, 1)
assert.Equal(t, "workspace", skills[0].Source)
assert.Equal(t, "workspace version", skills[0].Description)
}
func TestListSkillsGlobalOverridesBuiltin(t *testing.T) {
tmp := t.TempDir()
ws := filepath.Join(tmp, "workspace")
global := filepath.Join(tmp, "global")
builtin := filepath.Join(tmp, "builtin")
createSkillDir(t, global, "my-skill", "my-skill", "global version")
createSkillDir(t, builtin, "my-skill", "my-skill", "builtin version")
sl := NewSkillsLoader(ws, global, builtin)
skills := sl.ListSkills()
assert.Len(t, skills, 1)
assert.Equal(t, "global", skills[0].Source)
assert.Equal(t, "global version", skills[0].Description)
}
func TestListSkillsMetadataNameDedup(t *testing.T) {
tmp := t.TempDir()
ws := filepath.Join(tmp, "workspace")
global := filepath.Join(tmp, "global")
// Different directory names but same metadata name
createSkillDir(t, filepath.Join(ws, "skills"), "dir-a", "shared-name", "workspace version")
createSkillDir(t, global, "dir-b", "shared-name", "global version")
sl := NewSkillsLoader(ws, global, "")
skills := sl.ListSkills()
assert.Len(t, skills, 1)
assert.Equal(t, "shared-name", skills[0].Name)
assert.Equal(t, "workspace", skills[0].Source)
}
func TestListSkillsMultipleDistinctSkills(t *testing.T) {
tmp := t.TempDir()
ws := filepath.Join(tmp, "workspace")
global := filepath.Join(tmp, "global")
builtin := filepath.Join(tmp, "builtin")
createSkillDir(t, filepath.Join(ws, "skills"), "skill-a", "skill-a", "desc a")
createSkillDir(t, global, "skill-b", "skill-b", "desc b")
createSkillDir(t, builtin, "skill-c", "skill-c", "desc c")
sl := NewSkillsLoader(ws, global, builtin)
skills := sl.ListSkills()
assert.Len(t, skills, 3)
names := map[string]string{}
for _, s := range skills {
names[s.Name] = s.Source
}
assert.Equal(t, "workspace", names["skill-a"])
assert.Equal(t, "global", names["skill-b"])
assert.Equal(t, "builtin", names["skill-c"])
}
func TestListSkillsInvalidSkillSkipped(t *testing.T) {
tmp := t.TempDir()
ws := filepath.Join(tmp, "workspace")
global := filepath.Join(tmp, "global")
// Invalid name (underscore)
createSkillDir(t, filepath.Join(ws, "skills"), "bad_skill", "bad_skill", "desc")
// Valid skill
createSkillDir(t, global, "good-skill", "good-skill", "desc")
sl := NewSkillsLoader(ws, global, "")
skills := sl.ListSkills()
assert.Len(t, skills, 1)
assert.Equal(t, "good-skill", skills[0].Name)
}
func TestListSkillsEmptyAndNonexistentDirs(t *testing.T) {
tmp := t.TempDir()
ws := filepath.Join(tmp, "workspace")
emptyDir := filepath.Join(tmp, "empty")
require.NoError(t, os.MkdirAll(emptyDir, 0o755))
sl := NewSkillsLoader(ws, emptyDir, filepath.Join(tmp, "nonexistent"))
skills := sl.ListSkills()
assert.Empty(t, skills)
}
func TestListSkillsDirWithoutSkillMD(t *testing.T) {
tmp := t.TempDir()
ws := filepath.Join(tmp, "workspace")
global := filepath.Join(tmp, "global")
// Directory exists but has no SKILL.md
require.NoError(t, os.MkdirAll(filepath.Join(global, "no-skillmd"), 0o755))
// Valid skill alongside
createSkillDir(t, global, "real-skill", "real-skill", "desc")
sl := NewSkillsLoader(ws, global, "")
skills := sl.ListSkills()
assert.Len(t, skills, 1)
assert.Equal(t, "real-skill", skills[0].Name)
}
func TestStripFrontmatter(t *testing.T) {
sl := &SkillsLoader{}