feat(skills): list available skills when install runs non-interactively#13548
Conversation
There was a problem hiding this comment.
Pull request overview
Changes gh skill install <repo> behavior so that running it non-interactively without a skill name now lists the discovered skills (via tableprinter, falling back to tab-separated output when piped) instead of erroring out. This improves discoverability and composes with Unix tools.
Changes:
- Replace the non-interactive "must specify a skill name" error with a new
listAvailableSkillshelper that prints skills usingtableprinter; a sentinelerrSkillsListedletsinstallRun/runLocalInstallexit cleanly. - Update long help text and add a piping example to reflect the new behavior.
- Replace the old "errors" test with a remote listing assertion and add an equivalent local-install listing test.
Show a summary per file
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 0
Previously, running 'gh skill install <repo>' without a skill name in a non-interactive context returned an error. This made discovery awkward when piping or scripting: callers had to first run the command interactively to see what skills were available. Now, when no skill name is given and stdin/stdout is not a TTY, the discovered skills are printed as tab-separated 'SKILL\tDESCRIPTION' rows to stdout (matching the interactive picker contents) so they can be piped into grep, awk, fzf, etc. Re-running with a specific skill name performs the install as before. The same behavior applies to --from-local installs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
BagToad
force-pushed
the
sammorrowdrums/skill-install-nontty-listing
branch
from
aa28bca to
5af799f
Compare
| if opts.IO.IsStdoutTTY() { | ||
| fmt.Fprintf(opts.IO.ErrOut, "Showing %s from %s. Re-run with a skill name to install.\n\n", | ||
| text.Pluralize(len(skills), "skill"), sel.sourceHint) | ||
| } |
There was a problem hiding this comment.
🤔 I don't think this could ever fire in practice because we check if we can prompt before calling listAvailableSkills
There was a problem hiding this comment.
If I were being nit-picky, I'd say this decision should be one level higher up the stack - that's not really something we're causing here, but we are deepening it by making a longer branch in the noninteractive case. I think the noninteractive branch is different enough that it would be more natural to see it diverge before entering a function whose name suggests that we're selecting stuff. It also would eliminate the need for a sentinel error; we'd just do a print and early return without falling into this function instead.
BagToad
left a comment
BagToad
left a comment
There was a problem hiding this comment.
LGTM, nothing blocking.
If we could clean some of my comments up in a follow up eventually, I think that'd help tear down a bit of tech debt for editing these flows in the future 🙌
Summary
Running
gh skill install <repo>(or--from-local <dir>) without a skill name in a non-interactive context previously errored withmust specify a skill name when not running interactively. That made discovery awkward when piping or scripting — users had to switch to an interactive shell just to see what skills a repo offered.This PR changes that behavior so the discovered skills are printed instead, mirroring (approximately) what the interactive picker shows. The list is rendered through
tableprinter, so it auto-switches between a friendly TTY table and tab-separatedSKILL\tDESCRIPTIONrows when piped — making it easy to grep / fzf / awk.Before
After
Re-running with a specific skill name installs as before. Same behavior for
--from-local.Context
Came out of a Slack thread where this was identified as a UX gap: interactive mode supports browsing-then-installing, and non-interactive mode should provide the same discoverability so it composes with standard Unix tooling.
Changes
selectSkillsWithSelectornow prints discovered skills via a newlistAvailableSkillshelper instead of erroring when!canPrompt.errSkillsListedletsinstallRun/runLocalInstallbail cleanly without going into the install path.fetchDescriptionscallback the picker uses) so the listed output is useful, not bare.grepupdated.Verification
go test ./pkg/cmd/skills/...✅make lint✅