diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index 808d12c07..094e38fa3 100644 --- a/pkg/agent/loop.go +++ b/pkg/agent/loop.go @@ -335,6 +335,12 @@ func registerSharedTools( } } + // Skill management tool (create/update/delete workspace skills). + if cfg.Tools.IsToolEnabled("skill_manage") { + skillMgr := skills.NewSkillManager(filepath.Join(agent.Workspace, "skills")) + agent.Tools.Register(tools.NewSkillManageTool(skillMgr)) + } + // Spawn and spawn_status tools share a SubagentManager. // Construct it when either tool is enabled (both require subagent). spawnEnabled := cfg.Tools.IsToolEnabled("spawn") diff --git a/pkg/config/config.go b/pkg/config/config.go index 7165246e5..a30a2045b 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -863,6 +863,7 @@ type ToolsConfig struct { FindSkills ToolConfig `json:"find_skills" yaml:"-" envPrefix:"PICOCLAW_TOOLS_FIND_SKILLS_"` I2C ToolConfig `json:"i2c" yaml:"-" envPrefix:"PICOCLAW_TOOLS_I2C_"` InstallSkill ToolConfig `json:"install_skill" yaml:"-" envPrefix:"PICOCLAW_TOOLS_INSTALL_SKILL_"` + SkillManage ToolConfig `json:"skill_manage" yaml:"-" envPrefix:"PICOCLAW_TOOLS_SKILL_MANAGE_"` ListDir ToolConfig `json:"list_dir" yaml:"-" envPrefix:"PICOCLAW_TOOLS_LIST_DIR_"` Message ToolConfig `json:"message" yaml:"-" envPrefix:"PICOCLAW_TOOLS_MESSAGE_"` ReadFile ReadFileToolConfig `json:"read_file" yaml:"-" envPrefix:"PICOCLAW_TOOLS_READ_FILE_"` @@ -1337,6 +1338,8 @@ func (t *ToolsConfig) IsToolEnabled(name string) bool { return t.I2C.Enabled case "install_skill": return t.InstallSkill.Enabled + case "skill_manage": + return t.SkillManage.Enabled case "list_dir": return t.ListDir.Enabled case "message": diff --git a/pkg/skills/manager.go b/pkg/skills/manager.go new file mode 100644 index 000000000..993b1d554 --- /dev/null +++ b/pkg/skills/manager.go @@ -0,0 +1,273 @@ +package skills + +import ( + "fmt" + "os" + "path/filepath" + "strings" + "sync" + + "github.com/sipeed/picoclaw/pkg/logger" +) + +// SkillManager handles dynamic creation, patching, and discovery of skills. +// It writes SKILL.md files to the workspace skills directory using atomic +// writes (temp file + rename) to prevent partial files. +// +// Inspired by Hermes Agent's tools/skill_manager_tool.py — ported to Go +// for PicoClaw's self-improvement system. +type SkillManager struct { + mu sync.Mutex + skillsDir string // e.g. ~/.picoclaw/workspace/skills/ +} + +// NewSkillManager creates a manager that writes skills to the given directory. +func NewSkillManager(skillsDir string) *SkillManager { + return &SkillManager{skillsDir: skillsDir} +} + +// CreateSkill validates and atomically writes a new skill. +// category is optional — if provided, creates a subdirectory. +func (sm *SkillManager) CreateSkill(name, content, category string) error { + sm.mu.Lock() + defer sm.mu.Unlock() + + // Validate inputs. + if err := ValidateName(name); err != nil { + return fmt.Errorf("validate name: %w", err) + } + if err := ValidateFrontmatter(content); err != nil { + return fmt.Errorf("validate frontmatter: %w", err) + } + if err := ValidateSize(content); err != nil { + return fmt.Errorf("validate size: %w", err) + } + + // Check for duplicates. + if _, exists := sm.findSkillLocked(name); exists { + return fmt.Errorf("skill %q already exists", name) + } + + // Build target path. + dir := sm.skillsDir + if category != "" { + if err := validateCategory(category); err != nil { + return fmt.Errorf("validate category: %w", err) + } + dir = filepath.Join(dir, category) + } + skillDir := filepath.Join(dir, name) + skillFile := filepath.Join(skillDir, "SKILL.md") + + // Create directory. + if err := os.MkdirAll(skillDir, 0o755); err != nil { + return fmt.Errorf("create skill directory: %w", err) + } + + // Atomic write: temp file + rename. + if err := atomicWrite(skillFile, content); err != nil { + // Cleanup empty directory on failure. + os.Remove(skillDir) + return fmt.Errorf("write skill: %w", err) + } + + logger.DebugCF("skills", "skill created", map[string]any{ + "name": name, + "category": category, + "path": skillFile, + "size": len(content), + }) + + return nil +} + +// PatchSkill applies a find-and-replace to an existing skill's SKILL.md. +func (sm *SkillManager) PatchSkill(name, oldStr, newStr string) error { + sm.mu.Lock() + defer sm.mu.Unlock() + + info, exists := sm.findSkillLocked(name) + if !exists { + return fmt.Errorf("skill %q not found", name) + } + + data, err := os.ReadFile(info.Path) + if err != nil { + return fmt.Errorf("read skill: %w", err) + } + + content := string(data) + if !strings.Contains(content, oldStr) { + return fmt.Errorf("old_string not found in %s", info.Path) + } + + updated := strings.Replace(content, oldStr, newStr, 1) + + // Validate updated content. + if err := ValidateFrontmatter(updated); err != nil { + return fmt.Errorf("patch breaks frontmatter: %w", err) + } + + if err := atomicWrite(info.Path, updated); err != nil { + return fmt.Errorf("write patched skill: %w", err) + } + + logger.DebugCF("skills", "skill patched", map[string]any{ + "name": name, + "path": info.Path, + }) + + return nil +} + +// EditSkill replaces the entire content of a skill's SKILL.md. +func (sm *SkillManager) EditSkill(name, content string) error { + sm.mu.Lock() + defer sm.mu.Unlock() + + info, exists := sm.findSkillLocked(name) + if !exists { + return fmt.Errorf("skill %q not found", name) + } + + if err := ValidateFrontmatter(content); err != nil { + return fmt.Errorf("validate frontmatter: %w", err) + } + if err := ValidateSize(content); err != nil { + return fmt.Errorf("validate size: %w", err) + } + + if err := atomicWrite(info.Path, content); err != nil { + return fmt.Errorf("write skill: %w", err) + } + + return nil +} + +// DeleteSkill removes a skill directory and all its contents. +func (sm *SkillManager) DeleteSkill(name string) error { + sm.mu.Lock() + defer sm.mu.Unlock() + + info, exists := sm.findSkillLocked(name) + if !exists { + return fmt.Errorf("skill %q not found", name) + } + + skillDir := filepath.Dir(info.Path) + if err := os.RemoveAll(skillDir); err != nil { + return fmt.Errorf("remove skill directory: %w", err) + } + + logger.DebugCF("skills", "skill deleted", map[string]any{ + "name": name, + "path": skillDir, + }) + + return nil +} + +// FindSkill looks up a skill by name in the skills directory. +func (sm *SkillManager) FindSkill(name string) (*SkillInfo, bool) { + sm.mu.Lock() + defer sm.mu.Unlock() + return sm.findSkillLocked(name) +} + +// ListSkills returns all skills in the managed directory. +func (sm *SkillManager) ListSkills() []SkillInfo { + sm.mu.Lock() + defer sm.mu.Unlock() + + var skills []SkillInfo + filepath.WalkDir(sm.skillsDir, func(path string, d os.DirEntry, err error) error { + if err != nil || d.IsDir() || d.Name() != "SKILL.md" { + return nil + } + info, err := loadSkillInfo(path) + if err != nil { + return nil + } + skills = append(skills, *info) + return nil + }) + return skills +} + +// findSkillLocked searches for a skill by name. Caller must hold sm.mu. +func (sm *SkillManager) findSkillLocked(name string) (*SkillInfo, bool) { + var found *SkillInfo + filepath.WalkDir(sm.skillsDir, func(path string, d os.DirEntry, err error) error { + if err != nil || d.IsDir() || d.Name() != "SKILL.md" { + return nil + } + if filepath.Base(filepath.Dir(path)) == name { + info, err := loadSkillInfo(path) + if err == nil { + found = info + return filepath.SkipAll + } + } + return nil + }) + if found != nil { + return found, true + } + return nil, false +} + +// loadSkillInfo reads a SKILL.md and extracts its metadata. +func loadSkillInfo(path string) (*SkillInfo, error) { + data, err := os.ReadFile(path) + if err != nil { + return nil, err + } + content := string(data) + + // Extract name from directory. + name := filepath.Base(filepath.Dir(path)) + + // Extract description from frontmatter. + desc := "" + if strings.HasPrefix(content, "---\n") { + end := strings.Index(content[4:], "\n---") + if end > 0 { + // Simple extraction — look for description field. + for _, line := range strings.Split(content[4:4+end], "\n") { + if strings.HasPrefix(strings.TrimSpace(line), "description:") { + desc = strings.TrimSpace(strings.TrimPrefix(strings.TrimSpace(line), "description:")) + desc = strings.Trim(desc, "\"'>") + break + } + } + } + } + + return &SkillInfo{ + Name: name, + Path: path, + Source: "workspace", + Description: desc, + }, nil +} + +// atomicWrite writes content to path via temp file + rename. +func atomicWrite(path, content string) error { + tmp := path + ".tmp." + fmt.Sprint(os.Getpid()) + if err := os.WriteFile(tmp, []byte(content), 0o644); err != nil { + return err + } + if err := os.Rename(tmp, path); err != nil { + os.Remove(tmp) + return err + } + return nil +} + +// validateCategory checks that a category name is safe. +func validateCategory(cat string) error { + if strings.Contains(cat, "..") || strings.Contains(cat, "/") || strings.Contains(cat, "\\") { + return fmt.Errorf("invalid category %q: must not contain path separators or ..", cat) + } + return nil +} diff --git a/pkg/skills/validator.go b/pkg/skills/validator.go new file mode 100644 index 000000000..dfcee53df --- /dev/null +++ b/pkg/skills/validator.go @@ -0,0 +1,71 @@ +package skills + +import ( + "fmt" + "strings" + + "gopkg.in/yaml.v3" +) + +// MaxContentSize is the maximum allowed size for SKILL.md content (~36k tokens). +const MaxContentSize = 100_000 + +// ValidateName checks that a skill name follows PicoClaw's naming convention: +// alphanumeric segments separated by hyphens (e.g. "my-skill", "go-code-improve"). +// Uses the same namePattern as loader.go. +func ValidateName(name string) error { + if name == "" { + return fmt.Errorf("skill name must not be empty") + } + if len(name) > MaxNameLength { + return fmt.Errorf("skill name too long: %d chars (max %d)", len(name), MaxNameLength) + } + if !namePattern.MatchString(name) { + return fmt.Errorf("invalid skill name %q: must be alphanumeric with hyphens", name) + } + return nil +} + +// ValidateFrontmatter checks that SKILL.md content has valid YAML +// frontmatter with required "name" and "description" fields. +func ValidateFrontmatter(content string) error { + if !strings.HasPrefix(content, "---\n") { + return fmt.Errorf("SKILL.md must start with YAML frontmatter (---)") + } + + end := strings.Index(content[4:], "\n---") + if end < 0 { + return fmt.Errorf("SKILL.md frontmatter not closed (missing ---)") + } + + yamlBlock := content[4 : 4+end] + + var meta map[string]any + if err := yaml.Unmarshal([]byte(yamlBlock), &meta); err != nil { + return fmt.Errorf("invalid YAML frontmatter: %w", err) + } + + name, ok := meta["name"] + if !ok || fmt.Sprint(name) == "" { + return fmt.Errorf("frontmatter missing required field: name") + } + + desc, ok := meta["description"] + if !ok || fmt.Sprint(desc) == "" { + return fmt.Errorf("frontmatter missing required field: description") + } + + if len(fmt.Sprint(desc)) > MaxDescriptionLength { + return fmt.Errorf("description too long: %d chars (max %d)", len(fmt.Sprint(desc)), MaxDescriptionLength) + } + + return nil +} + +// ValidateSize checks that content doesn't exceed the maximum size. +func ValidateSize(content string) error { + if len(content) > MaxContentSize { + return fmt.Errorf("content too large: %d chars (max %d)", len(content), MaxContentSize) + } + return nil +} diff --git a/pkg/tools/skill_manage.go b/pkg/tools/skill_manage.go new file mode 100644 index 000000000..1468361ed --- /dev/null +++ b/pkg/tools/skill_manage.go @@ -0,0 +1,112 @@ +package tools + +import ( + "context" + "fmt" + "strings" + + "github.com/sipeed/picoclaw/pkg/skills" +) + +// SkillManageTool exposes the SkillManager to the LLM agent so it can +// create, read, update, and delete workspace skills dynamically. +type SkillManageTool struct { + mgr *skills.SkillManager +} + +// NewSkillManageTool creates a new SkillManageTool backed by the given manager. +func NewSkillManageTool(mgr *skills.SkillManager) *SkillManageTool { + return &SkillManageTool{mgr: mgr} +} + +func (t *SkillManageTool) Name() string { return "skill_manage" } + +func (t *SkillManageTool) Description() string { + return "Create, read, update, or delete workspace skills. Use this to persist reusable procedures the agent discovers during conversations." +} + +func (t *SkillManageTool) Parameters() map[string]any { + return map[string]any{ + "type": "object", + "properties": map[string]any{ + "operation": map[string]any{ + "type": "string", + "enum": []string{"create", "read", "update", "delete", "list"}, + "description": "The operation to perform", + }, + "name": map[string]any{ + "type": "string", + "description": "Skill name (required for create/read/update/delete)", + }, + "content": map[string]any{ + "type": "string", + "description": "SKILL.md content with YAML frontmatter (required for create/update)", + }, + "category": map[string]any{ + "type": "string", + "description": "Optional subdirectory category for create", + }, + }, + "required": []string{"operation"}, + } +} + +func (t *SkillManageTool) Execute(_ context.Context, args map[string]any) *ToolResult { + op, _ := args["operation"].(string) + name, _ := args["name"].(string) + content, _ := args["content"].(string) + category, _ := args["category"].(string) + + switch op { + case "create": + if name == "" || content == "" { + return ErrorResult("create requires 'name' and 'content'") + } + if err := t.mgr.CreateSkill(name, content, category); err != nil { + return ErrorResult(fmt.Sprintf("create failed: %v", err)) + } + return NewToolResult(fmt.Sprintf("Skill %q created successfully", name)) + + case "read": + if name == "" { + return ErrorResult("read requires 'name'") + } + info, ok := t.mgr.FindSkill(name) + if !ok { + return ErrorResult(fmt.Sprintf("skill %q not found", name)) + } + return NewToolResult(fmt.Sprintf("Name: %s\nPath: %s\nDescription: %s", info.Name, info.Path, info.Description)) + + case "update": + if name == "" || content == "" { + return ErrorResult("update requires 'name' and 'content'") + } + if err := t.mgr.EditSkill(name, content); err != nil { + return ErrorResult(fmt.Sprintf("update failed: %v", err)) + } + return NewToolResult(fmt.Sprintf("Skill %q updated successfully", name)) + + case "delete": + if name == "" { + return ErrorResult("delete requires 'name'") + } + if err := t.mgr.DeleteSkill(name); err != nil { + return ErrorResult(fmt.Sprintf("delete failed: %v", err)) + } + return NewToolResult(fmt.Sprintf("Skill %q deleted", name)) + + case "list": + allSkills := t.mgr.ListSkills() + if len(allSkills) == 0 { + return NewToolResult("No skills found in workspace") + } + var sb strings.Builder + for _, s := range allSkills { + fmt.Fprintf(&sb, "- %s: %s\n", s.Name, s.Description) + } + return NewToolResult(sb.String()) + + default: + return ErrorResult(fmt.Sprintf("unknown operation %q — use create/read/update/delete/list", op)) + } +}