feat(ui): add skills discovery status to sidebar and landing page (#2384)

huaiyuWangh created

- Display loaded/errored skills in sidebar and landing page
- Use pubsub event to carry skill states directly to UI, avoiding
  global mutable state
- Subscribe to skills discovery events via broker, consistent with
  MCP/LSP patterns
- Rename State to DiscoveryState for clarity

Change summary

internal/app/app.go              |   2 
internal/skills/skills.go        |  48 ++++++++++++
internal/skills/skills_test.go   |  58 ++++++++++++++
internal/ui/model/landing.go     |   5 
internal/ui/model/sidebar.go     | 114 +++++++++++++++++++++---------
internal/ui/model/skills.go      | 127 ++++++++++++++++++++++++++++++++++
internal/ui/model/skills_test.go |  58 +++++++++++++++
internal/ui/model/ui.go          |  13 ++
8 files changed, 383 insertions(+), 42 deletions(-)

Detailed changes

internal/app/app.go πŸ”—

@@ -34,6 +34,7 @@ import (
 	"github.com/charmbracelet/crush/internal/pubsub"
 	"github.com/charmbracelet/crush/internal/session"
 	"github.com/charmbracelet/crush/internal/shell"
+	"github.com/charmbracelet/crush/internal/skills"
 	"github.com/charmbracelet/crush/internal/ui/anim"
 	"github.com/charmbracelet/crush/internal/ui/styles"
 	"github.com/charmbracelet/crush/internal/update"
@@ -481,6 +482,7 @@ func (app *App) setupEvents() {
 	setupSubscriber(ctx, app.serviceEventsWG, "agent-notifications", app.agentNotifications.Subscribe, app.events)
 	setupSubscriber(ctx, app.serviceEventsWG, "mcp", mcp.SubscribeEvents, app.events)
 	setupSubscriber(ctx, app.serviceEventsWG, "lsp", SubscribeLSPEvents, app.events)
+	setupSubscriber(ctx, app.serviceEventsWG, "skills", skills.SubscribeEvents, app.events)
 	cleanupFunc := func(context.Context) error {
 		cancel()
 		app.serviceEventsWG.Wait()

internal/skills/skills.go πŸ”—

@@ -3,6 +3,7 @@
 package skills
 
 import (
+	"context"
 	"errors"
 	"fmt"
 	"log/slog"
@@ -15,6 +16,7 @@ import (
 	"sync"
 
 	"github.com/charlievieth/fastwalk"
+	"github.com/charmbracelet/crush/internal/pubsub"
 	"gopkg.in/yaml.v3"
 )
 
@@ -43,6 +45,36 @@ type Skill struct {
 	Builtin       bool              `yaml:"-" json:"builtin"`
 }
 
+// DiscoveryState represents the outcome of discovering a single skill file.
+type DiscoveryState int
+
+const (
+	// StateNormal indicates the skill was parsed and validated successfully.
+	StateNormal DiscoveryState = iota
+	// StateError indicates discovery encountered a scan/parse/validate error.
+	StateError
+)
+
+// SkillState represents the latest discovery status of a skill file.
+type SkillState struct {
+	Name  string
+	Path  string
+	State DiscoveryState
+	Err   error
+}
+
+// Event is published when skill discovery completes.
+type Event struct {
+	States []*SkillState
+}
+
+var broker = pubsub.NewBroker[Event]()
+
+// SubscribeEvents returns a channel that receives events when skill discovery state changes.
+func SubscribeEvents(ctx context.Context) <-chan pubsub.Event[Event] {
+	return broker.Subscribe(ctx)
+}
+
 // Validate checks if the skill meets spec requirements.
 func (s *Skill) Validate() error {
 	var errs []error
@@ -141,8 +173,19 @@ func splitFrontmatter(content string) (frontmatter, body string, err error) {
 // Discover finds all valid skills in the given paths.
 func Discover(paths []string) []*Skill {
 	var skills []*Skill
+	var states []*SkillState
 	var mu sync.Mutex
 	seen := make(map[string]bool)
+	addState := func(name, path string, state DiscoveryState, err error) {
+		mu.Lock()
+		states = append(states, &SkillState{
+			Name:  name,
+			Path:  path,
+			State: state,
+			Err:   err,
+		})
+		mu.Unlock()
+	}
 
 	for _, base := range paths {
 		// We use fastwalk with Follow: true instead of filepath.WalkDir because
@@ -156,6 +199,7 @@ func Discover(paths []string) []*Skill {
 		err := fastwalk.Walk(&conf, base, func(path string, d os.DirEntry, err error) error {
 			if err != nil {
 				slog.Warn("Failed to walk skills path entry", "base", base, "path", path, "error", err)
+				addState("", path, StateError, err)
 				return nil
 			}
 			if d.IsDir() || d.Name() != SkillFileName {
@@ -171,16 +215,19 @@ func Discover(paths []string) []*Skill {
 			skill, err := Parse(path)
 			if err != nil {
 				slog.Warn("Failed to parse skill file", "path", path, "error", err)
+				addState("", path, StateError, err)
 				return nil
 			}
 			if err := skill.Validate(); err != nil {
 				slog.Warn("Skill validation failed", "path", path, "error", err)
+				addState(skill.Name, path, StateError, err)
 				return nil
 			}
 			slog.Debug("Successfully loaded skill", "name", skill.Name, "path", path)
 			mu.Lock()
 			skills = append(skills, skill)
 			mu.Unlock()
+			addState(skill.Name, path, StateNormal, nil)
 			return nil
 		})
 		if err != nil {
@@ -198,6 +245,7 @@ func Discover(paths []string) []*Skill {
 		return left < right
 	})
 
+	broker.Publish(pubsub.UpdatedEvent, Event{States: states})
 	return skills
 }
 

internal/skills/skills_test.go πŸ”—

@@ -1,6 +1,7 @@
 package skills
 
 import (
+	"context"
 	"os"
 	"path/filepath"
 	"strings"
@@ -215,8 +216,7 @@ func TestSkillValidate(t *testing.T) {
 }
 
 func TestDiscover(t *testing.T) {
-	t.Parallel()
-
+	// Not parallel: shares global broker with other Discover tests.
 	tmpDir := t.TempDir()
 
 	// Create valid skill 1.
@@ -248,7 +248,31 @@ description: Name doesn't match directory.
 ---
 `), 0o644))
 
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	ch := SubscribeEvents(ctx)
+
 	skills := Discover([]string{tmpDir})
+
+	evt := <-ch
+	states := evt.Payload.States
+	var normalCount int
+	var errorCount int
+	var hasInvalidDir bool
+	for _, state := range states {
+		if state.State == StateNormal {
+			normalCount++
+		}
+		if state.State == StateError {
+			errorCount++
+			if strings.Contains(state.Path, "invalid-dir") {
+				hasInvalidDir = true
+			}
+		}
+	}
+	require.Equal(t, 2, normalCount)
+	require.Equal(t, 1, errorCount)
+	require.True(t, hasInvalidDir)
 	require.Len(t, skills, 2)
 	require.Equal(t, []string{"skill-two", "skill-one"}, []string{skills[0].Name, skills[1].Name})
 
@@ -260,6 +284,36 @@ description: Name doesn't match directory.
 	require.True(t, names["skill-two"])
 }
 
+func TestDiscoverEmptyDir(t *testing.T) {
+	// Not parallel: shares global broker with other Discover tests.
+
+	tmpDir := t.TempDir()
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	ch := SubscribeEvents(ctx)
+
+	skills := Discover([]string{tmpDir})
+
+	evt := <-ch
+	require.Empty(t, evt.Payload.States)
+	require.Empty(t, skills)
+}
+
+func TestDiscoverMissingPath(t *testing.T) {
+	// Not parallel: shares global broker with other Discover tests.
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	ch := SubscribeEvents(ctx)
+
+	skills := Discover([]string{filepath.Join(t.TempDir(), "missing")})
+
+	evt := <-ch
+	require.Empty(t, evt.Payload.States)
+	require.Empty(t, skills)
+}
+
 func TestToPromptXML(t *testing.T) {
 	t.Parallel()
 

internal/ui/model/landing.go πŸ”—

@@ -39,12 +39,13 @@ func (m *UI) landingView() string {
 		layout.Fill(1),
 	).Split(m.layout.main).Assign(new(image.Rectangle), &remainingHeightArea)
 
-	mcpLspSectionWidth := min(30, (width-1)/2)
+	mcpLspSectionWidth := min(30, (width-2)/3)
 
 	lspSection := m.lspInfo(mcpLspSectionWidth, max(1, remainingHeightArea.Dy()), false)
 	mcpSection := m.mcpInfo(mcpLspSectionWidth, max(1, remainingHeightArea.Dy()), false)
+	skillsSection := m.skillsInfo(mcpLspSectionWidth, max(1, remainingHeightArea.Dy()), false)
 
-	content := lipgloss.JoinHorizontal(lipgloss.Left, lspSection, " ", mcpSection)
+	content := lipgloss.JoinHorizontal(lipgloss.Left, lspSection, " ", mcpSection, " ", skillsSection)
 
 	return lipgloss.NewStyle().
 		Width(width).

internal/ui/model/sidebar.go πŸ”—

@@ -56,51 +56,73 @@ func (m *UI) modelInfo(width int) string {
 	return common.ModelInfo(m.com.Styles, modelName, providerName, reasoningInfo, modelContext, width)
 }
 
-// getDynamicHeightLimits will give us the num of items to show in each section based on the hight
+// getDynamicHeightLimits will give us the num of items to show in each section based on the height
 // some items are more important than others.
-func getDynamicHeightLimits(availableHeight int) (maxFiles, maxLSPs, maxMCPs int) {
+func getDynamicHeightLimits(availableHeight, fileCount, lspCount, mcpCount, skillCount int) (maxFiles, maxLSPs, maxMCPs, maxSkills int) {
 	const (
-		minItemsPerSection      = 2
-		defaultMaxFilesShown    = 10
-		defaultMaxLSPsShown     = 8
-		defaultMaxMCPsShown     = 8
+		minItemsPerSection = 2
+		// Keep these high so dynamic layout uses available sidebar space
+		// instead of hitting small hard limits.
+		defaultMaxFilesShown    = 1000
+		defaultMaxLSPsShown     = 1000
+		defaultMaxMCPsShown     = 1000
+		defaultMaxSkillsShown   = 1000
 		minAvailableHeightLimit = 10
 	)
 
-	// If we have very little space, use minimum values
 	if availableHeight < minAvailableHeightLimit {
-		return minItemsPerSection, minItemsPerSection, minItemsPerSection
+		return minItemsPerSection, minItemsPerSection, minItemsPerSection, minItemsPerSection
 	}
 
-	// Distribute available height among the three sections
-	// Give priority to files, then LSPs, then MCPs
-	totalSections := 3
-	heightPerSection := availableHeight / totalSections
-
-	// Calculate limits for each section, ensuring minimums
-	maxFiles = max(minItemsPerSection, min(defaultMaxFilesShown, heightPerSection))
-	maxLSPs = max(minItemsPerSection, min(defaultMaxLSPsShown, heightPerSection))
-	maxMCPs = max(minItemsPerSection, min(defaultMaxMCPsShown, heightPerSection))
-
-	// If we have extra space, give it to files first
-	remainingHeight := availableHeight - (maxFiles + maxLSPs + maxMCPs)
-	if remainingHeight > 0 {
-		extraForFiles := min(remainingHeight, defaultMaxFilesShown-maxFiles)
-		maxFiles += extraForFiles
-		remainingHeight -= extraForFiles
-
-		if remainingHeight > 0 {
-			extraForLSPs := min(remainingHeight, defaultMaxLSPsShown-maxLSPs)
-			maxLSPs += extraForLSPs
-			remainingHeight -= extraForLSPs
-
-			if remainingHeight > 0 {
-				maxMCPs += min(remainingHeight, defaultMaxMCPsShown-maxMCPs)
+	maxFiles = minItemsPerSection
+	maxLSPs = minItemsPerSection
+	maxMCPs = minItemsPerSection
+	maxSkills = minItemsPerSection
+
+	remainingHeight := max(0, availableHeight-(minItemsPerSection*4))
+
+	sectionValues := []*int{&maxFiles, &maxLSPs, &maxMCPs, &maxSkills}
+	sectionCaps := []int{defaultMaxFilesShown, defaultMaxLSPsShown, defaultMaxMCPsShown, defaultMaxSkillsShown}
+	sectionNeeds := []int{max(0, fileCount-maxFiles), max(0, lspCount-maxLSPs), max(0, mcpCount-maxMCPs), max(0, skillCount-maxSkills)}
+
+	for remainingHeight > 0 {
+		allocated := false
+		for i, section := range sectionValues {
+			if remainingHeight == 0 {
+				break
+			}
+			if sectionNeeds[i] == 0 || *section >= sectionCaps[i] {
+				continue
+			}
+			*section = *section + 1
+			sectionNeeds[i]--
+			remainingHeight--
+			allocated = true
+		}
+		if !allocated {
+			break
+		}
+	}
+
+	for remainingHeight > 0 {
+		allocated := false
+		for i, section := range sectionValues {
+			if remainingHeight == 0 {
+				break
+			}
+			if *section >= sectionCaps[i] {
+				continue
 			}
+			*section = *section + 1
+			remainingHeight--
+			allocated = true
+		}
+		if !allocated {
+			break
 		}
 	}
 
-	return maxFiles, maxLSPs, maxMCPs
+	return maxFiles, maxLSPs, maxMCPs, maxSkills
 }
 
 // sidebar renders the chat sidebar containing session title, working
@@ -142,11 +164,31 @@ func (m *UI) drawSidebar(scr uv.Screen, area uv.Rectangle) {
 		layout.Len(lipgloss.Height(sidebarHeader)),
 		layout.Fill(1),
 	).Split(m.layout.sidebar).Assign(new(image.Rectangle), &remainingHeightArea)
-	remainingHeight := remainingHeightArea.Dy() - 10
-	maxFiles, maxLSPs, maxMCPs := getDynamicHeightLimits(remainingHeight)
+	remainingHeight := remainingHeightArea.Dy() - 6
+	filesCount := 0
+	for _, f := range m.sessionFiles {
+		if f.Additions == 0 && f.Deletions == 0 {
+			continue
+		}
+		filesCount++
+	}
+
+	lspsCount := len(m.lspStates)
+
+	mcpsCount := 0
+	for _, mcpCfg := range m.com.Config().MCP.Sorted() {
+		if _, ok := m.mcpStates[mcpCfg.Name]; ok {
+			mcpsCount++
+		}
+	}
+
+	skillsCount := len(m.skillStatusItems())
+
+	maxFiles, maxLSPs, maxMCPs, maxSkills := getDynamicHeightLimits(remainingHeight, filesCount, lspsCount, mcpsCount, skillsCount)
 
 	lspSection := m.lspInfo(width, maxLSPs, true)
 	mcpSection := m.mcpInfo(width, maxMCPs, true)
+	skillsSection := m.skillsInfo(width, maxSkills, true)
 	filesSection := m.filesInfo(m.com.Workspace.WorkingDir(), width, maxFiles, true)
 
 	uv.NewStyledString(
@@ -162,6 +204,8 @@ func (m *UI) drawSidebar(scr uv.Screen, area uv.Rectangle) {
 					lspSection,
 					"",
 					mcpSection,
+					"",
+					skillsSection,
 				),
 			),
 	).Draw(scr, area)

internal/ui/model/skills.go πŸ”—

@@ -0,0 +1,127 @@
+package model
+
+import (
+	"fmt"
+	"path/filepath"
+	"slices"
+	"strings"
+	"sync"
+
+	"charm.land/lipgloss/v2"
+	"github.com/charmbracelet/crush/internal/skills"
+	"github.com/charmbracelet/crush/internal/ui/common"
+	"github.com/charmbracelet/crush/internal/ui/styles"
+)
+
+type skillStatusItem struct {
+	icon  string
+	name  string
+	title string
+	// description is reserved for future use (e.g. showing error details).
+	description string
+}
+
+var builtinSkillsCache struct {
+	once   sync.Once
+	skills []*skills.Skill
+}
+
+func cachedBuiltinSkills() []*skills.Skill {
+	builtinSkillsCache.once.Do(func() {
+		builtinSkillsCache.skills = skills.DiscoverBuiltin()
+	})
+	return builtinSkillsCache.skills
+}
+
+// skillsInfo renders the skill discovery status section showing loaded and
+// invalid skills.
+func (m *UI) skillsInfo(width, maxItems int, isSection bool) string {
+	t := m.com.Styles
+
+	title := t.ResourceGroupTitle.Render("Skills")
+	if isSection {
+		title = common.Section(t, title, width)
+	}
+
+	items := m.skillStatusItems()
+	if len(items) == 0 {
+		list := t.ResourceAdditionalText.Render("None")
+		return lipgloss.NewStyle().Width(width).Render(fmt.Sprintf("%s\n\n%s", title, list))
+	}
+
+	list := skillsList(t, items, width, maxItems)
+	return lipgloss.NewStyle().Width(width).Render(fmt.Sprintf("%s\n\n%s", title, list))
+}
+
+func (m *UI) skillStatusItems() []skillStatusItem {
+	t := m.com.Styles
+	var items []skillStatusItem
+	stateNames := make(map[string]struct{}, len(m.skillStates))
+
+	states := slices.Clone(m.skillStates)
+	slices.SortStableFunc(states, func(a, b *skills.SkillState) int {
+		return strings.Compare(a.Path, b.Path)
+	})
+	for _, state := range states {
+		name := state.Name
+		if name == "" {
+			name = filepath.Base(filepath.Dir(state.Path))
+		}
+		stateNames[name] = struct{}{}
+		icon := t.ResourceOnlineIcon.String()
+		if state.State == skills.StateError {
+			icon = t.ResourceErrorIcon.String()
+		}
+		items = append(items, skillStatusItem{
+			icon:  icon,
+			name:  name,
+			title: t.ResourceName.Render(name),
+		})
+	}
+
+	builtin := cachedBuiltinSkills()
+	slices.SortStableFunc(builtin, func(a, b *skills.Skill) int {
+		return strings.Compare(a.Name, b.Name)
+	})
+	for _, skill := range builtin {
+		if _, ok := stateNames[skill.Name]; ok {
+			continue
+		}
+		items = append(items, skillStatusItem{
+			icon:  t.ResourceOnlineIcon.String(),
+			name:  skill.Name,
+			title: t.ResourceName.Render(skill.Name),
+		})
+	}
+
+	slices.SortStableFunc(items, func(a, b skillStatusItem) int {
+		return strings.Compare(a.name, b.name)
+	})
+
+	return items
+}
+
+func skillsList(t *styles.Styles, items []skillStatusItem, width, maxItems int) string {
+	if maxItems <= 0 {
+		return ""
+	}
+
+	if len(items) > maxItems {
+		visibleItems := items[:maxItems-1]
+		remaining := len(items) - (maxItems - 1)
+		items = append(visibleItems, skillStatusItem{
+			name:  "more",
+			title: t.ResourceAdditionalText.Render(fmt.Sprintf("…and %d more", remaining)),
+		})
+	}
+
+	renderedItems := make([]string, 0, len(items))
+	for _, item := range items {
+		renderedItems = append(renderedItems, common.Status(t, common.StatusOpts{
+			Icon:        item.icon,
+			Title:       item.title,
+			Description: item.description,
+		}, width))
+	}
+	return lipgloss.JoinVertical(lipgloss.Left, renderedItems...)
+}

internal/ui/model/skills_test.go πŸ”—

@@ -0,0 +1,58 @@
+package model
+
+import (
+	"testing"
+
+	"github.com/charmbracelet/crush/internal/skills"
+	"github.com/charmbracelet/crush/internal/ui/common"
+	uistyles "github.com/charmbracelet/crush/internal/ui/styles"
+	"github.com/stretchr/testify/require"
+)
+
+// TestSkillStatusItemsIncludesBuiltinSkills verifies sidebar skills include
+// both runtime-discovered skill states and builtin skills that may not have
+// emitted a SkillState event yet.
+func TestSkillStatusItemsIncludesBuiltinSkills(t *testing.T) {
+	t.Parallel()
+
+	st := uistyles.DefaultStyles()
+	ui := &UI{
+		com: &common.Common{Styles: &st},
+		skillStates: []*skills.SkillState{
+			{Name: "go-doc", Path: "/tmp/go-doc/SKILL.md", State: skills.StateNormal},
+		},
+	}
+
+	items := ui.skillStatusItems()
+	require.NotEmpty(t, items)
+
+	var hasGoDoc bool
+	for _, item := range items {
+		if item.title == st.ResourceName.Render("go-doc") {
+			hasGoDoc = true
+			break
+		}
+	}
+	require.True(t, hasGoDoc)
+
+	builtinSkills := skills.DiscoverBuiltin()
+	require.NotEmpty(t, builtinSkills)
+
+	var hasBuiltin bool
+	for _, skill := range builtinSkills {
+		if skill.Name == "go-doc" {
+			continue
+		}
+		expected := st.ResourceName.Render(skill.Name)
+		for _, item := range items {
+			if item.title == expected {
+				hasBuiltin = true
+				break
+			}
+		}
+		if hasBuiltin {
+			break
+		}
+	}
+	require.True(t, hasBuiltin)
+}

internal/ui/model/ui.go πŸ”—

@@ -38,6 +38,7 @@ import (
 	"github.com/charmbracelet/crush/internal/permission"
 	"github.com/charmbracelet/crush/internal/pubsub"
 	"github.com/charmbracelet/crush/internal/session"
+	"github.com/charmbracelet/crush/internal/skills"
 	"github.com/charmbracelet/crush/internal/ui/anim"
 	"github.com/charmbracelet/crush/internal/ui/attachments"
 	"github.com/charmbracelet/crush/internal/ui/chat"
@@ -220,6 +221,9 @@ type UI struct {
 	// mcp
 	mcpStates map[string]mcp.ClientInfo
 
+	// skills
+	skillStates []*skills.SkillState
+
 	// sidebarLogo keeps a cached version of the sidebar sidebarLogo.
 	sidebarLogo string
 
@@ -619,6 +623,8 @@ func (m *UI) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 		cmds = append(cmds, m.handleFileEvent(msg.Payload))
 	case pubsub.Event[app.LSPEvent]:
 		m.lspStates = app.GetLSPStates()
+	case pubsub.Event[skills.Event]:
+		m.skillStates = msg.Payload.States
 	case pubsub.Event[mcp.Event]:
 		switch msg.Payload.Type {
 		case mcp.EventStateChanged:
@@ -3535,13 +3541,14 @@ func (m *UI) drawSessionDetails(scr uv.Screen, area uv.Rectangle) {
 	remainingHeight := height - lipgloss.Height(detailsHeader) - lipgloss.Height(version)
 
 	const maxSectionWidth = 50
-	sectionWidth := min(maxSectionWidth, width/3-2) // account for 2 spaces
-	maxItemsPerSection := remainingHeight - 3       // Account for section title and spacing
+	sectionWidth := max(1, min(maxSectionWidth, width/4-2)) // account for spacing between sections
+	maxItemsPerSection := remainingHeight - 3               // Account for section title and spacing
 
 	lspSection := m.lspInfo(sectionWidth, maxItemsPerSection, false)
 	mcpSection := m.mcpInfo(sectionWidth, maxItemsPerSection, false)
+	skillsSection := m.skillsInfo(sectionWidth, maxItemsPerSection, false)
 	filesSection := m.filesInfo(m.com.Workspace.WorkingDir(), sectionWidth, maxItemsPerSection, false)
-	sections := lipgloss.JoinHorizontal(lipgloss.Top, filesSection, " ", lspSection, " ", mcpSection)
+	sections := lipgloss.JoinHorizontal(lipgloss.Top, filesSection, " ", lspSection, " ", mcpSection, " ", skillsSection)
 	uv.NewStyledString(
 		s.CompactDetails.View.
 			Width(area.Dx()).