feat(ui): interactive Skills dialog with live enable/disable toggle - #3329
Open
joestump-agent wants to merge 14 commits into
Open
feat(ui): interactive Skills dialog with live enable/disable toggle#3329joestump-agent wants to merge 14 commits into
joestump-agent wants to merge 14 commits into
Conversation
Contributor
Author
|
Pushed one more commit: builtin skills ( |
Skill discovery was re-run from scratch on every prompt build, ignoring the already-built Manager. Combined with symlink-unaware dedup (keyed by raw path string), a single invalid skill could log "Skill validation failed" up to 10× per invocation. Three changes: 1. DiscoverWithStates: resolve symlinks before keying the seen map so the same file reached via two paths is parsed/validated/warned once. 2. Prompt: add WithSkills option so callers pass pre-discovered skills instead of re-walking the filesystem on every Build. 3. Coordinator + agentic-fetch: pass activeSkills via WithSkills at both prompt construction sites. Closes charmbracelet#2683 (symlink UI duplicate — same root cause). See charmbracelet#3318 for the full analysis.
Lets users pick up skill changes (new files, edits, deletions) without restarting the session. Available as "Reload Skills" in the command palette. The reload chain mirrors UpdateAgentModel: Manager.Reload re-runs discovery and publishes, coordinator.ReloadSkills propagates to the system prompt + tools + tracker, and the full Workspace interface (local + client-server) is wired. Wires WithDiscoveryConfig into both Manager construction sites so Reload has the config it needs.
Discovery involves filesystem I/O that could hang on a stalled NFS mount or broken symlink. 10s timeout matches the pattern used by UpdateAgentModel and other UI handlers.
logDiscoveryStats was only called from discoverSkills (the coordinator's fallback path that no production caller hits). Move the discovery-level logging into DiscoverFromConfig itself so startup AND reloads are logged. Split the coordinator-specific prompt/token stats into logPromptSkillStats and call it from NewCoordinator and ReloadSkills.
- Manager.Reload now takes context.Context and returns error (cancellation checked after discovery, before swap) - Add swagger annotations to /skills/reload endpoint - Add tests: Manager.Reload (discovery + cancellation), WithSkills (stores + nil fallback), ReloadSkills (nil manager guard) - Fix misleading skillsMgr comment - gofmt clean on all changed files
Both production callers (root.go, backend.go) always pass a Manager. The fallback path was dead code that existed only to handle a case that never happens in production. Removing it makes the Manager requirement explicit.
- agenttest.NewCoordinator now passes an empty skills.Manager instead of nil, matching the requirement that NewCoordinator receives one. - Fix gofumpt formatting on all one-liner ReloadSkills test stubs.
Two separate calls to AllSkills() + ActiveSkills() were a torn read — Reload could swap one slice between them. Reload now returns both, and SkillSnapshot provides the same atomic read without reload.
Three fixes from code review: 1. ReloadSkills now builds models+prompt BEFORE swapping coordinator state, so a mid-reload error leaves the agent on the old skills instead of a half-committed mix. 2. InitializePrompt now accepts pre-discovered skills via WithSkills, eliminating the filesystem walk that caused duplicate "Skill validation failed" warnings on /init. 3. Manager.Reload no longer sets m.states before PublishStates overwrites it — PublishStates is now the single mutation point as documented. Added TestManager_SkillSnapshot and TestManager_SkillSnapshot_AfterReload to verify atomic reads of both skill slices. 💘 Generated with Crush Assisted-by: Crush:glm-5.2
Adds a full-screen Skills management dialog accessible from the command palette. Lists all discovered skills — including disabled and errored ones — with their source, state, and user-invocable flag. Key bindings: - enter / ctrl+d: toggle a skill on/off (writes options.disabled_skills) - ctrl+r: reload skills from disk - type to filter, up/down to navigate The toggle updates the config and triggers an immediate skill reload, so changes take effect without restarting the session or editing config files by hand. Builds on the ReloadSkills / SkillSnapshot infrastructure from charmbracelet#3319.
Builtin skills (crush-config, crush-hooks, jq) now declare user-invocable: true so they surface in the palette like discovered user skills, with coverage for the catalog filter. Assisted-by: Claude Fable 5
joestump-agent
force-pushed
the
feat/skills-dialog
branch
from
August 16, 2026 06:15
92977da to
4fb1b0e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds an interactive Skills management dialog that lets users browse all discovered skills and toggle individual skills on/off live — no config-file editing, no restart.
Builds on the
ReloadSkills/SkillSnapshotinfrastructure from #3319.What it does
on/off/error), and user-invocable flagenter/ctrl+d: toggle the selected skill's enabled state by updatingoptions.disabled_skillsin config, then triggers an immediateReloadSkillsso the change takes effect instantlyctrl+r: reload skills from disk (picks up new/edited/deleted skill files)↑/↓to navigateWhy
Currently the only way to disable a skill is to hand-edit
options.disabled_skillsin the config file and restart the session. A live, per-skill toggle in a browsable dialog is a significant UX improvement — users can experiment with skills without leaving the TUI.Changes
internal/ui/dialog/skills.gointernal/ui/dialog/actions.goActionSkillToggleaction typeinternal/ui/dialog/commands.gointernal/ui/model/ui.goopenSkillsDialog()+toggleSkill()handler +ActionReloadSkillsalso closes Skills dialoginternal/workspace/workspace.goGetSkillStates()to theWorkspaceinterfaceinternal/workspace/app_workspace.goGetSkillStates()viaManager.States()internal/workspace/client_workspace.goGetSkillStates()via localManager.States()Dependencies
ReloadSkills(ctx),Manager.Reload(), andManager.SkillSnapshot()APIs introduced there. The branch includes fix: eliminate duplicate skill-discovery warnings; add /skills reload #3319's commits so CI passes; once fix: eliminate duplicate skill-discovery warnings; add /skills reload #3319 merges, this can rebase cleanly ontomain.Test plan
go build ./...passesgo test ./internal/ui/dialog/... ./internal/workspace/... ./internal/skills/...passesgo vetclean<available_skills>, toggle it back onctrl+rin the dialog, verify it appears💘 Generated with Crush