diff --git a/internal/skills/collisions.go b/internal/skills/collisions.go new file mode 100644 index 000000000..87e4705c9 --- /dev/null +++ b/internal/skills/collisions.go @@ -0,0 +1,55 @@ +package skills + +import ( + "fmt" + "sort" + "strings" + + "github.com/cli/cli/v2/internal/skills/discovery" +) + +// NameCollision represents a group of skills that share the same InstallName +// and would overwrite each other when installed to the same directory. +type NameCollision struct { + Name string // the conflicting install name (may include namespace prefix) + DisplayNames []string // display names of each conflicting skill +} + +// FindNameCollisions detects skills that share the same InstallName and returns a +// sorted slice of collisions. Callers decide how to present the conflict to +// the user (different flows need different error messages). +func FindNameCollisions(skills []discovery.Skill) []NameCollision { + byName := make(map[string][]discovery.Skill) + for _, s := range skills { + byName[s.InstallName()] = append(byName[s.InstallName()], s) + } + + var collisions []NameCollision + for name, group := range byName { + if len(group) <= 1 { + continue + } + names := make([]string, len(group)) + for i, s := range group { + names[i] = s.DisplayName() + } + collisions = append(collisions, NameCollision{Name: name, DisplayNames: names}) + } + + sort.Slice(collisions, func(i, j int) bool { + return collisions[i].Name < collisions[j].Name + }) + return collisions +} + +// FormatCollisions builds a human-readable string listing each collision, +// suitable for embedding in an error message. Each collision is formatted as +// "name: display1, display2" and collisions are separated by newlines with +// leading indentation. +func FormatCollisions(collisions []NameCollision) string { + lines := make([]string, len(collisions)) + for i, c := range collisions { + lines[i] = fmt.Sprintf("%s: %s", c.Name, strings.Join(c.DisplayNames, ", ")) + } + return strings.Join(lines, "\n ") +} diff --git a/internal/skills/discovery/discovery.go b/internal/skills/discovery/discovery.go new file mode 100644 index 000000000..e0e881088 --- /dev/null +++ b/internal/skills/discovery/discovery.go @@ -0,0 +1,769 @@ +package discovery + +import ( + "encoding/base64" + "fmt" + "io" + "os" + "path" + "path/filepath" + "regexp" + "strings" + "sync" + + "github.com/cli/cli/v2/internal/skills/frontmatter" +) + +// specNamePattern matches the strict agentskills.io name spec: +// 1-64 chars, lowercase alphanumeric + hyphens, no leading/trailing/consecutive hyphens. +var specNamePattern = regexp.MustCompile(`^[a-z0-9]([a-z0-9-]*[a-z0-9])?$`) + +// safeNamePattern matches names that are safe for filesystem use during discovery. +// Allows letters (any case), numbers, hyphens, underscores, dots, and spaces. +// Must start with a letter or number. This matches copilot-agent-runtime's SKILL_NAME_REGEX. +var safeNamePattern = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9._\- ]*$`) + +// Skill represents a discovered skill in a repository. +type Skill struct { + Name string + Namespace string // author/scope prefix for namespaced skills + Description string + Path string // path within the repo, e.g. "skills/git-commit" + BlobSHA string // SHA of the SKILL.md blob + TreeSHA string // SHA of the skill directory tree + Convention string // which directory convention matched +} + +// DisplayName returns the skill name, prefixed with namespace if present +// to disambiguate skills from different authors in the same repository. +// Skills discovered via non-standard conventions (plugins, root) include +// a convention tag to distinguish them from identically-named skills in +// the standard skills/ directory. +func (s Skill) DisplayName() string { + name := s.Name + if s.Namespace != "" { + name = s.Namespace + "/" + name + } + switch s.Convention { + case "plugins": + return "[plugins] " + name + case "root": + return "[root] " + name + default: + return name + } +} + +// InstallName returns the relative path used for the install directory. +// For namespaced skills it returns "namespace/name" (creating a nested directory), +// otherwise it returns the plain name. Callers should use filepath.FromSlash +// when building OS-specific paths from this value. +func (s Skill) InstallName() string { + if s.Namespace != "" { + return s.Namespace + "/" + s.Name + } + return s.Name +} + +// ResolvedRef contains the resolved git reference and its SHA. +type ResolvedRef struct { + Ref string // tag name, branch name, or SHA + SHA string // commit SHA +} + +// RESTClient is the interface for making GitHub REST API calls. +// It mirrors the subset of api.Client used by discovery. +type RESTClient interface { + // REST performs a REST API call. + // hostname is the GitHub host (e.g. "github.com"). + // method is the HTTP method (e.g. "GET"). + // path is the API path (e.g. "repos/owner/repo/releases/latest"). + // body is the request body (nil for GET). + // data is the response data to unmarshal into. + REST(hostname string, method string, path string, body io.Reader, data interface{}) error +} + +type treeEntry struct { + Path string `json:"path"` + Mode string `json:"mode"` + Type string `json:"type"` + SHA string `json:"sha"` + Size int `json:"size"` +} + +// SkillFile represents a file within a skill directory. +type SkillFile struct { + Path string // relative path within the skill directory + SHA string // blob SHA for fetching content + Size int // file size in bytes +} + +type treeResponse struct { + SHA string `json:"sha"` + Tree []treeEntry `json:"tree"` + Truncated bool `json:"truncated"` +} + +type blobResponse struct { + SHA string `json:"sha"` + Content string `json:"content"` + Encoding string `json:"encoding"` +} + +type releaseResponse struct { + TagName string `json:"tag_name"` +} + +type repoResponse struct { + DefaultBranch string `json:"default_branch"` +} + +// ResolveRef determines the git ref to use for a given owner/repo. +// Priority: explicit version → latest release tag → default branch. +func ResolveRef(client RESTClient, host, owner, repo, version string) (*ResolvedRef, error) { + if version != "" { + return resolveExplicitRef(client, host, owner, repo, version) + } + ref, err := resolveLatestRelease(client, host, owner, repo) + if err == nil { + return ref, nil + } + return resolveDefaultBranch(client, host, owner, repo) +} + +// resolveExplicitRef resolves a user-supplied --pin value. It tries, in order: +// tag → commit SHA. Branches are deliberately excluded because they are mutable +// and pinning to one gives a false sense of reproducibility. +func resolveExplicitRef(client RESTClient, host, owner, repo, ref string) (*ResolvedRef, error) { + tagPath := fmt.Sprintf("repos/%s/%s/git/ref/tags/%s", owner, repo, ref) + var refResp struct { + Object struct { + SHA string `json:"sha"` + Type string `json:"type"` + } `json:"object"` + } + if err := client.REST(host, "GET", tagPath, nil, &refResp); err == nil { + sha := refResp.Object.SHA + if refResp.Object.Type == "tag" { + derefPath := fmt.Sprintf("repos/%s/%s/git/tags/%s", owner, repo, sha) + var tagResp struct { + Object struct { + SHA string `json:"sha"` + } `json:"object"` + } + if err := client.REST(host, "GET", derefPath, nil, &tagResp); err != nil { + return nil, fmt.Errorf("could not dereference annotated tag %q: %w", ref, err) + } + sha = tagResp.Object.SHA + } + return &ResolvedRef{Ref: ref, SHA: sha}, nil + } + + commitPath := fmt.Sprintf("repos/%s/%s/commits/%s", owner, repo, ref) + var commitResp struct { + SHA string `json:"sha"` + } + if err := client.REST(host, "GET", commitPath, nil, &commitResp); err == nil { + return &ResolvedRef{Ref: commitResp.SHA, SHA: commitResp.SHA}, nil + } + + return nil, fmt.Errorf("ref %q not found as tag or commit in %s/%s", ref, owner, repo) +} + +func resolveLatestRelease(client RESTClient, host, owner, repo string) (*ResolvedRef, error) { + apiPath := fmt.Sprintf("repos/%s/%s/releases/latest", owner, repo) + var release releaseResponse + if err := client.REST(host, "GET", apiPath, nil, &release); err != nil { + return nil, fmt.Errorf("no releases found: %w", err) + } + if release.TagName == "" { + return nil, fmt.Errorf("latest release has no tag") + } + return resolveExplicitRef(client, host, owner, repo, release.TagName) +} + +func resolveDefaultBranch(client RESTClient, host, owner, repo string) (*ResolvedRef, error) { + apiPath := fmt.Sprintf("repos/%s/%s", owner, repo) + var repoResp repoResponse + if err := client.REST(host, "GET", apiPath, nil, &repoResp); err != nil { + return nil, fmt.Errorf("could not determine default branch: %w", err) + } + branch := repoResp.DefaultBranch + if branch == "" { + branch = "main" + } + + refPath := fmt.Sprintf("repos/%s/%s/git/ref/heads/%s", owner, repo, branch) + var refResp struct { + Object struct { + SHA string `json:"sha"` + } `json:"object"` + } + if err := client.REST(host, "GET", refPath, nil, &refResp); err != nil { + return nil, fmt.Errorf("could not resolve branch %q: %w", branch, err) + } + + return &ResolvedRef{Ref: branch, SHA: refResp.Object.SHA}, nil +} + +// skillMatch represents a matched SKILL.md file and its convention. +type skillMatch struct { + entry treeEntry + name string + namespace string + skillDir string + convention string +} + +// MatchesSkillPath checks if a file path matches any known skill convention +// and returns the skill name. Returns empty string if the path doesn't match. +func MatchesSkillPath(filePath string) string { + m := matchSkillConventions(treeEntry{Path: filePath}) + if m == nil { + return "" + } + return m.name +} + +// matchSkillConventions checks if a blob path matches any known skill convention. +func matchSkillConventions(entry treeEntry) *skillMatch { + if path.Base(entry.Path) != "SKILL.md" { + return nil + } + + dir := path.Dir(entry.Path) + parentDir := path.Dir(dir) + skillName := path.Base(dir) + + if !ValidateName(skillName) { + return nil + } + + if parentDir == "skills" { + return &skillMatch{entry: entry, name: skillName, skillDir: dir, convention: "skills"} + } + + grandparentDir := path.Dir(parentDir) + if grandparentDir == "skills" { + namespace := path.Base(parentDir) + if !ValidateName(namespace) { + return nil + } + return &skillMatch{entry: entry, name: skillName, namespace: namespace, skillDir: dir, convention: "skills-namespaced"} + } + + if path.Base(parentDir) == "skills" && path.Dir(grandparentDir) == "plugins" { + namespace := path.Base(grandparentDir) + if !ValidateName(namespace) { + return nil + } + return &skillMatch{entry: entry, name: skillName, namespace: namespace, skillDir: dir, convention: "plugins"} + } + + if parentDir == "." && skillName != "skills" && skillName != "plugins" && !strings.HasPrefix(skillName, ".") { + return &skillMatch{entry: entry, name: skillName, skillDir: dir, convention: "root"} + } + + return nil +} + +// DiscoverSkills finds all skills in a repository at the given commit SHA. +func DiscoverSkills(client RESTClient, host, owner, repo, commitSHA string) ([]Skill, error) { + apiPath := fmt.Sprintf("repos/%s/%s/git/trees/%s?recursive=true", owner, repo, commitSHA) + var tree treeResponse + if err := client.REST(host, "GET", apiPath, nil, &tree); err != nil { + return nil, fmt.Errorf("could not fetch repository tree: %w", err) + } + + if tree.Truncated { + return nil, fmt.Errorf( + "repository tree for %s/%s is too large for full discovery\n"+ + " Use path-based install instead: gh skills install %s/%s skills/", + owner, repo, owner, repo, + ) + } + + treeSHAs := make(map[string]string) + for _, entry := range tree.Tree { + if entry.Type == "tree" { + treeSHAs[entry.Path] = entry.SHA + } + } + + seen := make(map[string]bool) + var matches []skillMatch + for _, entry := range tree.Tree { + if entry.Type != "blob" { + continue + } + m := matchSkillConventions(entry) + if m == nil { + continue + } + if seen[m.skillDir] { + continue + } + seen[m.skillDir] = true + matches = append(matches, *m) + } + + if len(matches) == 0 { + return nil, fmt.Errorf( + "no skills found in %s/%s\n"+ + " Expected skills in skills/*/SKILL.md, skills/{scope}/*/SKILL.md,\n"+ + " */SKILL.md, or plugins/*/skills/*/SKILL.md\n"+ + " This repository may be a curated list rather than a skills publisher", + owner, repo, + ) + } + + var skills []Skill + for _, m := range matches { + skills = append(skills, Skill{ + Name: m.name, + Namespace: m.namespace, + Path: m.skillDir, + BlobSHA: m.entry.SHA, + TreeSHA: treeSHAs[m.skillDir], + Convention: m.convention, + }) + } + + return skills, nil +} + +// FetchDescription fetches and parses the frontmatter description for a skill. +func FetchDescription(client RESTClient, host, owner, repo string, skill *Skill) string { + if skill.BlobSHA == "" { + return "" + } + content, err := FetchBlob(client, host, owner, repo, skill.BlobSHA) + if err != nil { + return "" + } + result, err := frontmatter.Parse(content) + if err != nil { + return "" + } + return result.Metadata.Description +} + +// FetchDescriptions fetches descriptions for a batch of skills. +func FetchDescriptions(client RESTClient, host, owner, repo string, skills []Skill) { + for i := range skills { + if skills[i].Description == "" { + skills[i].Description = FetchDescription(client, host, owner, repo, &skills[i]) + } + } +} + +// FetchDescriptionsConcurrent fetches descriptions with bounded concurrency. +func FetchDescriptionsConcurrent(client RESTClient, host, owner, repo string, skills []Skill, onProgress func(done, total int)) { + total := 0 + for _, s := range skills { + if s.Description == "" { + total++ + } + } + if total == 0 { + return + } + + const maxWorkers = 10 + sem := make(chan struct{}, maxWorkers) + var mu sync.Mutex + done := 0 + + var wg sync.WaitGroup + for i := range skills { + if skills[i].Description != "" { + continue + } + wg.Add(1) + go func(idx int) { + defer wg.Done() + sem <- struct{}{} + defer func() { <-sem }() + + desc := FetchDescription(client, host, owner, repo, &skills[idx]) + + mu.Lock() + skills[idx].Description = desc + done++ + d := done + mu.Unlock() + if onProgress != nil { + onProgress(d, total) + } + }(i) + } + wg.Wait() +} + +// DiscoverSkillByPath looks up a single skill by its exact path in the repository. +func DiscoverSkillByPath(client RESTClient, host, owner, repo, commitSHA, skillPath string) (*Skill, error) { + skillPath = strings.TrimSuffix(skillPath, "/SKILL.md") + skillPath = strings.TrimSuffix(skillPath, "/") + + skillName := path.Base(skillPath) + if !ValidateName(skillName) { + return nil, fmt.Errorf("invalid skill name %q", skillName) + } + + parentPath := path.Dir(skillPath) + apiPath := fmt.Sprintf("repos/%s/%s/contents/%s?ref=%s", owner, repo, parentPath, commitSHA) + + var contents []struct { + Name string `json:"name"` + Path string `json:"path"` + SHA string `json:"sha"` + Type string `json:"type"` + } + if err := client.REST(host, "GET", apiPath, nil, &contents); err != nil { + return nil, fmt.Errorf("path %q not found in %s/%s: %w", parentPath, owner, repo, err) + } + + var treeSHA string + for _, entry := range contents { + if entry.Name == skillName && entry.Type == "dir" { + treeSHA = entry.SHA + break + } + } + if treeSHA == "" { + return nil, fmt.Errorf("skill directory %q not found in %s/%s", skillPath, owner, repo) + } + + skillTreePath := fmt.Sprintf("repos/%s/%s/git/trees/%s", owner, repo, treeSHA) + var skillTree treeResponse + if err := client.REST(host, "GET", skillTreePath, nil, &skillTree); err != nil { + return nil, fmt.Errorf("could not read skill directory: %w", err) + } + + var blobSHA string + for _, entry := range skillTree.Tree { + if entry.Path == "SKILL.md" && entry.Type == "blob" { + blobSHA = entry.SHA + break + } + } + if blobSHA == "" { + return nil, fmt.Errorf("no SKILL.md found in %s", skillPath) + } + + var namespace string + parts := strings.Split(skillPath, "/") + if len(parts) >= 3 && parts[0] == "skills" { + namespace = parts[1] + } + + skill := &Skill{ + Name: skillName, + Namespace: namespace, + Path: skillPath, + BlobSHA: blobSHA, + TreeSHA: treeSHA, + } + + skill.Description = FetchDescription(client, host, owner, repo, skill) + + return skill, nil +} + +// DiscoverSkillFiles returns all file paths belonging to a skill directory +// by fetching the skill's subtree directly using its tree SHA. +func DiscoverSkillFiles(client RESTClient, host, owner, repo, treeSHA, skillPath string) ([]treeEntry, error) { + apiPath := fmt.Sprintf("repos/%s/%s/git/trees/%s?recursive=true", owner, repo, treeSHA) + var tree treeResponse + if err := client.REST(host, "GET", apiPath, nil, &tree); err != nil { + return nil, fmt.Errorf("could not fetch skill tree: %w", err) + } + + if tree.Truncated { + // Recursive fetch was truncated — fall back to walking subtrees individually. + return walkTree(client, host, owner, repo, treeSHA, skillPath) + } + + var files []treeEntry + for _, entry := range tree.Tree { + if entry.Type == "blob" { + files = append(files, treeEntry{ + Path: skillPath + "/" + entry.Path, + SHA: entry.SHA, + Size: entry.Size, + }) + } + } + + return files, nil +} + +// ListSkillFiles returns all files in a skill directory as public SkillFile +// structs with paths relative to the skill root. +func ListSkillFiles(client RESTClient, host, owner, repo, treeSHA string) ([]SkillFile, error) { + apiPath := fmt.Sprintf("repos/%s/%s/git/trees/%s?recursive=true", owner, repo, treeSHA) + var tree treeResponse + if err := client.REST(host, "GET", apiPath, nil, &tree); err != nil { + return nil, fmt.Errorf("could not fetch skill tree: %w", err) + } + + if tree.Truncated { + // Fall back to non-recursive traversal when the tree is too large. + entries, err := walkTree(client, host, owner, repo, treeSHA, "") + if err != nil { + return nil, err + } + var files []SkillFile + for _, e := range entries { + // walkTree prefixes with "/{path}", trim the leading slash. + p := strings.TrimPrefix(e.Path, "/") + files = append(files, SkillFile{Path: p, SHA: e.SHA, Size: e.Size}) + } + return files, nil + } + + var files []SkillFile + for _, entry := range tree.Tree { + if entry.Type == "blob" { + files = append(files, SkillFile{ + Path: entry.Path, + SHA: entry.SHA, + Size: entry.Size, + }) + } + } + return files, nil +} + +// walkTree enumerates files by fetching each tree level individually, +// avoiding the truncation limit of the recursive tree API. +func walkTree(client RESTClient, host, owner, repo, sha, prefix string) ([]treeEntry, error) { + apiPath := fmt.Sprintf("repos/%s/%s/git/trees/%s", owner, repo, sha) + var tree treeResponse + if err := client.REST(host, "GET", apiPath, nil, &tree); err != nil { + return nil, fmt.Errorf("could not fetch tree %s: %w", prefix, err) + } + + var files []treeEntry + for _, entry := range tree.Tree { + entryPath := prefix + "/" + entry.Path + switch entry.Type { + case "blob": + files = append(files, treeEntry{Path: entryPath, SHA: entry.SHA, Size: entry.Size}) + case "tree": + sub, err := walkTree(client, host, owner, repo, entry.SHA, entryPath) + if err != nil { + return nil, err + } + files = append(files, sub...) + } + } + return files, nil +} + +// FetchBlob retrieves the content of a blob by SHA. +func FetchBlob(client RESTClient, host, owner, repo, sha string) (string, error) { + apiPath := fmt.Sprintf("repos/%s/%s/git/blobs/%s", owner, repo, sha) + var blob blobResponse + if err := client.REST(host, "GET", apiPath, nil, &blob); err != nil { + return "", fmt.Errorf("could not fetch blob: %w", err) + } + + if blob.Encoding != "base64" { + 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. + 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) + } + + return string(decoded), nil +} + +// DiscoverLocalSkills finds skills in a local directory using the same +// conventions as remote discovery. +func DiscoverLocalSkills(dir string) ([]Skill, error) { + absDir, err := filepath.Abs(dir) + if err != nil { + return nil, fmt.Errorf("could not resolve path: %w", err) + } + + info, err := os.Stat(absDir) + if err != nil { + return nil, fmt.Errorf("could not access %s: %w", dir, err) + } + if !info.IsDir() { + return nil, fmt.Errorf("%s is not a directory", dir) + } + + if _, err := os.Stat(filepath.Join(absDir, "SKILL.md")); err == nil { + skill, err := localSkillFromDir(absDir) + if err != nil { + return nil, err + } + skill.Path = "." + return []Skill{*skill}, nil + } + + var skills []Skill + seen := make(map[string]bool) + + err = filepath.Walk(absDir, func(p string, info os.FileInfo, walkErr error) error { + if walkErr != nil { + return walkErr + } + if info.IsDir() || info.Name() != "SKILL.md" { + return nil + } + + relPath, relErr := filepath.Rel(absDir, p) + if relErr != nil { + return relErr + } + relPath = filepath.ToSlash(relPath) + + entry := treeEntry{Path: relPath, Type: "blob"} + m := matchSkillConventions(entry) + if m == nil { + return nil + } + if seen[m.skillDir] { + return nil + } + seen[m.skillDir] = true + + skill, skillErr := localSkillFromDir(filepath.Join(absDir, filepath.FromSlash(m.skillDir))) + if skillErr != nil { + return nil //nolint:nilerr // intentionally skip files that aren't valid skills + } + skill.Path = m.skillDir + skill.Namespace = m.namespace + skill.Convention = m.convention + skills = append(skills, *skill) + return nil + }) + if err != nil { + return nil, fmt.Errorf("could not walk directory: %w", err) + } + + if len(skills) == 0 { + return nil, fmt.Errorf( + "no skills found in %s\n"+ + " Expected SKILL.md in the directory, or skills in skills/*/SKILL.md,\n"+ + " skills/{scope}/*/SKILL.md, */SKILL.md, or plugins/*/skills/*/SKILL.md", + dir, + ) + } + + return skills, nil +} + +func localSkillFromDir(dir string) (*Skill, error) { + skillFile := filepath.Join(dir, "SKILL.md") + data, err := os.ReadFile(skillFile) + if err != nil { + return nil, fmt.Errorf("could not read %s: %w", skillFile, err) + } + + name := filepath.Base(dir) + var description string + + result, parseErr := frontmatter.Parse(string(data)) + if parseErr == nil { + if result.Metadata.Name != "" { + name = result.Metadata.Name + } + description = result.Metadata.Description + } + + if !ValidateName(name) { + return nil, fmt.Errorf("invalid skill name %q in %s", name, dir) + } + + return &Skill{ + Name: name, + Description: description, + Path: filepath.Base(dir), + }, nil +} + +// ValidateName checks if a skill name is safe for use (filesystem-safe). +func ValidateName(name string) bool { + if len(name) == 0 || len(name) > 64 { + return false + } + if strings.Contains(name, "/") || strings.Contains(name, "..") { + return false + } + return safeNamePattern.MatchString(name) +} + +// IsSpecCompliant checks if a skill name matches the strict agentskills.io spec. +func IsSpecCompliant(name string) bool { + if len(name) == 0 || len(name) > 64 { + return false + } + if strings.Contains(name, "--") { + return false + } + return specNamePattern.MatchString(name) +} + +// verifyBatchSize controls how many repos are checked per code-search API call. +const verifyBatchSize = 8 + +type codeSearchResponse struct { + Items []codeSearchItem `json:"items"` +} + +type codeSearchItem struct { + Repository codeSearchRepo `json:"repository"` +} + +type codeSearchRepo struct { + FullName string `json:"full_name"` +} + +// VerifySkillRepos filters a list of repository names to only those that +// actually contain SKILL.md files. It uses the GitHub code search API with +// batched repo: qualifiers. +// +// If a verification call fails (e.g. rate limit), repos in that batch are +// kept rather than silently dropped — we fail open. +func VerifySkillRepos(client RESTClient, host string, repos []string) map[string]bool { + verified := make(map[string]bool) + + for i := 0; i < len(repos); i += verifyBatchSize { + end := i + verifyBatchSize + if end > len(repos) { + end = len(repos) + } + batch := repos[i:end] + + var queryParts []string + queryParts = append(queryParts, "filename:SKILL.md") + for _, r := range batch { + queryParts = append(queryParts, "repo:"+r) + } + query := strings.Join(queryParts, "+") + apiPath := fmt.Sprintf("search/code?q=%s&per_page=%d", query, verifyBatchSize*3) + + var resp codeSearchResponse + if err := client.REST(host, "GET", apiPath, nil, &resp); err != nil { + // Fail open: if we can't verify, assume all repos in the batch are valid + for _, r := range batch { + verified[r] = true + } + continue + } + + for _, item := range resp.Items { + verified[item.Repository.FullName] = true + } + } + + return verified +} diff --git a/internal/skills/discovery/discovery_test.go b/internal/skills/discovery/discovery_test.go new file mode 100644 index 000000000..b5fe2410d --- /dev/null +++ b/internal/skills/discovery/discovery_test.go @@ -0,0 +1,109 @@ +package discovery + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestInstallName(t *testing.T) { + tests := []struct { + name string + skill Skill + wantName string + }{ + { + name: "plain skill", + skill: Skill{Name: "git-commit"}, + wantName: "git-commit", + }, + { + name: "namespaced skill", + skill: Skill{Name: "xlsx-pro", Namespace: "alice"}, + wantName: "alice/xlsx-pro", + }, + { + name: "plugin skill with namespace", + skill: Skill{Name: "code-review", Namespace: "bob", Convention: "plugins"}, + wantName: "bob/code-review", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.wantName, tt.skill.InstallName()) + }) + } +} + +func TestMatchSkillConventions_PluginNamespace(t *testing.T) { + entry := treeEntry{ + Path: "plugins/bob/skills/code-review/SKILL.md", + Type: "blob", + } + m := matchSkillConventions(entry) + assert.NotNil(t, m) + assert.Equal(t, "code-review", m.name) + assert.Equal(t, "bob", m.namespace) + assert.Equal(t, "plugins", m.convention) +} + +func TestMatchSkillConventions_NamespacedSkill(t *testing.T) { + entry := treeEntry{ + Path: "skills/alice/xlsx-pro/SKILL.md", + Type: "blob", + } + m := matchSkillConventions(entry) + assert.NotNil(t, m) + assert.Equal(t, "xlsx-pro", m.name) + assert.Equal(t, "alice", m.namespace) + assert.Equal(t, "skills-namespaced", m.convention) +} + +func TestMatchSkillConventions_RegularSkill(t *testing.T) { + entry := treeEntry{ + Path: "skills/git-commit/SKILL.md", + Type: "blob", + } + m := matchSkillConventions(entry) + assert.NotNil(t, m) + assert.Equal(t, "git-commit", m.name) + assert.Equal(t, "", m.namespace) + assert.Equal(t, "skills", m.convention) +} + +func TestDuplicatePluginSkills_DifferentAuthors(t *testing.T) { + // Simulates a repo with the same skill name under two different plugin authors. + // Previously this caused a collision error; now each gets a distinct namespace. + entries := []treeEntry{ + {Path: "plugins/author1/skills/azure-diag/SKILL.md", Type: "blob"}, + {Path: "plugins/author2/skills/azure-diag/SKILL.md", Type: "blob"}, + } + + seen := make(map[string]bool) + var matches []skillMatch + for _, e := range entries { + m := matchSkillConventions(e) + if m == nil || seen[m.skillDir] { + continue + } + seen[m.skillDir] = true + matches = append(matches, *m) + } + + assert.Len(t, matches, 2) + assert.Equal(t, "author1", matches[0].namespace) + assert.Equal(t, "author2", matches[1].namespace) + + // Build skills and verify they have different InstallNames + var skills []Skill + for _, m := range matches { + skills = append(skills, Skill{ + Name: m.name, + Namespace: m.namespace, + Convention: m.convention, + }) + } + assert.Equal(t, "author1/azure-diag", skills[0].InstallName()) + assert.Equal(t, "author2/azure-diag", skills[1].InstallName()) + assert.NotEqual(t, skills[0].InstallName(), skills[1].InstallName()) +} diff --git a/internal/skills/frontmatter/frontmatter.go b/internal/skills/frontmatter/frontmatter.go new file mode 100644 index 000000000..034068884 --- /dev/null +++ b/internal/skills/frontmatter/frontmatter.go @@ -0,0 +1,148 @@ +package frontmatter + +import ( + "bytes" + "fmt" + "strings" + + "gopkg.in/yaml.v3" +) + +const delimiter = "---" + +// Metadata represents the parsed YAML frontmatter of a SKILL.md file. +type Metadata struct { + Name string `yaml:"name"` + Description string `yaml:"description"` + License string `yaml:"license,omitempty"` + Meta map[string]interface{} `yaml:"metadata,omitempty"` +} + +// ParseResult contains the parsed frontmatter and remaining body. +type ParseResult struct { + Metadata Metadata + Body string + RawYAML map[string]interface{} +} + +// Parse extracts YAML frontmatter from a SKILL.md file. +// Frontmatter is delimited by --- on its own lines. +func Parse(content string) (*ParseResult, error) { + trimmed := strings.TrimLeft(content, "\r\n") + if !strings.HasPrefix(trimmed, delimiter) { + return &ParseResult{Body: content}, nil + } + + rest := trimmed[len(delimiter):] + rest = strings.TrimLeft(rest, "\r\n") + endIdx := strings.Index(rest, "\n"+delimiter) + if endIdx == -1 { + return &ParseResult{Body: content}, nil + } + + yamlContent := rest[:endIdx] + body := rest[endIdx+len("\n"+delimiter):] + body = strings.TrimLeft(body, "\r\n") + + var rawYAML map[string]interface{} + if err := yaml.Unmarshal([]byte(yamlContent), &rawYAML); err != nil { + return nil, fmt.Errorf("invalid frontmatter YAML: %w", err) + } + + var meta Metadata + if err := yaml.Unmarshal([]byte(yamlContent), &meta); err != nil { + return nil, fmt.Errorf("invalid frontmatter YAML: %w", err) + } + + return &ParseResult{ + Metadata: meta, + Body: body, + RawYAML: rawYAML, + }, nil +} + +// InjectGitHubMetadata adds GitHub tracking metadata to the spec-defined +// "metadata" map in frontmatter. Keys are prefixed with "github-" to avoid +// collisions with other tools' metadata. +// pinnedRef is the user's explicit --pin value; empty string means unpinned. +// skillPath is the skill's source path in the repo (e.g. "skills/author/my-skill"). +func InjectGitHubMetadata(content string, owner, repo, ref, sha, treeSHA, pinnedRef, skillPath string) (string, error) { + result, err := Parse(content) + if err != nil { + return "", err + } + + if result.RawYAML == nil { + result.RawYAML = make(map[string]interface{}) + } + + meta, _ := result.RawYAML["metadata"].(map[string]interface{}) + if meta == nil { + meta = make(map[string]interface{}) + } + meta["github-owner"] = owner + meta["github-repo"] = repo + meta["github-ref"] = ref + meta["github-sha"] = sha + meta["github-tree-sha"] = treeSHA + meta["github-path"] = skillPath + if pinnedRef != "" { + meta["github-pinned"] = pinnedRef + } else { + delete(meta, "github-pinned") + } + result.RawYAML["metadata"] = meta + + return Serialize(result.RawYAML, result.Body) +} + +// InjectLocalMetadata adds local-source tracking metadata to frontmatter. +// sourcePath is the absolute path to the source skill directory. +func InjectLocalMetadata(content string, sourcePath string) (string, error) { + result, err := Parse(content) + if err != nil { + return "", err + } + + if result.RawYAML == nil { + result.RawYAML = make(map[string]interface{}) + } + + meta, _ := result.RawYAML["metadata"].(map[string]interface{}) + if meta == nil { + meta = make(map[string]interface{}) + } + delete(meta, "github-owner") + delete(meta, "github-repo") + delete(meta, "github-ref") + delete(meta, "github-sha") + delete(meta, "github-tree-sha") + delete(meta, "github-pinned") + delete(meta, "github-path") + meta["local-path"] = sourcePath + result.RawYAML["metadata"] = meta + + return Serialize(result.RawYAML, result.Body) +} + +// Serialize writes a frontmatter map and body back to a SKILL.md string. +func Serialize(frontmatter map[string]interface{}, body string) (string, error) { + var buf bytes.Buffer + + yamlBytes, err := yaml.Marshal(frontmatter) + if err != nil { + return "", fmt.Errorf("failed to serialize frontmatter: %w", err) + } + + buf.WriteString(delimiter + "\n") + buf.Write(yamlBytes) + buf.WriteString(delimiter + "\n") + if body != "" { + buf.WriteString(body) + if !strings.HasSuffix(body, "\n") { + buf.WriteString("\n") + } + } + + return buf.String(), nil +} diff --git a/internal/skills/frontmatter/frontmatter_test.go b/internal/skills/frontmatter/frontmatter_test.go new file mode 100644 index 000000000..02bd1ee0e --- /dev/null +++ b/internal/skills/frontmatter/frontmatter_test.go @@ -0,0 +1,178 @@ +package frontmatter + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestParse(t *testing.T) { + tests := []struct { + name string + content string + wantName string + wantDesc string + wantBody string + wantErr bool + }{ + { + name: "valid frontmatter", + content: "---\nname: test-skill\ndescription: A test skill\n---\n# Body\n", + wantName: "test-skill", + wantDesc: "A test skill", + wantBody: "# Body\n", + }, + { + name: "no frontmatter", + content: "# Just a markdown file\n", + wantBody: "# Just a markdown file\n", + }, + { + name: "invalid YAML", + content: "---\n: invalid yaml [[\n---\n", + wantErr: true, + }, + { + name: "no closing delimiter", + content: "---\nname: test\n", + wantBody: "---\nname: test\n", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := Parse(tt.content) + if tt.wantErr { + assert.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, tt.wantName, result.Metadata.Name) + assert.Equal(t, tt.wantDesc, result.Metadata.Description) + assert.Equal(t, tt.wantBody, result.Body) + }) + } +} + +func TestInjectGitHubMetadata(t *testing.T) { + tests := []struct { + name string + content string + owner string + repo string + ref string + sha string + treeSHA string + pinnedRef string + skillPath string + wantContains []string + wantNotContain []string + }{ + { + name: "injects metadata without pin", + content: "---\nname: my-skill\ndescription: desc\n---\n# Body\n", + owner: "owner", + repo: "repo", + ref: "v1.0.0", + sha: "abc123", + treeSHA: "tree456", + pinnedRef: "", + skillPath: "skills/my-skill", + wantContains: []string{ + "github-owner: owner", + "github-repo: repo", + "github-ref: v1.0.0", + "github-sha: abc123", + "github-tree-sha: tree456", + "github-path: skills/my-skill", + "# Body", + }, + wantNotContain: []string{ + "github-pinned", + }, + }, + { + name: "injects pinned ref", + content: "---\nname: my-skill\n---\n# Body\n", + owner: "owner", + repo: "repo", + ref: "v1.0.0", + sha: "abc", + treeSHA: "tree", + pinnedRef: "v1.0.0", + skillPath: "skills/my-skill", + wantContains: []string{ + "github-pinned: v1.0.0", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := InjectGitHubMetadata(tt.content, tt.owner, tt.repo, tt.ref, tt.sha, tt.treeSHA, tt.pinnedRef, tt.skillPath) + require.NoError(t, err) + for _, s := range tt.wantContains { + assert.Contains(t, got, s) + } + for _, s := range tt.wantNotContain { + assert.NotContains(t, got, s) + } + }) + } +} + +func TestInjectLocalMetadata(t *testing.T) { + content := "---\nname: my-skill\nmetadata:\n github-owner: old\n github-repo: old\n---\n# Body\n" + got, err := InjectLocalMetadata(content, "/home/user/skills/my-skill") + require.NoError(t, err) + + assert.Contains(t, got, "local-path: /home/user/skills/my-skill") + assert.NotContains(t, got, "github-owner") + assert.NotContains(t, got, "github-repo") +} + +func TestSerialize(t *testing.T) { + tests := []struct { + name string + frontmatter map[string]interface{} + body string + wantPrefix string + wantSuffix string + wantContains []string + }{ + { + name: "with body", + frontmatter: map[string]interface{}{"name": "test"}, + body: "# Body content", + wantPrefix: "---\n", + wantContains: []string{ + "name: test", + "# Body content", + }, + }, + { + name: "empty body", + frontmatter: map[string]interface{}{"name": "test"}, + body: "", + wantSuffix: "---\n", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := Serialize(tt.frontmatter, tt.body) + require.NoError(t, err) + if tt.wantPrefix != "" { + assert.True(t, strings.HasPrefix(got, tt.wantPrefix)) + } + if tt.wantSuffix != "" { + assert.True(t, strings.HasSuffix(got, tt.wantSuffix)) + } + for _, s := range tt.wantContains { + assert.Contains(t, got, s) + } + }) + } +} diff --git a/internal/skills/gitclient/gitclient.go b/internal/skills/gitclient/gitclient.go new file mode 100644 index 000000000..99735db90 --- /dev/null +++ b/internal/skills/gitclient/gitclient.go @@ -0,0 +1,149 @@ +// Package gitclient provides a shared adapter from the cli/cli git.Client +// (via cmdutil.Factory) to the narrow interfaces used by skills commands. +package gitclient + +import ( + "context" + "os" + "strings" + + "github.com/cli/cli/v2/pkg/cmdutil" +) + +// RootResolver can resolve the git repository root directory. +type RootResolver interface { + ToplevelDir() (string, error) +} + +// RemoteResolver can resolve git remote URLs. +type RemoteResolver interface { + RemoteURL(name string) (string, error) +} + +// Client is the full git operations interface used by skills commands. +type Client interface { + RootResolver + RemoteResolver + GitDir(dir string) error + Remotes() ([]string, error) + CurrentBranch(dir string) (string, error) + IsIgnored(dir, path string) bool +} + +// FactoryClient adapts the cli/cli git.Client to the Client interface. +type FactoryClient struct { + F *cmdutil.Factory +} + +// ToplevelDir returns the root directory of the current git repository. +func (g *FactoryClient) ToplevelDir() (string, error) { + cmd, err := g.F.GitClient.Command(context.Background(), "rev-parse", "--show-toplevel") + if err != nil { + return "", err + } + out, err := cmd.Output() + if err != nil { + return "", err + } + return strings.TrimSpace(string(out)), nil +} + +// RemoteURL returns the URL configured for the named git remote. +func (g *FactoryClient) RemoteURL(name string) (string, error) { + cmd, err := g.F.GitClient.Command(context.Background(), "remote", "get-url", name) + if err != nil { + return "", err + } + out, err := cmd.Output() + if err != nil { + return "", err + } + return strings.TrimSpace(string(out)), nil +} + +// GitDir validates that the given directory is inside a git repository. +func (g *FactoryClient) GitDir(dir string) error { + cmd, err := g.F.GitClient.Command(context.Background(), "-C", dir, "rev-parse", "--git-dir") + if err != nil { + return err + } + _, err = cmd.Output() + return err +} + +// Remotes returns the list of configured git remote names. +func (g *FactoryClient) Remotes() ([]string, error) { + cmd, err := g.F.GitClient.Command(context.Background(), "remote") + if err != nil { + return nil, err + } + out, err := cmd.Output() + if err != nil { + return nil, err + } + return strings.Fields(string(out)), nil +} + +// CurrentBranch returns the current branch name, or "" if HEAD is detached. +func (g *FactoryClient) CurrentBranch(dir string) (string, error) { + cmd, err := g.F.GitClient.Command(context.Background(), "-C", dir, "rev-parse", "--abbrev-ref", "HEAD") + if err != nil { + return "", err + } + out, err := cmd.Output() + if err != nil { + return "", err + } + branch := strings.TrimSpace(string(out)) + if branch == "HEAD" { + return "", nil // detached HEAD + } + return branch, nil +} + +// IsIgnored reports whether the given path is git-ignored in the given directory. +func (g *FactoryClient) IsIgnored(dir, path string) bool { + cmd, err := g.F.GitClient.Command(context.Background(), "-C", dir, "check-ignore", "-q", path) + if err != nil { + return false + } + _, err = cmd.Output() + return err == nil +} + +// ResolveGitRoot returns the git repository root using the provided resolver, +// falling back to the current working directory on error. +func ResolveGitRoot(resolver RootResolver) string { + if resolver == nil { + if cwd, err := os.Getwd(); err == nil { + return cwd + } + return "" + } + root, err := resolver.ToplevelDir() + if err != nil { + if cwd, cwdErr := os.Getwd(); cwdErr == nil { + return cwd + } + return "" + } + return root +} + +// ResolveHomeDir returns the user's home directory, or "" on error. +func ResolveHomeDir() string { + home, err := os.UserHomeDir() + if err != nil { + return "" + } + return home +} + +// TruncateSHA returns the first 8 characters of a SHA, or the full string +// if it is shorter. +func TruncateSHA(sha string) string { + if len(sha) > 8 { + return sha[:8] + } + return sha +} diff --git a/internal/skills/gitclient/gitclient_test.go b/internal/skills/gitclient/gitclient_test.go new file mode 100644 index 000000000..0b8a2cfff --- /dev/null +++ b/internal/skills/gitclient/gitclient_test.go @@ -0,0 +1,49 @@ +package gitclient + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" +) + +type mockResolver struct { + root string + err error +} + +func (m *mockResolver) ToplevelDir() (string, error) { + if m.err != nil { + return "", m.err + } + return m.root, nil +} + +func TestResolveGitRoot(t *testing.T) { + t.Run("returns root on success", func(t *testing.T) { + got := ResolveGitRoot(&mockResolver{root: "/my/repo"}) + assert.Equal(t, "/my/repo", got) + }) + + t.Run("falls back to cwd on error", func(t *testing.T) { + got := ResolveGitRoot(&mockResolver{err: fmt.Errorf("not a git repo")}) + assert.NotEmpty(t, got) // falls back to cwd + }) + + t.Run("nil resolver falls back to cwd", func(t *testing.T) { + got := ResolveGitRoot(nil) + assert.NotEmpty(t, got) // falls back to cwd + }) +} + +func TestResolveHomeDir(t *testing.T) { + got := ResolveHomeDir() + assert.NotEmpty(t, got) +} + +func TestTruncateSHA(t *testing.T) { + assert.Equal(t, "abcdef12", TruncateSHA("abcdef1234567890")) + assert.Equal(t, "short", TruncateSHA("short")) + assert.Equal(t, "12345678", TruncateSHA("12345678")) + assert.Equal(t, "", TruncateSHA("")) +} diff --git a/internal/skills/hosts/hosts.go b/internal/skills/hosts/hosts.go new file mode 100644 index 000000000..bee20b0f0 --- /dev/null +++ b/internal/skills/hosts/hosts.go @@ -0,0 +1,175 @@ +package hosts + +import ( + "fmt" + "path/filepath" + + "github.com/cli/cli/v2/git" + "github.com/cli/cli/v2/internal/ghrepo" +) + +// Host represents an AI agent that can use skills. +type Host struct { + // ID is the canonical identifier for this host. + ID string + // Name is the human-readable display name. + Name string + // ProjectDir is the relative path within a project for skills. + ProjectDir string + // UserDir is the relative path within the user's home directory for skills. + UserDir string +} + +// Scope determines where skills are installed. +type Scope string + +const ( + ScopeProject Scope = "project" + ScopeUser Scope = "user" +) + +// Registry contains all known agent hosts. +var Registry = []Host{ + { + ID: "github-copilot", + Name: "GitHub Copilot", + ProjectDir: ".github/skills", + UserDir: ".copilot/skills", + }, + { + ID: "claude-code", + Name: "Claude Code", + ProjectDir: ".claude/skills", + UserDir: ".claude/skills", + }, + { + ID: "cursor", + Name: "Cursor", + ProjectDir: ".cursor/skills", + UserDir: ".cursor/skills", + }, + { + ID: "codex", + Name: "Codex", + ProjectDir: ".agents/skills", + UserDir: ".codex/skills", + }, + { + ID: "gemini", + Name: "Gemini CLI", + ProjectDir: ".agent/skills", + UserDir: ".gemini/skills", + }, + { + ID: "antigravity", + Name: "Antigravity", + ProjectDir: ".agent/skills", + UserDir: ".gemini/antigravity/skills", + }, +} + +// FindByID returns the host with the given ID, or an error if not found. +func FindByID(id string) (*Host, error) { + for i := range Registry { + if Registry[i].ID == id { + return &Registry[i], nil + } + } + return nil, fmt.Errorf("unknown host %q, valid hosts: %s", id, ValidHostIDs()) +} + +// ValidHostIDs returns a comma-separated list of valid host IDs. +func ValidHostIDs() string { + ids := "" + for i, h := range Registry { + if i > 0 { + ids += ", " + } + ids += h.ID + } + return ids +} + +// HostIDs returns the IDs of all known hosts as a slice. +func HostIDs() []string { + ids := make([]string, len(Registry)) + for i, h := range Registry { + ids[i] = h.ID + } + return ids +} + +// HostNames returns the display names of all hosts for prompting. +func HostNames() []string { + names := make([]string, len(Registry)) + for i, h := range Registry { + names[i] = h.Name + } + return names +} + +// UniqueProjectDirs returns the deduplicated set of project-scope skill +// directories from the Registry, preserving insertion order. +func UniqueProjectDirs() []string { + seen := map[string]bool{} + var dirs []string + for _, h := range Registry { + if !seen[h.ProjectDir] { + seen[h.ProjectDir] = true + dirs = append(dirs, h.ProjectDir) + } + } + return dirs +} + +// InstallDir resolves the absolute installation directory for a host and scope. +// For project scope, it uses the provided git root directory so that skills are +// installed at the top level regardless of which subdirectory the user is in. +// Returns an error when gitRoot is empty (not in a git repository). +// For user scope, it uses the home directory. +func (h *Host) InstallDir(scope Scope, gitRoot, homeDir string) (string, error) { + switch scope { + case ScopeProject: + if gitRoot == "" { + return "", fmt.Errorf("could not determine project root directory") + } + return filepath.Join(gitRoot, h.ProjectDir), nil + case ScopeUser: + if homeDir == "" { + return "", fmt.Errorf("could not determine home directory") + } + return filepath.Join(homeDir, h.UserDir), nil + default: + return "", fmt.Errorf("invalid scope %q", scope) + } +} + +// ScopeLabels returns the display labels for the scope selection prompt. +// If repoName is non-empty, it is included in the project-scope label +// for additional context. +func ScopeLabels(repoName string) []string { + projectLabel := "Project — install in current repository (recommended)" + if repoName != "" { + projectLabel = fmt.Sprintf("Project — %s (recommended)", repoName) + } + return []string{ + projectLabel, + "Global — install in home directory (available everywhere)", + } +} + +// RepoNameFromRemote extracts "owner/repo" from a git remote URL. +func RepoNameFromRemote(remote string) string { + if remote == "" { + return "" + } + u, err := git.ParseURL(remote) + if err != nil { + return "" + } + repo, err := ghrepo.FromURL(u) + if err != nil { + return "" + } + return ghrepo.FullName(repo) +} diff --git a/internal/skills/hosts/hosts_test.go b/internal/skills/hosts/hosts_test.go new file mode 100644 index 000000000..78c2a3e9d --- /dev/null +++ b/internal/skills/hosts/hosts_test.go @@ -0,0 +1,113 @@ +package hosts + +import ( + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestFindByID(t *testing.T) { + host, err := FindByID("github-copilot") + require.NoError(t, err) + assert.Equal(t, "GitHub Copilot", host.Name) + assert.Equal(t, ".github/skills", host.ProjectDir) +} + +func TestFindByID_Invalid(t *testing.T) { + _, err := FindByID("nonexistent") + assert.Error(t, err) + assert.Contains(t, err.Error(), "unknown host") +} + +func TestValidHostIDs(t *testing.T) { + ids := ValidHostIDs() + assert.Contains(t, ids, "github-copilot") + assert.Contains(t, ids, "claude-code") + assert.Contains(t, ids, "cursor") +} + +func TestHostNames(t *testing.T) { + names := HostNames() + assert.Contains(t, names, "GitHub Copilot") + assert.Contains(t, names, "Claude Code") +} + +func TestInstallDir_Project(t *testing.T) { + host, _ := FindByID("github-copilot") + dir, err := host.InstallDir(ScopeProject, "/tmp/myrepo", "/home/user") + require.NoError(t, err) + assert.Equal(t, filepath.Join("/tmp/myrepo", ".github", "skills"), dir) +} + +func TestInstallDir_User(t *testing.T) { + host, _ := FindByID("github-copilot") + dir, err := host.InstallDir(ScopeUser, "/tmp/myrepo", "/home/user") + require.NoError(t, err) + assert.Equal(t, filepath.Join("/home/user", ".copilot", "skills"), dir) +} + +func TestInstallDir_NoGitRoot(t *testing.T) { + host, _ := FindByID("github-copilot") + _, err := host.InstallDir(ScopeProject, "", "/home/user") + assert.Error(t, err) +} + +func TestRepoNameFromRemote(t *testing.T) { + tests := []struct { + remote string + want string + }{ + {"https://github.com/owner/repo.git", "owner/repo"}, + {"https://github.com/owner/repo", "owner/repo"}, + {"git@github.com:owner/repo.git", "owner/repo"}, + {"git@github.com:owner/repo", "owner/repo"}, + {"ssh://git@github.com/owner/repo.git", "owner/repo"}, + {"ssh://git@github.com/owner/repo", "owner/repo"}, + {"", ""}, + } + for _, tt := range tests { + t.Run(tt.remote, func(t *testing.T) { + assert.Equal(t, tt.want, RepoNameFromRemote(tt.remote)) + }) + } +} + +func TestUniqueProjectDirs(t *testing.T) { + dirs := UniqueProjectDirs() + + // Should contain all known project dirs + assert.Contains(t, dirs, ".github/skills") + assert.Contains(t, dirs, ".claude/skills") + assert.Contains(t, dirs, ".cursor/skills") + assert.Contains(t, dirs, ".agents/skills") + assert.Contains(t, dirs, ".agent/skills") + + // Should deduplicate — gemini and antigravity share .agent/skills + seen := map[string]int{} + for _, d := range dirs { + seen[d]++ + } + for dir, count := range seen { + assert.Equalf(t, 1, count, "directory %q appears %d times, expected 1", dir, count) + } +} + +func TestScopeLabels(t *testing.T) { + t.Run("without repo name", func(t *testing.T) { + labels := ScopeLabels("") + require.Len(t, labels, 2) + assert.Contains(t, labels[0], "Project") + assert.Contains(t, labels[0], "recommended") + assert.Contains(t, labels[1], "Global") + }) + + t.Run("with repo name", func(t *testing.T) { + labels := ScopeLabels("owner/repo") + require.Len(t, labels, 2) + assert.Contains(t, labels[0], "owner/repo") + assert.Contains(t, labels[0], "recommended") + assert.Contains(t, labels[1], "Global") + }) +} diff --git a/internal/skills/installer/installer.go b/internal/skills/installer/installer.go new file mode 100644 index 000000000..fa2854a7c --- /dev/null +++ b/internal/skills/installer/installer.go @@ -0,0 +1,296 @@ +package installer + +import ( + "fmt" + "os" + "path/filepath" + "strings" + "sync" + + "github.com/cli/cli/v2/internal/skills/discovery" + "github.com/cli/cli/v2/internal/skills/frontmatter" + "github.com/cli/cli/v2/internal/skills/hosts" + "github.com/cli/cli/v2/internal/skills/lockfile" +) + +// maxConcurrency limits parallel API requests to avoid rate limiting. +const maxConcurrency = 5 + +// Options configures an installation. +type Options struct { + Host string // GitHub API hostname + Owner string + Repo string + Ref string // resolved ref name + SHA string // resolved commit SHA + PinnedRef string // user-supplied --pin value (empty if unpinned) + Skills []discovery.Skill + AgentHost *hosts.Host + Scope hosts.Scope + Dir string // explicit target directory (overrides AgentHost+Scope) + GitRoot string // git repository root (for project scope) + HomeDir string // user home directory (for user scope) + Client discovery.RESTClient + OnProgress func(done, total int) // called after each skill is installed +} + +// Result tracks what was installed. +type Result struct { + Installed []string + Dir string + Warnings []string +} + +type skillResult struct { + name string + err error +} + +// Install fetches and writes skills to the target directory. +func Install(opts *Options) (*Result, error) { + targetDir := opts.Dir + if targetDir == "" { + var err error + targetDir, err = opts.AgentHost.InstallDir(opts.Scope, opts.GitRoot, opts.HomeDir) + if err != nil { + return nil, err + } + } + + if len(opts.Skills) == 1 { + skill := opts.Skills[0] + if opts.OnProgress != nil { + opts.OnProgress(0, 1) + } + if err := installSkill(opts, skill, targetDir); err != nil { + return nil, fmt.Errorf("failed to install skill %q: %w", skill.InstallName(), err) + } + var warnings []string + if err := lockfile.RecordInstall(skill.InstallName(), opts.Owner, opts.Repo, skill.Path+"/SKILL.md", skill.TreeSHA, opts.PinnedRef); err != nil { + warnings = append(warnings, fmt.Sprintf("could not record install for %s: %v", skill.InstallName(), err)) + } + if opts.OnProgress != nil { + opts.OnProgress(1, 1) + } + return &Result{Installed: []string{skill.InstallName()}, Dir: targetDir, Warnings: warnings}, nil + } + + total := len(opts.Skills) + if opts.OnProgress != nil { + opts.OnProgress(0, total) + } + + sem := make(chan struct{}, maxConcurrency) + results := make([]skillResult, total) + var wg sync.WaitGroup + var mu sync.Mutex + done := 0 + + for i, skill := range opts.Skills { + wg.Add(1) + go func(idx int, s discovery.Skill) { + defer wg.Done() + sem <- struct{}{} + defer func() { <-sem }() + + err := installSkill(opts, s, targetDir) + results[idx] = skillResult{name: s.InstallName(), err: err} + + if opts.OnProgress != nil { + mu.Lock() + done++ + d := done + mu.Unlock() + opts.OnProgress(d, total) + } + }(i, skill) + } + wg.Wait() + + var installed []string + var warnings []string + var firstErr error + for i, r := range results { + if r.err != nil { + if firstErr == nil { + firstErr = fmt.Errorf("failed to install skill %q: %w", r.name, r.err) + } + continue + } + installed = append(installed, r.name) + skill := opts.Skills[i] + if err := lockfile.RecordInstall(skill.InstallName(), opts.Owner, opts.Repo, skill.Path+"/SKILL.md", skill.TreeSHA, opts.PinnedRef); err != nil { + warnings = append(warnings, fmt.Sprintf("could not record install for %s: %v", skill.InstallName(), err)) + } + } + + if firstErr != nil { + return &Result{Installed: installed, Dir: targetDir, Warnings: warnings}, firstErr + } + + return &Result{Installed: installed, Dir: targetDir, Warnings: warnings}, nil +} + +// LocalOptions configures a local directory installation. +type LocalOptions struct { + SourceDir string + Skills []discovery.Skill + AgentHost *hosts.Host + Scope hosts.Scope + Dir string + GitRoot string + HomeDir string +} + +// InstallLocal copies skills from a local directory to the target install location. +func InstallLocal(opts *LocalOptions) (*Result, error) { + targetDir := opts.Dir + if targetDir == "" { + var err error + targetDir, err = opts.AgentHost.InstallDir(opts.Scope, opts.GitRoot, opts.HomeDir) + if err != nil { + return nil, err + } + } + + var installed []string + for _, skill := range opts.Skills { + if err := installLocalSkill(opts.SourceDir, skill, targetDir); err != nil { + return nil, fmt.Errorf("failed to install skill %q: %w", skill.InstallName(), err) + } + installed = append(installed, skill.InstallName()) + } + + return &Result{Installed: installed, Dir: targetDir}, nil +} + +func installLocalSkill(sourceRoot string, skill discovery.Skill, baseDir string) error { + skillDir := filepath.Join(baseDir, filepath.FromSlash(skill.InstallName())) + if err := os.MkdirAll(skillDir, 0o755); err != nil { + return fmt.Errorf("could not create directory %s: %w", skillDir, err) + } + + srcDir := filepath.Join(sourceRoot, filepath.FromSlash(skill.Path)) + absSource, err := filepath.Abs(srcDir) + if err != nil { + return fmt.Errorf("could not resolve source path: %w", err) + } + + absSkillDir, err := filepath.Abs(skillDir) + if err != nil { + return fmt.Errorf("could not resolve target path: %w", err) + } + + return filepath.WalkDir(srcDir, func(p string, d os.DirEntry, walkErr error) error { + if walkErr != nil { + return walkErr + } + if d.Type()&os.ModeSymlink != 0 { + return nil + } + if d.IsDir() { + return nil + } + + relPath, err := filepath.Rel(srcDir, p) + if err != nil { + return err + } + + cleaned := filepath.Clean(relPath) + if cleaned == ".." || strings.HasPrefix(cleaned, ".."+string(filepath.Separator)) { + return nil + } + + destPath := filepath.Join(skillDir, cleaned) + + absDest, err := filepath.Abs(destPath) + if err != nil { + return fmt.Errorf("could not resolve destination path: %w", err) + } + if !strings.HasPrefix(absDest, absSkillDir+string(filepath.Separator)) && absDest != absSkillDir { + return nil + } + + if dir := filepath.Dir(destPath); dir != skillDir { + if err := os.MkdirAll(dir, 0o755); err != nil { + return fmt.Errorf("could not create directory: %w", err) + } + } + + content, err := os.ReadFile(p) + if err != nil { + return fmt.Errorf("could not read %s: %w", p, err) + } + + if filepath.Base(relPath) == "SKILL.md" { + injected, injectErr := frontmatter.InjectLocalMetadata(string(content), absSource) + if injectErr != nil { + return fmt.Errorf("could not inject metadata: %w", injectErr) + } + content = []byte(injected) + } + + return os.WriteFile(destPath, content, 0o644) + }) +} + +func installSkill(opts *Options, skill discovery.Skill, baseDir string) error { + skillDir := filepath.Join(baseDir, filepath.FromSlash(skill.InstallName())) + if err := os.MkdirAll(skillDir, 0o755); err != nil { + return fmt.Errorf("could not create directory %s: %w", skillDir, err) + } + + files, err := discovery.DiscoverSkillFiles(opts.Client, opts.Host, opts.Owner, opts.Repo, skill.TreeSHA, skill.Path) + if err != nil { + return fmt.Errorf("could not list skill files: %w", err) + } + + absSkillDir, err := filepath.Abs(skillDir) + if err != nil { + return fmt.Errorf("could not resolve skill directory path: %w", err) + } + + for _, file := range files { + content, err := discovery.FetchBlob(opts.Client, opts.Host, opts.Owner, opts.Repo, file.SHA) + if err != nil { + return fmt.Errorf("could not fetch %s: %w", file.Path, err) + } + + relPath := strings.TrimPrefix(file.Path, skill.Path+"/") + + cleaned := filepath.Clean(relPath) + if cleaned == ".." || strings.HasPrefix(cleaned, ".."+string(filepath.Separator)) { + continue + } + + destPath := filepath.Join(skillDir, cleaned) + + absDest, err := filepath.Abs(destPath) + if err != nil { + return fmt.Errorf("could not resolve destination path: %w", err) + } + if !strings.HasPrefix(absDest, absSkillDir+string(filepath.Separator)) && absDest != absSkillDir { + continue + } + + if dir := filepath.Dir(destPath); dir != skillDir { + if err := os.MkdirAll(dir, 0o755); err != nil { + return fmt.Errorf("could not create directory: %w", err) + } + } + + if filepath.Base(relPath) == "SKILL.md" { + content, err = frontmatter.InjectGitHubMetadata(content, opts.Owner, opts.Repo, opts.Ref, file.SHA, skill.TreeSHA, opts.PinnedRef, skill.Path) + if err != nil { + return fmt.Errorf("could not inject metadata: %w", err) + } + } + + if err := os.WriteFile(destPath, []byte(content), 0o644); err != nil { + return fmt.Errorf("could not write %s: %w", destPath, err) + } + } + + return nil +} diff --git a/internal/skills/lockfile/lockfile.go b/internal/skills/lockfile/lockfile.go new file mode 100644 index 000000000..4ceed7872 --- /dev/null +++ b/internal/skills/lockfile/lockfile.go @@ -0,0 +1,165 @@ +package lockfile + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + "time" +) + +const ( + // lockVersion must match Vercel's CURRENT_LOCK_VERSION for interop. + lockVersion = 3 + agentsDir = ".agents" + lockFile = ".skill-lock.json" +) + +// Entry represents a single installed skill in the lock file. +type Entry struct { + Source string `json:"source"` + SourceType string `json:"sourceType"` + SourceURL string `json:"sourceUrl"` + SkillPath string `json:"skillPath,omitempty"` + SkillFolderHash string `json:"skillFolderHash"` + InstalledAt string `json:"installedAt"` + UpdatedAt string `json:"updatedAt"` + PinnedRef string `json:"pinnedRef,omitempty"` +} + +// File is the top-level structure of .skill-lock.json. +type File struct { + Version int `json:"version"` + Skills map[string]Entry `json:"skills"` + Dismissed map[string]bool `json:"dismissed,omitempty"` +} + +// Path returns the absolute path to the lock file. +func Path() (string, error) { + home, err := os.UserHomeDir() + if err != nil { + return "", err + } + return filepath.Join(home, agentsDir, lockFile), nil +} + +// Read loads the lock file, returning an empty file if it doesn't exist +// or if it's an incompatible version. +func Read() (*File, error) { + lockPath, err := Path() + if err != nil { + return newFile(), nil //nolint:nilerr // graceful: no home dir means fresh state + } + + data, err := os.ReadFile(lockPath) + if err != nil { + if os.IsNotExist(err) { + return newFile(), nil + } + return nil, fmt.Errorf("could not read lock file: %w", err) + } + + var f File + if err := json.Unmarshal(data, &f); err != nil { + return newFile(), nil //nolint:nilerr // graceful: corrupt file means fresh state + } + + if f.Version != lockVersion || f.Skills == nil { + return newFile(), nil + } + + return &f, nil +} + +// Write persists the lock file to disk. +func Write(f *File) error { + lockPath, err := Path() + if err != nil { + return err + } + + if err := os.MkdirAll(filepath.Dir(lockPath), 0o755); err != nil { + return err + } + + data, err := json.MarshalIndent(f, "", " ") + if err != nil { + return err + } + + return os.WriteFile(lockPath, data, 0o644) +} + +// RecordInstall adds or updates a skill entry in the lock file. +// It uses a file-based lock to prevent concurrent read-modify-write races +// when multiple install processes run simultaneously. +func RecordInstall(skillName, owner, repo, skillPath, treeSHA, pinnedRef string) error { + unlock := acquireLock() + defer unlock() + + f, err := Read() + if err != nil { + return err + } + + now := time.Now().UTC().Format(time.RFC3339) + + existing, exists := f.Skills[skillName] + installedAt := now + if exists { + installedAt = existing.InstalledAt + } + + f.Skills[skillName] = Entry{ + Source: owner + "/" + repo, + SourceType: "github", + SourceURL: "https://github.com/" + owner + "/" + repo + ".git", + SkillPath: skillPath, + SkillFolderHash: treeSHA, + InstalledAt: installedAt, + UpdatedAt: now, + PinnedRef: pinnedRef, + } + + return Write(f) +} + +func newFile() *File { + return &File{ + Version: lockVersion, + Skills: make(map[string]Entry), + } +} + +// acquireLock creates an exclusive lock file to serialize concurrent access. +// Returns an unlock function. If locking fails after retries, it proceeds +// unlocked rather than blocking the user indefinitely. +func acquireLock() (unlock func()) { + lockPath, pathErr := Path() + if pathErr != nil { + return func() {} + } + lkPath := lockPath + ".lk" + + // Ensure the parent directory exists (fresh machine may lack ~/.agents). + if err := os.MkdirAll(filepath.Dir(lockPath), 0o755); err != nil { + return func() {} + } + + for i := 0; i < 30; i++ { + f, createErr := os.OpenFile(lkPath, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0o644) + if createErr == nil { + f.Close() + return func() { os.Remove(lkPath) } + } + // 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) + continue + } + time.Sleep(100 * time.Millisecond) + } + + // Best-effort: proceed without lock. + return func() {} +}