diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 98642afaf..7c3c6f6ce 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -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")) +} diff --git a/acceptance/testdata/skills/.gitkeep b/acceptance/testdata/skills/.gitkeep new file mode 100644 index 000000000..e69de29bb diff --git a/acceptance/testdata/skills/skills-install-alias.txtar b/acceptance/testdata/skills/skills-install-alias.txtar new file mode 100644 index 000000000..089474b3a --- /dev/null +++ b/acceptance/testdata/skills/skills-install-alias.txtar @@ -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' diff --git a/acceptance/testdata/skills/skills-install-all.txtar b/acceptance/testdata/skills/skills-install-all.txtar new file mode 100644 index 000000000..6efd7747e --- /dev/null +++ b/acceptance/testdata/skills/skills-install-all.txtar @@ -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' diff --git a/acceptance/testdata/skills/skills-install-conflict.txtar b/acceptance/testdata/skills/skills-install-conflict.txtar new file mode 100644 index 000000000..9e79e5a5e --- /dev/null +++ b/acceptance/testdata/skills/skills-install-conflict.txtar @@ -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 diff --git a/acceptance/testdata/skills/skills-install-force.txtar b/acceptance/testdata/skills/skills-install-force.txtar new file mode 100644 index 000000000..5623fce84 --- /dev/null +++ b/acceptance/testdata/skills/skills-install-force.txtar @@ -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' diff --git a/acceptance/testdata/skills/skills-install-pin.txtar b/acceptance/testdata/skills/skills-install-pin.txtar new file mode 100644 index 000000000..43d780e3e --- /dev/null +++ b/acceptance/testdata/skills/skills-install-pin.txtar @@ -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' diff --git a/acceptance/testdata/skills/skills-install-scope.txtar b/acceptance/testdata/skills/skills-install-scope.txtar new file mode 100644 index 000000000..9b8048ab5 --- /dev/null +++ b/acceptance/testdata/skills/skills-install-scope.txtar @@ -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' diff --git a/acceptance/testdata/skills/skills-install.txtar b/acceptance/testdata/skills/skills-install.txtar new file mode 100644 index 000000000..c04ced9d2 --- /dev/null +++ b/acceptance/testdata/skills/skills-install.txtar @@ -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 diff --git a/internal/skills/discovery/discovery.go b/internal/skills/discovery/discovery.go index e0e881088..fc234716a 100644 --- a/internal/skills/discovery/discovery.go +++ b/internal/skills/discovery/discovery.go @@ -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 } diff --git a/internal/skills/installer/installer.go b/internal/skills/installer/installer.go index fa2854a7c..4c2a39256 100644 --- a/internal/skills/installer/installer.go +++ b/internal/skills/installer/installer.go @@ -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 { diff --git a/internal/skills/lockfile/lockfile.go b/internal/skills/lockfile/lockfile.go index 4ceed7872..ad5fd4d4b 100644 --- a/internal/skills/lockfile/lockfile.go +++ b/internal/skills/lockfile/lockfile.go @@ -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) diff --git a/pkg/cmd/skills/install/install.go b/pkg/cmd/skills/install/install.go new file mode 100644 index 000000000..38613800d --- /dev/null +++ b/pkg/cmd/skills/install/install.go @@ -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 []", + 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) +} diff --git a/pkg/cmd/skills/install/install_test.go b/pkg/cmd/skills/install/install_test.go new file mode 100644 index 000000000..f53fd2267 --- /dev/null +++ b/pkg/cmd/skills/install/install_test.go @@ -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 []", 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)") +} diff --git a/pkg/cmd/skills/install/install_windows_test.go b/pkg/cmd/skills/install/install_windows_test.go new file mode 100644 index 000000000..8a184fac4 --- /dev/null +++ b/pkg/cmd/skills/install/install_windows_test.go @@ -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) +} diff --git a/pkg/cmd/skills/skills.go b/pkg/cmd/skills/skills.go index e3f1c286f..61afc12a4 100644 --- a/pkg/cmd/skills/skills.go +++ b/pkg/cmd/skills/skills.go @@ -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 }