From 0e8b879720d219e74970525e4c9d2615ca414519 Mon Sep 17 00:00:00 2001 From: Amolith Date: Tue, 2 Jun 2026 07:57:55 -0600 Subject: [PATCH] refactor(commands): centralise skill discovery The UI for invoking skills duplicated skill discovery, but missed some of the nuances like following symlinks. This removes some of that duplicative code and wires other bits together so the UI shares the same discovery logic as the backend, fixing at least the symlink behaviour. --- internal/backend/config.go | 11 ++-- internal/commands/commands.go | 69 ++++++-------------------- internal/commands/commands_test.go | 61 +++++++++++++++++++++++ internal/proto/proto.go | 11 ++-- internal/skills/catalog.go | 22 ++++---- internal/ui/model/ui.go | 10 ++-- internal/workspace/client_workspace.go | 11 ++-- 7 files changed, 112 insertions(+), 83 deletions(-) diff --git a/internal/backend/config.go b/internal/backend/config.go index 4e7ce27dd11db51758fc564a458a0527ca21c499..0401f818e5b607a5dad49039da92179479fb4c9c 100644 --- a/internal/backend/config.go +++ b/internal/backend/config.go @@ -196,11 +196,12 @@ func (b *Backend) ListSkills(workspaceID string) ([]proto.SkillInfo, error) { result := make([]proto.SkillInfo, len(entries)) for i, entry := range entries { result[i] = proto.SkillInfo{ - ID: entry.ID, - Name: entry.Name, - Description: entry.Description, - Label: entry.Label, - Source: string(entry.Source), + ID: entry.ID, + Name: entry.Name, + Description: entry.Description, + Label: entry.Label, + Source: string(entry.Source), + UserInvocable: entry.UserInvocable, } } return result, nil diff --git a/internal/commands/commands.go b/internal/commands/commands.go index 58a5c2e123a8ad655097c189213a4f5e8e847920..1de4576d897c6d58ca3b3680771882f69fb0124d 100644 --- a/internal/commands/commands.go +++ b/internal/commands/commands.go @@ -61,67 +61,28 @@ func LoadCustomCommands(cfg *config.Config) ([]CustomCommand, error) { return loadAll(buildCommandSources(cfg)) } -// LoadSkillCommands loads user-invocable skills as custom commands. -func LoadSkillCommands() []CustomCommand { - var commands []CustomCommand - - // Load from global skills directories with "user:" prefix - for _, dir := range config.GlobalSkillsDirs() { - commands = append(commands, loadInvocableSkillsFromDir(dir, userCommandPrefix)...) - } - - return commands -} - -// LoadProjectSkillCommands loads user-invocable skills from project directories as custom commands. -func LoadProjectSkillCommands(workingDir string) []CustomCommand { - var commands []CustomCommand - - // Load from project skills directories with "project:" prefix - for _, dir := range config.ProjectSkillsDir(workingDir) { - commands = append(commands, loadInvocableSkillsFromDir(dir, projectCommandPrefix)...) - } - - return commands -} - -func loadInvocableSkillsFromDir(dir, prefix string) []CustomCommand { - if _, err := os.Stat(dir); os.IsNotExist(err) { - return nil - } - - var commands []CustomCommand - - entries, err := os.ReadDir(dir) - if err != nil { - return nil - } - +// FromSkillCatalog converts user-invocable catalog entries into custom +// command entries for the command palette. +func FromSkillCatalog(entries []skills.CatalogEntry) []CustomCommand { + commands := make([]CustomCommand, 0, len(entries)) for _, entry := range entries { - if !entry.IsDir() { + if !entry.UserInvocable { continue } - - skillPath := filepath.Join(dir, entry.Name(), skills.SkillFileName) - skill, err := skills.Parse(skillPath) - if err != nil { - continue + name := entry.Label + if name == "" { + name = userCommandPrefix + entry.Name } - - if !skill.UserInvocable { - continue - } - - name := prefix + skill.Name commands = append(commands, CustomCommand{ - ID: name, - Name: name, - Content: skill.Instructions, - Arguments: nil, - Skill: skill, + ID: name, + Name: name, + Skill: &skills.Skill{ + Name: entry.Name, + Description: entry.Description, + SkillFilePath: entry.ID, + }, }) } - return commands } diff --git a/internal/commands/commands_test.go b/internal/commands/commands_test.go index b3be29c4d1aceae99b372f5890713c6691bd1c0e..147f63119026fa6f70c0b1ed3a86ec492e912bdb 100644 --- a/internal/commands/commands_test.go +++ b/internal/commands/commands_test.go @@ -3,8 +3,10 @@ package commands import ( "os" "path/filepath" + "runtime" "testing" + "github.com/charmbracelet/crush/internal/skills" "github.com/stretchr/testify/require" ) @@ -51,3 +53,62 @@ func TestLoadAll_MixedSources(t *testing.T) { require.Len(t, cmds, 1) require.Equal(t, "user:cmd", cmds[0].ID) } + +func TestFromSkillCatalog_UserInvocableOnly(t *testing.T) { + t.Parallel() + + cmds := FromSkillCatalog([]skills.CatalogEntry{ + { + ID: "/skills/on/SKILL.md", + Name: "on", + Description: "Enabled.", + Label: "user:on", + UserInvocable: true, + }, + { + ID: "/skills/off/SKILL.md", + Name: "off", + Description: "Not invocable.", + Label: "user:off", + UserInvocable: false, + }, + }) + + require.Len(t, cmds, 1) + require.Equal(t, "user:on", cmds[0].ID) + require.Equal(t, "user:on", cmds[0].Name) + require.Equal(t, "on", cmds[0].Skill.Name) + require.Equal(t, "Enabled.", cmds[0].Skill.Description) + require.Equal(t, "/skills/on/SKILL.md", cmds[0].Skill.SkillFilePath) +} + +func TestFromSkillCatalog_UsesDiscoveredSymlinkedSkills(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("symlink creation requires special privileges on Windows") + } + t.Parallel() + + root := t.TempDir() + targetParent := t.TempDir() + targetSkillDir := filepath.Join(targetParent, "linked-skill") + require.NoError(t, os.MkdirAll(targetSkillDir, 0o755)) + require.NoError(t, os.WriteFile( + filepath.Join(targetSkillDir, skills.SkillFileName), + []byte("---\nname: linked-skill\ndescription: Symlinked.\nuser-invocable: true\n---\nUse me.\n"), + 0o644, + )) + + link := filepath.Join(root, "linked-skill") + require.NoError(t, os.Symlink(targetSkillDir, link)) + + _, activeSkills, _ := skills.DiscoverFromConfig(skills.DiscoveryConfig{ + SkillsPaths: []string{root}, + }) + entries := skills.Catalog(activeSkills, []string{root}, "") + cmds := FromSkillCatalog(entries) + + require.Len(t, cmds, 1) + require.Equal(t, "user:linked-skill", cmds[0].ID) + require.Equal(t, "linked-skill", cmds[0].Skill.Name) + require.Equal(t, filepath.Join(link, skills.SkillFileName), cmds[0].Skill.SkillFilePath) +} diff --git a/internal/proto/proto.go b/internal/proto/proto.go index 3e37f61def9cd15ea4884ca6535fb62af82431e8..2e2cebc6cad33b999ab2704c139ad0c4e86010ff 100644 --- a/internal/proto/proto.go +++ b/internal/proto/proto.go @@ -73,11 +73,12 @@ type RunComplete struct { // SkillInfo describes a visible skill exposed to a frontend. type SkillInfo struct { - ID string `json:"id"` - Name string `json:"name"` - Description string `json:"description"` - Label string `json:"label"` - Source string `json:"source"` + ID string `json:"id"` + Name string `json:"name"` + Description string `json:"description"` + Label string `json:"label"` + Source string `json:"source"` + UserInvocable bool `json:"user_invocable"` } // ReadSkillRequest is the request body for reading a skill's content. diff --git a/internal/skills/catalog.go b/internal/skills/catalog.go index c1af90ca8bf11e9be1f5993338362b19f4749d22..6dd6c9e3e817336c7340049db692f3db0598ad40 100644 --- a/internal/skills/catalog.go +++ b/internal/skills/catalog.go @@ -19,11 +19,12 @@ const ( // CatalogEntry describes an effective visible skill for frontend display. type CatalogEntry struct { - ID string `json:"id"` - Name string `json:"name"` - Description string `json:"description"` - Label string `json:"label"` - Source SourceType `json:"source"` + ID string `json:"id"` + Name string `json:"name"` + Description string `json:"description"` + Label string `json:"label"` + Source SourceType `json:"source"` + UserInvocable bool `json:"user_invocable"` } // SkillReadResult holds metadata about a skill returned alongside its @@ -48,11 +49,12 @@ func Catalog(active []*Skill, skillPaths []string, workingDir string) []CatalogE for _, skill := range active { label, source := skillLabel(skillPaths, workingDir, skill) entries = append(entries, CatalogEntry{ - ID: skill.SkillFilePath, - Name: skill.Name, - Description: skill.Description, - Label: label, - Source: source, + ID: skill.SkillFilePath, + Name: skill.Name, + Description: skill.Description, + Label: label, + Source: source, + UserInvocable: skill.UserInvocable, }) } return entries diff --git a/internal/ui/model/ui.go b/internal/ui/model/ui.go index 890dfc7de8a97eae13c4ecbd56ca07b566061408..63226d0a2f9ec951ccc0cf1167f4e06ac6c88c16 100644 --- a/internal/ui/model/ui.go +++ b/internal/ui/model/ui.go @@ -527,10 +527,12 @@ func (m *UI) loadCustomCommands() tea.Cmd { if err != nil { slog.Error("Failed to load custom commands", "error", err) } - // Append user-invocable skills as commands - skillCommands := commands.LoadSkillCommands() - skillCommands = append(skillCommands, commands.LoadProjectSkillCommands(m.com.Workspace.WorkingDir())...) - customCommands = append(customCommands, skillCommands...) + // Append user-invocable skills as commands. + skillEntries, err := m.com.Workspace.ListSkills(context.Background()) + if err != nil { + slog.Error("Failed to load skill commands", "error", err) + } + customCommands = append(customCommands, commands.FromSkillCatalog(skillEntries)...) return userCommandsLoadedMsg{Commands: customCommands} } } diff --git a/internal/workspace/client_workspace.go b/internal/workspace/client_workspace.go index 2018fab8a7dcc2cb3aeb0f44fc0920c1db72d852..3653046709c66f87c6b496e11dce2373a5ea40f2 100644 --- a/internal/workspace/client_workspace.go +++ b/internal/workspace/client_workspace.go @@ -506,11 +506,12 @@ func (w *ClientWorkspace) ListSkills(ctx context.Context) ([]skills.CatalogEntry result := make([]skills.CatalogEntry, len(entries)) for i, entry := range entries { result[i] = skills.CatalogEntry{ - ID: entry.ID, - Name: entry.Name, - Description: entry.Description, - Label: entry.Label, - Source: skills.SourceType(entry.Source), + ID: entry.ID, + Name: entry.Name, + Description: entry.Description, + Label: entry.Label, + Source: skills.SourceType(entry.Source), + UserInvocable: entry.UserInvocable, } } return result, nil