From a37217a5b9ef7b7f3a39875b753ff0fa6744d237 Mon Sep 17 00:00:00 2001 From: Cody Hart Date: Mon, 26 Jan 2026 17:03:40 -0500 Subject: [PATCH 1/2] add skills feature --- internal/cli/info.go | 60 ++ internal/cli/root.go | 1 + internal/cli/skills.go | 438 +++++++++++ internal/cli/sync.go | 274 ++++++- internal/cli/team.go | 44 ++ internal/config/paths.go | 29 + internal/config/source.go | 22 + internal/integration/harness.go | 167 ++++- internal/integration/integration_test.go | 313 ++++++++ internal/integration/live_test.go | 221 ++++++ internal/skills/claude.go | 244 ++++++ internal/skills/claude_test.go | 442 +++++++++++ internal/skills/registry.go | 222 ++++++ internal/skills/skill.go | 385 ++++++++++ internal/skills/skill_test.go | 701 ++++++++++++++++++ internal/starter/skills.go | 258 +++++++ internal/starter/skills/code-review/SKILL.md | 51 ++ .../starter/skills/security-audit/SKILL.md | 59 ++ internal/starter/skills/test-gen/SKILL.md | 72 ++ internal/starter/skills_test.go | 198 +++++ 20 files changed, 4197 insertions(+), 4 deletions(-) create mode 100644 internal/cli/skills.go create mode 100644 internal/skills/claude.go create mode 100644 internal/skills/claude_test.go create mode 100644 internal/skills/registry.go create mode 100644 internal/skills/skill.go create mode 100644 internal/skills/skill_test.go create mode 100644 internal/starter/skills.go create mode 100644 internal/starter/skills/code-review/SKILL.md create mode 100644 internal/starter/skills/security-audit/SKILL.md create mode 100644 internal/starter/skills/test-gen/SKILL.md create mode 100644 internal/starter/skills_test.go diff --git a/internal/cli/info.go b/internal/cli/info.go index d2e60d5..ffba60d 100644 --- a/internal/cli/info.go +++ b/internal/cli/info.go @@ -8,11 +8,13 @@ import ( "strings" "github.com/HartBrook/staghorn/internal/cache" + "github.com/HartBrook/staghorn/internal/commands" "github.com/HartBrook/staghorn/internal/config" "github.com/HartBrook/staghorn/internal/github" "github.com/HartBrook/staghorn/internal/language" "github.com/HartBrook/staghorn/internal/merge" "github.com/HartBrook/staghorn/internal/optimize" + "github.com/HartBrook/staghorn/internal/skills" "github.com/spf13/cobra" ) @@ -271,6 +273,42 @@ func showCompactStatus(cfg *config.Config, paths *config.Paths, owner, repo stri langStatus = strings.Join(activeLanguages, ", ") } + // Commands count + cmdRegistry, _ := loadCommandRegistryForInfo(paths, owner, repo, projectRoot) + cmdStatus := dim("none") + if cmdRegistry != nil && cmdRegistry.Count() > 0 { + counts := cmdRegistry.CountBySource() + var parts []string + if counts[commands.SourceTeam] > 0 { + parts = append(parts, fmt.Sprintf("%d team", counts[commands.SourceTeam])) + } + if counts[commands.SourcePersonal] > 0 { + parts = append(parts, fmt.Sprintf("%d personal", counts[commands.SourcePersonal])) + } + if counts[commands.SourceProject] > 0 { + parts = append(parts, fmt.Sprintf("%d project", counts[commands.SourceProject])) + } + cmdStatus = fmt.Sprintf("%d (%s)", cmdRegistry.Count(), strings.Join(parts, ", ")) + } + + // Skills count + skillRegistry, _ := loadSkillRegistryForInfo(paths, owner, repo, projectRoot) + skillStatus := dim("none") + if skillRegistry != nil && skillRegistry.Count() > 0 { + counts := skillRegistry.CountBySource() + var parts []string + if counts[skills.SourceTeam] > 0 { + parts = append(parts, fmt.Sprintf("%d team", counts[skills.SourceTeam])) + } + if counts[skills.SourcePersonal] > 0 { + parts = append(parts, fmt.Sprintf("%d personal", counts[skills.SourcePersonal])) + } + if counts[skills.SourceProject] > 0 { + parts = append(parts, fmt.Sprintf("%d project", counts[skills.SourceProject])) + } + skillStatus = fmt.Sprintf("%d (%s)", skillRegistry.Count(), strings.Join(parts, ", ")) + } + // Calculate merged token count mergedTokens := calculateMergedTokens(cfg, paths, owner, repo, activeLanguages) tokenStatus := fmt.Sprintf("%d tokens", mergedTokens) @@ -283,6 +321,8 @@ func showCompactStatus(cfg *config.Config, paths *config.Paths, owner, repo stri fmt.Printf(" %s: %s\n", dim("Personal"), personalStatus) fmt.Printf(" %s: %s\n", dim("Project"), projectStatus) fmt.Printf(" %s: %s\n", dim("Languages"), langStatus) + fmt.Printf(" %s: %s\n", dim("Commands"), cmdStatus) + fmt.Printf(" %s: %s\n", dim("Skills"), skillStatus) fmt.Printf(" %s: %s\n", dim("Size"), tokenStatus) // Suggest optimization if large @@ -470,3 +510,23 @@ func calculateMergedTokens(cfg *config.Config, paths *config.Paths, owner, repo merged := merge.MergeWithLanguages(layers, mergeOpts) return optimize.CountTokens(merged) } + +// loadCommandRegistryForInfo loads commands from all sources for info display. +func loadCommandRegistryForInfo(paths *config.Paths, owner, repo, projectRoot string) (*commands.Registry, error) { + teamCommandsDir := paths.TeamCommandsDir(owner, repo) + projectCommandsDir := "" + if projectRoot != "" { + projectCommandsDir = config.ProjectCommandsDir(projectRoot) + } + return commands.LoadRegistry(teamCommandsDir, paths.PersonalCommands, projectCommandsDir) +} + +// loadSkillRegistryForInfo loads skills from all sources for info display. +func loadSkillRegistryForInfo(paths *config.Paths, owner, repo, projectRoot string) (*skills.Registry, error) { + teamSkillsDir := paths.TeamSkillsDir(owner, repo) + projectSkillsDir := "" + if projectRoot != "" { + projectSkillsDir = config.ProjectSkillsDir(projectRoot) + } + return skills.LoadRegistry(teamSkillsDir, paths.PersonalSkills, projectSkillsDir) +} diff --git a/internal/cli/root.go b/internal/cli/root.go index a367f2e..843a045 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -49,6 +49,7 @@ configs on top, and outputs a merged file for Claude to consume.`, rootCmd.AddCommand(NewCommandsCmd()) rootCmd.AddCommand(NewRunCmd()) rootCmd.AddCommand(NewLanguagesCmd()) + rootCmd.AddCommand(NewSkillsCmd()) rootCmd.AddCommand(NewTeamCmd()) rootCmd.AddCommand(NewEvalCmd()) rootCmd.AddCommand(NewVersionCmd()) diff --git a/internal/cli/skills.go b/internal/cli/skills.go new file mode 100644 index 0000000..997ebce --- /dev/null +++ b/internal/cli/skills.go @@ -0,0 +1,438 @@ +package cli + +import ( + "fmt" + "strings" + + "github.com/HartBrook/staghorn/internal/config" + "github.com/HartBrook/staghorn/internal/skills" + "github.com/HartBrook/staghorn/internal/starter" + "github.com/spf13/cobra" +) + +// NewSkillsCmd creates the skills command. +func NewSkillsCmd() *cobra.Command { + var tag string + var source string + var verbose bool + + cmd := &cobra.Command{ + Use: "skills [name]", + Short: "List skills or show info for a specific skill", + Long: `Lists all available skills from team, personal, and project sources. + +If a skill name is provided, shows detailed information about that skill. +Skills are directories containing SKILL.md plus optional supporting files +like templates, scripts, and references. + +Skills follow the Agent Skills standard (agentskills.io) and support extended +Claude Code features like tool restrictions, subagent execution, and hooks.`, + Example: ` staghorn skills # List all skills + staghorn skills -v # List with details + staghorn skills code-review # Show info for specific skill + staghorn skills --tag review # Filter by tag`, + Args: cobra.MaximumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + if len(args) == 1 { + // Show info for specific skill + return runSkillInfo(args[0]) + } + return runSkillsList(tag, source, verbose) + }, + } + + cmd.Flags().StringVar(&tag, "tag", "", "Filter by tag") + cmd.Flags().StringVar(&source, "source", "", "Filter by source (team, personal, project)") + cmd.Flags().BoolVarP(&verbose, "verbose", "v", false, "Show detailed information including supporting files") + + // Add subcommands + cmd.AddCommand(NewSkillsInitCmd()) + + return cmd +} + +// NewSkillsInitCmd creates the 'skills init' command to bootstrap starter skills. +func NewSkillsInitCmd() *cobra.Command { + var project bool + var claude bool + var claudeProject bool + + cmd := &cobra.Command{ + Use: "init", + Short: "Install starter skills", + Long: `Installs staghorn's built-in starter skills to your personal or project config. + +Starter skills include common workflows like code-review, test-gen, and +security-audit with enhanced features like tool restrictions and subagent execution. + +Use --claude to install skills directly to Claude Code's skills directory.`, + Example: ` staghorn skills init # Install to ~/.config/staghorn/skills/ + staghorn skills init --project # Install to .staghorn/skills/ + staghorn skills init --claude # Install to ~/.claude/skills/ + staghorn skills init --claude-project # Install to .claude/skills/`, + RunE: func(cmd *cobra.Command, args []string) error { + if claude || claudeProject { + return runSkillsInitClaude(claudeProject) + } + return runSkillsInit(project) + }, + } + + cmd.Flags().BoolVar(&project, "project", false, "Install to project directory (.staghorn/skills/)") + cmd.Flags().BoolVar(&claude, "claude", false, "Install to Claude Code skills (~/.claude/skills/)") + cmd.Flags().BoolVar(&claudeProject, "claude-project", false, "Install to project Claude skills (.claude/skills/)") + + return cmd +} + +func runSkillsInit(project bool) error { + paths := config.NewPaths() + + var targetDir string + var targetLabel string + + if project { + projectRoot := findProjectRoot() + if projectRoot == "" { + return fmt.Errorf("no project root found (looking for .git or .staghorn directory)") + } + targetDir = config.ProjectSkillsDir(projectRoot) + targetLabel = ".staghorn/skills/" + } else { + targetDir = paths.PersonalSkills + targetLabel = "~/.config/staghorn/skills/" + } + + fmt.Printf("Installing starter skills to %s\n", targetLabel) + fmt.Println() + + // Show available skills + skillNames := starter.SkillNames() + fmt.Printf("Available starter skills (%d):\n", len(skillNames)) + for _, name := range skillNames { + fmt.Printf(" - %s\n", info(name)) + } + fmt.Println() + + // Install starter skills + count, installed, err := starter.BootstrapSkillsWithSkip(targetDir, nil) + if err != nil { + return fmt.Errorf("failed to install starter skills: %w", err) + } + + if count == 0 { + fmt.Println(dim("All starter skills already installed.")) + } else { + printSuccess("Installed %d starter skills:", count) + for _, name := range installed { + fmt.Printf(" - %s\n", info(name)) + } + } + + fmt.Println() + fmt.Printf("Skills are invoked via %s in Claude Code.\n", info("/skill-name")) + + return nil +} + +func runSkillsInitClaude(project bool) error { + paths := config.NewPaths() + + var targetDir string + var targetLabel string + + if project { + projectRoot := findProjectRoot() + if projectRoot == "" { + return fmt.Errorf("no project root found (looking for .git or .staghorn directory)") + } + targetDir = config.ProjectClaudeSkillsDir(projectRoot) + targetLabel = ".claude/skills/" + } else { + targetDir = paths.ClaudeSkillsDir() + targetLabel = "~/.claude/skills/" + } + + fmt.Printf("Installing starter skills to %s\n", targetLabel) + fmt.Println() + + // Show available skills + skillNames := starter.SkillNames() + fmt.Printf("Available starter skills (%d):\n", len(skillNames)) + for _, name := range skillNames { + fmt.Printf(" - %s\n", info(name)) + } + fmt.Println() + + // Install starter skills + count, installed, err := starter.BootstrapSkillsWithSkip(targetDir, nil) + if err != nil { + return fmt.Errorf("failed to install starter skills: %w", err) + } + + if count == 0 { + fmt.Println(dim("All starter skills already installed.")) + } else { + printSuccess("Installed %d starter skills:", count) + for _, name := range installed { + fmt.Printf(" - %s\n", info(name)) + } + } + + fmt.Println() + fmt.Printf("Skills are invoked via %s in Claude Code.\n", info("/skill-name")) + + return nil +} + +func runSkillsList(tagFilter, sourceFilter string, verbose bool) error { + registry, err := loadSkillRegistry() + if err != nil { + return err + } + + if registry.Count() == 0 { + fmt.Println("No skills found.") + fmt.Println() + fmt.Println("Skills are directories containing SKILL.md plus optional supporting files") + fmt.Println("like templates, scripts, and references. They follow the Agent Skills") + fmt.Println("standard (agentskills.io) and support Claude Code's extended features.") + fmt.Println() + fmt.Println(dim("To create a personal skill:")) + fmt.Println() + fmt.Println(" 1. Create directory ~/.config/staghorn/skills/my-skill/") + fmt.Println(" 2. Add SKILL.md with YAML frontmatter:") + fmt.Println() + fmt.Println(dim(" ---")) + fmt.Println(dim(" name: my-skill")) + fmt.Println(dim(" description: What this skill does")) + fmt.Println(dim(" allowed-tools: Read Grep Glob")) + fmt.Println(dim(" ---")) + fmt.Println(dim(" Instructions for the skill...")) + fmt.Println() + fmt.Println(" 3. Optionally add templates/, scripts/, references/ directories") + fmt.Println() + fmt.Println(dim("Skills can also come from:")) + fmt.Println(dim(" - Team repo (skills/ directory, synced via 'staghorn sync')")) + fmt.Println(dim(" - Project (.staghorn/skills/)")) + fmt.Println(dim(" - Community repos (via multi-source config)")) + return nil + } + + // Apply filters + var filtered []*skills.Skill + if tagFilter != "" { + filtered = registry.ByTag(tagFilter) + } else { + filtered = registry.All() + } + + if sourceFilter != "" { + var src skills.Source + switch sourceFilter { + case "team": + src = skills.SourceTeam + case "personal": + src = skills.SourcePersonal + case "project": + src = skills.SourceProject + default: + return fmt.Errorf("invalid source: %s (use team, personal, or project)", sourceFilter) + } + + var sourceFiltered []*skills.Skill + for _, s := range filtered { + if s.Source == src { + sourceFiltered = append(sourceFiltered, s) + } + } + filtered = sourceFiltered + } + + if len(filtered) == 0 { + fmt.Println("No skills match the filter.") + return nil + } + + // Group by source for display + teamSkills := filterSkillsBySource(filtered, skills.SourceTeam) + personalSkills := filterSkillsBySource(filtered, skills.SourcePersonal) + projectSkills := filterSkillsBySource(filtered, skills.SourceProject) + + if len(teamSkills) > 0 { + printSkillGroup("TEAM SKILLS", teamSkills, verbose) + } + + if len(personalSkills) > 0 { + if len(teamSkills) > 0 { + fmt.Println() + } + printSkillGroup("PERSONAL SKILLS", personalSkills, verbose) + } + + if len(projectSkills) > 0 { + if len(teamSkills) > 0 || len(personalSkills) > 0 { + fmt.Println() + } + printSkillGroup("PROJECT SKILLS", projectSkills, verbose) + } + + fmt.Println() + fmt.Printf("Skills are invoked via %s in Claude Code.\n", info("/skill-name")) + + return nil +} + +func filterSkillsBySource(skillList []*skills.Skill, source skills.Source) []*skills.Skill { + var result []*skills.Skill + for _, s := range skillList { + if s.Source == source { + result = append(result, s) + } + } + return result +} + +func printSkillGroup(title string, skillList []*skills.Skill, verbose bool) { + fmt.Println(dim(title)) + for _, s := range skillList { + name := s.Name + desc := s.Description + if desc == "" { + desc = "(no description)" + } + + // Truncate description if too long (unless verbose) + if !verbose && len(desc) > 50 { + desc = desc[:47] + "..." + } + + fmt.Printf(" %-20s %s\n", info(name), desc) + + if verbose { + // Show tags + if len(s.Tags) > 0 { + fmt.Printf(" %s %s\n", dim("Tags:"), strings.Join(s.Tags, ", ")) + } + // Show allowed tools + if s.AllowedTools != "" { + fmt.Printf(" %s %s\n", dim("Tools:"), s.AllowedTools) + } + // Show context + if s.Context != "" { + fmt.Printf(" %s %s\n", dim("Context:"), s.Context) + } + // Show supporting files count + if len(s.SupportingFiles) > 0 { + fmt.Printf(" %s %d supporting files\n", dim("Files:"), len(s.SupportingFiles)) + } + fmt.Println() + } + } +} + +// loadSkillRegistry loads skills from all sources. +func loadSkillRegistry() (*skills.Registry, error) { + paths := config.NewPaths() + + // Get team skills directory + var teamSkillsDir string + if config.Exists() { + cfg, err := config.Load() + if err == nil { + owner, repo, err := cfg.DefaultOwnerRepo() + if err == nil { + teamSkillsDir = paths.TeamSkillsDir(owner, repo) + } + } + } + + // Find project root + projectSkillsDir := "" + if projectRoot := findProjectRoot(); projectRoot != "" { + projectSkillsDir = config.ProjectSkillsDir(projectRoot) + } + + return skills.LoadRegistry(teamSkillsDir, paths.PersonalSkills, projectSkillsDir) +} + +func runSkillInfo(skillName string) error { + registry, err := loadSkillRegistry() + if err != nil { + return err + } + + skill := registry.Get(skillName) + if skill == nil { + return fmt.Errorf("skill '%s' not found", skillName) + } + + fmt.Println(dim("Name:"), info(skill.Name)) + fmt.Println(dim("Source:"), skill.Source.Label()) + + if skill.Description != "" { + fmt.Println(dim("Description:"), skill.Description) + } + + if len(skill.Tags) > 0 { + fmt.Println(dim("Tags:"), strings.Join(skill.Tags, ", ")) + } + + // Agent Skills standard fields + if skill.License != "" { + fmt.Println(dim("License:"), skill.License) + } + if skill.Compatibility != "" { + fmt.Println(dim("Compatibility:"), skill.Compatibility) + } + + // Claude Code extensions + fmt.Println() + fmt.Println(dim("Claude Code Settings:")) + if skill.AllowedTools != "" { + fmt.Println(" Allowed Tools:", skill.AllowedTools) + } + if skill.Context != "" { + fmt.Println(" Context:", skill.Context) + } + if skill.Agent != "" { + fmt.Println(" Agent:", skill.Agent) + } + fmt.Println(" User Invocable:", skill.IsUserInvocable()) + if skill.DisableModelInvocation { + fmt.Println(" Model Invocation: disabled") + } + if skill.Hooks != nil { + if skill.Hooks.Pre != "" { + fmt.Println(" Pre Hook:", skill.Hooks.Pre) + } + if skill.Hooks.Post != "" { + fmt.Println(" Post Hook:", skill.Hooks.Post) + } + } + + // Supporting files + if len(skill.SupportingFiles) > 0 { + fmt.Println() + fmt.Println(dim("Supporting Files:")) + for relPath := range skill.SupportingFiles { + fmt.Printf(" %s\n", relPath) + } + } + + // Show if overridden + versions := registry.GetAllVersions(skillName) + if len(versions) > 1 { + fmt.Println() + fmt.Println(dim("Versions:")) + for _, v := range versions { + active := "" + if v == skill { + active = " (active)" + } + fmt.Printf(" %s%s\n", v.Source.Label(), active) + } + } + + return nil +} diff --git a/internal/cli/sync.go b/internal/cli/sync.go index 1ce1531..5ae17d0 100644 --- a/internal/cli/sync.go +++ b/internal/cli/sync.go @@ -18,6 +18,7 @@ import ( "github.com/HartBrook/staghorn/internal/merge" "github.com/HartBrook/staghorn/internal/optimize" "github.com/HartBrook/staghorn/internal/rules" + "github.com/HartBrook/staghorn/internal/skills" "github.com/spf13/cobra" ) @@ -28,6 +29,7 @@ type syncOptions struct { commandsOnly bool languagesOnly bool rulesOnly bool + skillsOnly bool fetchOnly bool applyOnly bool claudeOnly bool @@ -70,7 +72,17 @@ func (o *syncOptions) shouldSyncClaudeCommands() bool { // shouldSyncClaudeRules returns true if rules should be synced to Claude Code. func (o *syncOptions) shouldSyncClaudeRules() bool { - return !o.configOnly && !o.languagesOnly && !o.commandsOnly && !o.fetchOnly + return !o.configOnly && !o.languagesOnly && !o.commandsOnly && !o.skillsOnly && !o.fetchOnly +} + +// shouldSyncSkills returns true if skills should be synced. +func (o *syncOptions) shouldSyncSkills() bool { + return !o.configOnly && !o.commandsOnly && !o.languagesOnly && !o.rulesOnly && !o.claudeOnly +} + +// shouldSyncClaudeSkills returns true if skills should be synced to Claude Code. +func (o *syncOptions) shouldSyncClaudeSkills() bool { + return !o.configOnly && !o.languagesOnly && !o.commandsOnly && !o.rulesOnly && !o.fetchOnly } // repoContext holds the branch info for a single repo. @@ -108,7 +120,8 @@ This is the main command for keeping your Claude Code config up to date.`, cmd.Flags().BoolVar(&opts.commandsOnly, "commands-only", false, "Only sync commands, skip config, languages, and rules") cmd.Flags().BoolVar(&opts.languagesOnly, "languages-only", false, "Only sync languages, skip config, commands, and rules") cmd.Flags().BoolVar(&opts.rulesOnly, "rules-only", false, "Only sync rules, skip config, commands, and languages") - cmd.Flags().BoolVar(&opts.claudeOnly, "claude-only", false, "Only sync commands and rules to ~/.claude/, skip config apply") + cmd.Flags().BoolVar(&opts.skillsOnly, "skills-only", false, "Only sync skills, skip config, commands, languages, and rules") + cmd.Flags().BoolVar(&opts.claudeOnly, "claude-only", false, "Only sync commands, rules, and skills to ~/.claude/, skip config apply") return cmd } @@ -270,6 +283,18 @@ func runSync(ctx context.Context, opts *syncOptions) error { } } + // Sync skills + if opts.shouldSyncSkills() { + skillCount, err := syncSkills(ctx, client, owner, repo, branch, paths) + if err != nil { + printWarning("Failed to sync skills: %v", err) + } else if skillCount > 0 { + printSuccess("Synced %d skills", skillCount) + } else if opts.skillsOnly { + fmt.Println("No skills found in team repository") + } + } + // Apply to ~/.claude/CLAUDE.md if opts.shouldApplyConfig() { fmt.Println() @@ -299,6 +324,17 @@ func runSync(ctx context.Context, opts *syncOptions) error { } } + // Sync skills to Claude Code + if opts.shouldSyncClaudeSkills() { + claudeSkillCount, err := syncClaudeSkills(paths, owner, repo) + if err != nil { + printWarning("Failed to sync Claude skills: %v", err) + } else if claudeSkillCount > 0 { + printSuccess("Synced %d skills to Claude Code", claudeSkillCount) + fmt.Printf(" %s Skills are available via /skill-name in Claude Code\n", dim("Tip:")) + } + } + // Check merged config size and suggest optimization if large if !opts.fetchOnly { checkConfigSizeAndSuggestOptimize(cfg, paths, owner, repo) @@ -1111,6 +1147,16 @@ func runMultiSourceSync(ctx context.Context, cfg *config.Config, paths *config.P } } + // Sync skills with multi-source support + if opts.shouldSyncSkills() { + skillCount, err := syncSkillsMultiSource(ctx, client, cfg, repoContexts, paths) + if err != nil { + printWarning("Failed to sync skills: %v", err) + } else if skillCount > 0 { + printSuccess("Synced %d skills", skillCount) + } + } + // Apply config if opts.shouldApplyConfig() { fmt.Println() @@ -1139,6 +1185,17 @@ func runMultiSourceSync(ctx context.Context, cfg *config.Config, paths *config.P } } + // Sync skills to Claude Code + if opts.shouldSyncClaudeSkills() { + claudeSkillCount, err := syncClaudeSkills(paths, defaultCtx.owner, defaultCtx.repo) + if err != nil { + printWarning("Failed to sync Claude skills: %v", err) + } else if claudeSkillCount > 0 { + printSuccess("Synced %d skills to Claude Code", claudeSkillCount) + fmt.Printf(" %s Skills are available via /skill-name in Claude Code\n", dim("Tip:")) + } + } + // Check config size if !opts.fetchOnly { checkConfigSizeAndSuggestOptimize(cfg, paths, defaultCtx.owner, defaultCtx.repo) @@ -1387,3 +1444,216 @@ func loadMultiSourceLanguageFiles(cfg *config.Config, paths *config.Paths, repoC } return languageFiles } + +// syncSkills fetches skills from the team repo's skills/ directory. +// Skills are directories containing SKILL.md plus optional supporting files. +func syncSkills(ctx context.Context, client *github.Client, owner, repo, branch string, paths *config.Paths) (int, error) { + // List skills directory (top-level entries are skill directories) + entries, err := client.ListDirectory(ctx, owner, repo, "skills", branch) + if err != nil { + return 0, err + } + + if entries == nil { + return 0, nil + } + + // Create local skills cache directory + skillsDir := paths.TeamSkillsDir(owner, repo) + + // Clear existing cache to handle deletions + if err := os.RemoveAll(skillsDir); err != nil { + return 0, fmt.Errorf("failed to clear skills cache: %w", err) + } + + // Sync each skill directory + count := 0 + for _, entry := range entries { + if entry.Type != "dir" { + continue + } + + // Sync this skill directory recursively + skillLocalDir := filepath.Join(skillsDir, entry.Name) + fileCount, err := syncSkillDir(ctx, client, owner, repo, branch, entry.Path, skillLocalDir) + if err != nil { + printWarning("Failed to sync skill %s: %v", entry.Name, err) + continue + } + + if fileCount > 0 { + count++ + } + } + + return count, nil +} + +// syncSkillDir syncs a single skill directory recursively. +func syncSkillDir(ctx context.Context, client *github.Client, owner, repo, branch, remotePath, localDir string) (int, error) { + entries, err := client.ListDirectory(ctx, owner, repo, remotePath, branch) + if err != nil { + return 0, err + } + + if entries == nil { + return 0, nil + } + + // Ensure local directory exists + if err := os.MkdirAll(localDir, 0755); err != nil { + return 0, fmt.Errorf("failed to create skill directory: %w", err) + } + + count := 0 + for _, entry := range entries { + if entry.Type == "dir" { + // Recurse into subdirectory + subLocalDir := filepath.Join(localDir, entry.Name) + subCount, err := syncSkillDir(ctx, client, owner, repo, branch, entry.Path, subLocalDir) + if err != nil { + printWarning("Failed to sync skill subdirectory %s: %v", entry.Name, err) + continue + } + count += subCount + } else if entry.Type == "file" { + // Fetch and cache file + result, err := client.FetchFile(ctx, owner, repo, entry.Path, branch) + if err != nil { + printWarning("Failed to fetch skill file %s: %v", entry.Name, err) + continue + } + + localPath := filepath.Join(localDir, entry.Name) + if err := os.WriteFile(localPath, []byte(result.Content), 0644); err != nil { + printWarning("Failed to write skill file %s: %v", entry.Name, err) + continue + } + + count++ + } + } + + return count, nil +} + +// syncClaudeSkills syncs staghorn skills to Claude Code skills directory. +func syncClaudeSkills(paths *config.Paths, owner, repo string) (int, error) { + // Load skills from all sources using the registry + registry, err := skills.LoadRegistry( + paths.TeamSkillsDir(owner, repo), + paths.PersonalSkills, + "", // No project dir for global sync + ) + if err != nil { + return 0, fmt.Errorf("failed to load skills: %w", err) + } + + allSkills := registry.All() + if len(allSkills) == 0 { + return 0, nil + } + + // Create Claude skills directory + claudeDir := paths.ClaudeSkillsDir() + if err := os.MkdirAll(claudeDir, 0755); err != nil { + return 0, fmt.Errorf("failed to create Claude skills directory: %w", err) + } + + // Sync each skill + count := 0 + for _, skill := range allSkills { + filesWritten, err := skills.SyncToClaude(skill, claudeDir) + if err != nil { + if strings.Contains(err.Error(), "not managed by staghorn") { + printWarning("Skipping skill %s: existing skill not managed by staghorn", skill.Name) + } else { + printWarning("Failed to sync skill %s: %v", skill.Name, err) + } + continue + } + if filesWritten > 0 { + count++ + } + } + + return count, nil +} + +// isExplicitlyConfiguredSkill returns true if the skill has an explicit source configured. +func isExplicitlyConfiguredSkill(cfg *config.Config, skill string) bool { + if cfg.Source.Multi != nil && cfg.Source.Multi.Skills != nil { + _, ok := cfg.Source.Multi.Skills[skill] + return ok + } + return false +} + +// syncSkillsMultiSource fetches skills from their configured source repos. +func syncSkillsMultiSource(ctx context.Context, client *github.Client, cfg *config.Config, repoContexts map[string]*repoContext, paths *config.Paths) (int, error) { + // First, discover all skills from the default repo + defaultRepoStr := cfg.Source.DefaultRepo() + defaultCtx := repoContexts[defaultRepoStr] + if defaultCtx == nil { + return 0, fmt.Errorf("no context for default repo %s", defaultRepoStr) + } + + // Get skills from default repo + allSkills := make(map[string]bool) + entries, err := client.ListDirectory(ctx, defaultCtx.owner, defaultCtx.repo, "skills", defaultCtx.branch) + if err == nil && entries != nil { + for _, entry := range entries { + if entry.Type == "dir" { + allSkills[entry.Name] = true + } + } + } + + // Add any explicitly configured skill sources + if cfg.Source.Multi != nil && cfg.Source.Multi.Skills != nil { + for skill := range cfg.Source.Multi.Skills { + allSkills[skill] = true + } + } + + // Sync each skill from its configured source + count := 0 + for skill := range allSkills { + sourceRepoStr := cfg.Source.RepoForSkill(skill) + repoCtx := repoContexts[sourceRepoStr] + if repoCtx == nil { + printWarning("No context for skill %s source %s", skill, sourceRepoStr) + continue + } + + // Sync this skill from its source + skillPath := fmt.Sprintf("skills/%s", skill) + skillLocalDir := filepath.Join(paths.TeamSkillsDir(repoCtx.owner, repoCtx.repo), skill) + + // Check if skill exists in remote + entries, err := client.ListDirectory(ctx, repoCtx.owner, repoCtx.repo, skillPath, repoCtx.branch) + if err != nil { + handleMultiSourceFetchError("skill", skill, sourceRepoStr, err, isExplicitlyConfiguredSkill(cfg, skill)) + continue + } + if entries == nil { + if isExplicitlyConfiguredSkill(cfg, skill) { + printWarning("Skill %s not found in explicitly configured source %s", skill, sourceRepoStr) + } + continue + } + + // Sync the skill directory + fileCount, err := syncSkillDir(ctx, client, repoCtx.owner, repoCtx.repo, repoCtx.branch, skillPath, skillLocalDir) + if err != nil { + printWarning("Failed to sync skill %s from %s: %v", skill, sourceRepoStr, err) + continue + } + + if fileCount > 0 { + count++ + } + } + + return count, nil +} diff --git a/internal/cli/team.go b/internal/cli/team.go index 2cd847f..2041bf3 100644 --- a/internal/cli/team.go +++ b/internal/cli/team.go @@ -11,6 +11,7 @@ import ( "github.com/HartBrook/staghorn/internal/commands" "github.com/HartBrook/staghorn/internal/config" "github.com/HartBrook/staghorn/internal/eval" + "github.com/HartBrook/staghorn/internal/skills" "github.com/HartBrook/staghorn/internal/starter" "github.com/spf13/cobra" ) @@ -371,6 +372,24 @@ func runTeamValidate() error { fmt.Printf("%s evals/ - directory not found (optional)\n", warningIcon) } + // Check skills/ (optional) + if _, err := os.Stat("skills"); err == nil { + skillsValid, skillsTotal, skillErrs := validateSkills("skills") + if skillsTotal == 0 { + fmt.Printf("%s skills/ - directory empty\n", warningIcon) + warnings++ + } else if len(skillErrs) > 0 { + for _, e := range skillErrs { + printError("%s", e) + } + errors += len(skillErrs) + } else { + printSuccess("skills/ - %d valid skills", skillsValid) + } + } else { + fmt.Printf("%s skills/ - directory not found (optional)\n", warningIcon) + } + // Summary fmt.Println() if errors > 0 { @@ -697,3 +716,28 @@ func validateEvals(dir string) (valid, total int, errs []string) { return valid, total, errs } + +func validateSkills(dir string) (valid, total int, errs []string) { + entries, err := os.ReadDir(dir) + if err != nil { + return 0, 0, nil + } + + for _, entry := range entries { + // Skills are directories containing SKILL.md + if !entry.IsDir() { + continue + } + total++ + + skillDir := filepath.Join(dir, entry.Name()) + _, err := skills.ParseDir(skillDir, skills.SourceTeam) + if err != nil { + errs = append(errs, fmt.Sprintf("%s - %v", skillDir, err)) + } else { + valid++ + } + } + + return valid, total, errs +} diff --git a/internal/config/paths.go b/internal/config/paths.go index b8f7453..baf4b80 100644 --- a/internal/config/paths.go +++ b/internal/config/paths.go @@ -17,6 +17,7 @@ type Paths struct { PersonalLanguages string // ~/.config/staghorn/languages PersonalEvals string // ~/.config/staghorn/evals PersonalRules string // ~/.config/staghorn/rules + PersonalSkills string // ~/.config/staghorn/skills } // NewPaths creates Paths using ~/.config and ~/.cache directories. @@ -36,6 +37,7 @@ func NewPaths() *Paths { PersonalLanguages: filepath.Join(configDir, "languages"), PersonalEvals: filepath.Join(configDir, "evals"), PersonalRules: filepath.Join(configDir, "rules"), + PersonalSkills: filepath.Join(configDir, "skills"), } } @@ -50,6 +52,7 @@ func NewPathsWithOverrides(configDir, cacheDir string) *Paths { PersonalLanguages: filepath.Join(configDir, "languages"), PersonalEvals: filepath.Join(configDir, "evals"), PersonalRules: filepath.Join(configDir, "rules"), + PersonalSkills: filepath.Join(configDir, "skills"), } } @@ -88,6 +91,11 @@ func (p *Paths) TeamRulesDir(owner, repo string) string { return filepath.Join(p.CacheDir, fmt.Sprintf("%s-%s-rules", owner, repo)) } +// TeamSkillsDir returns the path for cached team skills. +func (p *Paths) TeamSkillsDir(owner, repo string) string { + return filepath.Join(p.CacheDir, fmt.Sprintf("%s-%s-skills", owner, repo)) +} + // OptimizedDir returns the path for optimized config storage. func (p *Paths) OptimizedDir() string { return filepath.Join(p.ConfigDir, "optimized") @@ -121,6 +129,15 @@ func (p *Paths) ClaudeRulesDir() string { return filepath.Join(home, ".claude", "rules") } +// ClaudeSkillsDir returns the path for Claude Code user-level skills. +func (p *Paths) ClaudeSkillsDir() string { + home, err := os.UserHomeDir() + if err != nil { + home = os.Getenv("HOME") + } + return filepath.Join(home, ".claude", "skills") +} + // ProjectClaudeCommandsDir returns the path for project-level Claude Code commands. func ProjectClaudeCommandsDir(projectRoot string) string { return filepath.Join(projectRoot, ".claude", "commands") @@ -142,6 +159,7 @@ type ProjectPaths struct { LanguagesDir string // .staghorn/languages/ EvalsDir string // .staghorn/evals/ RulesDir string // .staghorn/rules/ + SkillsDir string // .staghorn/skills/ ConfigFile string // .staghorn/config.yaml (optional project config) } @@ -157,6 +175,7 @@ func NewProjectPaths(projectRoot string) *ProjectPaths { LanguagesDir: filepath.Join(staghornDir, "languages"), EvalsDir: filepath.Join(staghornDir, "evals"), RulesDir: filepath.Join(staghornDir, "rules"), + SkillsDir: filepath.Join(staghornDir, "skills"), ConfigFile: filepath.Join(staghornDir, "config.yaml"), } } @@ -175,3 +194,13 @@ func ProjectRulesDir(projectRoot string) string { func ProjectClaudeRulesDir(projectRoot string) string { return filepath.Join(projectRoot, ".claude", "rules") } + +// ProjectSkillsDir returns the path for project-specific skills. +func ProjectSkillsDir(projectRoot string) string { + return filepath.Join(projectRoot, ".staghorn", "skills") +} + +// ProjectClaudeSkillsDir returns the path for project-level Claude Code skills. +func ProjectClaudeSkillsDir(projectRoot string) string { + return filepath.Join(projectRoot, ".claude", "skills") +} diff --git a/internal/config/source.go b/internal/config/source.go index 17a59ed..22a5364 100644 --- a/internal/config/source.go +++ b/internal/config/source.go @@ -29,6 +29,10 @@ type SourceConfig struct { // Commands maps command names to their source repos. // Example: { "code-review": "acme/internal-commands" } Commands map[string]string `yaml:"commands,omitempty"` + + // Skills maps skill names to their source repos. + // Example: { "react": "vercel-labs/agent-skills/skills/react" } + Skills map[string]string `yaml:"skills,omitempty"` } // Source wraps the flexible source configuration. @@ -110,6 +114,16 @@ func (s *Source) RepoForCommand(cmd string) string { return s.DefaultRepo() } +// RepoForSkill returns the repository to use for a specific skill. +func (s *Source) RepoForSkill(skill string) string { + if s.Multi != nil && s.Multi.Skills != nil { + if repo, ok := s.Multi.Skills[skill]; ok { + return repo + } + } + return s.DefaultRepo() +} + // AllRepos returns all unique repositories referenced by this source config. // Useful for syncing all sources at once. func (s *Source) AllRepos() []string { @@ -133,6 +147,9 @@ func (s *Source) AllRepos() []string { for _, repo := range s.Multi.Commands { addRepo(repo) } + for _, repo := range s.Multi.Skills { + addRepo(repo) + } } return repos @@ -214,6 +231,11 @@ func (s *Source) Validate() error { return fmt.Errorf("invalid source for command %q: %w", cmd, err) } } + for skill, repo := range s.Multi.Skills { + if _, _, err := ParseRepo(repo); err != nil { + return fmt.Errorf("invalid source for skill %q: %w", skill, err) + } + } } return nil diff --git a/internal/integration/harness.go b/internal/integration/harness.go index 1f949a0..3e0f8af 100644 --- a/internal/integration/harness.go +++ b/internal/integration/harness.go @@ -11,6 +11,7 @@ import ( "github.com/HartBrook/staghorn/internal/language" "github.com/HartBrook/staghorn/internal/merge" "github.com/HartBrook/staghorn/internal/rules" + "github.com/HartBrook/staghorn/internal/skills" "gopkg.in/yaml.v3" ) @@ -260,7 +261,7 @@ func (e *TestEnv) RunSync(owner, repo string, cfg *config.Config) error { } output := merge.MergeWithLanguages(layers, mergeOpts) - // Write to output + // Write to output (RunSync) return os.WriteFile(e.GetOutputPath(), []byte(output), 0644) } @@ -361,6 +362,168 @@ func (e *TestEnv) RunMultiSourceSync(cfg *config.Config) error { } output := merge.MergeWithLanguages(layers, mergeOpts) - // Write to output + // Write to output (RunMultiSourceSync) return os.WriteFile(e.GetOutputPath(), []byte(output), 0644) } + +// SetupTeamSkill writes a team skill to cache. +// The skill is a directory containing SKILL.md and optional supporting files. +func (e *TestEnv) SetupTeamSkill(owner, repo, skillName, skillMD string) error { + skillsDir := e.Paths.TeamSkillsDir(owner, repo) + skillDir := filepath.Join(skillsDir, skillName) + if err := os.MkdirAll(skillDir, 0755); err != nil { + return err + } + return os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte(skillMD), 0644) +} + +// SetupTeamSkillWithFiles writes a team skill with supporting files. +func (e *TestEnv) SetupTeamSkillWithFiles(owner, repo, skillName, skillMD string, files map[string]string) error { + skillsDir := e.Paths.TeamSkillsDir(owner, repo) + skillDir := filepath.Join(skillsDir, skillName) + if err := os.MkdirAll(skillDir, 0755); err != nil { + return err + } + + // Write SKILL.md + if err := os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte(skillMD), 0644); err != nil { + return err + } + + // Write supporting files + for relPath, content := range files { + fullPath := filepath.Join(skillDir, relPath) + if err := os.MkdirAll(filepath.Dir(fullPath), 0755); err != nil { + return err + } + if err := os.WriteFile(fullPath, []byte(content), 0644); err != nil { + return err + } + } + + return nil +} + +// SetupPersonalSkill writes a personal skill. +func (e *TestEnv) SetupPersonalSkill(skillName, skillMD string) error { + skillDir := filepath.Join(e.Paths.PersonalSkills, skillName) + if err := os.MkdirAll(skillDir, 0755); err != nil { + return err + } + return os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte(skillMD), 0644) +} + +// GetClaudeSkillsDir returns the path to ~/.claude/skills. +func (e *TestEnv) GetClaudeSkillsDir() string { + return filepath.Join(e.ClaudeDir, "skills") +} + +// ReadClaudeSkill reads SKILL.md from ~/.claude/skills//. +func (e *TestEnv) ReadClaudeSkill(skillName string) (string, error) { + content, err := os.ReadFile(filepath.Join(e.GetClaudeSkillsDir(), skillName, "SKILL.md")) + if err != nil { + return "", err + } + return string(content), nil +} + +// ReadClaudeSkillFile reads a supporting file from a Claude skill directory. +func (e *TestEnv) ReadClaudeSkillFile(skillName, relPath string) (string, error) { + content, err := os.ReadFile(filepath.Join(e.GetClaudeSkillsDir(), skillName, relPath)) + if err != nil { + return "", err + } + return string(content), nil +} + +// RunSyncSkills syncs skills from team/personal sources to Claude skills directory. +func (e *TestEnv) RunSyncSkills(owner, repo string) (int, error) { + // Load skills from all sources using the registry + registry, err := skills.LoadRegistry( + e.Paths.TeamSkillsDir(owner, repo), + e.Paths.PersonalSkills, + "", // No project dir for global sync + ) + if err != nil { + return 0, err + } + + allSkills := registry.All() + if len(allSkills) == 0 { + return 0, nil + } + + // Create Claude skills directory + claudeSkillsDir := e.GetClaudeSkillsDir() + if err := os.MkdirAll(claudeSkillsDir, 0755); err != nil { + return 0, err + } + + // Sync each skill + count := 0 + for _, skill := range allSkills { + _, err := skills.SyncToClaude(skill, claudeSkillsDir) + if err != nil { + return count, err + } + count++ + } + + return count, nil +} + +// RunSyncSkillsMultiSource syncs skills from multiple source repos. +func (e *TestEnv) RunSyncSkillsMultiSource(cfg *config.Config) (int, error) { + // Collect team skills directories from all repos + var teamSkillsDirs []string + for _, repoStr := range cfg.Source.AllRepos() { + owner, repo, err := config.ParseRepo(repoStr) + if err != nil { + continue + } + teamSkillsDirs = append(teamSkillsDirs, e.Paths.TeamSkillsDir(owner, repo)) + } + + // Load skills from all sources + registry, err := skills.LoadRegistryWithMultipleDirs( + teamSkillsDirs, + e.Paths.PersonalSkills, + "", // No project dir for global sync + ) + if err != nil { + return 0, err + } + + allSkills := registry.All() + if len(allSkills) == 0 { + return 0, nil + } + + // Create Claude skills directory + claudeSkillsDir := e.GetClaudeSkillsDir() + if err := os.MkdirAll(claudeSkillsDir, 0755); err != nil { + return 0, err + } + + // Sync each skill + count := 0 + for _, skill := range allSkills { + _, err := skills.SyncToClaude(skill, claudeSkillsDir) + if err != nil { + continue // Skip errors for multi-source (may have collision warnings) + } + count++ + } + + return count, nil +} + +// SetupExistingClaudeSkill creates a skill in ~/.claude/skills that is NOT managed by staghorn. +// Used for testing collision detection. +func (e *TestEnv) SetupExistingClaudeSkill(skillName, skillMD string) error { + skillDir := filepath.Join(e.GetClaudeSkillsDir(), skillName) + if err := os.MkdirAll(skillDir, 0755); err != nil { + return err + } + return os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte(skillMD), 0644) +} diff --git a/internal/integration/integration_test.go b/internal/integration/integration_test.go index d6cd121..acb3d84 100644 --- a/internal/integration/integration_test.go +++ b/internal/integration/integration_test.go @@ -661,3 +661,316 @@ Default Rust.` assert.True(t, asserter.ContainsText("Default Rust"), "rust should fallback to default repo") } + +// TestIntegration_SkillsBasicSync tests basic skill sync to Claude directory. +func TestIntegration_SkillsBasicSync(t *testing.T) { + env := NewTestEnv(t) + defer env.Cleanup() + + owner, repo := "acme", "standards" + + // Setup team skill + teamSkill := `--- +name: code-review +description: Thorough code review +allowed-tools: Read Grep Glob +--- + +# Code Review + +Review the code carefully.` + + err := env.SetupTeamSkill(owner, repo, "code-review", teamSkill) + require.NoError(t, err) + + // Run sync + count, err := env.RunSyncSkills(owner, repo) + require.NoError(t, err) + assert.Equal(t, 1, count, "should sync 1 skill") + + // Read output and verify + output, err := env.ReadClaudeSkill("code-review") + require.NoError(t, err) + + assert.Contains(t, output, "Managed by staghorn", "should have managed header") + assert.Contains(t, output, "Source: team", "should have team source") + assert.Contains(t, output, "name: code-review", "should contain skill name") + assert.Contains(t, output, "Review the code carefully", "should contain skill body") +} + +// TestIntegration_SkillsPrecedence tests that personal skills override team skills. +func TestIntegration_SkillsPrecedence(t *testing.T) { + env := NewTestEnv(t) + defer env.Cleanup() + + owner, repo := "acme", "standards" + + // Setup team skill + teamSkill := `--- +name: code-review +description: Team code review +allowed-tools: Read Grep +--- + +Team review instructions.` + + // Setup personal skill with same name (should override) + personalSkill := `--- +name: code-review +description: My personal code review +allowed-tools: Read Grep Glob WebSearch +--- + +My custom review instructions.` + + err := env.SetupTeamSkill(owner, repo, "code-review", teamSkill) + require.NoError(t, err) + + err = env.SetupPersonalSkill("code-review", personalSkill) + require.NoError(t, err) + + count, err := env.RunSyncSkills(owner, repo) + require.NoError(t, err) + assert.Equal(t, 1, count, "should have 1 unique skill (personal overrides team)") + + output, err := env.ReadClaudeSkill("code-review") + require.NoError(t, err) + + // Personal should win + assert.Contains(t, output, "Source: personal", "should have personal source (higher precedence)") + assert.Contains(t, output, "My custom review instructions", "should contain personal content") + assert.NotContains(t, output, "Team review instructions", "should NOT contain team content") +} + +// TestIntegration_SkillsWithSupportingFiles tests skills with templates and scripts. +func TestIntegration_SkillsWithSupportingFiles(t *testing.T) { + env := NewTestEnv(t) + defer env.Cleanup() + + owner, repo := "acme", "standards" + + skillMD := `--- +name: test-gen +description: Generate tests with templates +allowed-tools: Read Grep Glob Write +--- + +# Test Generation + +Use the templates in templates/ directory.` + + supportingFiles := map[string]string{ + "templates/jest.md": "# Jest Template\n\nUse describe blocks.", + "templates/pytest.md": "# Pytest Template\n\nUse fixtures.", + "scripts/validate.sh": "#!/bin/bash\necho 'Validating...'", + } + + err := env.SetupTeamSkillWithFiles(owner, repo, "test-gen", skillMD, supportingFiles) + require.NoError(t, err) + + count, err := env.RunSyncSkills(owner, repo) + require.NoError(t, err) + assert.Equal(t, 1, count, "should sync 1 skill") + + // Verify SKILL.md + output, err := env.ReadClaudeSkill("test-gen") + require.NoError(t, err) + assert.Contains(t, output, "Use the templates in templates/ directory", "should contain skill body") + + // Verify supporting files were copied + jestTemplate, err := env.ReadClaudeSkillFile("test-gen", "templates/jest.md") + require.NoError(t, err) + assert.Contains(t, jestTemplate, "Use describe blocks", "jest template should be copied") + + pytestTemplate, err := env.ReadClaudeSkillFile("test-gen", "templates/pytest.md") + require.NoError(t, err) + assert.Contains(t, pytestTemplate, "Use fixtures", "pytest template should be copied") + + script, err := env.ReadClaudeSkillFile("test-gen", "scripts/validate.sh") + require.NoError(t, err) + assert.Contains(t, script, "Validating", "script should be copied") +} + +// TestIntegration_SkillsCollisionDetection tests that staghorn won't overwrite user skills. +func TestIntegration_SkillsCollisionDetection(t *testing.T) { + env := NewTestEnv(t) + defer env.Cleanup() + + owner, repo := "acme", "standards" + + // Create existing user skill (NOT managed by staghorn) + userSkill := `--- +name: my-custom-skill +description: User's own skill +--- + +My custom workflow that I created manually.` + + err := env.SetupExistingClaudeSkill("my-custom-skill", userSkill) + require.NoError(t, err) + + // Setup team skill with same name + teamSkill := `--- +name: my-custom-skill +description: Team version trying to overwrite +--- + +Team content.` + + err = env.SetupTeamSkill(owner, repo, "my-custom-skill", teamSkill) + require.NoError(t, err) + + // Sync should fail for this skill (collision) + _, err = env.RunSyncSkills(owner, repo) + // The sync returns error when collision is detected + require.Error(t, err, "should error when trying to overwrite non-staghorn skill") + + // Verify user skill was NOT overwritten + output, err := env.ReadClaudeSkill("my-custom-skill") + require.NoError(t, err) + assert.Contains(t, output, "My custom workflow that I created manually", + "user's skill should be preserved") + assert.NotContains(t, output, "Managed by staghorn", + "should NOT have staghorn header") +} + +// TestIntegration_SkillsEmptyDirs tests sync with no skills. +func TestIntegration_SkillsEmptyDirs(t *testing.T) { + env := NewTestEnv(t) + defer env.Cleanup() + + owner, repo := "acme", "standards" + + // No skills set up - directories don't exist + + count, err := env.RunSyncSkills(owner, repo) + require.NoError(t, err) + assert.Equal(t, 0, count, "should sync 0 skills") +} + +// TestIntegration_SkillsMultiSource tests skills from different source repos. +func TestIntegration_SkillsMultiSource(t *testing.T) { + env := NewTestEnv(t) + defer env.Cleanup() + + // Setup: code-review from default repo (acme/standards) + codeReviewSkill := `--- +name: code-review +description: Team code review from acme +allowed-tools: Read Grep Glob +--- + +Review code using team standards.` + err := env.SetupTeamSkill("acme", "standards", "code-review", codeReviewSkill) + require.NoError(t, err) + + // Setup: react skill from vercel-labs/agent-skills + reactSkill := `--- +name: react +description: React development patterns from Vercel +allowed-tools: Read Grep Glob Write +--- + +# React Patterns + +Use React Server Components when possible.` + err = env.SetupTeamSkill("vercel-labs", "agent-skills", "react", reactSkill) + require.NoError(t, err) + + // Setup: security-audit from community/security + securitySkill := `--- +name: security-audit +description: Security audit from community +allowed-tools: Read Grep Glob +context: fork +agent: Explore +--- + +# Security Audit + +Check for OWASP Top 10 vulnerabilities.` + err = env.SetupTeamSkill("community", "security", "security-audit", securitySkill) + require.NoError(t, err) + + // Create multi-source config + cfg := &config.Config{ + Version: 1, + Source: config.Source{ + Multi: &config.SourceConfig{ + Default: "acme/standards", + Skills: map[string]string{ + "react": "vercel-labs/agent-skills", + "security-audit": "community/security", + }, + }, + }, + } + + // Run multi-source sync + count, err := env.RunSyncSkillsMultiSource(cfg) + require.NoError(t, err) + assert.Equal(t, 3, count, "should sync 3 skills from different repos") + + // Verify code-review from default repo + codeReviewOutput, err := env.ReadClaudeSkill("code-review") + require.NoError(t, err) + assert.Contains(t, codeReviewOutput, "Review code using team standards", + "code-review should come from acme/standards") + + // Verify react from vercel-labs + reactOutput, err := env.ReadClaudeSkill("react") + require.NoError(t, err) + assert.Contains(t, reactOutput, "React Server Components", + "react should come from vercel-labs/agent-skills") + + // Verify security-audit from community + securityOutput, err := env.ReadClaudeSkill("security-audit") + require.NoError(t, err) + assert.Contains(t, securityOutput, "OWASP Top 10", + "security-audit should come from community/security") +} + +// TestIntegration_SkillsMultipleFromSameTeam tests multiple skills from default repo. +func TestIntegration_SkillsMultipleFromSameTeam(t *testing.T) { + env := NewTestEnv(t) + defer env.Cleanup() + + owner, repo := "acme", "standards" + + // Setup multiple skills from same repo + skills := map[string]string{ + "code-review": `--- +name: code-review +description: Code review +--- + +Review code.`, + "test-gen": `--- +name: test-gen +description: Generate tests +--- + +Generate tests.`, + "refactor": `--- +name: refactor +description: Refactoring helper +--- + +Refactor code.`, + } + + for name, content := range skills { + err := env.SetupTeamSkill(owner, repo, name, content) + require.NoError(t, err) + } + + count, err := env.RunSyncSkills(owner, repo) + require.NoError(t, err) + assert.Equal(t, 3, count, "should sync 3 skills") + + // Verify all skills exist + for name := range skills { + _, err := env.ReadClaudeSkill(name) + require.NoError(t, err, "%s should exist", name) + } +} diff --git a/internal/integration/live_test.go b/internal/integration/live_test.go index 87b7d6a..e0fdd23 100644 --- a/internal/integration/live_test.go +++ b/internal/integration/live_test.go @@ -4,11 +4,13 @@ package integration import ( "context" + "strings" "testing" "time" "github.com/HartBrook/staghorn/internal/config" "github.com/HartBrook/staghorn/internal/github" + "github.com/HartBrook/staghorn/internal/skills" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -128,3 +130,222 @@ func TestLive_WithLanguages(t *testing.T) { assert.True(t, asserter.HasManagedHeader(), "should have managed header") assert.True(t, asserter.HasProvenanceMarker("team"), "should have team marker") } + +// TestLive_VercelSkills tests fetching skills from vercel-labs/agent-skills. +// This validates that staghorn can parse real-world Agent Skills format from external repos. +// Run with: go test -tags=live ./internal/integration/... +func TestLive_VercelSkills(t *testing.T) { + if testing.Short() { + t.Skip("skipping live test in short mode") + } + + env := NewTestEnv(t) + defer env.Cleanup() + + // Create GitHub client + client, err := github.NewClient() + if err != nil { + t.Skip("GitHub auth not available, skipping live test") + } + + ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) + defer cancel() + + owner, repo := "vercel-labs", "agent-skills" + + // List skills directory to discover available skills + entries, err := client.ListDirectory(ctx, owner, repo, "skills", "") + if err != nil { + t.Skipf("Failed to list skills directory from GitHub: %v", err) + } + + if len(entries) == 0 { + t.Skip("No skills found in vercel-labs/agent-skills") + } + + // Fetch and parse each skill + var fetchedSkills []*skills.Skill + for _, entry := range entries { + if entry.Type != "dir" { + continue + } + + skillPath := "skills/" + entry.Name + "/SKILL.md" + result, err := client.FetchFile(ctx, owner, repo, skillPath, "") + if err != nil { + t.Logf("Skipping skill %s: %v", entry.Name, err) + continue + } + + // Parse the skill to validate format compatibility + skill, err := skills.Parse(result.Content, skills.SourceTeam, "") + if err != nil { + t.Errorf("Failed to parse skill %s: %v", entry.Name, err) + continue + } + + fetchedSkills = append(fetchedSkills, skill) + t.Logf("Successfully parsed skill: %s (%s)", skill.Name, skill.Description) + } + + require.NotEmpty(t, fetchedSkills, "should have fetched at least one skill") + + // Setup skills in test environment and sync to Claude + for _, skill := range fetchedSkills { + // Recreate SKILL.md content for setup + content := buildSkillMD(skill) + err := env.SetupTeamSkill(owner, repo, skill.Name, content) + require.NoError(t, err, "failed to setup skill %s", skill.Name) + } + + // Run sync + count, err := env.RunSyncSkills(owner, repo) + require.NoError(t, err) + assert.Equal(t, len(fetchedSkills), count, "should sync all fetched skills") + + // Verify skills were synced correctly + for _, skill := range fetchedSkills { + output, err := env.ReadClaudeSkill(skill.Name) + require.NoError(t, err, "should be able to read synced skill %s", skill.Name) + + assert.Contains(t, output, "Managed by staghorn", "skill %s should have staghorn header", skill.Name) + assert.Contains(t, output, "name: "+skill.Name, "skill %s should have name in frontmatter", skill.Name) + } + + t.Logf("Successfully synced %d skills from vercel-labs/agent-skills", count) +} + +// TestLive_VercelSkillsMultiSource tests a multi-source config with Vercel skills. +// Run with: go test -tags=live ./internal/integration/... +func TestLive_VercelSkillsMultiSource(t *testing.T) { + if testing.Short() { + t.Skip("skipping live test in short mode") + } + + env := NewTestEnv(t) + defer env.Cleanup() + + // Create GitHub client + client, err := github.NewClient() + if err != nil { + t.Skip("GitHub auth not available, skipping live test") + } + + ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) + defer cancel() + + // Fetch one skill from Vercel + vercelOwner, vercelRepo := "vercel-labs", "agent-skills" + + // List and pick the first available skill + entries, err := client.ListDirectory(ctx, vercelOwner, vercelRepo, "skills", "") + if err != nil { + t.Skipf("Failed to list Vercel skills: %v", err) + } + + // Find first valid skill (directory with SKILL.md) + var vercelSkill *skills.Skill + var vercelSkillContent string + for _, entry := range entries { + if entry.Type != "dir" { + continue + } + + skillPath := "skills/" + entry.Name + "/SKILL.md" + result, fetchErr := client.FetchFile(ctx, vercelOwner, vercelRepo, skillPath, "") + if fetchErr != nil { + t.Logf("Skipping %s: no SKILL.md", entry.Name) + continue + } + + skill, parseErr := skills.Parse(result.Content, skills.SourceTeam, "") + if parseErr != nil { + t.Logf("Skipping %s: failed to parse: %v", entry.Name, parseErr) + continue + } + + vercelSkill = skill + vercelSkillContent = result.Content + break + } + + if vercelSkill == nil { + t.Skip("No valid skills found in vercel-labs/agent-skills") + } + + // Setup Vercel skill + err = env.SetupTeamSkill(vercelOwner, vercelRepo, vercelSkill.Name, vercelSkillContent) + require.NoError(t, err) + + // Also fetch from staghorn-community if available + communityOwner, communityRepo := "HartBrook", "staghorn-community" + communityEntries, err := client.ListDirectory(ctx, communityOwner, communityRepo, "skills", "") + + var communitySkillName string + if err == nil && len(communityEntries) > 0 { + for _, entry := range communityEntries { + if entry.Type == "dir" { + communitySkillName = entry.Name + + communityResult, fetchErr := client.FetchFile(ctx, communityOwner, communityRepo, "skills/"+entry.Name+"/SKILL.md", "") + if fetchErr == nil { + err = env.SetupTeamSkill(communityOwner, communityRepo, entry.Name, communityResult.Content) + if err != nil { + t.Logf("Warning: failed to setup community skill %s: %v", entry.Name, err) + communitySkillName = "" + } + } + break + } + } + } + + // Create multi-source config + cfg := &config.Config{ + Version: 1, + Source: config.Source{ + Multi: &config.SourceConfig{ + Default: communityOwner + "/" + communityRepo, + Skills: map[string]string{ + vercelSkill.Name: vercelOwner + "/" + vercelRepo, + }, + }, + }, + } + + // Run multi-source sync + count, err := env.RunSyncSkillsMultiSource(cfg) + require.NoError(t, err) + assert.GreaterOrEqual(t, count, 1, "should sync at least the Vercel skill") + + // Verify Vercel skill was synced + output, err := env.ReadClaudeSkill(vercelSkill.Name) + require.NoError(t, err) + assert.Contains(t, output, "Managed by staghorn", "Vercel skill should have staghorn header") + + t.Logf("Successfully synced %d skills in multi-source config (Vercel: %s)", count, vercelSkill.Name) + if communitySkillName != "" { + t.Logf("Also included community skill: %s", communitySkillName) + } +} + +// buildSkillMD reconstructs a SKILL.md from a parsed skill. +// This is a simplified version for testing - the real content comes from GitHub. +func buildSkillMD(skill *skills.Skill) string { + var sb strings.Builder + sb.WriteString("---\n") + sb.WriteString("name: " + skill.Name + "\n") + sb.WriteString("description: " + skill.Description + "\n") + if skill.AllowedTools != "" { + sb.WriteString("allowed-tools: " + skill.AllowedTools + "\n") + } + if skill.Context != "" { + sb.WriteString("context: " + skill.Context + "\n") + } + if skill.Agent != "" { + sb.WriteString("agent: " + skill.Agent + "\n") + } + sb.WriteString("---\n\n") + sb.WriteString(skill.Body) + return sb.String() +} diff --git a/internal/skills/claude.go b/internal/skills/claude.go new file mode 100644 index 0000000..7fb5dce --- /dev/null +++ b/internal/skills/claude.go @@ -0,0 +1,244 @@ +package skills + +import ( + "fmt" + "io" + "os" + "path/filepath" + "strings" + + "gopkg.in/yaml.v3" +) + +// Header prefix used to identify staghorn-managed skills. +const HeaderManagedPrefix = "\n\n", HeaderManagedPrefix, skill.Source.Label())) + + // Add args hint if there are arguments + if len(skill.Args) > 0 { + sb.WriteString(buildArgsHint(skill)) + sb.WriteString("\n") + } + + // Add the body + sb.WriteString(skill.Body) + sb.WriteString("\n") + + return sb.String() +} + +// buildClaudeFrontmatter creates the frontmatter map for Claude Code. +func buildClaudeFrontmatter(skill *Skill) map[string]any { + fm := make(map[string]any) + + // Required fields + fm["name"] = skill.Name + fm["description"] = skill.Description + + // Optional Agent Skills standard fields + if skill.License != "" { + fm["license"] = skill.License + } + if skill.Compatibility != "" { + fm["compatibility"] = skill.Compatibility + } + if len(skill.Metadata) > 0 { + fm["metadata"] = skill.Metadata + } + if skill.AllowedTools != "" { + fm["allowed-tools"] = skill.AllowedTools + } + + // Claude Code extensions + if skill.DisableModelInvocation { + fm["disable-model-invocation"] = true + } + if skill.UserInvocable != nil { + fm["user-invocable"] = *skill.UserInvocable + } + if skill.Context != "" { + fm["context"] = skill.Context + } + if skill.Agent != "" { + fm["agent"] = skill.Agent + } + if skill.ArgumentHint != "" { + fm["argument-hint"] = skill.ArgumentHint + } + if skill.Model != "" { + fm["model"] = skill.Model + } + if skill.Hooks != nil { + fm["hooks"] = skill.Hooks + } + + return fm +} + +// buildArgsHint creates a usage hint comment for Claude to understand the args. +func buildArgsHint(skill *Skill) string { + var sb strings.Builder + + // Build args list + var argParts []string + for _, arg := range skill.Args { + part := arg.Name + if arg.Required { + part += " (required)" + } else if arg.Default != "" { + part += fmt.Sprintf(" (default: %s)", arg.Default) + } + argParts = append(argParts, part) + } + + sb.WriteString(fmt.Sprintf("\n", strings.Join(argParts, ", "))) + + // Build example usage + var exampleParts []string + for _, arg := range skill.Args { + val := arg.Default + if val == "" { + val = "" + } + exampleParts = append(exampleParts, fmt.Sprintf("%s=%q", arg.Name, val)) + } + sb.WriteString(fmt.Sprintf("\n", skill.Name, strings.Join(exampleParts, " "))) + + return sb.String() +} + +// SyncToClaude syncs a skill to Claude Code's skills directory. +// This copies the entire skill directory, preserving structure. +// Returns the number of files written. +func SyncToClaude(skill *Skill, claudeSkillsDir string) (int, error) { + destDir := filepath.Join(claudeSkillsDir, skill.Name) + + // Check for collision with non-staghorn skill + destSkillMD := filepath.Join(destDir, "SKILL.md") + if existingContent, err := os.ReadFile(destSkillMD); err == nil { + if !strings.Contains(string(existingContent), HeaderManagedPrefix) { + return 0, fmt.Errorf("existing skill not managed by staghorn") + } + } + + // Create the skill directory + if err := os.MkdirAll(destDir, 0755); err != nil { + return 0, fmt.Errorf("failed to create skill directory: %w", err) + } + + filesWritten := 0 + + // Write the converted SKILL.md + content := ConvertToClaude(skill) + if err := os.WriteFile(destSkillMD, []byte(content), 0644); err != nil { + return filesWritten, fmt.Errorf("failed to write SKILL.md: %w", err) + } + filesWritten++ + + // Copy supporting files + for relPath, srcPath := range skill.SupportingFiles { + destPath := filepath.Join(destDir, relPath) + + // Ensure parent directory exists + if err := os.MkdirAll(filepath.Dir(destPath), 0755); err != nil { + return filesWritten, fmt.Errorf("failed to create directory for %s: %w", relPath, err) + } + + // Copy the file + if err := copyFile(srcPath, destPath); err != nil { + return filesWritten, fmt.Errorf("failed to copy %s: %w", relPath, err) + } + filesWritten++ + } + + return filesWritten, nil +} + +// copyFile copies a file from src to dst. +func copyFile(src, dst string) error { + srcFile, err := os.Open(src) + if err != nil { + return err + } + defer srcFile.Close() + + srcInfo, err := srcFile.Stat() + if err != nil { + return err + } + + dstFile, err := os.OpenFile(dst, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, srcInfo.Mode()) + if err != nil { + return err + } + defer dstFile.Close() + + _, err = io.Copy(dstFile, srcFile) + return err +} + +// RemoveSkill removes a skill directory from Claude Code's skills directory. +func RemoveSkill(name, claudeSkillsDir string) error { + destDir := filepath.Join(claudeSkillsDir, name) + + // Check if it exists and is managed by staghorn + destSkillMD := filepath.Join(destDir, "SKILL.md") + if existingContent, err := os.ReadFile(destSkillMD); err == nil { + if !strings.Contains(string(existingContent), HeaderManagedPrefix) { + return fmt.Errorf("skill not managed by staghorn") + } + } else if os.IsNotExist(err) { + return nil // Nothing to remove + } else { + return err + } + + return os.RemoveAll(destDir) +} + +// ListManagedSkills returns a list of skill names in the Claude skills directory +// that are managed by staghorn. +func ListManagedSkills(claudeSkillsDir string) ([]string, error) { + entries, err := os.ReadDir(claudeSkillsDir) + if err != nil { + if os.IsNotExist(err) { + return nil, nil + } + return nil, err + } + + var names []string + for _, entry := range entries { + if !entry.IsDir() { + continue + } + + skillMD := filepath.Join(claudeSkillsDir, entry.Name(), "SKILL.md") + content, err := os.ReadFile(skillMD) + if err != nil { + continue + } + + if strings.Contains(string(content), HeaderManagedPrefix) { + names = append(names, entry.Name()) + } + } + + return names, nil +} diff --git a/internal/skills/claude_test.go b/internal/skills/claude_test.go new file mode 100644 index 0000000..b26df1c --- /dev/null +++ b/internal/skills/claude_test.go @@ -0,0 +1,442 @@ +package skills + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestConvertToClaude(t *testing.T) { + skill := &Skill{ + Frontmatter: Frontmatter{ + Name: "code-review", + Description: "Thorough code review", + AllowedTools: "Read Grep Glob", + Tags: []string{"review"}, + }, + Body: "Review the code carefully.", + Source: SourceTeam, + } + + result := ConvertToClaude(skill) + + // Check frontmatter is present + if !strings.Contains(result, "name: code-review") { + t.Error("result should contain name field") + } + if !strings.Contains(result, "description: Thorough code review") { + t.Error("result should contain description field") + } + if !strings.Contains(result, "allowed-tools: Read Grep Glob") { + t.Error("result should contain allowed-tools field") + } + + // Check staghorn header + if !strings.Contains(result, HeaderManagedPrefix) { + t.Error("result should contain staghorn managed header") + } + if !strings.Contains(result, "Source: team") { + t.Error("result should contain source label") + } + + // Check body is present + if !strings.Contains(result, "Review the code carefully.") { + t.Error("result should contain body") + } +} + +func TestConvertToClaudeWithArgs(t *testing.T) { + skill := &Skill{ + Frontmatter: Frontmatter{ + Name: "test-gen", + Description: "Generate tests", + Args: []Arg{ + {Name: "path", Default: ".", Required: true}, + {Name: "framework", Default: "jest"}, + }, + }, + Body: "Generate tests at {{path}}.", + Source: SourcePersonal, + } + + result := ConvertToClaude(skill) + + // Check args hint is added + if !strings.Contains(result, " + +Old content.` + if err := os.WriteFile(filepath.Join(destDir, "SKILL.md"), []byte(existingContent), 0644); err != nil { + t.Fatal(err) + } + + skill := &Skill{ + Frontmatter: Frontmatter{ + Name: "my-skill", + Description: "New version", + }, + Body: "New content.", + Source: SourceTeam, + } + + filesWritten, err := SyncToClaude(skill, claudeSkillsDir) + if err != nil { + t.Fatalf("SyncToClaude() error = %v", err) + } + if filesWritten != 1 { + t.Errorf("filesWritten = %d, want 1", filesWritten) + } + + // Check content was updated + content, err := os.ReadFile(filepath.Join(destDir, "SKILL.md")) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(content), "New version") { + t.Error("content should be updated") + } +} + +func TestRemoveSkill(t *testing.T) { + // Create temp directories + tempDir := t.TempDir() + claudeSkillsDir := filepath.Join(tempDir, ".claude", "skills") + destDir := filepath.Join(claudeSkillsDir, "my-skill") + + // Create managed skill + if err := os.MkdirAll(destDir, 0755); err != nil { + t.Fatal(err) + } + content := `--- +name: my-skill +description: Test +--- + +` + HeaderManagedPrefix + ` | Source: team --> + +Content.` + if err := os.WriteFile(filepath.Join(destDir, "SKILL.md"), []byte(content), 0644); err != nil { + t.Fatal(err) + } + + err := RemoveSkill("my-skill", claudeSkillsDir) + if err != nil { + t.Fatalf("RemoveSkill() error = %v", err) + } + + if _, err := os.Stat(destDir); !os.IsNotExist(err) { + t.Error("skill directory should be removed") + } +} + +func TestRemoveSkillNonManaged(t *testing.T) { + // Create temp directories + tempDir := t.TempDir() + claudeSkillsDir := filepath.Join(tempDir, ".claude", "skills") + destDir := filepath.Join(claudeSkillsDir, "user-skill") + + // Create non-managed skill + if err := os.MkdirAll(destDir, 0755); err != nil { + t.Fatal(err) + } + content := `--- +name: user-skill +description: User's own skill +--- + +Not managed by staghorn.` + if err := os.WriteFile(filepath.Join(destDir, "SKILL.md"), []byte(content), 0644); err != nil { + t.Fatal(err) + } + + err := RemoveSkill("user-skill", claudeSkillsDir) + if err == nil { + t.Error("expected error when trying to remove non-managed skill") + } + + // Directory should still exist + if _, err := os.Stat(destDir); os.IsNotExist(err) { + t.Error("non-managed skill should not be removed") + } +} + +func TestRemoveSkillNonexistent(t *testing.T) { + tempDir := t.TempDir() + claudeSkillsDir := filepath.Join(tempDir, ".claude", "skills") + + // Should not error for nonexistent skill + err := RemoveSkill("nonexistent", claudeSkillsDir) + if err != nil { + t.Errorf("RemoveSkill() for nonexistent skill should not error, got: %v", err) + } +} + +func TestListManagedSkills(t *testing.T) { + // Create temp directories + tempDir := t.TempDir() + claudeSkillsDir := filepath.Join(tempDir, ".claude", "skills") + + // Create managed skill + managedDir := filepath.Join(claudeSkillsDir, "managed-skill") + if err := os.MkdirAll(managedDir, 0755); err != nil { + t.Fatal(err) + } + managedContent := `--- +name: managed-skill +description: Managed +--- + +` + HeaderManagedPrefix + ` --> + +Content.` + if err := os.WriteFile(filepath.Join(managedDir, "SKILL.md"), []byte(managedContent), 0644); err != nil { + t.Fatal(err) + } + + // Create non-managed skill + userDir := filepath.Join(claudeSkillsDir, "user-skill") + if err := os.MkdirAll(userDir, 0755); err != nil { + t.Fatal(err) + } + userContent := `--- +name: user-skill +description: User +--- + +Not managed.` + if err := os.WriteFile(filepath.Join(userDir, "SKILL.md"), []byte(userContent), 0644); err != nil { + t.Fatal(err) + } + + // Create regular file (should be ignored) + if err := os.WriteFile(filepath.Join(claudeSkillsDir, "readme.txt"), []byte("ignore"), 0644); err != nil { + t.Fatal(err) + } + + names, err := ListManagedSkills(claudeSkillsDir) + if err != nil { + t.Fatalf("ListManagedSkills() error = %v", err) + } + + if len(names) != 1 { + t.Errorf("ListManagedSkills() = %d, want 1", len(names)) + } + if len(names) > 0 && names[0] != "managed-skill" { + t.Errorf("ListManagedSkills()[0] = %q, want %q", names[0], "managed-skill") + } +} + +func TestListManagedSkillsNonexistent(t *testing.T) { + names, err := ListManagedSkills("/nonexistent/path") + if err != nil { + t.Errorf("expected nil error for nonexistent directory, got %v", err) + } + if names != nil { + t.Errorf("expected nil names for nonexistent directory, got %v", names) + } +} diff --git a/internal/skills/registry.go b/internal/skills/registry.go new file mode 100644 index 0000000..a45f73b --- /dev/null +++ b/internal/skills/registry.go @@ -0,0 +1,222 @@ +package skills + +import ( + "fmt" + "os" + "sort" +) + +// Registry manages skills from multiple sources with precedence handling. +// Precedence (highest to lowest): project > personal > team > starter +type Registry struct { + skills map[string]*Skill // name -> skill (highest precedence wins) + bySource map[Source][]*Skill +} + +// NewRegistry creates an empty skill registry. +func NewRegistry() *Registry { + return &Registry{ + skills: make(map[string]*Skill), + bySource: make(map[Source][]*Skill), + } +} + +// Add adds a skill to the registry. +// If a skill with the same name exists from a lower precedence source, it's overridden. +func (r *Registry) Add(skill *Skill) { + r.bySource[skill.Source] = append(r.bySource[skill.Source], skill) + + // Check precedence before overriding + existing, exists := r.skills[skill.Name] + if !exists || sourcePrecedence(skill.Source) > sourcePrecedence(existing.Source) { + r.skills[skill.Name] = skill + } +} + +// AddAll adds multiple skills to the registry. +func (r *Registry) AddAll(skills []*Skill) { + for _, skill := range skills { + r.Add(skill) + } +} + +// Get returns a skill by name (highest precedence version). +func (r *Registry) Get(name string) *Skill { + return r.skills[name] +} + +// All returns all unique skills (highest precedence version of each). +func (r *Registry) All() []*Skill { + skills := make([]*Skill, 0, len(r.skills)) + for _, skill := range r.skills { + skills = append(skills, skill) + } + sort.Slice(skills, func(i, j int) bool { + return skills[i].Name < skills[j].Name + }) + return skills +} + +// BySource returns all skills from a specific source. +func (r *Registry) BySource(source Source) []*Skill { + // Make a copy to avoid mutating the original slice during sort + original := r.bySource[source] + skills := make([]*Skill, len(original)) + copy(skills, original) + sort.Slice(skills, func(i, j int) bool { + return skills[i].Name < skills[j].Name + }) + return skills +} + +// ByTag returns all skills that have a specific tag. +func (r *Registry) ByTag(tag string) []*Skill { + var result []*Skill + for _, skill := range r.skills { + for _, t := range skill.Tags { + if t == tag { + result = append(result, skill) + break + } + } + } + sort.Slice(result, func(i, j int) bool { + return result[i].Name < result[j].Name + }) + return result +} + +// Names returns all skill names. +func (r *Registry) Names() []string { + names := make([]string, 0, len(r.skills)) + for name := range r.skills { + names = append(names, name) + } + sort.Strings(names) + return names +} + +// Count returns the total number of unique skills. +func (r *Registry) Count() int { + return len(r.skills) +} + +// CountBySource returns counts per source. +func (r *Registry) CountBySource() map[Source]int { + counts := make(map[Source]int) + for source, skills := range r.bySource { + counts[source] = len(skills) + } + return counts +} + +// IsOverridden checks if a skill from a lower source is overridden. +func (r *Registry) IsOverridden(name string, source Source) bool { + skill := r.skills[name] + if skill == nil { + return false + } + return skill.Source != source +} + +// GetAllVersions returns all versions of a skill across sources. +func (r *Registry) GetAllVersions(name string) []*Skill { + var versions []*Skill + for _, skills := range r.bySource { + for _, skill := range skills { + if skill.Name == name { + versions = append(versions, skill) + } + } + } + // Sort by precedence (highest first) + sort.Slice(versions, func(i, j int) bool { + return sourcePrecedence(versions[i].Source) > sourcePrecedence(versions[j].Source) + }) + return versions +} + +// sourcePrecedence returns the precedence level of a source. +// Higher number = higher precedence. +func sourcePrecedence(s Source) int { + switch s { + case SourceProject: + return 3 + case SourcePersonal: + return 2 + case SourceTeam: + return 1 + case SourceStarter: + return 0 + default: + return -1 + } +} + +// LoadRegistry creates a registry by loading skills from all sources. +func LoadRegistry(teamDir, personalDir, projectDir string) (*Registry, error) { + registry := NewRegistry() + + // Load in precedence order (lowest first, so higher precedence overwrites) + sources := []struct { + dir string + source Source + }{ + {teamDir, SourceTeam}, + {personalDir, SourcePersonal}, + {projectDir, SourceProject}, + } + + for _, s := range sources { + if s.dir == "" { + continue + } + skills, err := LoadFromDirectory(s.dir, s.source) + if err != nil { + return nil, fmt.Errorf("failed to load %s skills: %w", s.source.Label(), err) + } + registry.AddAll(skills) + } + + return registry, nil +} + +// LoadRegistryWithMultipleDirs creates a registry by loading skills from multiple team directories. +// This supports multi-source configurations where different skills come from different repos. +func LoadRegistryWithMultipleDirs(teamDirs []string, personalDir, projectDir string) (*Registry, error) { + registry := NewRegistry() + + // Load team skills from all team directories + for _, teamDir := range teamDirs { + if teamDir == "" { + continue + } + skills, err := LoadFromDirectory(teamDir, SourceTeam) + if err != nil { + // Log warning but continue - some dirs may not have skills + fmt.Fprintf(os.Stderr, "Warning: failed to load team skills from %s: %v\n", teamDir, err) + continue + } + registry.AddAll(skills) + } + + // Load personal skills + if personalDir != "" { + skills, err := LoadFromDirectory(personalDir, SourcePersonal) + if err != nil { + return nil, fmt.Errorf("failed to load personal skills: %w", err) + } + registry.AddAll(skills) + } + + // Load project skills + if projectDir != "" { + skills, err := LoadFromDirectory(projectDir, SourceProject) + if err != nil { + return nil, fmt.Errorf("failed to load project skills: %w", err) + } + registry.AddAll(skills) + } + + return registry, nil +} diff --git a/internal/skills/skill.go b/internal/skills/skill.go new file mode 100644 index 0000000..f4445d5 --- /dev/null +++ b/internal/skills/skill.go @@ -0,0 +1,385 @@ +// Package skills handles staghorn skill parsing, registry, and syncing. +// Skills are directories containing SKILL.md plus optional supporting files. +// They follow the Agent Skills standard (agentskills.io) with Claude Code extensions. +package skills + +import ( + "bufio" + "fmt" + "os" + "path/filepath" + "strings" + + "gopkg.in/yaml.v3" +) + +// Source indicates where a skill came from. +type Source string + +const ( + SourceTeam Source = "team" + SourcePersonal Source = "personal" + SourceProject Source = "project" + SourceStarter Source = "starter" +) + +// Label returns a human-readable label for the source. +func (s Source) Label() string { + switch s { + case SourceTeam: + return "team" + case SourcePersonal: + return "personal" + case SourceProject: + return "project" + case SourceStarter: + return "starter" + default: + return string(s) + } +} + +// Arg defines a skill argument (same as commands.Arg). +type Arg struct { + Name string `yaml:"name"` + Description string `yaml:"description"` + Default string `yaml:"default"` + Options []string `yaml:"options,omitempty"` + Required bool `yaml:"required"` +} + +// Hooks defines pre/post execution hooks. +type Hooks struct { + Pre string `yaml:"pre,omitempty"` + Post string `yaml:"post,omitempty"` +} + +// Metadata is an arbitrary key-value map for additional skill metadata. +type Metadata map[string]string + +// Frontmatter contains the YAML frontmatter of a skill. +// Fields follow the Agent Skills standard (agentskills.io) with Claude Code extensions. +type Frontmatter struct { + // Agent Skills Standard fields (cross-tool compatible) + Name string `yaml:"name"` + Description string `yaml:"description"` + License string `yaml:"license,omitempty"` + Compatibility string `yaml:"compatibility,omitempty"` + Metadata Metadata `yaml:"metadata,omitempty"` + AllowedTools string `yaml:"allowed-tools,omitempty"` // Space-delimited per standard + + // Staghorn extensions (for backwards compatibility with commands) + Tags []string `yaml:"tags,omitempty"` + Args []Arg `yaml:"args,omitempty"` + + // Claude Code extensions (ignored by other tools) + DisableModelInvocation bool `yaml:"disable-model-invocation,omitempty"` + UserInvocable *bool `yaml:"user-invocable,omitempty"` // pointer to distinguish unset from false + Context string `yaml:"context,omitempty"` // "normal" or "fork" + Agent string `yaml:"agent,omitempty"` // Subagent type for context: fork + ArgumentHint string `yaml:"argument-hint,omitempty"` + Model string `yaml:"model,omitempty"` + Hooks *Hooks `yaml:"hooks,omitempty"` +} + +// Skill represents a staghorn skill. +type Skill struct { + Frontmatter + Body string // Markdown content after frontmatter + Source Source // Where this skill came from + DirPath string // Path to the skill directory + SupportingFiles map[string]string // Relative path -> absolute path +} + +// ParseDir parses a skill from its directory. +// The directory must contain a SKILL.md file. +func ParseDir(dirPath string, source Source) (*Skill, error) { + skillMDPath := filepath.Join(dirPath, "SKILL.md") + + content, err := os.ReadFile(skillMDPath) + if err != nil { + return nil, fmt.Errorf("failed to read SKILL.md: %w", err) + } + + skill, err := Parse(string(content), source, dirPath) + if err != nil { + return nil, err + } + + // Discover supporting files + supportingFiles, err := discoverSupportingFiles(dirPath) + if err != nil { + return nil, fmt.Errorf("failed to discover supporting files: %w", err) + } + skill.SupportingFiles = supportingFiles + + return skill, nil +} + +// Parse parses a skill from SKILL.md content. +func Parse(content string, source Source, dirPath string) (*Skill, error) { + lines := strings.Split(content, "\n") + + // Check for frontmatter delimiter + if len(lines) == 0 || strings.TrimSpace(lines[0]) != "---" { + return nil, fmt.Errorf("SKILL.md must start with YAML frontmatter (---)") + } + + // Find end of frontmatter + endIdx := -1 + for i := 1; i < len(lines); i++ { + if strings.TrimSpace(lines[i]) == "---" { + endIdx = i + break + } + } + + if endIdx == -1 { + return nil, fmt.Errorf("unterminated frontmatter (missing closing ---)") + } + + // Parse frontmatter + frontmatterYAML := strings.Join(lines[1:endIdx], "\n") + var fm Frontmatter + if err := yaml.Unmarshal([]byte(frontmatterYAML), &fm); err != nil { + return nil, fmt.Errorf("invalid frontmatter YAML: %w", err) + } + + if fm.Name == "" { + return nil, fmt.Errorf("skill must have a 'name' field in frontmatter") + } + + if fm.Description == "" { + return nil, fmt.Errorf("skill must have a 'description' field in frontmatter") + } + + // Validate name format per Agent Skills standard + if err := validateSkillName(fm.Name); err != nil { + return nil, err + } + + // Extract body (everything after frontmatter) + body := "" + if endIdx+1 < len(lines) { + body = strings.TrimSpace(strings.Join(lines[endIdx+1:], "\n")) + } + + return &Skill{ + Frontmatter: fm, + Body: body, + Source: source, + DirPath: dirPath, + }, nil +} + +// validateSkillName validates that the name follows the Agent Skills standard: +// - Max 64 characters +// - Lowercase letters, numbers, and hyphens only +// - Must not start or end with hyphen +// - No consecutive hyphens +func validateSkillName(name string) error { + if len(name) == 0 { + return fmt.Errorf("skill name cannot be empty") + } + if len(name) > 64 { + return fmt.Errorf("skill name exceeds 64 characters") + } + if name[0] == '-' || name[len(name)-1] == '-' { + return fmt.Errorf("skill name cannot start or end with hyphen") + } + if strings.Contains(name, "--") { + return fmt.Errorf("skill name cannot contain consecutive hyphens") + } + for _, r := range name { + if !((r >= 'a' && r <= 'z') || (r >= '0' && r <= '9') || r == '-') { + return fmt.Errorf("skill name contains invalid character '%c' (allowed: lowercase a-z, 0-9, -)", r) + } + } + return nil +} + +// discoverSupportingFiles walks the skill directory and returns all non-SKILL.md files. +func discoverSupportingFiles(dirPath string) (map[string]string, error) { + files := make(map[string]string) + + err := filepath.Walk(dirPath, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + + // Skip directories and SKILL.md + if info.IsDir() { + return nil + } + if info.Name() == "SKILL.md" { + return nil + } + + // Get relative path from skill directory + relPath, err := filepath.Rel(dirPath, path) + if err != nil { + return err + } + + files[relPath] = path + return nil + }) + + if err != nil { + return nil, err + } + + return files, nil +} + +// LoadFromDirectory loads all skills from a parent directory. +// Each subdirectory that contains a SKILL.md is treated as a skill. +func LoadFromDirectory(dir string, source Source) ([]*Skill, error) { + var skills []*Skill + + entries, err := os.ReadDir(dir) + if err != nil { + if os.IsNotExist(err) { + return nil, nil // Directory doesn't exist, return empty + } + return nil, fmt.Errorf("failed to read skills directory: %w", err) + } + + for _, entry := range entries { + if !entry.IsDir() { + continue + } + + skillDir := filepath.Join(dir, entry.Name()) + skillMD := filepath.Join(skillDir, "SKILL.md") + + // Check if SKILL.md exists + if _, err := os.Stat(skillMD); os.IsNotExist(err) { + continue // Not a skill directory + } + + skill, err := ParseDir(skillDir, source) + if err != nil { + // Log warning but continue loading other skills + fmt.Fprintf(os.Stderr, "Warning: failed to parse skill %s: %v\n", entry.Name(), err) + continue + } + + skills = append(skills, skill) + } + + return skills, nil +} + +// HasArg checks if the skill has a specific argument. +func (s *Skill) HasArg(name string) bool { + for _, arg := range s.Args { + if arg.Name == name { + return true + } + } + return false +} + +// GetArg returns an argument by name. +func (s *Skill) GetArg(name string) *Arg { + for i := range s.Args { + if s.Args[i].Name == name { + return &s.Args[i] + } + } + return nil +} + +// AllowedToolsList returns the allowed tools as a slice. +// The standard uses space-delimited format. +func (s *Skill) AllowedToolsList() []string { + if s.AllowedTools == "" { + return nil + } + return strings.Fields(s.AllowedTools) +} + +// IsUserInvocable returns true if the skill can be invoked by users. +// Defaults to true if not explicitly set. +func (s *Skill) IsUserInvocable() bool { + if s.UserInvocable == nil { + return true + } + return *s.UserInvocable +} + +// ReadFrontmatterOnly reads just the frontmatter without loading supporting files. +// Useful for listing skills without loading all content into memory. +func ReadFrontmatterOnly(dirPath string) (*Frontmatter, error) { + skillMDPath := filepath.Join(dirPath, "SKILL.md") + + file, err := os.Open(skillMDPath) + if err != nil { + return nil, err + } + defer file.Close() + + scanner := bufio.NewScanner(file) + + // First line must be --- + if !scanner.Scan() || strings.TrimSpace(scanner.Text()) != "---" { + return nil, fmt.Errorf("SKILL.md must start with YAML frontmatter (---)") + } + + // Read until closing --- + var yamlLines []string + for scanner.Scan() { + line := scanner.Text() + if strings.TrimSpace(line) == "---" { + break + } + yamlLines = append(yamlLines, line) + } + + if err := scanner.Err(); err != nil { + return nil, err + } + + var fm Frontmatter + if err := yaml.Unmarshal([]byte(strings.Join(yamlLines, "\n")), &fm); err != nil { + return nil, fmt.Errorf("invalid frontmatter YAML: %w", err) + } + + return &fm, nil +} + +// NewSkillTemplate returns a template for creating a new skill. +func NewSkillTemplate(name, description string) string { + return fmt.Sprintf(`--- +name: %s +description: %s +allowed-tools: Read Grep Glob +--- + +# %s + +%s + +## Instructions + +1. First step +2. Second step +3. Third step + +## Output Format + +Describe the expected output format here. +`, name, description, toTitleCase(name), description) +} + +// toTitleCase converts kebab-case to Title Case. +func toTitleCase(s string) string { + words := strings.Split(s, "-") + for i, word := range words { + if len(word) > 0 { + words[i] = strings.ToUpper(word[:1]) + word[1:] + } + } + return strings.Join(words, " ") +} diff --git a/internal/skills/skill_test.go b/internal/skills/skill_test.go new file mode 100644 index 0000000..55c29c6 --- /dev/null +++ b/internal/skills/skill_test.go @@ -0,0 +1,701 @@ +package skills + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestParse(t *testing.T) { + tests := []struct { + name string + content string + wantName string + wantDesc string + wantTags []string + wantTools string + wantBody string + wantErr bool + errContains string + }{ + { + name: "valid skill with all fields", + content: `--- +name: code-review +description: Thorough code review +tags: [review, quality] +allowed-tools: Read Grep Glob +context: normal +--- + +# Code Review + +Review the code carefully.`, + wantName: "code-review", + wantDesc: "Thorough code review", + wantTags: []string{"review", "quality"}, + wantTools: "Read Grep Glob", + wantBody: "# Code Review\n\nReview the code carefully.", + }, + { + name: "minimal skill", + content: `--- +name: simple +description: A simple skill +--- + +Do something.`, + wantName: "simple", + wantDesc: "A simple skill", + wantBody: "Do something.", + }, + { + name: "skill with claude code extensions", + content: `--- +name: security-audit +description: Scan for vulnerabilities +disable-model-invocation: true +user-invocable: false +context: fork +agent: Explore +--- + +Audit the code.`, + wantName: "security-audit", + wantDesc: "Scan for vulnerabilities", + wantBody: "Audit the code.", + }, + { + name: "missing frontmatter start", + content: "name: test\n---\nBody", + wantErr: true, + errContains: "must start with YAML frontmatter", + }, + { + name: "unterminated frontmatter", + content: "---\nname: test\nBody without closing", + wantErr: true, + errContains: "unterminated frontmatter", + }, + { + name: "missing name field", + content: `--- +description: No name provided +--- + +Body here.`, + wantErr: true, + errContains: "must have a 'name' field", + }, + { + name: "missing description field", + content: `--- +name: no-desc +--- + +Body here.`, + wantErr: true, + errContains: "must have a 'description' field", + }, + { + name: "invalid name - uppercase", + content: `--- +name: CodeReview +description: Has uppercase +--- + +Body.`, + wantErr: true, + errContains: "invalid character", + }, + { + name: "invalid name - starts with hyphen", + content: `--- +name: -review +description: Starts with hyphen +--- + +Body.`, + wantErr: true, + errContains: "cannot start or end with hyphen", + }, + { + name: "invalid name - consecutive hyphens", + content: `--- +name: code--review +description: Has consecutive hyphens +--- + +Body.`, + wantErr: true, + errContains: "cannot contain consecutive hyphens", + }, + { + name: "skill with hooks", + content: `--- +name: deploy +description: Deploy the application +hooks: + pre: ./scripts/validate.sh + post: ./scripts/notify.sh +--- + +Deploy steps here.`, + wantName: "deploy", + wantDesc: "Deploy the application", + wantBody: "Deploy steps here.", + }, + { + name: "skill with args", + content: `--- +name: test-gen +description: Generate tests +args: + - name: path + description: Target path + default: "." + - name: framework + options: [jest, vitest, pytest] + default: jest +--- + +Generate tests at {{path}}.`, + wantName: "test-gen", + wantDesc: "Generate tests", + wantBody: "Generate tests at {{path}}.", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + skill, err := Parse(tt.content, SourceTeam, "") + + if tt.wantErr { + if err == nil { + t.Error("expected error, got nil") + } else if tt.errContains != "" && !strings.Contains(err.Error(), tt.errContains) { + t.Errorf("error %q doesn't contain %q", err.Error(), tt.errContains) + } + return + } + + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if skill.Name != tt.wantName { + t.Errorf("Name = %q, want %q", skill.Name, tt.wantName) + } + + if tt.wantDesc != "" && skill.Description != tt.wantDesc { + t.Errorf("Description = %q, want %q", skill.Description, tt.wantDesc) + } + + if tt.wantTags != nil && len(skill.Tags) != len(tt.wantTags) { + t.Errorf("Tags = %v, want %v", skill.Tags, tt.wantTags) + } + + if tt.wantTools != "" && skill.AllowedTools != tt.wantTools { + t.Errorf("AllowedTools = %q, want %q", skill.AllowedTools, tt.wantTools) + } + + if tt.wantBody != "" && skill.Body != tt.wantBody { + t.Errorf("Body = %q, want %q", skill.Body, tt.wantBody) + } + }) + } +} + +func TestValidateSkillName(t *testing.T) { + tests := []struct { + name string + input string + wantErr bool + }{ + {"valid simple", "code-review", false}, + {"valid with numbers", "test-123", false}, + {"valid short", "a", false}, + {"valid long", "this-is-a-valid-skill-name-with-many-parts", false}, + {"empty", "", true}, + {"too long", "this-skill-name-is-way-too-long-and-exceeds-the-sixty-four-character-limit-set-by-standard", true}, + {"uppercase", "CodeReview", true}, + {"spaces", "code review", true}, + {"underscores", "code_review", true}, + {"starts with hyphen", "-review", true}, + {"ends with hyphen", "review-", true}, + {"consecutive hyphens", "code--review", true}, + {"special chars", "code@review", true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateSkillName(tt.input) + if (err != nil) != tt.wantErr { + t.Errorf("validateSkillName(%q) error = %v, wantErr %v", tt.input, err, tt.wantErr) + } + }) + } +} + +func TestAllowedToolsList(t *testing.T) { + tests := []struct { + name string + tools string + want []string + }{ + {"empty", "", nil}, + {"single tool", "Read", []string{"Read"}}, + {"multiple tools", "Read Grep Glob", []string{"Read", "Grep", "Glob"}}, + {"extra spaces", "Read Grep Glob", []string{"Read", "Grep", "Glob"}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + skill := &Skill{Frontmatter: Frontmatter{AllowedTools: tt.tools}} + got := skill.AllowedToolsList() + + if len(got) != len(tt.want) { + t.Errorf("AllowedToolsList() = %v, want %v", got, tt.want) + return + } + + for i, v := range got { + if v != tt.want[i] { + t.Errorf("AllowedToolsList()[%d] = %q, want %q", i, v, tt.want[i]) + } + } + }) + } +} + +func TestIsUserInvocable(t *testing.T) { + tests := []struct { + name string + value *bool + want bool + }{ + {"nil defaults to true", nil, true}, + {"explicit true", boolPtr(true), true}, + {"explicit false", boolPtr(false), false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + skill := &Skill{Frontmatter: Frontmatter{UserInvocable: tt.value}} + if got := skill.IsUserInvocable(); got != tt.want { + t.Errorf("IsUserInvocable() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestRegistry(t *testing.T) { + registry := NewRegistry() + + // Add team skill + teamSkill := &Skill{ + Frontmatter: Frontmatter{Name: "audit", Description: "Team audit"}, + Source: SourceTeam, + } + registry.Add(teamSkill) + + // Add personal override + personalSkill := &Skill{ + Frontmatter: Frontmatter{Name: "audit", Description: "Personal audit"}, + Source: SourcePersonal, + } + registry.Add(personalSkill) + + // Add project skill + projectSkill := &Skill{ + Frontmatter: Frontmatter{Name: "project-only", Description: "Project specific"}, + Source: SourceProject, + } + registry.Add(projectSkill) + + // Test Get returns highest precedence + got := registry.Get("audit") + if got.Description != "Personal audit" { + t.Errorf("Get(audit) returned %q, want personal override", got.Description) + } + + // Test Count + if registry.Count() != 2 { + t.Errorf("Count() = %d, want 2", registry.Count()) + } + + // Test GetAllVersions + versions := registry.GetAllVersions("audit") + if len(versions) != 2 { + t.Errorf("GetAllVersions(audit) = %d, want 2", len(versions)) + } + + // Test BySource + teamSkills := registry.BySource(SourceTeam) + if len(teamSkills) != 1 { + t.Errorf("BySource(team) = %d, want 1", len(teamSkills)) + } +} + +func TestRegistryPrecedence(t *testing.T) { + // Test that project > personal > team > starter + tests := []struct { + name string + sources []Source + wantSource Source + }{ + {"team only", []Source{SourceTeam}, SourceTeam}, + {"personal overrides team", []Source{SourceTeam, SourcePersonal}, SourcePersonal}, + {"project overrides personal", []Source{SourcePersonal, SourceProject}, SourceProject}, + {"project overrides all", []Source{SourceStarter, SourceTeam, SourcePersonal, SourceProject}, SourceProject}, + {"starter lowest", []Source{SourceStarter, SourceTeam}, SourceTeam}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + registry := NewRegistry() + for _, source := range tt.sources { + registry.Add(&Skill{ + Frontmatter: Frontmatter{Name: "test", Description: string(source)}, + Source: source, + }) + } + + got := registry.Get("test") + if got.Source != tt.wantSource { + t.Errorf("Get() source = %v, want %v", got.Source, tt.wantSource) + } + }) + } +} + +func TestByTag(t *testing.T) { + registry := NewRegistry() + registry.Add(&Skill{ + Frontmatter: Frontmatter{Name: "a", Description: "A", Tags: []string{"review", "quality"}}, + Source: SourceTeam, + }) + registry.Add(&Skill{ + Frontmatter: Frontmatter{Name: "b", Description: "B", Tags: []string{"security"}}, + Source: SourceTeam, + }) + registry.Add(&Skill{ + Frontmatter: Frontmatter{Name: "c", Description: "C", Tags: []string{"review"}}, + Source: SourceTeam, + }) + + got := registry.ByTag("review") + if len(got) != 2 { + t.Errorf("ByTag(review) = %d skills, want 2", len(got)) + } + + got = registry.ByTag("security") + if len(got) != 1 { + t.Errorf("ByTag(security) = %d skills, want 1", len(got)) + } + + got = registry.ByTag("nonexistent") + if len(got) != 0 { + t.Errorf("ByTag(nonexistent) = %d skills, want 0", len(got)) + } +} + +func TestParseDirWithSupportingFiles(t *testing.T) { + // Create temp directory structure + tempDir := t.TempDir() + skillDir := filepath.Join(tempDir, "my-skill") + if err := os.MkdirAll(filepath.Join(skillDir, "templates"), 0755); err != nil { + t.Fatal(err) + } + + // Create SKILL.md + skillMD := `--- +name: my-skill +description: Test skill with supporting files +--- + +Instructions here.` + if err := os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte(skillMD), 0644); err != nil { + t.Fatal(err) + } + + // Create supporting files + if err := os.WriteFile(filepath.Join(skillDir, "templates", "review.md"), []byte("template"), 0644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(skillDir, "script.sh"), []byte("echo hello"), 0644); err != nil { + t.Fatal(err) + } + + skill, err := ParseDir(skillDir, SourceTeam) + if err != nil { + t.Fatalf("ParseDir() error = %v", err) + } + + if skill.Name != "my-skill" { + t.Errorf("Name = %q, want %q", skill.Name, "my-skill") + } + + if len(skill.SupportingFiles) != 2 { + t.Errorf("SupportingFiles count = %d, want 2", len(skill.SupportingFiles)) + } + + // Check that relative paths are correct + if _, ok := skill.SupportingFiles["templates/review.md"]; !ok { + t.Error("expected templates/review.md in SupportingFiles") + } + if _, ok := skill.SupportingFiles["script.sh"]; !ok { + t.Error("expected script.sh in SupportingFiles") + } +} + +func TestLoadFromDirectory(t *testing.T) { + // Create temp directory with test skills + tempDir := t.TempDir() + + // Create valid skill + validSkillDir := filepath.Join(tempDir, "valid-skill") + if err := os.MkdirAll(validSkillDir, 0755); err != nil { + t.Fatal(err) + } + validSkill := `--- +name: valid-skill +description: A valid skill +--- + +Do something.` + if err := os.WriteFile(filepath.Join(validSkillDir, "SKILL.md"), []byte(validSkill), 0644); err != nil { + t.Fatal(err) + } + + // Create directory without SKILL.md (should be ignored) + notASkill := filepath.Join(tempDir, "not-a-skill") + if err := os.MkdirAll(notASkill, 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(notASkill, "README.md"), []byte("not a skill"), 0644); err != nil { + t.Fatal(err) + } + + // Create regular file (should be ignored) + if err := os.WriteFile(filepath.Join(tempDir, "readme.txt"), []byte("ignore me"), 0644); err != nil { + t.Fatal(err) + } + + skills, err := LoadFromDirectory(tempDir, SourceTeam) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(skills) != 1 { + t.Errorf("LoadFromDirectory() returned %d skills, want 1", len(skills)) + } + + if skills[0].Name != "valid-skill" { + t.Errorf("skill name = %q, want %q", skills[0].Name, "valid-skill") + } +} + +func TestLoadFromNonexistentDirectory(t *testing.T) { + skills, err := LoadFromDirectory("/nonexistent/path", SourceTeam) + if err != nil { + t.Errorf("expected nil error for nonexistent directory, got %v", err) + } + if skills != nil { + t.Errorf("expected nil skills for nonexistent directory, got %v", skills) + } +} + +func TestLoadRegistryWithMultipleDirs(t *testing.T) { + // Create temp directories for multiple team sources + tempDir := t.TempDir() + + // Team dir 1 + teamDir1 := filepath.Join(tempDir, "team1") + skillDir1 := filepath.Join(teamDir1, "skill-from-team1") + if err := os.MkdirAll(skillDir1, 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(skillDir1, "SKILL.md"), []byte(`--- +name: skill-from-team1 +description: From team 1 +--- + +Body.`), 0644); err != nil { + t.Fatal(err) + } + + // Team dir 2 + teamDir2 := filepath.Join(tempDir, "team2") + skillDir2 := filepath.Join(teamDir2, "skill-from-team2") + if err := os.MkdirAll(skillDir2, 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(skillDir2, "SKILL.md"), []byte(`--- +name: skill-from-team2 +description: From team 2 +--- + +Body.`), 0644); err != nil { + t.Fatal(err) + } + + // Personal dir with override + personalDir := filepath.Join(tempDir, "personal") + personalSkillDir := filepath.Join(personalDir, "skill-from-team1") + if err := os.MkdirAll(personalSkillDir, 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(personalSkillDir, "SKILL.md"), []byte(`--- +name: skill-from-team1 +description: Personal override +--- + +Body.`), 0644); err != nil { + t.Fatal(err) + } + + // Load with multiple team dirs + registry, err := LoadRegistryWithMultipleDirs( + []string{teamDir1, teamDir2}, + personalDir, + "", // no project dir + ) + if err != nil { + t.Fatalf("LoadRegistryWithMultipleDirs() error = %v", err) + } + + // Should have 2 unique skills + if registry.Count() != 2 { + t.Errorf("Count() = %d, want 2", registry.Count()) + } + + // skill-from-team1 should be the personal override + skill1 := registry.Get("skill-from-team1") + if skill1 == nil { + t.Fatal("expected skill-from-team1 to exist") + } + if skill1.Description != "Personal override" { + t.Errorf("skill-from-team1 description = %q, want %q", skill1.Description, "Personal override") + } + if skill1.Source != SourcePersonal { + t.Errorf("skill-from-team1 source = %v, want %v", skill1.Source, SourcePersonal) + } + + // skill-from-team2 should be from team + skill2 := registry.Get("skill-from-team2") + if skill2 == nil { + t.Fatal("expected skill-from-team2 to exist") + } + if skill2.Source != SourceTeam { + t.Errorf("skill-from-team2 source = %v, want %v", skill2.Source, SourceTeam) + } +} + +func TestLoadRegistryWithMultipleDirsNonexistent(t *testing.T) { + // Should handle nonexistent team dirs gracefully (logs warning, continues) + registry, err := LoadRegistryWithMultipleDirs( + []string{"/nonexistent/team1", "/nonexistent/team2"}, + "", + "", + ) + if err != nil { + t.Fatalf("LoadRegistryWithMultipleDirs() error = %v", err) + } + if registry.Count() != 0 { + t.Errorf("Count() = %d, want 0", registry.Count()) + } +} + +func TestHasArg(t *testing.T) { + skill := &Skill{ + Frontmatter: Frontmatter{ + Name: "test", + Description: "Test", + Args: []Arg{ + {Name: "path", Default: "."}, + {Name: "format", Options: []string{"json", "yaml"}}, + }, + }, + } + + if !skill.HasArg("path") { + t.Error("expected HasArg(path) to be true") + } + if !skill.HasArg("format") { + t.Error("expected HasArg(format) to be true") + } + if skill.HasArg("nonexistent") { + t.Error("expected HasArg(nonexistent) to be false") + } +} + +func TestGetArg(t *testing.T) { + skill := &Skill{ + Frontmatter: Frontmatter{ + Name: "test", + Description: "Test", + Args: []Arg{ + {Name: "path", Default: "."}, + }, + }, + } + + arg := skill.GetArg("path") + if arg == nil { + t.Fatal("expected GetArg(path) to return non-nil") + } + if arg.Default != "." { + t.Errorf("arg.Default = %q, want %q", arg.Default, ".") + } + + if skill.GetArg("nonexistent") != nil { + t.Error("expected GetArg(nonexistent) to return nil") + } +} + +func TestSourceLabel(t *testing.T) { + tests := []struct { + source Source + want string + }{ + {SourceTeam, "team"}, + {SourcePersonal, "personal"}, + {SourceProject, "project"}, + {SourceStarter, "starter"}, + {Source("unknown"), "unknown"}, + } + + for _, tt := range tests { + t.Run(tt.want, func(t *testing.T) { + if got := tt.source.Label(); got != tt.want { + t.Errorf("Label() = %q, want %q", got, tt.want) + } + }) + } +} + +func TestNewSkillTemplate(t *testing.T) { + template := NewSkillTemplate("code-review", "Review code for issues") + + // Check it contains expected content + if !strings.Contains(template, "name: code-review") { + t.Error("template should contain name field") + } + if !strings.Contains(template, "description: Review code for issues") { + t.Error("template should contain description field") + } + if !strings.Contains(template, "# Code Review") { + t.Error("template should contain title") + } + if !strings.Contains(template, "allowed-tools:") { + t.Error("template should contain allowed-tools field") + } +} + +// Helper functions + +func boolPtr(b bool) *bool { + return &b +} diff --git a/internal/starter/skills.go b/internal/starter/skills.go new file mode 100644 index 0000000..8d3c161 --- /dev/null +++ b/internal/starter/skills.go @@ -0,0 +1,258 @@ +// Package starter provides embedded starter skills that ship with staghorn. +package starter + +import ( + "embed" + "fmt" + "io/fs" + "os" + "path/filepath" + "strings" + + "github.com/HartBrook/staghorn/internal/skills" +) + +//go:embed skills/*/SKILL.md +var skillsFS embed.FS + +// SkillNames returns the list of available starter skill names. +func SkillNames() []string { + entries, err := skillsFS.ReadDir("skills") + if err != nil { + return nil + } + + var names []string + for _, entry := range entries { + if entry.IsDir() { + // Check if it has a SKILL.md + skillMD := filepath.Join("skills", entry.Name(), "SKILL.md") + if _, err := skillsFS.ReadFile(skillMD); err == nil { + names = append(names, entry.Name()) + } + } + } + return names +} + +// BootstrapSkills copies starter skills to the target directory. +// It skips skills that already exist. Returns the number of skills copied. +func BootstrapSkills(targetDir string) (int, error) { + count, _, err := BootstrapSkillsWithSkip(targetDir, nil) + return count, err +} + +// BootstrapSkillsWithSkip copies starter skills to the target directory, +// skipping skills in the skip list. Returns the count and names of installed skills. +func BootstrapSkillsWithSkip(targetDir string, skip []string) (int, []string, error) { + if err := os.MkdirAll(targetDir, 0755); err != nil { + return 0, nil, fmt.Errorf("failed to create skills directory: %w", err) + } + + // Build skip set + skipSet := make(map[string]bool) + for _, name := range skip { + skipSet[name] = true + } + + entries, err := skillsFS.ReadDir("skills") + if err != nil { + return 0, nil, fmt.Errorf("failed to read embedded skills: %w", err) + } + + copied := 0 + var installed []string + for _, entry := range entries { + if !entry.IsDir() { + continue + } + + name := entry.Name() + + // Skip if in skip list + if skipSet[name] { + continue + } + + // Check if skill already exists + targetPath := filepath.Join(targetDir, name) + if _, err := os.Stat(targetPath); err == nil { + continue + } + + // Create skill directory + if err := os.MkdirAll(targetPath, 0755); err != nil { + return copied, installed, fmt.Errorf("failed to create skill directory %s: %w", name, err) + } + + // Copy SKILL.md + skillMDPath := filepath.Join("skills", name, "SKILL.md") + content, err := skillsFS.ReadFile(skillMDPath) + if err != nil { + return copied, installed, fmt.Errorf("failed to read %s/SKILL.md: %w", name, err) + } + + if err := os.WriteFile(filepath.Join(targetPath, "SKILL.md"), content, 0644); err != nil { + return copied, installed, fmt.Errorf("failed to write %s/SKILL.md: %w", name, err) + } + + // Copy any supporting files (if they exist) + supportingFiles, _ := listSupportingFiles(name) + for _, relPath := range supportingFiles { + srcPath := filepath.Join("skills", name, relPath) + destPath := filepath.Join(targetPath, relPath) + + // Create parent directory if needed + if dir := filepath.Dir(destPath); dir != targetPath { + if err := os.MkdirAll(dir, 0755); err != nil { + return copied, installed, fmt.Errorf("failed to create directory for %s: %w", relPath, err) + } + } + + fileContent, err := skillsFS.ReadFile(srcPath) + if err != nil { + return copied, installed, fmt.Errorf("failed to read %s: %w", srcPath, err) + } + + if err := os.WriteFile(destPath, fileContent, 0644); err != nil { + return copied, installed, fmt.Errorf("failed to write %s: %w", destPath, err) + } + } + + copied++ + installed = append(installed, name) + } + + return copied, installed, nil +} + +// BootstrapSkillsSelective copies only the specified starter skills to the target directory. +// It skips skills that already exist. Returns the count and names of installed skills. +func BootstrapSkillsSelective(targetDir string, names []string) (int, []string, error) { + if err := os.MkdirAll(targetDir, 0755); err != nil { + return 0, nil, fmt.Errorf("failed to create skills directory: %w", err) + } + + // Build set of requested names + requested := make(map[string]bool) + for _, name := range names { + requested[name] = true + } + + entries, err := skillsFS.ReadDir("skills") + if err != nil { + return 0, nil, fmt.Errorf("failed to read embedded skills: %w", err) + } + + copied := 0 + var installed []string + for _, entry := range entries { + if !entry.IsDir() { + continue + } + + name := entry.Name() + + // Skip if not in requested list + if !requested[name] { + continue + } + + // Check if skill already exists + targetPath := filepath.Join(targetDir, name) + if _, err := os.Stat(targetPath); err == nil { + continue + } + + // Create skill directory + if err := os.MkdirAll(targetPath, 0755); err != nil { + return copied, installed, fmt.Errorf("failed to create skill directory %s: %w", name, err) + } + + // Copy SKILL.md + skillMDPath := filepath.Join("skills", name, "SKILL.md") + content, err := skillsFS.ReadFile(skillMDPath) + if err != nil { + return copied, installed, fmt.Errorf("failed to read %s/SKILL.md: %w", name, err) + } + + if err := os.WriteFile(filepath.Join(targetPath, "SKILL.md"), content, 0644); err != nil { + return copied, installed, fmt.Errorf("failed to write %s/SKILL.md: %w", name, err) + } + + copied++ + installed = append(installed, name) + } + + return copied, installed, nil +} + +// GetSkill returns the SKILL.md content for a starter skill by name. +func GetSkill(name string) ([]byte, error) { + return skillsFS.ReadFile(filepath.Join("skills", name, "SKILL.md")) +} + +// ListSkills returns all embedded skill directories. +func ListSkills() ([]fs.DirEntry, error) { + return skillsFS.ReadDir("skills") +} + +// LoadStarterSkills loads and parses all embedded starter skills. +func LoadStarterSkills() ([]*skills.Skill, error) { + entries, err := skillsFS.ReadDir("skills") + if err != nil { + return nil, fmt.Errorf("failed to read embedded skills: %w", err) + } + + var result []*skills.Skill + for _, entry := range entries { + if !entry.IsDir() { + continue + } + + skillMDPath := filepath.Join("skills", entry.Name(), "SKILL.md") + content, err := skillsFS.ReadFile(skillMDPath) + if err != nil { + continue // Skip if no SKILL.md + } + + skill, err := skills.Parse(string(content), skills.SourceStarter, "") + if err != nil { + return nil, fmt.Errorf("failed to parse %s: %w", entry.Name(), err) + } + + result = append(result, skill) + } + + return result, nil +} + +// listSupportingFiles returns all files in a skill directory except SKILL.md. +func listSupportingFiles(skillName string) ([]string, error) { + var files []string + + skillDir := filepath.Join("skills", skillName) + err := fs.WalkDir(skillsFS, skillDir, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + + // Skip the root and directories + if path == skillDir || d.IsDir() { + return nil + } + + // Skip SKILL.md + if d.Name() == "SKILL.md" { + return nil + } + + // Get relative path from skill directory + relPath := strings.TrimPrefix(path, skillDir+"/") + files = append(files, relPath) + + return nil + }) + + return files, err +} diff --git a/internal/starter/skills/code-review/SKILL.md b/internal/starter/skills/code-review/SKILL.md new file mode 100644 index 0000000..d48fe12 --- /dev/null +++ b/internal/starter/skills/code-review/SKILL.md @@ -0,0 +1,51 @@ +--- +name: code-review +description: Perform a thorough code review with structured feedback +tags: [review, quality] +allowed-tools: Read Grep Glob +args: + - name: path + description: File or directory to review + default: "." + - name: focus + description: What aspect to focus on + default: all + options: [all, logic, style, performance, security] +--- + +# Code Review + +Review the code at `{{path}}` with focus on: **{{focus}}** + +## Review Checklist + +### Logic & Correctness +- Does the code do what it's supposed to do? +- Are edge cases handled? +- Are there any off-by-one errors? +- Is error handling appropriate? + +### Code Quality +- Is the code readable and well-organized? +- Are functions and variables named clearly? +- Is there unnecessary duplication? +- Are comments helpful and accurate? + +### Performance +- Are there any obvious performance issues? +- Are expensive operations cached when appropriate? +- Are there N+1 query problems? +- Is memory usage reasonable? + +### Security +- Is user input validated? +- Are there potential injection vulnerabilities? +- Is sensitive data protected? + +## Output Format + +Provide feedback in these categories: +1. **Must Fix**: Issues that need to be addressed before merging +2. **Should Fix**: Improvements that would make the code better +3. **Consider**: Suggestions and minor improvements +4. **Praise**: Things done well (important for morale!) diff --git a/internal/starter/skills/security-audit/SKILL.md b/internal/starter/skills/security-audit/SKILL.md new file mode 100644 index 0000000..546af0c --- /dev/null +++ b/internal/starter/skills/security-audit/SKILL.md @@ -0,0 +1,59 @@ +--- +name: security-audit +description: Scan codebase for common security vulnerabilities +tags: [security, audit] +allowed-tools: Read Grep Glob +context: fork +agent: Explore +args: + - name: scope + description: What to audit + default: all + options: [all, injection, auth, secrets, dependencies] +--- + +# Security Audit + +Perform a security audit of the codebase, focusing on: **{{scope}}** + +## Audit Areas + +### Injection Vulnerabilities +- SQL injection (look for string concatenation in queries) +- Command injection (look for shell command construction) +- XSS vulnerabilities (look for unescaped user input in HTML) +- Path traversal (look for user input in file paths) + +### Authentication & Authorization +- Hardcoded credentials or API keys +- Weak password requirements +- Missing or improper session management +- Insecure token handling +- Missing authorization checks + +### Secrets & Configuration +- Secrets in source code or config files +- Sensitive data in logs +- Overly permissive file permissions +- Insecure default configurations + +### Dependencies +- Known vulnerable dependencies +- Outdated packages with security patches +- Unnecessary dependencies increasing attack surface + +## Output Format + +Report findings by severity: + +1. **Critical**: Immediate exploitation risk - fix before deploy +2. **High**: Significant security risk - fix soon +3. **Medium**: Security concern - address in next sprint +4. **Low**: Minor issue - fix when convenient +5. **Informational**: Best practice suggestion + +For each finding include: +- Location (file and line number) +- Description of the vulnerability +- Potential impact +- Recommended fix diff --git a/internal/starter/skills/test-gen/SKILL.md b/internal/starter/skills/test-gen/SKILL.md new file mode 100644 index 0000000..6ee1ecf --- /dev/null +++ b/internal/starter/skills/test-gen/SKILL.md @@ -0,0 +1,72 @@ +--- +name: test-gen +description: Generate unit tests for existing code +tags: [testing, quality] +allowed-tools: Read Grep Glob Write Edit +args: + - name: path + description: File or function to generate tests for + required: true + - name: framework + description: Testing framework to use + default: auto + options: [auto, jest, vitest, pytest, go, rust] + - name: coverage + description: What to cover + default: core + options: [core, edge-cases, full] +--- + +# Test Generation + +Generate unit tests for: `{{path}}` +Framework: **{{framework}}** | Coverage: **{{coverage}}** + +## Test Generation Guidelines + +### Test Structure +1. Read the source code to understand: + - Function inputs and outputs + - Side effects + - Error conditions + - Edge cases + +2. Generate tests that cover: + - Happy path (normal operation) + - Edge cases (empty inputs, boundaries) + - Error handling (invalid inputs, failures) + - Integration points (if applicable) + +### Framework-Specific Conventions + +#### JavaScript/TypeScript (Jest/Vitest) +- Use `describe` blocks to group related tests +- Use `it` or `test` with descriptive names +- Use `beforeEach`/`afterEach` for setup/cleanup +- Mock external dependencies + +#### Python (pytest) +- Use fixtures for test setup +- Use parametrize for multiple test cases +- Follow `test_` naming convention +- Use `@pytest.mark` for test categorization + +#### Go +- Use table-driven tests +- Follow `Test` prefix convention +- Use `t.Run` for subtests +- Use `testify` assertions if available + +#### Rust +- Use `#[test]` attribute +- Use `#[should_panic]` for error tests +- Use `#[ignore]` for slow tests +- Organize tests in a `tests` module + +## Output Format + +Generate tests that: +1. Are well-organized and readable +2. Have descriptive test names explaining what's tested +3. Include setup comments if complex +4. Follow the project's existing test patterns diff --git a/internal/starter/skills_test.go b/internal/starter/skills_test.go new file mode 100644 index 0000000..9cdd183 --- /dev/null +++ b/internal/starter/skills_test.go @@ -0,0 +1,198 @@ +package starter + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestSkillNames(t *testing.T) { + names := SkillNames() + if len(names) == 0 { + t.Error("expected at least one starter skill") + } + + // Check for expected starter skills + expected := []string{"code-review", "security-audit", "test-gen"} + for _, exp := range expected { + found := false + for _, name := range names { + if name == exp { + found = true + break + } + } + if !found { + t.Errorf("expected skill %q not found in starter skills", exp) + } + } +} + +func TestGetSkill(t *testing.T) { + content, err := GetSkill("code-review") + if err != nil { + t.Fatalf("GetSkill(code-review) error: %v", err) + } + + if len(content) == 0 { + t.Error("expected non-empty content") + } + + // Check it contains expected frontmatter + if !strings.Contains(string(content), "name: code-review") { + t.Error("expected skill to have name field") + } + if !strings.Contains(string(content), "allowed-tools:") { + t.Error("expected skill to have allowed-tools field") + } +} + +func TestGetSkillNotFound(t *testing.T) { + _, err := GetSkill("nonexistent") + if err == nil { + t.Error("expected error for nonexistent skill") + } +} + +func TestBootstrapSkills(t *testing.T) { + tempDir := t.TempDir() + + count, err := BootstrapSkills(tempDir) + if err != nil { + t.Fatalf("BootstrapSkills error: %v", err) + } + + if count == 0 { + t.Error("expected at least one skill to be installed") + } + + // Check that skill directories were created + entries, err := os.ReadDir(tempDir) + if err != nil { + t.Fatal(err) + } + + if len(entries) != count { + t.Errorf("expected %d directories, got %d", count, len(entries)) + } + + // Check that each directory has a SKILL.md + for _, entry := range entries { + if !entry.IsDir() { + continue + } + skillMD := filepath.Join(tempDir, entry.Name(), "SKILL.md") + if _, err := os.Stat(skillMD); os.IsNotExist(err) { + t.Errorf("expected SKILL.md in %s", entry.Name()) + } + } +} + +func TestBootstrapSkillsSkipExisting(t *testing.T) { + tempDir := t.TempDir() + + // Create a skill that already exists + existingSkill := filepath.Join(tempDir, "code-review") + if err := os.MkdirAll(existingSkill, 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(existingSkill, "SKILL.md"), []byte("existing"), 0644); err != nil { + t.Fatal(err) + } + + count, err := BootstrapSkills(tempDir) + if err != nil { + t.Fatalf("BootstrapSkills error: %v", err) + } + + // Should skip the existing one + totalSkills := len(SkillNames()) + expectedCount := totalSkills - 1 + if count != expectedCount { + t.Errorf("expected %d skills copied (skipping existing), got %d", expectedCount, count) + } + + // Existing skill should not be overwritten + content, _ := os.ReadFile(filepath.Join(existingSkill, "SKILL.md")) + if string(content) != "existing" { + t.Error("existing skill was overwritten") + } +} + +func TestBootstrapSkillsSelective(t *testing.T) { + tempDir := t.TempDir() + + count, installed, err := BootstrapSkillsSelective(tempDir, []string{"code-review"}) + if err != nil { + t.Fatalf("BootstrapSkillsSelective error: %v", err) + } + + if count != 1 { + t.Errorf("expected 1 skill, got %d", count) + } + + if len(installed) != 1 || installed[0] != "code-review" { + t.Errorf("expected [code-review], got %v", installed) + } + + // Check only code-review was installed + entries, _ := os.ReadDir(tempDir) + if len(entries) != 1 { + t.Errorf("expected 1 directory, got %d", len(entries)) + } +} + +func TestBootstrapSkillsWithSkip(t *testing.T) { + tempDir := t.TempDir() + + count, installed, err := BootstrapSkillsWithSkip(tempDir, []string{"code-review"}) + if err != nil { + t.Fatalf("BootstrapSkillsWithSkip error: %v", err) + } + + // Should install all except code-review + totalSkills := len(SkillNames()) + if count != totalSkills-1 { + t.Errorf("expected %d skills (skipping 1), got %d", totalSkills-1, count) + } + + // code-review should not be in installed list + for _, name := range installed { + if name == "code-review" { + t.Error("code-review should have been skipped") + } + } +} + +func TestLoadStarterSkills(t *testing.T) { + skills, err := LoadStarterSkills() + if err != nil { + t.Fatalf("LoadStarterSkills error: %v", err) + } + + if len(skills) == 0 { + t.Error("expected at least one starter skill") + } + + // Check skills are valid + for _, skill := range skills { + if skill.Name == "" { + t.Error("skill has empty name") + } + if skill.Description == "" { + t.Error("skill has empty description") + } + } +} + +func TestListSkills(t *testing.T) { + entries, err := ListSkills() + if err != nil { + t.Fatalf("ListSkills error: %v", err) + } + + if len(entries) == 0 { + t.Error("expected at least one skill entry") + } +} From 3e53d5e060f46704d16a892d23ecef0f96ec4dd5 Mon Sep 17 00:00:00 2001 From: Cody Hart Date: Mon, 26 Jan 2026 17:40:06 -0500 Subject: [PATCH 2/2] improvements --- internal/cli/sync.go | 205 ++++++++++++------------------------ internal/github/client.go | 16 +-- internal/skills/claude.go | 9 +- internal/skills/registry.go | 6 +- internal/skills/skill.go | 3 +- 5 files changed, 88 insertions(+), 151 deletions(-) diff --git a/internal/cli/sync.go b/internal/cli/sync.go index 5ae17d0..ae52138 100644 --- a/internal/cli/sync.go +++ b/internal/cli/sync.go @@ -343,85 +343,57 @@ func runSync(ctx context.Context, opts *syncOptions) error { return nil } -// syncCommands fetches commands from the team repo's commands/ directory. -func syncCommands(ctx context.Context, client *github.Client, owner, repo, branch string, paths *config.Paths) (int, error) { - // List commands directory - entries, err := client.ListDirectory(ctx, owner, repo, "commands", branch) +// syncDirectoryOpts configures the syncDirectoryContents helper. +type syncDirectoryOpts struct { + remoteDir string // Remote directory name (e.g., "commands") + localDir string // Local directory path to sync to + itemType string // Human-readable name for warnings (e.g., "command") + extensions []string // File extensions to sync (e.g., []string{".md"}) +} + +// syncDirectoryContents fetches files from a remote directory and saves them locally. +// This is a generic helper used by syncCommands, syncTemplates, syncLanguages, etc. +func syncDirectoryContents(ctx context.Context, client *github.Client, owner, repo, branch string, opts syncDirectoryOpts) (int, error) { + entries, err := client.ListDirectory(ctx, owner, repo, opts.remoteDir, branch) if err != nil { return 0, err } if entries == nil { - // No commands directory return 0, nil } - // Create local commands cache directory - commandsDir := paths.TeamCommandsDir(owner, repo) - if err := os.MkdirAll(commandsDir, 0755); err != nil { - return 0, fmt.Errorf("failed to create commands directory: %w", err) + if err := os.MkdirAll(opts.localDir, 0755); err != nil { + return 0, fmt.Errorf("failed to create %s directory: %w", opts.itemType, err) } - // Fetch each .md file count := 0 for _, entry := range entries { - if entry.Type != "file" || !strings.HasSuffix(entry.Name, ".md") { - continue - } - - result, err := client.FetchFile(ctx, owner, repo, entry.Path, branch) - if err != nil { - printWarning("Failed to fetch command %s: %v", entry.Name, err) + if entry.Type != "file" { continue } - localPath := filepath.Join(commandsDir, entry.Name) - if err := os.WriteFile(localPath, []byte(result.Content), 0644); err != nil { - printWarning("Failed to write command %s: %v", entry.Name, err) - continue + // Check if file matches any of the allowed extensions + hasValidExt := false + for _, ext := range opts.extensions { + if strings.HasSuffix(entry.Name, ext) { + hasValidExt = true + break + } } - - count++ - } - - return count, nil -} - -// syncTemplates fetches project templates from the team repo's templates/ directory. -func syncTemplates(ctx context.Context, client *github.Client, owner, repo, branch string, paths *config.Paths) (int, error) { - // List templates directory - entries, err := client.ListDirectory(ctx, owner, repo, "templates", branch) - if err != nil { - return 0, err - } - - if entries == nil { - // No templates directory - return 0, nil - } - - // Create local templates cache directory - templatesDir := paths.TeamTemplatesDir(owner, repo) - if err := os.MkdirAll(templatesDir, 0755); err != nil { - return 0, fmt.Errorf("failed to create templates directory: %w", err) - } - - // Fetch each .md file - count := 0 - for _, entry := range entries { - if entry.Type != "file" || !strings.HasSuffix(entry.Name, ".md") { + if !hasValidExt { continue } result, err := client.FetchFile(ctx, owner, repo, entry.Path, branch) if err != nil { - printWarning("Failed to fetch template %s: %v", entry.Name, err) + printWarning("Failed to fetch %s %s: %v", opts.itemType, entry.Name, err) continue } - localPath := filepath.Join(templatesDir, entry.Name) + localPath := filepath.Join(opts.localDir, entry.Name) if err := os.WriteFile(localPath, []byte(result.Content), 0644); err != nil { - printWarning("Failed to write template %s: %v", entry.Name, err) + printWarning("Failed to write %s %s: %v", opts.itemType, entry.Name, err) continue } @@ -431,95 +403,44 @@ func syncTemplates(ctx context.Context, client *github.Client, owner, repo, bran return count, nil } -// syncLanguages fetches language configs from the team repo's languages/ directory. -func syncLanguages(ctx context.Context, client *github.Client, owner, repo, branch string, paths *config.Paths) (int, error) { - // List languages directory - entries, err := client.ListDirectory(ctx, owner, repo, "languages", branch) - if err != nil { - return 0, err - } - - if entries == nil { - // No languages directory - return 0, nil - } - - // Create local languages cache directory - languagesDir := paths.TeamLanguagesDir(owner, repo) - if err := os.MkdirAll(languagesDir, 0755); err != nil { - return 0, fmt.Errorf("failed to create languages directory: %w", err) - } - - // Fetch each .md file - count := 0 - for _, entry := range entries { - if entry.Type != "file" || !strings.HasSuffix(entry.Name, ".md") { - continue - } - - result, err := client.FetchFile(ctx, owner, repo, entry.Path, branch) - if err != nil { - printWarning("Failed to fetch language config %s: %v", entry.Name, err) - continue - } - - localPath := filepath.Join(languagesDir, entry.Name) - if err := os.WriteFile(localPath, []byte(result.Content), 0644); err != nil { - printWarning("Failed to write language config %s: %v", entry.Name, err) - continue - } +// syncCommands fetches commands from the team repo's commands/ directory. +func syncCommands(ctx context.Context, client *github.Client, owner, repo, branch string, paths *config.Paths) (int, error) { + return syncDirectoryContents(ctx, client, owner, repo, branch, syncDirectoryOpts{ + remoteDir: "commands", + localDir: paths.TeamCommandsDir(owner, repo), + itemType: "command", + extensions: []string{".md"}, + }) +} - count++ - } +// syncTemplates fetches project templates from the team repo's templates/ directory. +func syncTemplates(ctx context.Context, client *github.Client, owner, repo, branch string, paths *config.Paths) (int, error) { + return syncDirectoryContents(ctx, client, owner, repo, branch, syncDirectoryOpts{ + remoteDir: "templates", + localDir: paths.TeamTemplatesDir(owner, repo), + itemType: "template", + extensions: []string{".md"}, + }) +} - return count, nil +// syncLanguages fetches language configs from the team repo's languages/ directory. +func syncLanguages(ctx context.Context, client *github.Client, owner, repo, branch string, paths *config.Paths) (int, error) { + return syncDirectoryContents(ctx, client, owner, repo, branch, syncDirectoryOpts{ + remoteDir: "languages", + localDir: paths.TeamLanguagesDir(owner, repo), + itemType: "language config", + extensions: []string{".md"}, + }) } // syncEvals fetches evals from the team repo's evals/ directory. func syncEvals(ctx context.Context, client *github.Client, owner, repo, branch string, paths *config.Paths) (int, error) { - // List evals directory - entries, err := client.ListDirectory(ctx, owner, repo, "evals", branch) - if err != nil { - return 0, err - } - - if entries == nil { - // No evals directory - return 0, nil - } - - // Create local evals cache directory - evalsDir := paths.TeamEvalsDir(owner, repo) - if err := os.MkdirAll(evalsDir, 0755); err != nil { - return 0, fmt.Errorf("failed to create evals directory: %w", err) - } - - // Fetch each .yaml/.yml file - count := 0 - for _, entry := range entries { - if entry.Type != "file" { - continue - } - if !strings.HasSuffix(entry.Name, ".yaml") && !strings.HasSuffix(entry.Name, ".yml") { - continue - } - - result, err := client.FetchFile(ctx, owner, repo, entry.Path, branch) - if err != nil { - printWarning("Failed to fetch eval %s: %v", entry.Name, err) - continue - } - - localPath := filepath.Join(evalsDir, entry.Name) - if err := os.WriteFile(localPath, []byte(result.Content), 0644); err != nil { - printWarning("Failed to write eval %s: %v", entry.Name, err) - continue - } - - count++ - } - - return count, nil + return syncDirectoryContents(ctx, client, owner, repo, branch, syncDirectoryOpts{ + remoteDir: "evals", + localDir: paths.TeamEvalsDir(owner, repo), + itemType: "eval", + extensions: []string{".yaml", ".yml"}, + }) } // syncRules fetches rules from the team repo's rules/ directory (recursive). @@ -1017,10 +938,16 @@ func checkConfigSizeAndSuggestOptimize(cfg *config.Config, paths *config.Paths, merged := merge.MergeWithLanguages(layers, mergeOpts) tokens := optimize.CountTokens(merged) + // Use configured threshold or default + threshold := cfg.Optimize.WarnThreshold + if threshold == 0 { + threshold = 3000 // Default threshold + } + // Warn if over threshold - if tokens > 3000 { + if tokens > threshold { fmt.Println() - printWarning("Merged config is %d tokens (threshold: 3,000)", tokens) + printWarning("Merged config is %d tokens (threshold: %d)", tokens, threshold) fmt.Printf(" Large configs may reduce Claude Code effectiveness.\n") fmt.Printf(" Run %s to compress.\n", info("staghorn optimize")) } diff --git a/internal/github/client.go b/internal/github/client.go index 9419807..88b8908 100644 --- a/internal/github/client.go +++ b/internal/github/client.go @@ -78,7 +78,7 @@ type fileContentsResponse struct { } // FetchFile fetches a file from a repo. -// If etag is provided and content hasn't changed, returns NotModified=true. +// The context is used for request cancellation and timeouts. func (c *Client) FetchFile(ctx context.Context, owner, repo, path, branch string) (*FetchResult, error) { if owner == "" || repo == "" || path == "" { return nil, fmt.Errorf("owner, repo, and path are required") @@ -90,7 +90,7 @@ func (c *Client) FetchFile(ctx context.Context, owner, repo, path, branch string } var response fileContentsResponse - err := c.rest.Get(endpoint, &response) + err := c.rest.DoWithContext(ctx, http.MethodGet, endpoint, nil, &response) if err != nil { return nil, err } @@ -108,6 +108,7 @@ func (c *Client) FetchFile(ctx context.Context, owner, repo, path, branch string } // GetDefaultBranch returns the repo's default branch. +// The context is used for request cancellation and timeouts. func (c *Client) GetDefaultBranch(ctx context.Context, owner, repo string) (string, error) { endpoint := fmt.Sprintf("repos/%s/%s", owner, repo) @@ -115,7 +116,7 @@ func (c *Client) GetDefaultBranch(ctx context.Context, owner, repo string) (stri DefaultBranch string `json:"default_branch"` } - err := c.rest.Get(endpoint, &response) + err := c.rest.DoWithContext(ctx, http.MethodGet, endpoint, nil, &response) if err != nil { return "", err } @@ -124,6 +125,7 @@ func (c *Client) GetDefaultBranch(ctx context.Context, owner, repo string) (stri } // RepoExists checks if a repository exists and is accessible. +// The context is used for request cancellation and timeouts. func (c *Client) RepoExists(ctx context.Context, owner, repo string) (bool, error) { endpoint := fmt.Sprintf("repos/%s/%s", owner, repo) @@ -131,7 +133,7 @@ func (c *Client) RepoExists(ctx context.Context, owner, repo string) (bool, erro ID int `json:"id"` } - err := c.rest.Get(endpoint, &response) + err := c.rest.DoWithContext(ctx, http.MethodGet, endpoint, nil, &response) if err != nil { // Check if it's a 404 if httpErr, ok := err.(*api.HTTPError); ok { @@ -146,6 +148,7 @@ func (c *Client) RepoExists(ctx context.Context, owner, repo string) (bool, erro } // FileExists checks if a file exists in a repo. +// The context is used for request cancellation and timeouts. func (c *Client) FileExists(ctx context.Context, owner, repo, path, branch string) (bool, error) { endpoint := fmt.Sprintf("repos/%s/%s/contents/%s", owner, repo, url.PathEscape(path)) if branch != "" { @@ -153,7 +156,7 @@ func (c *Client) FileExists(ctx context.Context, owner, repo, path, branch strin } var response fileContentsResponse - err := c.rest.Get(endpoint, &response) + err := c.rest.DoWithContext(ctx, http.MethodGet, endpoint, nil, &response) if err != nil { if httpErr, ok := err.(*api.HTTPError); ok { if httpErr.StatusCode == http.StatusNotFound { @@ -176,6 +179,7 @@ type DirectoryEntry struct { // ListDirectory lists contents of a directory in a repo. // Returns nil, nil if the directory doesn't exist. +// The context is used for request cancellation and timeouts. func (c *Client) ListDirectory(ctx context.Context, owner, repo, path, branch string) ([]DirectoryEntry, error) { endpoint := fmt.Sprintf("repos/%s/%s/contents/%s", owner, repo, url.PathEscape(path)) if branch != "" { @@ -183,7 +187,7 @@ func (c *Client) ListDirectory(ctx context.Context, owner, repo, path, branch st } var response []DirectoryEntry - err := c.rest.Get(endpoint, &response) + err := c.rest.DoWithContext(ctx, http.MethodGet, endpoint, nil, &response) if err != nil { if httpErr, ok := err.(*api.HTTPError); ok { if httpErr.StatusCode == http.StatusNotFound { diff --git a/internal/skills/claude.go b/internal/skills/claude.go index 7fb5dce..2de1155 100644 --- a/internal/skills/claude.go +++ b/internal/skills/claude.go @@ -23,8 +23,13 @@ func ConvertToClaude(skill *Skill) string { // Build frontmatter map to preserve field order and include all fields fm := buildClaudeFrontmatter(skill) - yamlBytes, _ := yaml.Marshal(fm) - sb.Write(yamlBytes) + yamlBytes, err := yaml.Marshal(fm) + if err != nil { + // Fallback to minimal frontmatter if marshal fails + sb.WriteString(fmt.Sprintf("name: %s\ndescription: %s\n", skill.Name, skill.Description)) + } else { + sb.Write(yamlBytes) + } sb.WriteString("---\n\n") // Add staghorn header after frontmatter diff --git a/internal/skills/registry.go b/internal/skills/registry.go index a45f73b..7a49c75 100644 --- a/internal/skills/registry.go +++ b/internal/skills/registry.go @@ -2,7 +2,7 @@ package skills import ( "fmt" - "os" + "log" "sort" ) @@ -173,7 +173,7 @@ func LoadRegistry(teamDir, personalDir, projectDir string) (*Registry, error) { } skills, err := LoadFromDirectory(s.dir, s.source) if err != nil { - return nil, fmt.Errorf("failed to load %s skills: %w", s.source.Label(), err) + return nil, fmt.Errorf("failed to load %s skills from %s: %w", s.source.Label(), s.dir, err) } registry.AddAll(skills) } @@ -194,7 +194,7 @@ func LoadRegistryWithMultipleDirs(teamDirs []string, personalDir, projectDir str skills, err := LoadFromDirectory(teamDir, SourceTeam) if err != nil { // Log warning but continue - some dirs may not have skills - fmt.Fprintf(os.Stderr, "Warning: failed to load team skills from %s: %v\n", teamDir, err) + log.Printf("Warning: failed to load team skills from %s: %v", teamDir, err) continue } registry.AddAll(skills) diff --git a/internal/skills/skill.go b/internal/skills/skill.go index f4445d5..10c52a3 100644 --- a/internal/skills/skill.go +++ b/internal/skills/skill.go @@ -6,6 +6,7 @@ package skills import ( "bufio" "fmt" + "log" "os" "path/filepath" "strings" @@ -261,7 +262,7 @@ func LoadFromDirectory(dir string, source Source) ([]*Skill, error) { skill, err := ParseDir(skillDir, source) if err != nil { // Log warning but continue loading other skills - fmt.Fprintf(os.Stderr, "Warning: failed to parse skill %s: %v\n", entry.Name(), err) + log.Printf("Warning: failed to parse skill %s: %v", entry.Name(), err) continue }