diff --git a/internal/skills/discovery/discovery.go b/internal/skills/discovery/discovery.go index df17c5b9f21..ff6f1286e6b 100644 --- a/internal/skills/discovery/discovery.go +++ b/internal/skills/discovery/discovery.go @@ -390,6 +390,28 @@ func MatchSkillPath(filePath string) (name, namespace string) { return m.name, m.namespace } +// IsSkillPath reports whether a skill selector looks like a repo-relative path +// rather than a simple skill name. +func IsSkillPath(name string) bool { + name = strings.TrimSuffix(name, "/") + if name == "" { + return false + } + if strings.HasSuffix(name, "/SKILL.md") { + return true + } + if strings.HasPrefix(name, "skills/") || strings.HasPrefix(name, "plugins/") { + return true + } + if strings.Contains(name, "/skills/") || strings.Contains(name, "/plugins/") { + return true + } + if strings.Count(name, "/") >= 2 { + return true + } + return false +} + // matchSkillConventions checks if a blob path matches any known skill convention. func matchSkillConventions(entry treeEntry) *skillMatch { if path.Base(entry.Path) != "SKILL.md" { @@ -451,18 +473,22 @@ func matchSkillConventions(entry treeEntry) *skillMatch { } // matchHiddenDirConventions checks if a blob path matches a skill convention -// under a hidden (dot-prefixed) root directory. These patterns mirror the -// standard skills/ conventions but rooted under .{host}/skills/: +// under a path that contains a hidden (dot-prefixed) directory. These patterns +// mirror the standard skills/ conventions, but only when a hidden segment +// appears anywhere in the ancestor path: // -// - .{host}/skills/*/SKILL.md -> "hidden-dir" -// - .{host}/skills/{scope}/*/SKILL.md -> "hidden-dir-namespaced" +// - {prefix}/.{host}/{suffix}/skills/*/SKILL.md -> "hidden-dir" +// - {prefix}/.{host}/{suffix}/skills/{scope}/*/SKILL.md -> "hidden-dir-namespaced" func matchHiddenDirConventions(entry treeEntry) *skillMatch { if path.Base(entry.Path) != "SKILL.md" { return nil } + if !hasHiddenSegment(entry.Path) { + return nil + } - // .{host}/skills/* - // .{host}/skills/{scope}/* + // {prefix}/.{host}/{suffix}/skills/* + // {prefix}/.{host}/{suffix}/skills/{scope}/* dir := path.Dir(entry.Path) skillName := path.Base(dir) @@ -470,29 +496,23 @@ func matchHiddenDirConventions(entry treeEntry) *skillMatch { return nil } - // .{host}/skills - // .{host}/skills/{scope} + // {prefix}/.{host}/{suffix}/skills + // {prefix}/.{host}/{suffix}/skills/{scope} parentDir := path.Dir(dir) - // .{host}/skills/*/SKILL.md + // {prefix}/.{host}/{suffix}/skills/*/SKILL.md if path.Base(parentDir) == "skills" { - hiddenRoot := path.Dir(parentDir) - if path.Dir(hiddenRoot) == "." && strings.HasPrefix(hiddenRoot, ".") { - return &skillMatch{entry: entry, name: skillName, skillDir: dir, convention: "hidden-dir"} - } + return &skillMatch{entry: entry, name: skillName, skillDir: dir, convention: "hidden-dir"} } - // .{host}/skills/{scope}/*/SKILL.md + // {prefix}/.{host}/{suffix}/skills/{scope}/*/SKILL.md grandparentDir := path.Dir(parentDir) if path.Base(grandparentDir) == "skills" { - hiddenRoot := path.Dir(grandparentDir) - if path.Dir(hiddenRoot) == "." && strings.HasPrefix(hiddenRoot, ".") { - namespace := path.Base(parentDir) - if !validateName(namespace) { - return nil - } - return &skillMatch{entry: entry, name: skillName, namespace: namespace, skillDir: dir, convention: "hidden-dir-namespaced"} + namespace := path.Base(parentDir) + if !validateName(namespace) { + return nil } + return &skillMatch{entry: entry, name: skillName, namespace: namespace, skillDir: dir, convention: "hidden-dir-namespaced"} } return nil @@ -655,8 +675,19 @@ func FetchDescriptionsConcurrent(client *api.Client, host, owner, repo string, s wg.Wait() } +// DiscoverSkillByPathOptions controls optional behavior for DiscoverSkillByPathWithOptions. +type DiscoverSkillByPathOptions struct { + SkipDescription bool +} + // DiscoverSkillByPath looks up a single skill by its exact path in the repository. func DiscoverSkillByPath(client *api.Client, host, owner, repo, commitSHA, skillPath string) (*Skill, error) { + return DiscoverSkillByPathWithOptions(client, host, owner, repo, commitSHA, skillPath, DiscoverSkillByPathOptions{}) +} + +// DiscoverSkillByPathWithOptions looks up a single skill by its exact path in +// the repository, applying the given options. +func DiscoverSkillByPathWithOptions(client *api.Client, host, owner, repo, commitSHA, skillPath string, opts DiscoverSkillByPathOptions) (*Skill, error) { skillPath = strings.TrimSuffix(skillPath, "/SKILL.md") skillPath = strings.TrimSuffix(skillPath, "/") @@ -737,7 +768,9 @@ func DiscoverSkillByPath(client *api.Client, host, owner, repo, commitSHA, skill TreeSHA: treeSHA, } - skill.Description = fetchDescription(client, host, owner, repo, skill) + if !opts.SkipDescription { + skill.Description = fetchDescription(client, host, owner, repo, skill) + } return skill, nil } diff --git a/internal/skills/discovery/discovery_test.go b/internal/skills/discovery/discovery_test.go index bc3ce979b54..8d1cff8c93e 100644 --- a/internal/skills/discovery/discovery_test.go +++ b/internal/skills/discovery/discovery_test.go @@ -202,6 +202,19 @@ func TestMatchHiddenDirConventions(t *testing.T) { wantNamespace: "monalisa", wantConvention: "hidden-dir-namespaced", }, + { + name: "nested hidden dir skills directory", + path: "foo/bar/.claude/skills/code-review/SKILL.md", + wantName: "code-review", + wantConvention: "hidden-dir", + }, + { + name: "nested hidden dir namespaced skill", + path: "foo/bar/.claude/skills/monalisa/code-review/SKILL.md", + wantName: "code-review", + wantNamespace: "monalisa", + wantConvention: "hidden-dir-namespaced", + }, { name: "not a SKILL.md file", path: ".claude/skills/code-review/README.md", @@ -228,9 +241,17 @@ func TestMatchHiddenDirConventions(t *testing.T) { wantNil: true, }, { - name: "too deeply nested hidden dir", - path: ".claude/nested/skills/code-review/SKILL.md", - wantNil: true, + name: "hidden dir with nested skills directory", + path: ".claude/nested/skills/code-review/SKILL.md", + wantName: "code-review", + wantConvention: "hidden-dir", + }, + { + name: "hidden dir with nested namespaced skills directory", + path: ".claude/nested/skills/monalisa/code-review/SKILL.md", + wantName: "code-review", + wantNamespace: "monalisa", + wantConvention: "hidden-dir-namespaced", }, { name: "invalid skill name", @@ -992,6 +1013,16 @@ func TestDiscoverSkillsWithOptions(t *testing.T) { }, } + nestedHiddenTree := map[string]interface{}{ + "sha": "abc123", "truncated": false, + "tree": []map[string]interface{}{ + {"path": "foo/bar/.claude/skills/hidden-skill", "type": "tree", "sha": "tree-sha-1"}, + {"path": "foo/bar/.claude/skills/hidden-skill/SKILL.md", "type": "blob", "sha": "blob-1"}, + {"path": "foo/bar/.claude/nested/skills/deep-hidden-skill", "type": "tree", "sha": "tree-sha-2"}, + {"path": "foo/bar/.claude/nested/skills/deep-hidden-skill/SKILL.md", "type": "blob", "sha": "blob-2"}, + }, + } + emptyTree := map[string]interface{}{ "sha": "abc123", "truncated": false, "tree": []map[string]interface{}{ @@ -1015,6 +1046,11 @@ func TestDiscoverSkillsWithOptions(t *testing.T) { tree: mixedTree, wantSkills: []string{"hidden-skill", "standard-skill"}, }, + { + name: "nested hidden-dir tree returns hidden skill", + tree: nestedHiddenTree, + wantSkills: []string{"deep-hidden-skill", "hidden-skill"}, + }, { name: "no skills at all", tree: emptyTree, @@ -1298,6 +1334,32 @@ func TestDiscoverSkillByPath(t *testing.T) { } } +func TestDiscoverSkillByPathWithOptionsSkipsDescription(t *testing.T) { + reg := &httpmock.Registry{} + defer reg.Verify(t) + + reg.Register( + httpmock.REST("GET", "repos/monalisa/octocat-skills/contents/skills"), + httpmock.JSONResponse([]map[string]interface{}{ + {"name": "code-review", "path": "skills/code-review", "sha": "tree-sha", "type": "dir"}, + })) + reg.Register( + httpmock.REST("GET", "repos/monalisa/octocat-skills/git/trees/tree-sha"), + httpmock.JSONResponse(map[string]interface{}{ + "sha": "tree-sha", "truncated": false, + "tree": []map[string]interface{}{ + {"path": "SKILL.md", "type": "blob", "sha": "blob-sha"}, + }, + })) + + client := api.NewClientFromHTTP(&http.Client{Transport: reg}) + skill, err := DiscoverSkillByPathWithOptions(client, "github.com", "monalisa", "octocat-skills", "abc123", "skills/code-review", DiscoverSkillByPathOptions{SkipDescription: true}) + + require.NoError(t, err) + assert.Equal(t, "code-review", skill.Name) + assert.Empty(t, skill.Description) +} + func TestDiscoverLocalSkills(t *testing.T) { tests := []struct { name string @@ -1415,6 +1477,20 @@ func TestDiscoverLocalSkillsWithOptions(t *testing.T) { }, wantSkills: []string{"standard", "hidden"}, }, + { + name: "nested hidden dir returns skill", + setup: func(t *testing.T, dir string) { + t.Helper() + skillDir := filepath.Join(dir, "foo", "bar", ".claude", "skills", "hidden") + require.NoError(t, os.MkdirAll(skillDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte("# hidden"), 0o644)) + + deepDir := filepath.Join(dir, "foo", "bar", ".claude", "nested", "skills", "deep-hidden") + require.NoError(t, os.MkdirAll(deepDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(deepDir, "SKILL.md"), []byte("# deep-hidden"), 0o644)) + }, + wantSkills: []string{"deep-hidden", "hidden"}, + }, { name: "no skills at all", setup: func(t *testing.T, _ string) { t.Helper() }, @@ -1489,6 +1565,34 @@ func TestMatchSkillPath(t *testing.T) { } } +func TestIsSkillPath(t *testing.T) { + tests := []struct { + name string + path string + want bool + }{ + {name: "empty string", path: "", want: false}, + {name: "plain skill name", path: "git-commit", want: false}, + {name: "bare SKILL.md", path: "SKILL.md", want: false}, + {name: "SKILL.md suffix", path: "skills/code-review/SKILL.md", want: true}, + {name: "starts with skills/", path: "skills/code-review", want: true}, + {name: "starts with plugins/", path: "plugins/hubot/skills/pr-summary", want: true}, + {name: "nested skills/ path", path: "terraform/code-generation/skills/terraform-style-guide", want: true}, + {name: "deeply nested skills/ path", path: "a/b/c/skills/my-skill", want: true}, + {name: "nested plugins/ path", path: "vendor/plugins/hubot/skills/pr-summary", want: true}, + {name: "arbitrary nested skill path", path: "packages/agent-skills/netsuite-ai-connector-instructions", want: true}, + {name: "arbitrary nested skill path with trailing slash", path: "skills-catalog/matlab-core/matlab-debugging/", want: true}, + {name: "name containing skills substring", path: "myskills", want: false}, + {name: "namespaced skill name", path: "monalisa/code-review", want: false}, + {name: "namespaced path", path: "skills/monalisa/issue-triage", want: true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, IsSkillPath(tt.path)) + }) + } +} + func TestDiscoverSkillFiles(t *testing.T) { tests := []struct { name string diff --git a/pkg/cmd/skills/install/install.go b/pkg/cmd/skills/install/install.go index a2153069fc7..b944c4c55b7 100644 --- a/pkg/cmd/skills/install/install.go +++ b/pkg/cmd/skills/install/install.go @@ -116,8 +116,10 @@ func NewCmdInstall(f *cmdutil.Factory, telemetry ghtelemetry.CommandRecorder, ru 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). + or an exact path within the repository (%[1]sskills/author/skill%[1]s, + %[1]spackages/agent-skills/code-review%[1]s, or any %[1]s.../SKILL.md%[1]s path). + Namespaced names with one slash are matched by name. Use a %[1]sSKILL.md%[1]s + suffix to force a one-directory path outside the standard conventions. Performance tip: when installing from a large repository with many skills, providing an exact path instead of a skill name avoids a @@ -159,6 +161,9 @@ func NewCmdInstall(f *cmdutil.Factory, telemetry ghtelemetry.CommandRecorder, ru # Install from a large namespaced repo by path (efficient, skips full discovery) $ gh skill install github/awesome-copilot skills/monalisa/code-review + # Install from a non-standard nested path (efficient, skips full discovery) + $ gh skill install monalisa/skills-repo packages/agent-skills/code-review + # Install from a local directory $ gh skill install ./my-skills-repo --from-local @@ -285,7 +290,7 @@ func installRun(opts *InstallOptions) error { var selectedSkills []discovery.Skill - if isSkillPath(opts.SkillName) { + if discovery.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() @@ -552,24 +557,6 @@ func runLocalInstall(opts *InstallOptions) error { 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 - } - if strings.Contains(name, "/skills/") || strings.Contains(name, "/plugins/") { - return true - } - return false -} - func resolveRepoArg(skillSource string, canPrompt bool, p prompter.Prompter) (ghrepo.Interface, string, error) { if skillSource == "" { if !canPrompt { diff --git a/pkg/cmd/skills/install/install_test.go b/pkg/cmd/skills/install/install_test.go index e9cb2d7dc58..64cded9bf67 100644 --- a/pkg/cmd/skills/install/install_test.go +++ b/pkg/cmd/skills/install/install_test.go @@ -817,6 +817,34 @@ func TestInstallRun(t *testing.T) { }, wantStdout: "Installed terraform-style-guide", }, + { + name: "remote install by arbitrary nested skill path skips full discovery", + isTTY: true, + stubs: func(reg *httpmock.Registry) { + stubResolveVersion(reg, "monalisa", "skills-repo", "v1.0.0", "abc123") + stubSkillByPath(reg, "monalisa", "skills-repo", "abc123", + "packages/agent-skills/code-review", "code-review", "treeSHA") + // DiscoverSkillByPath: tree + blob (for fetchDescription) + stubInstallFiles(reg, "monalisa", "skills-repo", "treeSHA", "blobSHA", gitCommitContent) + // installer.Install: tree + blob (again, for writing files) + stubInstallFiles(reg, "monalisa", "skills-repo", "treeSHA", "blobSHA", gitCommitContent) + }, + opts: func(ios *iostreams.IOStreams, reg *httpmock.Registry) *InstallOptions { + t.Helper() + return &InstallOptions{ + IO: ios, + HttpClient: func() (*http.Client, error) { return &http.Client{Transport: reg}, nil }, + GitClient: &git.Client{RepoDir: t.TempDir()}, + SkillSource: "monalisa/skills-repo", + SkillName: "packages/agent-skills/code-review", + Agent: "github-copilot", + Scope: "project", + ScopeChanged: true, + Dir: t.TempDir(), + } + }, + wantStdout: "Installed code-review", + }, { name: "remote install with URL repo argument", isTTY: true, @@ -2161,31 +2189,6 @@ func TestRunLocalInstall(t *testing.T) { } } -func Test_isSkillPath(t *testing.T) { - tests := []struct { - name string - path string - want bool - }{ - {name: "empty string", path: "", want: false}, - {name: "plain skill name", path: "git-commit", want: false}, - {name: "SKILL.md at root", path: "SKILL.md", want: true}, - {name: "SKILL.md suffix", path: "skills/code-review/SKILL.md", want: true}, - {name: "starts with skills/", path: "skills/code-review", want: true}, - {name: "starts with plugins/", path: "plugins/hubot/skills/pr-summary", want: true}, - {name: "nested skills/ path", path: "terraform/code-generation/skills/terraform-style-guide", want: true}, - {name: "deeply nested skills/ path", path: "a/b/c/skills/my-skill", want: true}, - {name: "nested plugins/ path", path: "vendor/plugins/hubot/skills/pr-summary", want: true}, - {name: "name containing skills substring", path: "myskills", want: false}, - {name: "namespaced path", path: "skills/monalisa/issue-triage", want: true}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - assert.Equal(t, tt.want, isSkillPath(tt.path)) - }) - } -} - func Test_printReviewHint(t *testing.T) { tests := []struct { name string diff --git a/pkg/cmd/skills/preview/preview.go b/pkg/cmd/skills/preview/preview.go index e6c202b0992..06d50154c91 100644 --- a/pkg/cmd/skills/preview/preview.go +++ b/pkg/cmd/skills/preview/preview.go @@ -69,6 +69,12 @@ func NewCmdPreview(f *cmdutil.Factory, telemetry ghtelemetry.CommandRecorder, ru When run with only a repository argument, lists available skills and prompts for selection. + 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, + %[1]spackages/agent-skills/code-review%[1]s, or any %[1]s.../SKILL.md%[1]s path). + Namespaced names with one slash are matched by name. Use a %[1]sSKILL.md%[1]s + suffix to force a one-directory path outside the standard conventions. + To preview a specific version of the skill, append %[1]s@VERSION%[1]s to the skill name. The version is resolved as a git tag, branch, or commit SHA. `, "`"), @@ -82,6 +88,9 @@ func NewCmdPreview(f *cmdutil.Factory, telemetry ghtelemetry.CommandRecorder, ru # Preview a skill at a specific commit SHA $ gh skill preview github/awesome-copilot documentation-writer@abc123def456 + # Preview from a non-standard nested path (efficient, skips full discovery) + $ gh skill preview monalisa/skills-repo packages/agent-skills/code-review + # Browse and preview interactively $ gh skill preview github/awesome-copilot `), @@ -153,25 +162,36 @@ func previewRun(opts *PreviewOptions) error { return fmt.Errorf("could not resolve version: %w", err) } - opts.IO.StartProgressIndicatorWithLabel("Discovering skills") - allSkills, err := discovery.DiscoverSkillsWithOptions(apiClient, hostname, owner, repoName, resolved.SHA, discovery.DiscoverOptions{}) - opts.IO.StopProgressIndicator() - if err != nil { - return err - } + var skill discovery.Skill + if discovery.IsSkillPath(opts.SkillName) { + opts.IO.StartProgressIndicatorWithLabel("Looking up skill") + found, err := discovery.DiscoverSkillByPathWithOptions(apiClient, hostname, owner, repoName, resolved.SHA, opts.SkillName, discovery.DiscoverSkillByPathOptions{SkipDescription: true}) + opts.IO.StopProgressIndicator() + if err != nil { + return err + } + skill = *found + } else { + opts.IO.StartProgressIndicatorWithLabel("Discovering skills") + allSkills, err := discovery.DiscoverSkillsWithOptions(apiClient, hostname, owner, repoName, resolved.SHA, discovery.DiscoverOptions{}) + opts.IO.StopProgressIndicator() + if err != nil { + return err + } - skills, err := filterHiddenDirSkills(opts, allSkills) - if err != nil { - return err - } + skills, err := filterHiddenDirSkills(opts, allSkills) + if err != nil { + return err + } - sort.Slice(skills, func(i, j int) bool { - return skills[i].DisplayName() < skills[j].DisplayName() - }) + sort.Slice(skills, func(i, j int) bool { + return skills[i].DisplayName() < skills[j].DisplayName() + }) - skill, err := selectSkill(opts, skills) - if err != nil { - return err + skill, err = selectSkill(opts, skills) + if err != nil { + return err + } } opts.IO.StartProgressIndicatorWithLabel("Fetching skill content") diff --git a/pkg/cmd/skills/preview/preview_test.go b/pkg/cmd/skills/preview/preview_test.go index 3bc98fece56..04fae62587e 100644 --- a/pkg/cmd/skills/preview/preview_test.go +++ b/pkg/cmd/skills/preview/preview_test.go @@ -261,6 +261,43 @@ func TestPreviewRun(t *testing.T) { }, wantStdout: "My Skill", }, + { + name: "preview by arbitrary nested skill path skips full discovery", + tty: true, + opts: &PreviewOptions{ + repo: ghrepo.New("owner", "repo"), + SkillName: "packages/agent-skills/code-review", + }, + httpStubs: func(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/contents/packages%2Fagent-skills"), + httpmock.StringResponse(`[ + {"name": "code-review", "path": "packages/agent-skills/code-review", "sha": "treeSHA4", "type": "dir"} + ]`), + ) + reg.Register( + httpmock.REST("GET", "repos/owner/repo/git/trees/treeSHA4"), + httpmock.StringResponse(`{ + "tree": [ + {"path": "SKILL.md", "type": "blob", "sha": "blob999", "size": 50} + ] + }`), + ) + reg.Register( + httpmock.REST("GET", "repos/owner/repo/git/blobs/blob999"), + httpmock.StringResponse(`{"sha": "blob999", "content": "`+encodedContent+`", "encoding": "base64"}`), + ) + }, + wantStdout: "My Skill", + }, { name: "skill not found", tty: true,