Skip to content

feat(skills): add dialog to command palette#2467

Closed
Amolith wants to merge 7 commits intocharmbracelet:builtin-skillsfrom
Amolith:add-skills-command-to-palette
Closed

feat(skills): add dialog to command palette#2467
Amolith wants to merge 7 commits intocharmbracelet:builtin-skillsfrom
Amolith:add-skills-command-to-palette

Conversation

@Amolith
Copy link
Copy Markdown
Contributor

@Amolith Amolith commented Mar 24, 2026

Adds a skills browsing dialog to the TUI command palette, letting users discover and attach both builtin and user-configured skills. Also renames the crush-config skill to gerund-prefixed configuring-crush and adds bits about checking versions and fetching more doc.

For now, just a demo of an idea for making skills user-invocable building on @meowgorithm's #2466 that embeds a skill for configuring crush. I'm sure there's more work to do and will mark it not a draft once I think it's ready.

Video_2026-03-23_19-27-50.mp4
  • I have read CONTRIBUTING.md.
  • I have created a discussion that was approved by a maintainer (for new features).

meowgorithm and others added 7 commits March 23, 2026 17:28
crush:// confused models into trying to load MCPs
Adds a skills browsing dialog to the TUI command palette, letting users
discover and attach both builtin and user-configured skills. Also renames the
crush-config skill to gerund-prefixed configuring-crush and adds bits about
checking versions and fetching more doc.
@Amolith
Copy link
Copy Markdown
Contributor Author

Amolith commented Apr 1, 2026

andreynering requested a review from meowgorithm yesterday

will rebase this and clean it up in the next few days 👍

@andreynering andreynering deleted the branch charmbracelet:builtin-skills April 2, 2026 20:37
@Amolith
Copy link
Copy Markdown
Contributor Author

Amolith commented Apr 2, 2026

@andreynering was closing this one intentional?

@andreynering
Copy link
Copy Markdown
Member

@Amolith Not really. This was a GitHub thing. It got closed because I merged the upstream branch:

Feel free to reopen targerting main, and sorry for the trouble.

}
}

func (s *Skills) previousSkillType() SkillSourceType { return s.nextSkillType() }
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a 3rd skill type is ever introduced then this is a bug waiting to happen. e.g. System, User, and Project.

Comment thread internal/skills/embed.go
relPath, _ := filepath.Rel("builtin", path)
relPath = filepath.ToSlash(relPath)
skill.SkillFilePath = BuiltinPrefix + relPath
skill.Path = BuiltinPrefix + filepath.Dir(relPath)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't DiscoverBuiltin also set the skill.Builtin bool?

require.Equal(t, "/crush/skills/configuring-crush/SKILL.md", s.SkillFilePath)
require.Equal(t, "/crush/skills/configuring-crush", s.Path)
require.NotEmpty(t, s.Description)
require.NotEmpty(t, s.Instructions)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should probably also assert s.Builtin

Comment thread internal/ui/model/ui.go
m.dialog.BringToFront(dialog.SkillsID)
return nil
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if i understand correctly we parse all the skills directory every time we open the dialog box. that seems fine for the initial work but we may want to have something that updates in the background only when something changes (fsnotify, ...).

@Amolith
Copy link
Copy Markdown
Contributor Author

Amolith commented Apr 14, 2026

@ehiggs Thanks for the feedback! This PR was closed and reopened as #2562, both marked as draft because it was a quick and dirty implementation. I have a heap of local changes cleaning it up that I've not pushed either here or there yet. I'll mark the other one ready for review once I feel it's at a good place :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants