improve test coverage/cleanup

This commit is contained in:
tommaso-moro 2026-03-30 17:32:23 +01:00 committed by Sam Morrow
parent 5d049cb897
commit 758785b8f4
No known key found for this signature in database
16 changed files with 2055 additions and 3 deletions

View file

@ -14,9 +14,9 @@ import (
"math/rand"
"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/v2/internal/ghcmd"
"github.com/cli/go-internal/testscript"
"github.com/MakeNowJust/heredoc"
)
func ghMain() int {
@ -434,3 +434,11 @@ func (e *testScriptEnv) fromEnv() error {
return nil
}
func TestSkills(t *testing.T) {
var tsEnv testScriptEnv
if err := tsEnv.fromEnv(); err != nil {
t.Fatal(err)
}
testscript.Run(t, testScriptParamsFor(tsEnv, "skills"))
}

0
acceptance/testdata/skills/.gitkeep vendored Normal file
View file

View file

@ -0,0 +1,3 @@
# Install with "add" alias
exec gh skills add github/awesome-copilot git-commit --scope user --force --agent github-copilot
stdout 'Installed git-commit'

View file

@ -0,0 +1,5 @@
# Install all skills from a repo with mixed conventions (skills/ + plugins/)
# This previously failed with "conflicting names" — now uses namespaced dirs
exec gh skills install github/awesome-copilot --all --scope user --force --agent github-copilot
stdout 'Installed'
! stderr 'conflicting names'

View file

@ -0,0 +1,8 @@
# Install --all should handle skills with same name across conventions
# (skills/ and plugins/ directories) without collision errors
exec gh skills install github/awesome-copilot --all --force --dir $WORK/scope-test --agent github-copilot
stdout 'Installed'
! stderr 'conflicting names'
# Verify skills were installed successfully
exists $WORK/scope-test/git-commit/SKILL.md

View file

@ -0,0 +1,11 @@
# Install with --force should overwrite an existing skill without error
exec gh skills install github/awesome-copilot git-commit --force --dir $WORK/force-test
stdout 'Installed git-commit'
# Install again with --force — should succeed (overwrite)
exec gh skills install github/awesome-copilot git-commit --force --dir $WORK/force-test
stdout 'Installed git-commit'
# Without --force, non-interactive should fail when skill exists
! exec gh skills install github/awesome-copilot git-commit --dir $WORK/force-test
stderr 'already installed'

View file

@ -0,0 +1,7 @@
# Install with --pin to a specific ref
exec gh skills install github/awesome-copilot git-commit --scope user --force --pin main
stdout 'Installed git-commit'
# Install without --pin should resolve latest version
exec gh skills install github/awesome-copilot git-commit --scope user --force
stdout 'Installed git-commit'

View file

@ -0,0 +1,9 @@
# Install with --scope project (default) inside a git repo
exec git init $WORK/myrepo
exec gh skills install github/awesome-copilot git-commit --scope project --force --agent github-copilot --dir $WORK/myrepo/.github/skills
stdout 'Installed git-commit'
exists $WORK/myrepo/.github/skills/git-commit/SKILL.md
# Install with --scope user
exec gh skills install github/awesome-copilot git-commit --scope user --force --agent github-copilot
stdout 'Installed git-commit'

View file

@ -0,0 +1,10 @@
# Install a single skill from a public repo
exec gh skills install github/awesome-copilot git-commit --scope user --force --agent github-copilot
stdout 'Installed git-commit'
# Install with --dir to a custom directory
exec gh skills install github/awesome-copilot git-commit --force --dir $WORK/custom-skills
stdout 'Installed git-commit'
# Verify the skill was written to the custom directory
exists $WORK/custom-skills/git-commit/SKILL.md

View file

@ -573,8 +573,8 @@ func FetchBlob(client RESTClient, host, owner, repo, sha string) (string, error)
return "", fmt.Errorf("unexpected blob encoding: %s", blob.Encoding)
}
// GitHub API returns base64 with embedded newlines; use the lenient
// RawStdEncoding decoder via a reader to handle them transparently.
// GitHub API returns base64 with embedded newlines; use the StdEncoding
// decoder via a reader to handle them transparently.
decoded, err := io.ReadAll(base64.NewDecoder(base64.StdEncoding, strings.NewReader(blob.Content)))
if err != nil {
return "", fmt.Errorf("could not decode blob content: %w", err)
@ -615,6 +615,10 @@ func DiscoverLocalSkills(dir string) ([]Skill, error) {
if walkErr != nil {
return walkErr
}
// Skip symlinks to avoid following links outside the source tree.
if info.Mode()&os.ModeSymlink != 0 {
return nil
}
if info.IsDir() || info.Name() != "SKILL.md" {
return nil
}

View file

@ -50,6 +50,9 @@ type skillResult struct {
func Install(opts *Options) (*Result, error) {
targetDir := opts.Dir
if targetDir == "" {
if opts.AgentHost == nil {
return nil, fmt.Errorf("either Dir or AgentHost must be specified")
}
var err error
targetDir, err = opts.AgentHost.InstallDir(opts.Scope, opts.GitRoot, opts.HomeDir)
if err != nil {
@ -146,6 +149,9 @@ type LocalOptions struct {
func InstallLocal(opts *LocalOptions) (*Result, error) {
targetDir := opts.Dir
if targetDir == "" {
if opts.AgentHost == nil {
return nil, fmt.Errorf("either Dir or AgentHost must be specified")
}
var err error
targetDir, err = opts.AgentHost.InstallDir(opts.Scope, opts.GitRoot, opts.HomeDir)
if err != nil {

View file

@ -152,6 +152,11 @@ func acquireLock() (unlock func()) {
f.Close()
return func() { os.Remove(lkPath) }
}
// Only retry when the lock file already exists (concurrent process).
// For other errors (permission denied, invalid path, etc.) give up immediately.
if !os.IsExist(createErr) {
return func() {}
}
// Break stale locks older than 30s (e.g. from a crashed process).
if info, statErr := os.Stat(lkPath); statErr == nil && time.Since(info.ModTime()) > 30*time.Second {
os.Remove(lkPath)

View file

@ -0,0 +1,993 @@
package install
import (
"errors"
"fmt"
"io"
"net/http"
"os"
"path/filepath"
"sort"
"strings"
"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/v2/api"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/internal/prompter"
"github.com/cli/cli/v2/internal/skills"
"github.com/cli/cli/v2/internal/skills/discovery"
"github.com/cli/cli/v2/internal/skills/frontmatter"
"github.com/cli/cli/v2/internal/skills/gitclient"
"github.com/cli/cli/v2/internal/skills/hosts"
"github.com/cli/cli/v2/internal/skills/installer"
"github.com/cli/cli/v2/internal/text"
"github.com/cli/cli/v2/pkg/cmdutil"
"github.com/cli/cli/v2/pkg/iostreams"
"github.com/spf13/cobra"
)
const (
// allSkillsKey is the persistent option label for selecting all skills.
allSkillsKey = "(all skills)"
// maxSearchResults caps how many skills are shown per search page in
// interactive selection, keeping the prompt readable.
maxSearchResults = 30
)
// installOptions holds all dependencies and user-provided flags for the install command.
type installOptions struct {
IO *iostreams.IOStreams
HttpClient func() (*http.Client, error)
Prompter prompter.Prompter
GitClient installGitClient
// Arguments
SkillSource string // owner/repo or local path
SkillName string // skill name, possibly with @version
// Flags
Agent string // --agent flag
Scope string // --scope flag
ScopeChanged bool // true when --scope was explicitly set
Pin string // --pin flag
Dir string // --dir flag (overrides host+scope)
All bool // --all flag
Force bool // --force flag
// Resolved at runtime
repo ghrepo.Interface // set when SkillSource is a GitHub repository
localPath string // set when SkillSource is a local directory
version string
}
// installGitClient is the git interface needed by the install command.
type installGitClient interface {
gitclient.RootResolver
gitclient.RemoteResolver
}
// NewCmdInstall creates the "skills install" command.
func NewCmdInstall(f *cmdutil.Factory, runF func(*installOptions) error) *cobra.Command {
opts := &installOptions{
IO: f.IOStreams,
Prompter: f.Prompter,
GitClient: &gitclient.FactoryClient{F: f},
HttpClient: f.HttpClient,
}
cmd := &cobra.Command{
Use: "install <repository> [<skill[@version]>]",
Short: "Install agent skills from a GitHub repository",
Long: heredoc.Docf(`
Install agent skills from a GitHub repository or local directory into
your local environment. Skills are placed in a host-specific directory
at either project scope (inside the current git repository) or user
scope (in your home directory, available everywhere):
Host Project User
GitHub Copilot .github/skills ~/.copilot/skills
Claude Code .claude/skills ~/.claude/skills
Cursor .cursor/skills ~/.cursor/skills
Codex .agents/skills ~/.codex/skills
Gemini CLI .agent/skills ~/.gemini/skills
Antigravity .agent/skills ~/.gemini/antigravity/skills
Use %[1]s--agent%[1]s and %[1]s--scope%[1]s to control placement, or %[1]s--dir%[1]s for a
custom directory. The default scope is %[1]sproject%[1]s, and the default
agent is %[1]sgithub-copilot%[1]s (when running non-interactively).
The first argument can be a GitHub repository in %[1]sOWNER/REPO%[1]s format
or a local directory path (e.g. %[1]s.%[1]s, %[1]s./my-skills%[1]s, %[1]s~/skills%[1]s).
For local directories, skills are auto-discovered using the same
conventions as remote repositories, and files are copied (not symlinked)
with local-path tracking metadata injected into frontmatter.
Skills are discovered automatically using the %[1]sskills/*/SKILL.md%[1]s convention
defined by the Agent Skills specification. For more information on the specification,
see: https://agentskills.io/specification
The skill argument can be a name, a namespaced name (%[1]sauthor/skill%[1]s),
or an exact path within the repository (%[1]sskills/author/skill%[1]s or
%[1]sskills/author/skill/SKILL.md%[1]s).
Performance tip: when installing from a large repository with many
skills, providing an exact path instead of a skill name avoids a
full tree traversal of the repository, making the install significantly faster.
When a skill name is provided without a version, the CLI resolves the
version in this order:
1. Latest tagged release in the repository
2. Default branch HEAD
To pin to a specific version, either append %[1]s@VERSION%[1]s to the skill
name or use the %[1]s--pin%[1]s flag. The version is resolved as a git tag or commit SHA.
Installed skills have GitHub tracking metadata injected into their
frontmatter (%[1]sgithub-owner%[1]s, %[1]sgithub-repo%[1]s, %[1]sgithub-ref%[1]s,
%[1]sgithub-sha%[1]s, %[1]sgithub-tree-sha%[1]s, %[1]sgithub-path%[1]s). This
metadata identifies the source repository and enables %[1]sgh skills update%[1]s
to detect changes the tree SHA serves as an ETag for staleness checks.
When run interactively, the command prompts for any missing arguments.
When run non-interactively, %[1]srepository%[1]s is required, and either a
skill name or %[1]s--all%[1]s must be specified.
`, "`"),
Example: heredoc.Doc(`
# Interactive: choose repo, skill, and agent
$ gh skills install
# Choose a skill from the repo interactively
$ gh skills install github/awesome-copilot
# Install a specific skill
$ gh skills install github/awesome-copilot git-commit
# Install a specific version
$ gh skills install github/awesome-copilot git-commit@v1.2.0
# Install all skills from a repo
$ gh skills install github/awesome-copilot --all
# Install from a large namespaced repo by path (efficient, skips full discovery)
$ gh skills install github/awesome-copilot skills/monalisa/code-review
# Install from a local directory (auto-discovers skills)
$ gh skills install ./my-skills-repo
# Install from current directory
$ gh skills install .
# Install a single local skill directory
$ gh skills install ./skills/git-commit
# Install for Claude Code at user scope
$ gh skills install github/awesome-copilot git-commit --agent claude-code --scope user
# Pin to a specific git ref
$ gh skills install github/awesome-copilot git-commit --pin v2.0.0
`),
Aliases: []string{"add"},
Args: cobra.MaximumNArgs(2),
RunE: func(cmd *cobra.Command, args []string) error {
if len(args) == 0 && !opts.IO.CanPrompt() {
return cmdutil.FlagErrorf("must specify a repository to install from")
}
if len(args) >= 1 {
opts.SkillSource = args[0]
}
if len(args) >= 2 {
opts.SkillName = args[1]
}
opts.ScopeChanged = cmd.Flags().Changed("scope")
// Resolve the source type early so installRun can branch directly.
if isLocalPath(opts.SkillSource) {
opts.localPath = opts.SkillSource
}
if opts.Agent != "" {
if _, err := hosts.FindByID(opts.Agent); err != nil {
return cmdutil.FlagErrorf("invalid value for --agent: %s", err)
}
}
if opts.Pin != "" && opts.SkillName != "" && strings.Contains(opts.SkillName, "@") {
return cmdutil.FlagErrorf("cannot use --pin with an inline @version in the skill name")
}
if runF != nil {
return runF(opts)
}
return installRun(opts)
},
}
cmd.Flags().StringVar(&opts.Agent, "agent", "", fmt.Sprintf("target agent (%s)", hosts.ValidHostIDs()))
_ = cmd.RegisterFlagCompletionFunc("agent", func(_ *cobra.Command, _ []string, _ string) ([]string, cobra.ShellCompDirective) {
return hosts.HostIDs(), cobra.ShellCompDirectiveNoFileComp
})
cmdutil.StringEnumFlag(cmd, &opts.Scope, "scope", "", "project", []string{"project", "user"}, "Installation scope")
cmd.Flags().StringVar(&opts.Pin, "pin", "", "pin to a specific git tag or commit SHA")
cmd.Flags().StringVar(&opts.Dir, "dir", "", "install to a custom directory (overrides --agent and --scope)")
cmd.Flags().BoolVar(&opts.All, "all", false, "install all skills from the repository")
cmd.Flags().BoolVarP(&opts.Force, "force", "f", false, "overwrite existing skills without prompting")
return cmd
}
func installRun(opts *installOptions) error {
cs := opts.IO.ColorScheme()
canPrompt := opts.IO.CanPrompt()
if opts.localPath != "" {
return runLocalInstall(opts)
}
repo, source, err := resolveRepoArg(opts.SkillSource, canPrompt, opts.Prompter)
if err != nil {
return err
}
opts.repo = repo
opts.SkillSource = source
parseSkillFromOpts(opts)
httpClient, err := opts.HttpClient()
if err != nil {
return err
}
apiClient := api.NewClientFromHTTP(httpClient)
hostname := opts.repo.RepoHost()
resolved, err := resolveVersion(opts, apiClient, hostname)
if err != nil {
return err
}
var selectedSkills []discovery.Skill
if isSkillPath(opts.SkillName) {
opts.IO.StartProgressIndicatorWithLabel("Looking up skill")
skill, err := discovery.DiscoverSkillByPath(apiClient, hostname, opts.repo.RepoOwner(), opts.repo.RepoName(), resolved.SHA, opts.SkillName)
opts.IO.StopProgressIndicator()
if err != nil {
return err
}
selectedSkills = []discovery.Skill{*skill}
} else {
skills, err := discoverSkills(opts, apiClient, hostname, resolved)
if err != nil {
return err
}
selectedSkills, err = selectSkillsWithSelector(opts, skills, canPrompt, skillSelector{
matchByName: matchSkillByName,
sourceHint: ghrepo.FullName(opts.repo),
fetchDescriptions: func() {
opts.IO.StartProgressIndicatorWithLabel("Fetching skill info")
discovery.FetchDescriptionsConcurrent(apiClient, hostname, opts.repo.RepoOwner(), opts.repo.RepoName(), skills, nil)
opts.IO.StopProgressIndicator()
},
})
if err != nil {
return err
}
}
selectedHosts, err := resolveHosts(opts, canPrompt)
if err != nil {
return err
}
scope, err := resolveScope(opts, canPrompt)
if err != nil {
return err
}
gitRoot := gitclient.ResolveGitRoot(opts.GitClient)
homeDir := gitclient.ResolveHomeDir()
source = ghrepo.FullName(opts.repo)
type hostPlan struct {
host *hosts.Host
skills []discovery.Skill
}
var plans []hostPlan
for _, host := range selectedHosts {
installSkills, err := checkOverwrite(opts, selectedSkills, host, scope, gitRoot, homeDir, canPrompt)
if err != nil {
return err
}
if len(installSkills) == 0 {
fmt.Fprintf(opts.IO.ErrOut, "No skills to install for %s.\n", host.Name)
continue
}
plans = append(plans, hostPlan{host: host, skills: installSkills})
}
for _, plan := range plans {
if len(plans) > 1 {
fmt.Fprintf(opts.IO.ErrOut, "\nInstalling to %s...\n", plan.host.Name)
}
result, err := installer.Install(&installer.Options{
Host: hostname,
Owner: opts.repo.RepoOwner(),
Repo: opts.repo.RepoName(),
Ref: resolved.Ref,
SHA: resolved.SHA,
PinnedRef: opts.Pin,
Skills: plan.skills,
AgentHost: plan.host,
Scope: scope,
Dir: opts.Dir,
GitRoot: gitRoot,
HomeDir: homeDir,
Client: apiClient,
OnProgress: installProgress(opts.IO, len(plan.skills)),
})
if result != nil {
for _, w := range result.Warnings {
fmt.Fprintf(opts.IO.ErrOut, "%s %s\n", cs.WarningIcon(), w)
}
for _, name := range result.Installed {
fmt.Fprintf(opts.IO.Out, "%s Installed %s (from %s@%s) in %s\n",
cs.SuccessIcon(), name, source, resolved.Ref, friendlyDir(result.Dir))
}
printFileTree(opts.IO.Out, cs, result.Dir, result.Installed)
printReviewHint(opts.IO.ErrOut, cs, source, result.Installed)
}
if err != nil {
return err
}
}
return nil
}
// isLocalPath returns true if the argument looks like a local filesystem path
// rather than a GitHub owner/repo reference.
func isLocalPath(arg string) bool {
if arg == "" {
return false
}
sep := string(filepath.Separator)
if arg == "." || arg == ".." ||
strings.HasPrefix(arg, "./") || strings.HasPrefix(arg, "../") ||
strings.HasPrefix(arg, "."+sep) || strings.HasPrefix(arg, ".."+sep) {
return true
}
// filepath.IsAbs on Windows requires a drive letter, so "/tmp/foo"
// would not be recognized. Check explicitly for a leading "/" so that
// Unix-style absolute paths are never mistaken for owner/repo refs.
if filepath.IsAbs(arg) || arg[0] == '/' || strings.HasPrefix(arg, "~") {
return true
}
info, err := os.Stat(arg)
if err == nil && info.IsDir() {
return true
}
return false
}
// runLocalInstall handles installation from a local directory path.
func runLocalInstall(opts *installOptions) error {
cs := opts.IO.ColorScheme()
canPrompt := opts.IO.CanPrompt()
sourcePath := opts.localPath
if sourcePath == "~" {
if home, err := os.UserHomeDir(); err == nil {
sourcePath = home
}
} else if after, ok := strings.CutPrefix(sourcePath, "~/"); ok {
if home, err := os.UserHomeDir(); err == nil {
sourcePath = filepath.Join(home, after)
}
}
absSource, err := filepath.Abs(sourcePath)
if err != nil {
return fmt.Errorf("could not resolve path: %w", err)
}
opts.IO.StartProgressIndicatorWithLabel("Discovering skills")
skills, err := discovery.DiscoverLocalSkills(absSource)
opts.IO.StopProgressIndicator()
if err != nil {
return err
}
if canPrompt {
fmt.Fprintf(opts.IO.ErrOut, "Found %d skill(s)\n", len(skills))
}
selectedSkills, err := selectSkillsWithSelector(opts, skills, canPrompt, skillSelector{
matchByName: matchLocalSkillByName,
sourceHint: absSource,
})
if err != nil {
return err
}
selectedHosts, err := resolveHosts(opts, canPrompt)
if err != nil {
return err
}
scope, err := resolveScope(opts, canPrompt)
if err != nil {
return err
}
gitRoot := gitclient.ResolveGitRoot(opts.GitClient)
homeDir := gitclient.ResolveHomeDir()
type hostPlan struct {
host *hosts.Host
skills []discovery.Skill
}
var plans []hostPlan
for _, host := range selectedHosts {
installSkills, err := checkOverwrite(opts, selectedSkills, host, scope, gitRoot, homeDir, canPrompt)
if err != nil {
return err
}
if len(installSkills) == 0 {
fmt.Fprintf(opts.IO.ErrOut, "No skills to install for %s.\n", host.Name)
continue
}
plans = append(plans, hostPlan{host: host, skills: installSkills})
}
for _, plan := range plans {
if len(plans) > 1 {
fmt.Fprintf(opts.IO.ErrOut, "\nInstalling to %s...\n", plan.host.Name)
}
result, err := installer.InstallLocal(&installer.LocalOptions{
SourceDir: absSource,
Skills: plan.skills,
AgentHost: plan.host,
Scope: scope,
Dir: opts.Dir,
GitRoot: gitRoot,
HomeDir: homeDir,
})
if err != nil {
return err
}
for _, name := range result.Installed {
fmt.Fprintf(opts.IO.Out, "Installed %s (from %s) in %s\n",
name, opts.SkillSource, friendlyDir(result.Dir))
}
printFileTree(opts.IO.Out, cs, result.Dir, result.Installed)
printReviewHint(opts.IO.ErrOut, cs, "", result.Installed)
}
return nil
}
// isSkillPath returns true if the argument looks like a repo-relative path
// rather than a simple skill name.
func isSkillPath(name string) bool {
if name == "" {
return false
}
if name == "SKILL.md" || strings.HasSuffix(name, "/SKILL.md") {
return true
}
if strings.HasPrefix(name, "skills/") || strings.HasPrefix(name, "plugins/") {
return true
}
return false
}
func resolveRepoArg(skillSource string, canPrompt bool, p prompter.Prompter) (ghrepo.Interface, string, error) {
if skillSource == "" {
if !canPrompt {
return nil, "", cmdutil.FlagErrorf("must specify a repository to install from")
}
repoInput, err := p.Input("Repository (owner/repo):", "")
if err != nil {
return nil, "", err
}
skillSource = strings.TrimSpace(repoInput)
if skillSource == "" {
return nil, "", fmt.Errorf("must specify a repository to install from")
}
}
repo, err := ghrepo.FromFullName(skillSource)
if err != nil {
return nil, "", cmdutil.FlagErrorf("invalid repository reference %q: expected OWNER/REPO, HOST/OWNER/REPO, or a full URL", skillSource)
}
return repo, skillSource, nil
}
func parseSkillFromOpts(opts *installOptions) {
if opts.SkillName != "" {
if name, version, ok := cutLast(opts.SkillName, "@"); ok && name != "" {
opts.version = version
opts.SkillName = name
return
}
}
if opts.Pin != "" {
opts.version = opts.Pin
}
}
// cutLast splits s around the last occurrence of sep,
// returning the text before and after sep, and whether sep was found.
func cutLast(s, sep string) (before, after string, found bool) {
if i := strings.LastIndex(s, sep); i >= 0 {
return s[:i], s[i+len(sep):], true
}
return s, "", false
}
func resolveVersion(opts *installOptions, client discovery.RESTClient, hostname string) (*discovery.ResolvedRef, error) {
opts.IO.StartProgressIndicatorWithLabel("Resolving version")
resolved, err := discovery.ResolveRef(client, hostname, opts.repo.RepoOwner(), opts.repo.RepoName(), opts.version)
opts.IO.StopProgressIndicator()
if err != nil {
return nil, fmt.Errorf("could not resolve version: %w", err)
}
fmt.Fprintf(opts.IO.ErrOut, "Using ref %s (%s)\n", resolved.Ref, gitclient.TruncateSHA(resolved.SHA))
return resolved, nil
}
func discoverSkills(opts *installOptions, client discovery.RESTClient, hostname string, resolved *discovery.ResolvedRef) ([]discovery.Skill, error) {
opts.IO.StartProgressIndicatorWithLabel("Discovering skills")
skills, err := discovery.DiscoverSkills(client, hostname, opts.repo.RepoOwner(), opts.repo.RepoName(), resolved.SHA)
opts.IO.StopProgressIndicator()
if err != nil {
return nil, err
}
logConventions(opts.IO, skills)
for _, s := range skills {
if !discovery.IsSpecCompliant(s.Name) {
fmt.Fprintf(opts.IO.ErrOut, "Warning: skill %q does not follow the agentskills.io naming convention\n", s.DisplayName())
}
}
sort.Slice(skills, func(i, j int) bool {
return skills[i].DisplayName() < skills[j].DisplayName()
})
return skills, nil
}
func logConventions(io *iostreams.IOStreams, skills []discovery.Skill) {
conventions := make(map[string]int)
for _, s := range skills {
conventions[s.Convention]++
}
if n, ok := conventions["skills-namespaced"]; ok {
fmt.Fprintf(io.ErrOut, "Note: found %d namespaced skill(s) in skills/{author}/ directories\n", n)
}
if n, ok := conventions["plugins"]; ok {
fmt.Fprintf(io.ErrOut, "Note: found %d skill(s) using the Claude Code plugins/ convention\n", n)
}
if n, ok := conventions["root"]; ok {
fmt.Fprintf(io.ErrOut, "Note: found %d skill(s) at the repository root\n", n)
}
}
// skillSelector holds the callbacks that differ between remote and local skill selection.
type skillSelector struct {
// matchByName resolves a skill name to matching skills.
matchByName func(opts *installOptions, skills []discovery.Skill) ([]discovery.Skill, error)
// sourceHint is shown in collision error guidance (e.g. "owner/repo" or "/path/to/skills").
sourceHint string
// fetchDescriptions, if non-nil, is called before prompting to pre-populate descriptions.
fetchDescriptions func()
}
func selectSkillsWithSelector(opts *installOptions, skills []discovery.Skill, canPrompt bool, sel skillSelector) ([]discovery.Skill, error) {
checkCollisions := func(ss []discovery.Skill) error {
return collisionError(ss, sel.sourceHint)
}
if opts.All {
if err := checkCollisions(skills); err != nil {
return nil, err
}
return skills, nil
}
if opts.SkillName != "" {
return sel.matchByName(opts, skills)
}
if !canPrompt {
return nil, cmdutil.FlagErrorf("must specify a skill name or use --all when not running interactively")
}
if sel.fetchDescriptions != nil {
sel.fetchDescriptions()
}
tw := opts.IO.TerminalWidth()
descWidth := tw - 35
if descWidth < 20 {
descWidth = 20
}
selected, err := opts.Prompter.MultiSelectWithSearch(
"Select skill(s) to install:",
"Filter skills",
nil,
[]string{allSkillsKey},
skillSearchFunc(skills, descWidth),
)
if err != nil {
return nil, err
}
if len(selected) == 0 {
return nil, fmt.Errorf("must select at least one skill")
}
for _, s := range selected {
if s == allSkillsKey {
if err := checkCollisions(skills); err != nil {
return nil, err
}
return skills, nil
}
}
result, err := matchSelectedSkills(skills, selected)
if err != nil {
return nil, err
}
return result, checkCollisions(result)
}
func matchSkillByName(opts *installOptions, skills []discovery.Skill) ([]discovery.Skill, error) {
for _, s := range skills {
if s.DisplayName() == opts.SkillName {
return []discovery.Skill{s}, nil
}
}
var matches []discovery.Skill
for _, s := range skills {
if s.Name == opts.SkillName {
matches = append(matches, s)
}
}
switch len(matches) {
case 0:
return nil, fmt.Errorf("skill %q not found in %s", opts.SkillName, ghrepo.FullName(opts.repo))
case 1:
return matches, nil
default:
names := make([]string, len(matches))
for i, m := range matches {
names[i] = m.DisplayName()
}
return nil, fmt.Errorf(
"skill name %q is ambiguous — multiple matches found:\n %s\n Specify the full name (e.g. %s) to disambiguate",
opts.SkillName, strings.Join(names, "\n "), names[0],
)
}
}
func matchLocalSkillByName(opts *installOptions, skills []discovery.Skill) ([]discovery.Skill, error) {
for _, s := range skills {
if s.DisplayName() == opts.SkillName || s.Name == opts.SkillName {
return []discovery.Skill{s}, nil
}
}
return nil, fmt.Errorf("skill %q not found in local directory", opts.SkillName)
}
// skillSearchFunc returns a search function for MultiSelectWithSearch that
// filters skills by case-insensitive substring match on name and description.
func skillSearchFunc(skills []discovery.Skill, descWidth int) func(string) prompter.MultiSelectSearchResult {
return func(query string) prompter.MultiSelectSearchResult {
var matched []discovery.Skill
if query == "" {
matched = skills
} else {
q := strings.ToLower(query)
for _, s := range skills {
if strings.Contains(strings.ToLower(s.DisplayName()), q) ||
strings.Contains(strings.ToLower(s.Description), q) {
matched = append(matched, s)
}
}
}
more := 0
if len(matched) > maxSearchResults {
more = len(matched) - maxSearchResults
matched = matched[:maxSearchResults]
}
keys := make([]string, len(matched))
labels := make([]string, len(matched))
for i, s := range matched {
keys[i] = s.DisplayName()
if s.Description != "" {
labels[i] = fmt.Sprintf("%s — %s", s.DisplayName(), truncateDescription(s.Description, descWidth))
} else {
labels[i] = s.DisplayName()
}
}
return prompter.MultiSelectSearchResult{
Keys: keys,
Labels: labels,
MoreResults: more,
}
}
}
// matchSelectedSkills maps display names back to skill structs.
func matchSelectedSkills(skills []discovery.Skill, selected []string) ([]discovery.Skill, error) {
nameSet := make(map[string]struct{}, len(selected))
for _, name := range selected {
nameSet[name] = struct{}{}
}
var result []discovery.Skill
for _, s := range skills {
if _, ok := nameSet[s.DisplayName()]; ok {
result = append(result, s)
}
}
if len(result) == 0 {
return nil, fmt.Errorf("no matching skills found")
}
return result, nil
}
// collisionError checks for name collisions and returns an error with
// guidance on how to install skills individually.
func collisionError(ss []discovery.Skill, sourceHint string) error {
collisions := skills.FindNameCollisions(ss)
if len(collisions) == 0 {
return nil
}
return errors.New(heredoc.Docf(`
cannot install skills with conflicting names they would overwrite each other:
%s
Install these skills individually using the full name:
gh skills install %s namespace/skill-name
`, skills.FormatCollisions(collisions), sourceHint))
}
func resolveHosts(opts *installOptions, canPrompt bool) ([]*hosts.Host, error) {
if opts.Agent != "" {
h, err := hosts.FindByID(opts.Agent)
if err != nil {
return nil, err
}
return []*hosts.Host{h}, nil
}
if !canPrompt {
h, err := hosts.FindByID("github-copilot")
if err != nil {
return nil, err
}
return []*hosts.Host{h}, nil
}
fmt.Fprintln(opts.IO.ErrOut)
names := hosts.HostNames()
indices, err := opts.Prompter.MultiSelect("Select target agent(s):", []string{names[0]}, names)
if err != nil {
return nil, err
}
if len(indices) == 0 {
return nil, fmt.Errorf("must select at least one target agent")
}
selected := make([]*hosts.Host, len(indices))
for i, idx := range indices {
selected[i] = &hosts.Registry[idx]
}
return selected, nil
}
func resolveScope(opts *installOptions, canPrompt bool) (hosts.Scope, error) {
if opts.Dir != "" {
return hosts.Scope(opts.Scope), nil
}
if opts.ScopeChanged || !canPrompt {
return hosts.Scope(opts.Scope), nil
}
var repoName string
if remote, err := opts.GitClient.RemoteURL("origin"); err == nil {
repoName = hosts.RepoNameFromRemote(remote)
}
idx, err := opts.Prompter.Select("Installation scope:", "", hosts.ScopeLabels(repoName))
if err != nil {
return "", err
}
if idx == 0 {
return hosts.ScopeProject, nil
}
return hosts.ScopeUser, nil
}
func truncateDescription(s string, maxWidth int) string {
return text.Truncate(maxWidth, text.RemoveExcessiveWhitespace(s))
}
func checkOverwrite(opts *installOptions, skills []discovery.Skill, host *hosts.Host, scope hosts.Scope, gitRoot, homeDir string, canPrompt bool) ([]discovery.Skill, error) {
targetDir := opts.Dir
if targetDir == "" {
var err error
targetDir, err = host.InstallDir(scope, gitRoot, homeDir)
if err != nil {
return nil, err
}
}
var existing, fresh []discovery.Skill
for _, s := range skills {
dir := filepath.Join(targetDir, filepath.FromSlash(s.InstallName()))
if _, err := os.Stat(dir); err == nil {
existing = append(existing, s)
} else {
fresh = append(fresh, s)
}
}
if len(existing) == 0 {
return skills, nil
}
if opts.Force {
return skills, nil
}
if !canPrompt {
names := make([]string, len(existing))
for i, s := range existing {
names[i] = s.DisplayName()
}
return nil, fmt.Errorf("skills already installed: %s (use --force to overwrite)", strings.Join(names, ", "))
}
var confirmed []discovery.Skill
for _, s := range existing {
prompt := existingSkillPrompt(targetDir, s)
ok, err := opts.Prompter.Confirm(prompt, false)
if err != nil {
return nil, err
}
if ok {
confirmed = append(confirmed, s)
} else {
fmt.Fprintf(opts.IO.ErrOut, "Skipping %s\n", s.DisplayName())
}
}
return append(fresh, confirmed...), nil
}
func existingSkillPrompt(targetDir string, incoming discovery.Skill) string {
skillFile := filepath.Join(targetDir, filepath.FromSlash(incoming.InstallName()), "SKILL.md")
data, err := os.ReadFile(skillFile)
if err != nil {
return fmt.Sprintf("Skill %q already exists. Overwrite?", incoming.DisplayName())
}
result, err := frontmatter.Parse(string(data))
if err != nil || result.Metadata.Meta == nil {
return fmt.Sprintf("Skill %q already exists. Overwrite?", incoming.DisplayName())
}
owner, _ := result.Metadata.Meta["github-owner"].(string)
repo, _ := result.Metadata.Meta["github-repo"].(string)
ref, _ := result.Metadata.Meta["github-ref"].(string)
if owner != "" && repo != "" {
source := owner + "/" + repo
if ref != "" {
source += "@" + ref
}
return fmt.Sprintf("Skill %q already installed from %s. Overwrite?", incoming.DisplayName(), source)
}
return fmt.Sprintf("Skill %q already exists. Overwrite?", incoming.DisplayName())
}
func installProgress(io *iostreams.IOStreams, total int) func(done, total int) {
if total <= 1 {
return nil
}
return func(done, total int) {
if done == 0 {
io.StartProgressIndicator()
} else if done >= total {
io.StopProgressIndicator()
}
}
}
func friendlyDir(dir string) string {
if cwd, err := os.Getwd(); err == nil {
if rel, err := filepath.Rel(cwd, dir); err == nil && rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator)) {
if rel == "." {
return filepath.Base(dir)
}
return rel
}
}
if home, err := os.UserHomeDir(); err == nil && (dir == home || strings.HasPrefix(dir, home+string(filepath.Separator))) {
return "~" + dir[len(home):]
}
return dir
}
// printFileTree renders a text tree of the on-disk contents of each skill directory.
func printFileTree(w io.Writer, cs *iostreams.ColorScheme, dir string, skillNames []string) {
if len(skillNames) == 0 {
return
}
fmt.Fprintln(w)
for _, name := range skillNames {
skillDir := filepath.Join(dir, filepath.FromSlash(name))
fmt.Fprintf(w, " %s\n", cs.Bold(name+"/"))
printTreeDir(w, cs, skillDir, " ")
}
}
func printTreeDir(w io.Writer, cs *iostreams.ColorScheme, dir, indent string) {
entries, err := os.ReadDir(dir)
if err != nil {
fmt.Fprintf(w, "%s%s\n", indent, cs.Gray("(could not read directory)"))
return
}
for i, entry := range entries {
isLast := i == len(entries)-1
connector := "├── "
childIndent := "│ "
if isLast {
connector = "└── "
childIndent = " "
}
name := entry.Name()
if entry.IsDir() {
fmt.Fprintf(w, "%s%s%s\n", indent, cs.Gray(connector), cs.Bold(name+"/"))
printTreeDir(w, cs, filepath.Join(dir, name), indent+cs.Gray(childIndent))
} else {
fmt.Fprintf(w, "%s%s%s\n", indent, cs.Gray(connector), name)
}
}
}
// printReviewHint warns the user to review installed skills and suggests preview commands.
func printReviewHint(w io.Writer, cs *iostreams.ColorScheme, repo string, skillNames []string) {
if len(skillNames) == 0 {
return
}
fmt.Fprintf(w, "\n%s Skills may contain prompt injections or malicious scripts.\n", cs.WarningIcon())
if repo == "" {
fmt.Fprintln(w, " Review the installed files before use.")
return
}
fmt.Fprintln(w, " Review installed content before use:")
fmt.Fprintln(w)
for _, name := range skillNames {
fmt.Fprintf(w, " gh skills preview %s %s\n", repo, name)
}
fmt.Fprintln(w)
}

View file

@ -0,0 +1,917 @@
package install
import (
"encoding/base64"
"fmt"
"net/http"
"os"
"path/filepath"
"strings"
"testing"
"github.com/cli/cli/v2/git"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/internal/prompter"
"github.com/cli/cli/v2/internal/skills/discovery"
"github.com/cli/cli/v2/internal/skills/gitclient"
"github.com/cli/cli/v2/internal/skills/hosts"
"github.com/cli/cli/v2/pkg/cmdutil"
"github.com/cli/cli/v2/pkg/httpmock"
"github.com/cli/cli/v2/pkg/iostreams"
"github.com/google/shlex"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// mockGitClient implements installGitClient for testing.
type mockGitClient struct {
root string
remote string
err error
}
func (m *mockGitClient) ToplevelDir() (string, error) {
if m.err != nil {
return "", m.err
}
return m.root, nil
}
func (m *mockGitClient) RemoteURL(_ string) (string, error) {
if m.err != nil {
return "", m.err
}
return m.remote, nil
}
func TestNewCmdInstall_Help(t *testing.T) {
ios, _, _, _ := iostreams.Test()
f := &cmdutil.Factory{
IOStreams: ios,
Prompter: &prompter.PrompterMock{},
GitClient: &git.Client{},
}
cmd := NewCmdInstall(f, func(opts *installOptions) error {
return nil
})
assert.Equal(t, "install <repository> [<skill[@version]>]", cmd.Use)
assert.NotEmpty(t, cmd.Short)
assert.NotEmpty(t, cmd.Long)
assert.NotEmpty(t, cmd.Example)
}
func TestNewCmdInstall_Alias(t *testing.T) {
ios, _, _, _ := iostreams.Test()
f := &cmdutil.Factory{IOStreams: ios, Prompter: &prompter.PrompterMock{}, GitClient: &git.Client{}}
cmd := NewCmdInstall(f, func(_ *installOptions) error { return nil })
assert.Contains(t, cmd.Aliases, "add")
}
func TestNewCmdInstall_Flags(t *testing.T) {
ios, _, _, _ := iostreams.Test()
f := &cmdutil.Factory{IOStreams: ios, Prompter: &prompter.PrompterMock{}, GitClient: &git.Client{}}
cmd := NewCmdInstall(f, func(_ *installOptions) error { return nil })
flags := []string{"agent", "scope", "pin", "all", "dir", "force"}
for _, name := range flags {
assert.NotNil(t, cmd.Flags().Lookup(name), "missing flag: --%s", name)
}
}
func TestNewCmdInstall_MaxArgs(t *testing.T) {
ios, _, _, _ := iostreams.Test()
f := &cmdutil.Factory{IOStreams: ios, Prompter: &prompter.PrompterMock{}, GitClient: &git.Client{}}
cmd := NewCmdInstall(f, func(_ *installOptions) error { return nil })
cmd.SetArgs([]string{"a", "b", "c"})
err := cmd.Execute()
assert.Error(t, err)
}
func TestResolveRepoArg(t *testing.T) {
tests := []struct {
input string
owner string
repo string
wantErr bool
}{
{"github/awesome-copilot", "github", "awesome-copilot", false},
{"owner/repo", "owner", "repo", false},
{"a/b", "a", "b", false},
{"https://github.com/owner/repo", "owner", "repo", false},
{"https://github.com/owner/repo.git", "owner", "repo", false},
{"invalid", "", "", true},
{"", "", "", true},
}
for _, tt := range tests {
t.Run(tt.input, func(t *testing.T) {
repo, _, err := resolveRepoArg(tt.input, false, nil)
if tt.wantErr {
assert.Error(t, err)
return
}
require.NoError(t, err)
assert.Equal(t, tt.owner, repo.RepoOwner())
assert.Equal(t, tt.repo, repo.RepoName())
})
}
}
func TestParseSkillFromOpts(t *testing.T) {
tests := []struct {
name string
skillName string
pin string
wantName string
wantVer string
}{
{
name: "name with version",
skillName: "git-commit@v1.2.0",
wantName: "git-commit",
wantVer: "v1.2.0",
},
{
name: "name without version",
skillName: "git-commit",
wantName: "git-commit",
wantVer: "",
},
{
name: "inline version takes precedence over pin",
skillName: "git-commit@v1.0.0",
pin: "v2.0.0",
wantName: "git-commit",
wantVer: "v1.0.0",
},
{
name: "pin flag alone",
skillName: "git-commit",
pin: "v3.0.0",
wantName: "git-commit",
wantVer: "v3.0.0",
},
{
name: "empty",
skillName: "",
wantName: "",
wantVer: "",
},
{
name: "@ at start is not version",
skillName: "@foo",
wantName: "@foo",
wantVer: "",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
opts := &installOptions{SkillName: tt.skillName, Pin: tt.pin}
parseSkillFromOpts(opts)
assert.Equal(t, tt.wantName, opts.SkillName)
assert.Equal(t, tt.wantVer, opts.version)
})
}
}
func TestInstallRun_NonInteractive_NoRepo(t *testing.T) {
ios, _, _, _ := iostreams.Test()
ios.SetStdoutTTY(false)
opts := &installOptions{
IO: ios,
GitClient: &mockGitClient{root: "/tmp", remote: ""},
}
err := installRun(opts)
assert.Error(t, err)
assert.Equal(t, "must specify a repository to install from", err.Error())
}
func TestInstallRun_NonInteractive_NoSkill(t *testing.T) {
ios, _, _, _ := iostreams.Test()
ios.SetStdoutTTY(false)
opts := &installOptions{IO: ios, repo: ghrepo.New("o", "r")}
skills := []discovery.Skill{{Name: "test-skill", Path: "skills/test-skill"}}
_, err := selectSkillsWithSelector(opts, skills, false, skillSelector{matchByName: matchSkillByName, sourceHint: "REPO"})
assert.Error(t, err)
assert.Contains(t, err.Error(), "must specify a skill name or use --all")
}
func TestSelectSkills_All(t *testing.T) {
ios, _, _, _ := iostreams.Test()
skills := []discovery.Skill{
{Name: "a"},
{Name: "b"},
}
opts := &installOptions{All: true, IO: ios, repo: ghrepo.New("o", "r")}
got, err := selectSkillsWithSelector(opts, skills, false, skillSelector{matchByName: matchSkillByName, sourceHint: "REPO"})
require.NoError(t, err)
assert.Len(t, got, 2)
}
func TestSelectSkills_ByName(t *testing.T) {
ios, _, _, _ := iostreams.Test()
skills := []discovery.Skill{
{Name: "alpha"},
{Name: "beta"},
}
opts := &installOptions{SkillName: "beta", IO: ios, repo: ghrepo.New("o", "r")}
got, err := selectSkillsWithSelector(opts, skills, false, skillSelector{matchByName: matchSkillByName, sourceHint: "REPO"})
require.NoError(t, err)
assert.Len(t, got, 1)
assert.Equal(t, "beta", got[0].Name)
}
func TestSelectSkills_NotFound(t *testing.T) {
ios, _, _, _ := iostreams.Test()
skills := []discovery.Skill{
{Name: "alpha"},
}
opts := &installOptions{SkillName: "nonexistent", IO: ios, repo: ghrepo.New("o", "r")}
_, err := selectSkillsWithSelector(opts, skills, false, skillSelector{matchByName: matchSkillByName, sourceHint: "REPO"})
assert.Error(t, err)
}
func TestSkillSearchFunc_EmptyQuery(t *testing.T) {
skills := []discovery.Skill{
{Name: "alpha", Description: "first skill"},
{Name: "beta", Description: "second skill"},
}
fn := skillSearchFunc(skills, 40)
result := fn("")
assert.Nil(t, result.Err)
assert.Len(t, result.Keys, 2)
assert.Equal(t, "alpha", result.Keys[0])
assert.Equal(t, "beta", result.Keys[1])
assert.Equal(t, 0, result.MoreResults)
}
func TestSkillSearchFunc_FilterByName(t *testing.T) {
skills := []discovery.Skill{
{Name: "git-commit"},
{Name: "code-review"},
{Name: "git-push"},
}
fn := skillSearchFunc(skills, 40)
result := fn("git")
assert.Nil(t, result.Err)
assert.Len(t, result.Keys, 2)
assert.Equal(t, "git-commit", result.Keys[0])
assert.Equal(t, "git-push", result.Keys[1])
}
func TestSkillSearchFunc_FilterByDescription(t *testing.T) {
skills := []discovery.Skill{
{Name: "alpha", Description: "handles authentication"},
{Name: "beta", Description: "builds docker images"},
}
fn := skillSearchFunc(skills, 40)
result := fn("docker")
assert.Nil(t, result.Err)
assert.Len(t, result.Keys, 1)
assert.Equal(t, "beta", result.Keys[0])
}
func TestSkillSearchFunc_CaseInsensitive(t *testing.T) {
skills := []discovery.Skill{
{Name: "Git-Commit"},
}
fn := skillSearchFunc(skills, 40)
result := fn("GIT")
assert.Nil(t, result.Err)
assert.Len(t, result.Keys, 1)
}
func TestSkillSearchFunc_MoreResults(t *testing.T) {
skills := make([]discovery.Skill, 50)
for i := range skills {
skills[i] = discovery.Skill{Name: fmt.Sprintf("skill-%d", i)}
}
fn := skillSearchFunc(skills, 40)
result := fn("")
assert.Equal(t, maxSearchResults, len(result.Keys))
assert.Equal(t, 50-maxSearchResults, result.MoreResults)
}
func TestMatchSelectedSkills(t *testing.T) {
skills := []discovery.Skill{
{Name: "alpha"},
{Name: "beta"},
{Name: "gamma"},
}
got, err := matchSelectedSkills(skills, []string{"alpha", "gamma"})
require.NoError(t, err)
assert.Len(t, got, 2)
assert.Equal(t, "alpha", got[0].Name)
assert.Equal(t, "gamma", got[1].Name)
}
func TestMatchSelectedSkills_NoMatch(t *testing.T) {
skills := []discovery.Skill{{Name: "alpha"}}
_, err := matchSelectedSkills(skills, []string{"nonexistent"})
assert.Error(t, err)
}
func TestResolveHosts_ByFlag(t *testing.T) {
ios, _, _, _ := iostreams.Test()
opts := &installOptions{Agent: "claude-code", IO: ios}
hosts, err := resolveHosts(opts, false)
require.NoError(t, err)
assert.Len(t, hosts, 1)
assert.Equal(t, "claude-code", hosts[0].ID)
}
func TestResolveHosts_InvalidFlag(t *testing.T) {
ios, _, _, _ := iostreams.Test()
opts := &installOptions{Agent: "nonexistent", IO: ios}
_, err := resolveHosts(opts, false)
assert.Error(t, err)
}
func TestResolveHosts_DefaultNonInteractive(t *testing.T) {
ios, _, _, _ := iostreams.Test()
opts := &installOptions{IO: ios}
hosts, err := resolveHosts(opts, false)
require.NoError(t, err)
assert.Len(t, hosts, 1)
assert.Equal(t, "github-copilot", hosts[0].ID)
}
func TestResolveHosts_MultiSelect(t *testing.T) {
ios, _, _, _ := iostreams.Test()
pm := &prompter.PrompterMock{
MultiSelectFunc: func(_ string, _ []string, _ []string) ([]int, error) {
return []int{0, 1}, nil
},
}
opts := &installOptions{IO: ios, Prompter: pm}
hosts, err := resolveHosts(opts, true)
require.NoError(t, err)
assert.Len(t, hosts, 2)
}
func TestResolveHosts_NoneSelected(t *testing.T) {
ios, _, _, _ := iostreams.Test()
pm := &prompter.PrompterMock{
MultiSelectFunc: func(_ string, _ []string, _ []string) ([]int, error) {
return []int{}, nil
},
}
opts := &installOptions{IO: ios, Prompter: pm}
_, err := resolveHosts(opts, true)
assert.Error(t, err)
}
func TestTruncateSHA(t *testing.T) {
assert.Equal(t, "abc123de", gitclient.TruncateSHA("abc123def456"))
assert.Equal(t, "short", gitclient.TruncateSHA("short"))
}
func TestTruncateDescription(t *testing.T) {
tests := []struct {
name string
input string
maxWidth int
}{
{"short stays short", "A short description", 60},
{"newlines collapsed", "Line one.\nLine two.\nLine three.", 60},
{"excessive whitespace", " lots of spaces ", 60},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := truncateDescription(tt.input, tt.maxWidth)
assert.NotContains(t, got, "\n")
})
}
long := "Execute git commit with conventional commit message analysis and intelligent staging"
got := truncateDescription(long, 30)
assert.LessOrEqual(t, len(got), 33) // allow room for ellipsis
}
func TestIsLocalPath(t *testing.T) {
tests := []struct {
arg string
want bool
}{
{".", true},
{"./skills", true},
{"../other", true},
{"/tmp/skills", true},
{"~/skills", true},
{"github/awesome-copilot", false},
{"owner/repo", false},
{"", false},
}
for _, tt := range tests {
t.Run(tt.arg, func(t *testing.T) {
got := isLocalPath(tt.arg)
assert.Equal(t, tt.want, got)
})
}
}
func TestIsSkillPath(t *testing.T) {
tests := []struct {
name string
want bool
}{
{"skills/test-skill", true},
{"skills/author/skill", true},
{"plugins/author/skills/skill", true},
{"skills/author/skill/SKILL.md", true},
{"git-commit", false},
{"", false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equal(t, tt.want, isSkillPath(tt.name))
})
}
}
func TestRunLocalInstall_NonInteractive(t *testing.T) {
dir := t.TempDir()
skillDir := filepath.Join(dir, "skills", "test-local")
require.NoError(t, os.MkdirAll(skillDir, 0o755))
content := "---\nname: test-local\ndescription: A local skill\n---\n# Test\n"
require.NoError(t, os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte(content), 0o644))
targetDir := t.TempDir()
ios, _, stdout, _ := iostreams.Test()
ios.SetStdoutTTY(false)
ios.SetColorEnabled(false)
opts := &installOptions{
IO: ios,
SkillSource: dir,
localPath: dir,
All: true,
Force: true,
Agent: "github-copilot",
Scope: "project",
Dir: targetDir,
GitClient: &mockGitClient{root: t.TempDir(), remote: ""},
}
err := installRun(opts)
require.NoError(t, err)
assert.Contains(t, stdout.String(), "Installed test-local")
installed, err := os.ReadFile(filepath.Join(targetDir, "test-local", "SKILL.md"))
require.NoError(t, err)
assert.Contains(t, string(installed), "local-path")
}
func TestRunLocalInstall_SingleSkillDir(t *testing.T) {
dir := t.TempDir()
content := "---\nname: direct-skill\ndescription: Direct\n---\n# Direct\n"
require.NoError(t, os.WriteFile(filepath.Join(dir, "SKILL.md"), []byte(content), 0o644))
targetDir := t.TempDir()
ios, _, stdout, _ := iostreams.Test()
ios.SetStdoutTTY(false)
ios.SetColorEnabled(false)
opts := &installOptions{
IO: ios,
SkillSource: dir,
localPath: dir,
All: true,
Force: true,
Agent: "github-copilot",
Scope: "project",
Dir: targetDir,
GitClient: &mockGitClient{root: t.TempDir(), remote: ""},
}
err := installRun(opts)
require.NoError(t, err)
assert.Contains(t, stdout.String(), "Installed direct-skill")
}
func TestCollisionError(t *testing.T) {
t.Run("no collisions", func(t *testing.T) {
skills := []discovery.Skill{
{Name: "a"},
{Name: "b"},
}
assert.NoError(t, collisionError(skills, "REPO"))
})
t.Run("no collisions with different namespaces", func(t *testing.T) {
skills := []discovery.Skill{
{Name: "xlsx-pro", Namespace: "author1"},
{Name: "xlsx-pro", Namespace: "author2"},
}
assert.NoError(t, collisionError(skills, "REPO"))
})
t.Run("has collisions same name no namespace", func(t *testing.T) {
skills := []discovery.Skill{
{Name: "xlsx-pro", Convention: "skills"},
{Name: "xlsx-pro", Convention: "root"},
}
err := collisionError(skills, "REPO")
assert.Error(t, err)
assert.Contains(t, err.Error(), "conflicting names")
assert.Contains(t, err.Error(), "gh skills install REPO")
})
t.Run("local source hint", func(t *testing.T) {
skills := []discovery.Skill{
{Name: "xlsx-pro", Convention: "skills"},
{Name: "xlsx-pro", Convention: "root"},
}
err := collisionError(skills, "PATH")
assert.Error(t, err)
assert.Contains(t, err.Error(), "conflicting names")
assert.Contains(t, err.Error(), "gh skills install PATH")
})
}
func TestMatchSkillByName_Ambiguous(t *testing.T) {
ios, _, _, _ := iostreams.Test()
skills := []discovery.Skill{
{Name: "xlsx-pro", Namespace: "alice"},
{Name: "xlsx-pro", Namespace: "bob"},
}
opts := &installOptions{SkillName: "xlsx-pro", IO: ios, repo: ghrepo.New("o", "r")}
_, err := matchSkillByName(opts, skills)
assert.Error(t, err)
assert.Contains(t, err.Error(), "ambiguous")
}
func TestMatchSkillByName_NamespacedExact(t *testing.T) {
ios, _, _, _ := iostreams.Test()
skills := []discovery.Skill{
{Name: "xlsx-pro", Namespace: "alice"},
{Name: "xlsx-pro", Namespace: "bob"},
}
opts := &installOptions{SkillName: "bob/xlsx-pro", IO: ios, repo: ghrepo.New("o", "r")}
got, err := matchSkillByName(opts, skills)
require.NoError(t, err)
assert.Len(t, got, 1)
assert.Equal(t, "bob", got[0].Namespace)
}
func TestFriendlyDir(t *testing.T) {
// Test home directory path
home, err := os.UserHomeDir()
require.NoError(t, err)
got := friendlyDir(filepath.Join(home, ".github", "skills"))
assert.True(t, strings.HasPrefix(got, "~"), "expected ~ prefix, got %q", got)
}
func TestResolveScope_ExplicitFlag(t *testing.T) {
ios, _, _, _ := iostreams.Test()
opts := &installOptions{
IO: ios,
Scope: "user",
ScopeChanged: true,
GitClient: &mockGitClient{root: "/tmp", remote: ""},
}
scope, err := resolveScope(opts, true)
require.NoError(t, err)
assert.Equal(t, "user", string(scope))
}
func TestResolveScope_DirBypasses(t *testing.T) {
ios, _, _, _ := iostreams.Test()
opts := &installOptions{
IO: ios,
Dir: "/tmp/custom",
Scope: "project",
GitClient: &mockGitClient{root: "/tmp", remote: ""},
}
scope, err := resolveScope(opts, true)
require.NoError(t, err)
assert.Equal(t, "project", string(scope))
}
func TestCheckOverwrite_NoExisting(t *testing.T) {
ios, _, _, _ := iostreams.Test()
targetDir := t.TempDir()
skills := []discovery.Skill{{Name: "new-skill"}}
host := &hosts.Host{ID: "test", ProjectDir: "skills"}
opts := &installOptions{IO: ios, Dir: targetDir}
got, err := checkOverwrite(opts, skills, host, hosts.ScopeProject, "/tmp", "/home", false)
require.NoError(t, err)
assert.Len(t, got, 1)
}
func TestCheckOverwrite_ExistingWithForce(t *testing.T) {
targetDir := t.TempDir()
require.NoError(t, os.MkdirAll(filepath.Join(targetDir, "existing-skill"), 0o755))
ios, _, _, _ := iostreams.Test()
skills := []discovery.Skill{{Name: "existing-skill"}}
host := &hosts.Host{ID: "test", ProjectDir: "skills"}
opts := &installOptions{IO: ios, Dir: targetDir, Force: true}
got, err := checkOverwrite(opts, skills, host, hosts.ScopeProject, "/tmp", "/home", false)
require.NoError(t, err)
assert.Len(t, got, 1)
}
func TestCheckOverwrite_ExistingNonInteractive(t *testing.T) {
targetDir := t.TempDir()
require.NoError(t, os.MkdirAll(filepath.Join(targetDir, "existing-skill"), 0o755))
ios, _, _, _ := iostreams.Test()
skills := []discovery.Skill{{Name: "existing-skill"}}
host := &hosts.Host{ID: "test", ProjectDir: "skills"}
opts := &installOptions{IO: ios, Dir: targetDir}
_, err := checkOverwrite(opts, skills, host, hosts.ScopeProject, "/tmp", "/home", false)
assert.Error(t, err)
assert.Contains(t, err.Error(), "already installed")
}
func TestNewCmdInstall(t *testing.T) {
tests := []struct {
name string
input string
wantOpts installOptions
wantErr bool
}{
{
name: "repo argument only",
input: "owner/repo",
wantOpts: installOptions{SkillSource: "owner/repo", Scope: "project"},
},
{
name: "repo and skill",
input: "owner/repo my-skill",
wantOpts: installOptions{SkillSource: "owner/repo", SkillName: "my-skill", Scope: "project"},
},
{
name: "with all flags",
input: "owner/repo my-skill --agent github-copilot --scope user --pin v1.0.0 --force",
wantOpts: installOptions{SkillSource: "owner/repo", SkillName: "my-skill", Agent: "github-copilot", Scope: "user", Pin: "v1.0.0", Force: true},
},
{
name: "all flag",
input: "owner/repo --all",
wantOpts: installOptions{SkillSource: "owner/repo", All: true, Scope: "project"},
},
{
name: "dir flag",
input: "owner/repo my-skill --dir /tmp/skills",
wantOpts: installOptions{SkillSource: "owner/repo", SkillName: "my-skill", Dir: "/tmp/skills", Scope: "project"},
},
{
name: "too many args",
input: "a b c",
wantErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ios, _, _, _ := iostreams.Test()
f := &cmdutil.Factory{
IOStreams: ios,
Prompter: &prompter.PrompterMock{},
GitClient: &git.Client{},
}
var gotOpts *installOptions
cmd := NewCmdInstall(f, func(opts *installOptions) error {
gotOpts = opts
return nil
})
args, err := shlex.Split(tt.input)
require.NoError(t, err)
cmd.SetArgs(args)
cmd.SetIn(&strings.Reader{})
cmd.SetOut(&strings.Builder{})
cmd.SetErr(&strings.Builder{})
_, err = cmd.ExecuteC()
if tt.wantErr {
assert.Error(t, err)
return
}
require.NoError(t, err)
assert.Equal(t, tt.wantOpts.SkillSource, gotOpts.SkillSource)
assert.Equal(t, tt.wantOpts.SkillName, gotOpts.SkillName)
assert.Equal(t, tt.wantOpts.Agent, gotOpts.Agent)
assert.Equal(t, tt.wantOpts.Scope, gotOpts.Scope)
assert.Equal(t, tt.wantOpts.Pin, gotOpts.Pin)
assert.Equal(t, tt.wantOpts.Dir, gotOpts.Dir)
assert.Equal(t, tt.wantOpts.All, gotOpts.All)
assert.Equal(t, tt.wantOpts.Force, gotOpts.Force)
})
}
}
func TestInstallRun_RemoteInstall(t *testing.T) {
t.Setenv("HOME", t.TempDir())
skillContent := "---\nname: test-skill\ndescription: A test\n---\n# Test\n"
encodedContent := base64.StdEncoding.EncodeToString([]byte(skillContent))
reg := &httpmock.Registry{}
reg.Register(
httpmock.REST("GET", "repos/owner/repo/releases/latest"),
httpmock.StringResponse(`{"tag_name": "v1.0.0"}`),
)
reg.Register(
httpmock.REST("GET", "repos/owner/repo/git/ref/tags/v1.0.0"),
httpmock.StringResponse(`{"object": {"sha": "abc123", "type": "commit"}}`),
)
reg.Register(
httpmock.REST("GET", "repos/owner/repo/git/trees/abc123"),
httpmock.StringResponse(`{"sha": "abc123", "tree": [{"path": "skills/test-skill", "type": "tree", "sha": "treeSHA"}, {"path": "skills/test-skill/SKILL.md", "type": "blob", "sha": "blobSHA"}]}`),
)
reg.Register(
httpmock.REST("GET", "repos/owner/repo/git/trees/treeSHA"),
httpmock.StringResponse(`{"tree": [{"path": "SKILL.md", "type": "blob", "sha": "blobSHA", "size": 50}]}`),
)
reg.Register(
httpmock.REST("GET", "repos/owner/repo/git/blobs/blobSHA"),
httpmock.StringResponse(fmt.Sprintf(`{"sha": "blobSHA", "content": "%s", "encoding": "base64"}`, encodedContent)),
)
targetDir := t.TempDir()
ios, _, stdout, _ := iostreams.Test()
ios.SetStdoutTTY(true)
ios.SetColorEnabled(false)
opts := &installOptions{
IO: ios,
HttpClient: func() (*http.Client, error) {
return &http.Client{Transport: reg}, nil
},
GitClient: &mockGitClient{root: t.TempDir(), remote: ""},
SkillSource: "owner/repo",
SkillName: "test-skill",
Agent: "github-copilot",
Scope: "project",
Dir: targetDir,
}
defer reg.Verify(t)
err := installRun(opts)
require.NoError(t, err)
assert.Contains(t, stdout.String(), "Installed test-skill")
installed, readErr := os.ReadFile(filepath.Join(targetDir, "test-skill", "SKILL.md"))
require.NoError(t, readErr)
assert.Contains(t, string(installed), "github-owner: owner")
assert.Contains(t, string(installed), "github-repo: repo")
}
func TestPrintFileTree(t *testing.T) {
dir := t.TempDir()
skillDir := filepath.Join(dir, "my-skill")
require.NoError(t, os.MkdirAll(filepath.Join(skillDir, "scripts"), 0o755))
require.NoError(t, os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte("# test"), 0o644))
require.NoError(t, os.WriteFile(filepath.Join(skillDir, "scripts", "run.sh"), []byte("#!/bin/bash"), 0o644))
ios, _, stdout, _ := iostreams.Test()
cs := ios.ColorScheme()
printFileTree(stdout, cs, dir, []string{"my-skill"})
out := stdout.String()
assert.Contains(t, out, "my-skill/")
assert.Contains(t, out, "SKILL.md")
assert.Contains(t, out, "scripts/")
assert.Contains(t, out, "run.sh")
}
func TestPrintFileTree_Empty(t *testing.T) {
ios, _, stdout, _ := iostreams.Test()
cs := ios.ColorScheme()
printFileTree(stdout, cs, t.TempDir(), nil)
assert.Empty(t, stdout.String())
}
func TestPrintTreeDir_Unreadable(t *testing.T) {
ios, _, stdout, _ := iostreams.Test()
cs := ios.ColorScheme()
printTreeDir(stdout, cs, filepath.Join(t.TempDir(), "nonexistent"), " ")
assert.Contains(t, stdout.String(), "(could not read directory)")
}
func TestPrintReviewHint_Remote(t *testing.T) {
ios, _, _, stderr := iostreams.Test()
cs := ios.ColorScheme()
printReviewHint(stderr, cs, "owner/repo", []string{"my-skill", "other-skill"})
out := stderr.String()
assert.Contains(t, out, "prompt injections or malicious scripts")
assert.Contains(t, out, "gh skills preview owner/repo my-skill")
assert.Contains(t, out, "gh skills preview owner/repo other-skill")
}
func TestPrintReviewHint_Local(t *testing.T) {
ios, _, _, stderr := iostreams.Test()
cs := ios.ColorScheme()
printReviewHint(stderr, cs, "", []string{"my-skill"})
out := stderr.String()
assert.Contains(t, out, "prompt injections or malicious scripts")
assert.Contains(t, out, "Review the installed files before use.")
assert.NotContains(t, out, "gh skills preview")
}
func TestPrintReviewHint_Empty(t *testing.T) {
ios, _, _, stderr := iostreams.Test()
cs := ios.ColorScheme()
printReviewHint(stderr, cs, "owner/repo", nil)
assert.Empty(t, stderr.String())
}
func TestSelectSkills_AllWithNamespacedSkills(t *testing.T) {
ios, _, _, _ := iostreams.Test()
skills := []discovery.Skill{
{Name: "xlsx-pro", Namespace: "alice", Convention: "skills-namespaced"},
{Name: "xlsx-pro", Namespace: "bob", Convention: "skills-namespaced"},
{Name: "other-skill", Convention: "skills"},
}
opts := &installOptions{All: true, IO: ios, repo: ghrepo.New("o", "r")}
got, err := selectSkillsWithSelector(opts, skills, false, skillSelector{matchByName: matchSkillByName, sourceHint: "REPO"})
require.NoError(t, err)
assert.Len(t, got, 3)
}
func TestRunLocalInstall_NamespacedSkills(t *testing.T) {
dir := t.TempDir()
// Create two skills with the same name under different namespaces
for _, ns := range []string{"alice", "bob"} {
skillDir := filepath.Join(dir, "skills", ns, "xlsx-pro")
require.NoError(t, os.MkdirAll(skillDir, 0o755))
content := fmt.Sprintf("---\nname: xlsx-pro\ndescription: %s xlsx-pro\n---\n# Test\n", ns)
require.NoError(t, os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte(content), 0o644))
}
targetDir := t.TempDir()
ios, _, stdout, _ := iostreams.Test()
ios.SetStdoutTTY(false)
ios.SetColorEnabled(false)
opts := &installOptions{
IO: ios,
SkillSource: dir,
localPath: dir,
All: true,
Force: true,
Agent: "github-copilot",
Scope: "project",
Dir: targetDir,
GitClient: &mockGitClient{root: t.TempDir(), remote: ""},
}
err := installRun(opts)
require.NoError(t, err)
out := stdout.String()
assert.Contains(t, out, "Installed alice/xlsx-pro")
assert.Contains(t, out, "Installed bob/xlsx-pro")
// Both should be installed in separate directories
_, err = os.Stat(filepath.Join(targetDir, "alice", "xlsx-pro", "SKILL.md"))
assert.NoError(t, err, "alice/xlsx-pro should be installed")
_, err = os.Stat(filepath.Join(targetDir, "bob", "xlsx-pro", "SKILL.md"))
assert.NoError(t, err, "bob/xlsx-pro should be installed")
}
func TestCheckOverwrite_NamespacedSkill(t *testing.T) {
ios, _, _, _ := iostreams.Test()
targetDir := t.TempDir()
// Pre-create a namespaced skill directory
require.NoError(t, os.MkdirAll(filepath.Join(targetDir, "alice", "xlsx-pro"), 0o755))
skills := []discovery.Skill{
{Name: "xlsx-pro", Namespace: "alice"},
{Name: "xlsx-pro", Namespace: "bob"},
}
host := &hosts.Host{ID: "test", ProjectDir: "skills"}
opts := &installOptions{IO: ios, Dir: targetDir, Force: true}
got, err := checkOverwrite(opts, skills, host, hosts.ScopeProject, "/tmp", "/home", false)
require.NoError(t, err)
assert.Len(t, got, 2, "both skills should be installable (force mode)")
}

View file

@ -0,0 +1,63 @@
//go:build windows
package install
import (
"os"
"path/filepath"
"testing"
"github.com/stretchr/testify/assert"
)
func TestIsLocalPath_Windows(t *testing.T) {
tests := []struct {
name string
arg string
want bool
}{
// Backslash-relative paths that only exist on Windows.
{`dot-backslash prefix`, `.\skills`, true},
{`dotdot-backslash prefix`, `..\other`, true},
{`drive-absolute path`, `C:\Users\me\skills`, true},
{`drive-relative path`, `D:\projects`, true},
{`UNC path`, `\\server\share\skills`, true},
// Forward-slash forms should still work on Windows.
{`dot-slash prefix`, `./skills`, true},
{`dotdot-slash prefix`, `../other`, true},
{`current dir`, `.`, true},
{`absolute unix-style`, `/tmp/skills`, true},
{`tilde prefix`, `~/skills`, true},
// owner/repo should never be treated as local.
{`owner-repo`, `github/awesome-copilot`, false},
{`simple name`, `awesome-copilot`, false},
{`empty string`, ``, false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := isLocalPath(tt.arg)
assert.Equal(t, tt.want, got, "isLocalPath(%q)", tt.arg)
})
}
}
func TestIsLocalPath_WindowsExistingDir(t *testing.T) {
// A directory that exists on disk should be detected as local even when
// its name looks like owner/repo (the os.Stat safety-net).
dir := t.TempDir()
nested := filepath.Join(dir, "owner", "repo")
if err := os.MkdirAll(nested, 0o755); err != nil {
t.Fatal(err)
}
// Use a relative path that happens to contain a backslash separator.
rel, err := filepath.Rel(".", nested)
if err != nil {
// If we can't compute a relative path, just use the absolute one.
rel = nested
}
assert.True(t, isLocalPath(rel), "existing dir should be detected as local: %s", rel)
}

View file

@ -1,6 +1,7 @@
package skills
import (
"github.com/cli/cli/v2/pkg/cmd/skills/install"
"github.com/cli/cli/v2/pkg/cmdutil"
"github.com/spf13/cobra"
)
@ -14,5 +15,7 @@ func NewCmdSkills(f *cmdutil.Factory) *cobra.Command {
GroupID: "core",
}
cmd.AddCommand(install.NewCmdInstall(f, nil))
return cmd
}