From 2832d5277fe92dad9e577ba9cabadaf4535c4432 Mon Sep 17 00:00:00 2001 From: Christian Rocha Date: Thu, 2 Apr 2026 16:37:46 -0400 Subject: [PATCH] feat(skills): builtin skills + skill disabling + crush-config builtin (#2466) --- README.md | 55 ++++-- internal/agent/common_test.go | 1 + internal/agent/prompt/prompt.go | 27 ++- internal/agent/templates/coder.md.tpl | 5 +- internal/agent/tools/view.go | 59 ++++++ internal/agent/tools/view_test.go | 52 ++++++ internal/config/config.go | 1 + internal/skills/builtin/crush-config/SKILL.md | 175 ++++++++++++++++++ internal/skills/embed.go | 66 +++++++ internal/skills/skills.go | 59 +++++- internal/skills/skills_test.go | 133 +++++++++++++ 11 files changed, 609 insertions(+), 24 deletions(-) create mode 100644 internal/skills/builtin/crush-config/SKILL.md create mode 100644 internal/skills/embed.go diff --git a/README.md b/README.md index 9b4f8e49218ba2982d04e56b22a4bd480bc110d9..924b48ae88978390ce0a32f7fbd0b6a95eedb431 100644 --- a/README.md +++ b/README.md @@ -224,6 +224,10 @@ Crush’s default model listing is managed in [Catwalk](https://github.com/charm ## Configuration +> [!TIP] +> Crush ships with a builtin `crush-config` skill for configuring itself. In +> many cases you can simply ask Crush to configure itself. + Crush runs great with no configuration. That said, if you do need or want to customize Crush, configuration can be added either local to the project itself, or globally, with the following priority: @@ -241,7 +245,8 @@ Configuration itself is stored as a JSON object: } ``` -As an additional note, Crush also stores ephemeral data, such as application state, in one additional location: +As an additional note, Crush also stores ephemeral data, such as application +state, in one additional location: ```bash # Unix @@ -253,8 +258,9 @@ $HOME/.local/share/crush/crush.json > [!TIP] > You can override the user and data config locations by setting: -> * `CRUSH_GLOBAL_CONFIG` -> * `CRUSH_GLOBAL_DATA` +> +> - `CRUSH_GLOBAL_CONFIG` +> - `CRUSH_GLOBAL_DATA` ### LSPs @@ -371,16 +377,28 @@ completely hidden from the agent. { "$schema": "https://charm.land/crush.json", "options": { - "disabled_tools": [ - "bash", - "sourcegraph" - ] + "disabled_tools": ["bash", "sourcegraph"] } } ``` To disable tools from MCP servers, see the [MCP config section](#mcps). +### Disabling Skills + +If you'd like to prevent Crush from using certain skills entirely, you can +disable them via the `options.disabled_skills` list. Disabled skills are hidden +from the agent, including builtin skills and skills discovered from disk. + +```json +{ + "$schema": "https://charm.land/crush.json", + "options": { + "disabled_skills": ["crush-config"] + } +} +``` + ### Agent Skills Crush supports the [Agent Skills](https://agentskills.io) open standard for @@ -412,9 +430,9 @@ relative paths: "options": { "skills_paths": [ "~/.config/crush/skills", // Windows: "%LOCALAPPDATA%\\crush\\skills", - "./project-skills" - ] - } + "./project-skills", + ], + }, } ``` @@ -446,8 +464,8 @@ focused _and_ your terminal supports reporting the focus state. { "$schema": "https://charm.land/crush.json", "options": { - "disable_notifications": false // default - } + "disable_notifications": false, // default + }, } ``` @@ -496,10 +514,10 @@ it creates. You can customize this behavior with the `attribution` option: - `trailer_style`: Controls the attribution trailer added to commit messages (default: `assisted-by`) - - `assisted-by`: Adds `Assisted-by: [Model Name] via Crush ` - (includes the model name) - - `co-authored-by`: Adds `Co-Authored-By: Crush ` - - `none`: No attribution trailer + - `assisted-by`: Adds `Assisted-by: [Model Name] via Crush ` + (includes the model name) + - `co-authored-by`: Adds `Co-Authored-By: Crush ` + - `none`: No attribution trailer - `generated_with`: When true (default), adds `💘 Generated with Crush` line to commit messages and PR descriptions @@ -511,8 +529,9 @@ Anthropic-compatible APIs. > [!NOTE] > Note that we support two "types" for OpenAI. Make sure to choose the right one > to ensure the best experience! -> * `openai` should be used when proxying or routing requests through OpenAI. -> * `openai-compat` should be used when using non-OpenAI providers that have OpenAI-compatible APIs. +> +> - `openai` should be used when proxying or routing requests through OpenAI. +> - `openai-compat` should be used when using non-OpenAI providers that have OpenAI-compatible APIs. #### OpenAI-Compatible APIs diff --git a/internal/agent/common_test.go b/internal/agent/common_test.go index 132c27d21aee81bd3930c469963f1d73885d58a7..5fddc3c10f7c68986ec64b6ccd9bdf10a1a6de91 100644 --- a/internal/agent/common_test.go +++ b/internal/agent/common_test.go @@ -192,6 +192,7 @@ func coderAgent(r *vcr.Recorder, env fakeEnv, large, small fantasy.LanguageModel // Clear some fields to avoid issues with VCR cassette matching. cfg.Config().Options.SkillsPaths = nil + cfg.Config().Options.DisabledSkills = []string{"crush-config"} cfg.Config().Options.ContextPaths = nil cfg.Config().LSP = nil diff --git a/internal/agent/prompt/prompt.go b/internal/agent/prompt/prompt.go index c8f488319f04238c476aae4719728fb94521695e..070f29c18d02f220d309d5827f8d0c8879e0e5a2 100644 --- a/internal/agent/prompt/prompt.go +++ b/internal/agent/prompt/prompt.go @@ -4,6 +4,7 @@ import ( "cmp" "context" "fmt" + "log/slog" "os" "path/filepath" "runtime" @@ -167,16 +168,38 @@ func (p *Prompt) promptData(ctx context.Context, provider, model string, store * // Discover and load skills metadata. var availSkillXML string + + // Start with builtin skills. + allSkills := skills.DiscoverBuiltin() + builtinNames := make(map[string]bool, len(allSkills)) + for _, s := range allSkills { + builtinNames[s.Name] = true + } + + // Discover user skills from configured paths. if len(cfg.Options.SkillsPaths) > 0 { expandedPaths := make([]string, 0, len(cfg.Options.SkillsPaths)) for _, pth := range cfg.Options.SkillsPaths { expandedPaths = append(expandedPaths, expandPath(pth, store)) } - if discoveredSkills := skills.Discover(expandedPaths); len(discoveredSkills) > 0 { - availSkillXML = skills.ToPromptXML(discoveredSkills) + for _, userSkill := range skills.Discover(expandedPaths) { + if builtinNames[userSkill.Name] { + slog.Warn("User skill overrides builtin skill", "name", userSkill.Name) + } + allSkills = append(allSkills, userSkill) } } + // Deduplicate: user skills override builtins with the same name. + allSkills = skills.Deduplicate(allSkills) + + // Filter out disabled skills. + allSkills = skills.Filter(allSkills, cfg.Options.DisabledSkills) + + if len(allSkills) > 0 { + availSkillXML = skills.ToPromptXML(allSkills) + } + isGit := isGitRepo(store.WorkingDir()) data := PromptDat{ Provider: provider, diff --git a/internal/agent/templates/coder.md.tpl b/internal/agent/templates/coder.md.tpl index 458ee7f1e2b70fbe938f7dc1e818cabdae33a921..054964dcb4b30063a675f33b9cdfad32fe96bb2e 100644 --- a/internal/agent/templates/coder.md.tpl +++ b/internal/agent/templates/coder.md.tpl @@ -377,7 +377,10 @@ Diagnostics (lint/typecheck) included in tool output. When a user task matches a skill's description, read the skill's SKILL.md file to get full instructions. -Skills are activated by reading their location path. Follow the skill's instructions to complete the task. +Skills are activated by reading their **exact** location path as shown above using the View tool. Always pass the location value directly to the View tool's file_path parameter — never guess, modify, or construct skill paths yourself. +Builtin skills (type=builtin) have virtual location identifiers starting with "crush://skills/". The "crush://" prefix is NOT a URL or network address — it is a special internal identifier that the View tool understands natively. Pass them verbatim to the View tool. Do not treat them as URLs, MCP resources, or filesystem paths. +Do not use MCP tools (including read_mcp_resource) to load skills. +Follow the skill's instructions to complete the task. If a skill mentions scripts, references, or assets, they are placed in the same folder as the skill itself (e.g., scripts/, references/, assets/ subdirectories within the skill's folder). {{end}} diff --git a/internal/agent/tools/view.go b/internal/agent/tools/view.go index db6eb2b5a92d778eeebe211bd2ce013ab316e5de..01b405056eaad03a0328f23bcb28cb94c975ea26 100644 --- a/internal/agent/tools/view.go +++ b/internal/agent/tools/view.go @@ -7,6 +7,7 @@ import ( "encoding/base64" "fmt" "io" + "io/fs" "os" "path/filepath" "strings" @@ -73,6 +74,11 @@ func NewViewTool( return fantasy.NewTextErrorResponse("file_path is required"), nil } + // Handle builtin skill files (crush: prefix). + if strings.HasPrefix(params.FilePath, skills.BuiltinPrefix) { + return readBuiltinFile(params) + } + // Handle relative paths filePath := filepathext.SmartJoin(workingDir, params.FilePath) @@ -373,3 +379,56 @@ func isInSkillsPath(filePath string, skillsPaths []string) bool { return false } + +// readBuiltinFile reads a file from the embedded builtin skills filesystem. +func readBuiltinFile(params ViewParams) (fantasy.ToolResponse, error) { + embeddedPath := "builtin/" + strings.TrimPrefix(params.FilePath, skills.BuiltinPrefix) + builtinFS := skills.BuiltinFS() + + data, err := fs.ReadFile(builtinFS, embeddedPath) + if err != nil { + return fantasy.NewTextErrorResponse(fmt.Sprintf("Builtin file not found: %s", params.FilePath)), nil + } + + content := string(data) + if !utf8.ValidString(content) { + return fantasy.NewTextErrorResponse("File content is not valid UTF-8"), nil + } + + limit := params.Limit + if limit <= 0 { + limit = 1000000 // Effectively no limit for skill files. + } + + lines := strings.Split(content, "\n") + offset := min(params.Offset, len(lines)) + lines = lines[offset:] + + hasMore := len(lines) > limit + if hasMore { + lines = lines[:limit] + } + + output := "\n" + output += addLineNumbers(strings.Join(lines, "\n"), offset+1) + if hasMore { + output += fmt.Sprintf("\n\n(File has more lines. Use 'offset' parameter to read beyond line %d)", + offset+len(lines)) + } + output += "\n\n" + + meta := ViewResponseMetadata{ + FilePath: params.FilePath, + Content: strings.Join(lines, "\n"), + } + if skill, err := skills.ParseContent(data); err == nil { + meta.ResourceType = ViewResourceSkill + meta.ResourceName = skill.Name + meta.ResourceDescription = skill.Description + } + + return fantasy.WithResponseMetadata( + fantasy.NewTextResponse(output), + meta, + ), nil +} diff --git a/internal/agent/tools/view_test.go b/internal/agent/tools/view_test.go index 18b61f4e5012b4a685405fa1d1e31b90f6c790bc..c821f9e2eabd697acef442dd900d93524e4f9d7f 100644 --- a/internal/agent/tools/view_test.go +++ b/internal/agent/tools/view_test.go @@ -1,6 +1,7 @@ package tools import ( + "encoding/json" "fmt" "os" "path/filepath" @@ -85,3 +86,54 @@ func TestReadTextFileTruncatesLongLines(t *testing.T) { require.False(t, hasMore) require.Equal(t, strings.Repeat("a", MaxLineLength)+"...", content) } + +func TestReadBuiltinFile(t *testing.T) { + t.Parallel() + + t.Run("reads crush-config skill", func(t *testing.T) { + t.Parallel() + + resp, err := readBuiltinFile(ViewParams{ + FilePath: "crush://skills/crush-config/SKILL.md", + }) + require.NoError(t, err) + require.NotEmpty(t, resp.Content) + require.Contains(t, resp.Content, "Crush Configuration") + }) + + t.Run("not found", func(t *testing.T) { + t.Parallel() + + resp, err := readBuiltinFile(ViewParams{ + FilePath: "crush://skills/nonexistent/SKILL.md", + }) + require.NoError(t, err) + require.True(t, resp.IsError) + }) + + t.Run("metadata has skill info", func(t *testing.T) { + t.Parallel() + + resp, err := readBuiltinFile(ViewParams{ + FilePath: "crush://skills/crush-config/SKILL.md", + }) + require.NoError(t, err) + + var meta ViewResponseMetadata + require.NoError(t, json.Unmarshal([]byte(resp.Metadata), &meta)) + require.Equal(t, ViewResourceSkill, meta.ResourceType) + require.Equal(t, "crush-config", meta.ResourceName) + require.NotEmpty(t, meta.ResourceDescription) + }) + + t.Run("respects offset", func(t *testing.T) { + t.Parallel() + + resp, err := readBuiltinFile(ViewParams{ + FilePath: "crush://skills/crush-config/SKILL.md", + Offset: 5, + }) + require.NoError(t, err) + require.NotContains(t, resp.Content, " 1|") + }) +} diff --git a/internal/config/config.go b/internal/config/config.go index 7aece9cdc2b38085f40306028050c96b8f17101c..71a517a5398151f346998b4c3ff1d91afadd4ee1 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -256,6 +256,7 @@ type Options struct { AutoLSP *bool `json:"auto_lsp,omitempty" jsonschema:"description=Automatically setup LSPs based on root markers,default=true"` Progress *bool `json:"progress,omitempty" jsonschema:"description=Show indeterminate progress updates during long operations,default=true"` DisableNotifications bool `json:"disable_notifications,omitempty" jsonschema:"description=Disable desktop notifications,default=false"` + DisabledSkills []string `json:"disabled_skills,omitempty" jsonschema:"description=List of skill names to disable and hide from the agent,example=crush-config"` } type MCPs map[string]MCPConfig diff --git a/internal/skills/builtin/crush-config/SKILL.md b/internal/skills/builtin/crush-config/SKILL.md new file mode 100644 index 0000000000000000000000000000000000000000..b4b86e24fe9252331cce866c2c0310f5dfda237a --- /dev/null +++ b/internal/skills/builtin/crush-config/SKILL.md @@ -0,0 +1,175 @@ +--- +name: crush-config +description: Configure Crush settings including providers, LSPs, MCPs, skills, permissions, and behavior options. Use when the user needs help with crush.json configuration, setting up providers, configuring LSPs, adding MCP servers, or changing Crush behavior. +--- + +# Crush Configuration + +Crush uses JSON configuration files with the following priority (highest to lowest): + +1. `.crush.json` (project-local, hidden) +2. `crush.json` (project-local) +3. `$XDG_CONFIG_HOME/crush/crush.json` or `$HOME/.config/crush/crush.json` (global) + +## Basic Structure + +```json +{ + "$schema": "https://charm.land/crush.json", + "options": {} +} +``` + +The `$schema` property enables IDE autocomplete but is optional. + +## Common Configurations + +### Project-Local Skills + +Add a relative path to keep project-specific skills alongside your code: + +```json +{ + "options": { + "skills_paths": ["./skills"] + } +} +``` + +> [!IMPORTANT] +> Keep in mind that the following paths are loaded by default, so they DO NOT NEED to be added to `skill_paths`: +> +> * `.agents/skills` +> * `.crush/skills` +> * `.claude/skills` +> * `.cursor/skills` + +### LSP Configuration + +```json +{ + "lsp": { + "go": { + "command": "gopls", + "env": { + "GOTOOLCHAIN": "go1.24.5" + } + }, + "typescript": { + "command": "typescript-language-server", + "args": ["--stdio"] + } + } +} +``` + +### MCP Servers + +```json +{ + "mcp": { + "filesystem": { + "type": "stdio", + "command": "node", + "args": ["/path/to/mcp-server.js"] + }, + "github": { + "type": "http", + "url": "https://api.githubcopilot.com/mcp/", + "headers": { + "Authorization": "Bearer $GH_PAT" + } + } + } +} +``` + +### Custom Provider + +```json +{ + "providers": { + "deepseek": { + "type": "openai-compat", + "base_url": "https://api.deepseek.com/v1", + "api_key": "$DEEPSEEK_API_KEY", + "models": [ + { + "id": "deepseek-chat", + "name": "Deepseek V3", + "context_window": 64000 + } + ] + } + } +} +``` + +### Tool Permissions + +```json +{ + "permissions": { + "allowed_tools": ["view", "ls", "grep", "edit"] + } +} +``` + +### Disable Built-in Tools + +```json +{ + "options": { + "disabled_tools": ["bash", "sourcegraph"] + } +} +``` + +### Disable Skills + +```json +{ + "options": { + "disabled_skills": ["crush-config"] + } +} +``` + +`disabled_skills` disables skills by name, including both builtin skills and +skills discovered from disk paths. + +### Debug Options + +```json +{ + "options": { + "debug": true, + "debug_lsp": true + } +} +``` + +### Attribution Settings + +```json +{ + "options": { + "attribution": { + "trailer_style": "assisted-by", + "generated_with": true + } + } +} +``` + +## Environment Variables + +- `CRUSH_GLOBAL_CONFIG` - Override global config location +- `CRUSH_GLOBAL_DATA` - Override data directory location +- `CRUSH_SKILLS_DIR` - Override default skills directory + +## Provider Types + +- `openai` - For OpenAI or OpenAI-compatible APIs that route through OpenAI +- `openai-compat` - For non-OpenAI providers with OpenAI-compatible APIs +- `anthropic` - For Anthropic-compatible APIs diff --git a/internal/skills/embed.go b/internal/skills/embed.go new file mode 100644 index 0000000000000000000000000000000000000000..fc3b41f21cc8a09287eadc6956c4557fdf475079 --- /dev/null +++ b/internal/skills/embed.go @@ -0,0 +1,66 @@ +package skills + +import ( + "embed" + "io/fs" + "log/slog" + "path/filepath" +) + +// BuiltinPrefix is the path prefix for builtin skill files. It is used by +// the View tool to distinguish embedded files from disk files. +const BuiltinPrefix = "crush://skills/" + +//go:embed builtin/* +var builtinFS embed.FS + +// BuiltinFS returns the embedded filesystem containing builtin skills. +func BuiltinFS() embed.FS { + return builtinFS +} + +// DiscoverBuiltin finds all valid skills embedded in the binary. +func DiscoverBuiltin() []*Skill { + var discovered []*Skill + + fs.WalkDir(builtinFS, "builtin", func(path string, d fs.DirEntry, err error) error { + if err != nil { + return nil + } + if d.IsDir() || d.Name() != SkillFileName { + return nil + } + + content, err := builtinFS.ReadFile(path) + if err != nil { + slog.Warn("Failed to read builtin skill file", "path", path, "error", err) + return nil + } + + skill, err := ParseContent(content) + if err != nil { + slog.Warn("Failed to parse builtin skill file", "path", path, "error", err) + return nil + } + + // Set paths using the crush prefix. Strip the leading "builtin/" + // so the path is relative to the embedded root + // (e.g., "crush://skills/crush-config/SKILL.md"). + relPath, _ := filepath.Rel("builtin", path) + relPath = filepath.ToSlash(relPath) + skill.SkillFilePath = BuiltinPrefix + relPath + skill.Path = BuiltinPrefix + filepath.Dir(relPath) + skill.Builtin = true + + if err := skill.Validate(); err != nil { + slog.Warn("Builtin skill validation failed", "path", path, "error", err) + return nil + } + + slog.Debug("Successfully loaded builtin skill", "name", skill.Name, "path", skill.SkillFilePath) + discovered = append(discovered, skill) + return nil + }) + + return discovered +} diff --git a/internal/skills/skills.go b/internal/skills/skills.go index e0488cff4a8c42ceee68a6a89608bd90acafdb2e..607cdc779e10138ef1225323cfffc7d5884004ec 100644 --- a/internal/skills/skills.go +++ b/internal/skills/skills.go @@ -35,6 +35,7 @@ type Skill struct { Instructions string `yaml:"-" json:"instructions"` Path string `yaml:"-" json:"path"` SkillFilePath string `yaml:"-" json:"skill_file_path"` + Builtin bool `yaml:"-" json:"builtin"` } // Validate checks if the skill meets spec requirements. @@ -68,13 +69,26 @@ func (s *Skill) Validate() error { return errors.Join(errs...) } -// Parse parses a SKILL.md file. +// Parse parses a SKILL.md file from disk. func Parse(path string) (*Skill, error) { content, err := os.ReadFile(path) if err != nil { return nil, err } + skill, err := ParseContent(content) + if err != nil { + return nil, err + } + + skill.Path = filepath.Dir(path) + skill.SkillFilePath = path + + return skill, nil +} + +// ParseContent parses a SKILL.md from raw bytes. +func ParseContent(content []byte) (*Skill, error) { frontmatter, body, err := splitFrontmatter(string(content)) if err != nil { return nil, err @@ -86,8 +100,6 @@ func Parse(path string) (*Skill, error) { } skill.Instructions = strings.TrimSpace(body) - skill.Path = filepath.Dir(path) - skill.SkillFilePath = path return &skill, nil } @@ -171,6 +183,9 @@ func ToPromptXML(skills []*Skill) string { fmt.Fprintf(&sb, " %s\n", escape(s.Name)) fmt.Fprintf(&sb, " %s\n", escape(s.Description)) fmt.Fprintf(&sb, " %s\n", escape(s.SkillFilePath)) + if s.Builtin { + sb.WriteString(" builtin\n") + } sb.WriteString(" \n") } sb.WriteString("") @@ -181,3 +196,41 @@ func escape(s string) string { r := strings.NewReplacer("&", "&", "<", "<", ">", ">", "\"", """, "'", "'") return r.Replace(s) } + +// Deduplicate removes duplicate skills by name. When duplicates exist, the +// last occurrence wins. This means user skills (appended after builtins) +// override builtin skills with the same name. +func Deduplicate(all []*Skill) []*Skill { + seen := make(map[string]int, len(all)) + for i, s := range all { + seen[s.Name] = i + } + + result := make([]*Skill, 0, len(seen)) + for i, s := range all { + if seen[s.Name] == i { + result = append(result, s) + } + } + return result +} + +// Filter removes skills whose names appear in the disabled list. +func Filter(all []*Skill, disabled []string) []*Skill { + if len(disabled) == 0 { + return all + } + + disabledSet := make(map[string]bool, len(disabled)) + for _, name := range disabled { + disabledSet[name] = true + } + + result := make([]*Skill, 0, len(all)) + for _, s := range all { + if !disabledSet[s.Name] { + result = append(result, s) + } + } + return result +} diff --git a/internal/skills/skills_test.go b/internal/skills/skills_test.go index f90d7cb341d85e85945efc06f46dd372a7cf4725..fcd5e71e2cb73ae5ba143ff62cc0628bda7dd9b0 100644 --- a/internal/skills/skills_test.go +++ b/internal/skills/skills_test.go @@ -247,3 +247,136 @@ func TestToPromptXMLEmpty(t *testing.T) { require.Empty(t, ToPromptXML(nil)) require.Empty(t, ToPromptXML([]*Skill{})) } + +func TestToPromptXMLBuiltinType(t *testing.T) { + t.Parallel() + + skills := []*Skill{ + {Name: "builtin-skill", Description: "A builtin.", SkillFilePath: "crush://skills/builtin-skill/SKILL.md", Builtin: true}, + {Name: "user-skill", Description: "A user skill.", SkillFilePath: "/home/user/.config/crush/skills/user-skill/SKILL.md"}, + } + xml := ToPromptXML(skills) + require.Contains(t, xml, "builtin") + require.Equal(t, 1, strings.Count(xml, "builtin")) +} + +func TestParseContent(t *testing.T) { + t.Parallel() + + content := []byte(`--- +name: my-skill +description: A test skill. +--- + +# My Skill + +Instructions here. +`) + skill, err := ParseContent(content) + require.NoError(t, err) + require.Equal(t, "my-skill", skill.Name) + require.Equal(t, "A test skill.", skill.Description) + require.Equal(t, "# My Skill\n\nInstructions here.", skill.Instructions) + require.Empty(t, skill.Path) + require.Empty(t, skill.SkillFilePath) +} + +func TestParseContent_NoFrontmatter(t *testing.T) { + t.Parallel() + + _, err := ParseContent([]byte("# Just Markdown")) + require.Error(t, err) +} + +func TestDiscoverBuiltin(t *testing.T) { + t.Parallel() + + discovered := DiscoverBuiltin() + require.NotEmpty(t, discovered) + + var found bool + for _, s := range discovered { + if s.Name == "crush-config" { + found = true + require.True(t, strings.HasPrefix(s.SkillFilePath, BuiltinPrefix)) + require.True(t, strings.HasPrefix(s.Path, BuiltinPrefix)) + require.Equal(t, "crush://skills/crush-config/SKILL.md", s.SkillFilePath) + require.Equal(t, "crush://skills/crush-config", s.Path) + require.NotEmpty(t, s.Description) + require.NotEmpty(t, s.Instructions) + require.True(t, s.Builtin) + } + } + require.True(t, found, "crush-config builtin skill not found") +} + +func TestDeduplicate(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + input []*Skill + wantLen int + wantName string + wantPath string + }{ + { + name: "no duplicates", + input: []*Skill{{Name: "a", Path: "/a"}, {Name: "b", Path: "/b"}}, + wantLen: 2, + }, + { + name: "user overrides builtin", + input: []*Skill{{Name: "crush-config", Path: "crush://skills/crush-config"}, {Name: "crush-config", Path: "/user/crush-config"}}, + wantLen: 1, + wantName: "crush-config", + wantPath: "/user/crush-config", + }, + { + name: "empty", + input: nil, + wantLen: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + result := Deduplicate(tt.input) + require.Len(t, result, tt.wantLen) + if tt.wantName != "" { + require.Equal(t, tt.wantName, result[0].Name) + require.Equal(t, tt.wantPath, result[0].Path) + } + }) + } +} + +func TestFilter(t *testing.T) { + t.Parallel() + + all := []*Skill{ + {Name: "a"}, + {Name: "b"}, + {Name: "c"}, + } + + tests := []struct { + name string + disabled []string + wantLen int + }{ + {"no filter", nil, 3}, + {"filter one", []string{"b"}, 2}, + {"filter all", []string{"a", "b", "c"}, 0}, + {"filter nonexistent", []string{"d"}, 3}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + result := Filter(all, tt.disabled) + require.Len(t, result, tt.wantLen) + }) + } +}