refactor: decouple hidden-dir filtering from discovery layer
Move --allow-hidden-dirs filtering logic from the discovery package to the install command, addressing review feedback. Discovery functions now always return all skills (including hidden-dir), and callers decide how to handle them. Changes: - DiscoverSkillsWithOptions/DiscoverLocalSkillsWithOptions always return hidden-dir skills; callers filter using IsHiddenDirConvention() - DiscoverSkills/DiscoverLocalSkills (convenience wrappers) auto-filter hidden-dir skills for backward compatibility with preview/update/publish - Remove --allow-hidden-dirs reference from discovery error messages - Add filterHiddenDirSkills in install.go with caller-side flag logic - Inline warning using heredoc.Docf, remove printHiddenDirWarning - Add inline comments in matchHiddenDirConventions (babakks nitpicks) - Add non-hidden-namespaced dir and no-skills-at-all test cases - Add --allow-hidden-dirs tests in TestNewCmdInstall, TestInstallRun, and TestRunLocalInstall Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Sam Morrow
parent
082f15a8fd
commit
eaa018545a
4 changed files with 659 additions and 14 deletions
|
|
@ -66,6 +66,8 @@ func (s Skill) DisplayName() string {
|
|||
return "[plugins] " + name
|
||||
case "root":
|
||||
return "[root] " + name
|
||||
case "hidden-dir", "hidden-dir-namespaced":
|
||||
return "[hidden-dir] " + name
|
||||
default:
|
||||
return name
|
||||
}
|
||||
|
|
@ -82,6 +84,23 @@ func (s Skill) InstallName() string {
|
|||
return s.Name
|
||||
}
|
||||
|
||||
// IsHiddenDirConvention returns true if the skill was discovered in a hidden
|
||||
// (dot-prefixed) directory such as .claude/skills/ or .agents/skills/.
|
||||
func (s Skill) IsHiddenDirConvention() bool {
|
||||
return s.Convention == "hidden-dir" || s.Convention == "hidden-dir-namespaced"
|
||||
}
|
||||
|
||||
// HasHiddenDirSkills returns true if any of the given skills were discovered
|
||||
// in hidden directories.
|
||||
func HasHiddenDirSkills(skills []Skill) bool {
|
||||
for _, s := range skills {
|
||||
if s.IsHiddenDirConvention() {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// ResolvedRef contains the resolved git reference and its SHA.
|
||||
type ResolvedRef struct {
|
||||
Ref string // fully qualified ref (refs/heads/*, refs/tags/*) or commit SHA
|
||||
|
|
@ -393,8 +412,87 @@ func matchSkillConventions(entry treeEntry) *skillMatch {
|
|||
return nil
|
||||
}
|
||||
|
||||
// DiscoverSkills finds all skills in a repository at the given commit SHA.
|
||||
// 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/:
|
||||
//
|
||||
// - .{host}/skills/*/SKILL.md -> "hidden-dir"
|
||||
// - .{host}/skills/{scope}/*/SKILL.md -> "hidden-dir-namespaced"
|
||||
func matchHiddenDirConventions(entry treeEntry) *skillMatch {
|
||||
if path.Base(entry.Path) != "SKILL.md" {
|
||||
return nil
|
||||
}
|
||||
|
||||
// .{host}/skills/*
|
||||
// .{host}/skills/{scope}/*
|
||||
dir := path.Dir(entry.Path)
|
||||
skillName := path.Base(dir)
|
||||
|
||||
if !validateName(skillName) {
|
||||
return nil
|
||||
}
|
||||
|
||||
// .{host}/skills
|
||||
// .{host}/skills/{scope}
|
||||
parentDir := path.Dir(dir)
|
||||
|
||||
// .{host}/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"}
|
||||
}
|
||||
}
|
||||
|
||||
// .{host}/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"}
|
||||
}
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// DiscoverOptions controls optional discovery behaviors.
|
||||
type DiscoverOptions struct {
|
||||
}
|
||||
|
||||
// DiscoverSkills finds all non-hidden-dir skills in a repository at the given
|
||||
// commit SHA. Hidden-dir skills are excluded; use DiscoverSkillsWithOptions to
|
||||
// retrieve all skills including those in hidden directories.
|
||||
func DiscoverSkills(client *api.Client, host, owner, repo, commitSHA string) ([]Skill, error) {
|
||||
all, err := DiscoverSkillsWithOptions(client, host, owner, repo, commitSHA, DiscoverOptions{})
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
var skills []Skill
|
||||
for _, s := range all {
|
||||
if !s.IsHiddenDirConvention() {
|
||||
skills = append(skills, s)
|
||||
}
|
||||
}
|
||||
if len(skills) == 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,
|
||||
)
|
||||
}
|
||||
return skills, nil
|
||||
}
|
||||
|
||||
// DiscoverSkillsWithOptions finds all skills in a repository at the given
|
||||
// commit SHA, with configurable discovery behavior.
|
||||
func DiscoverSkillsWithOptions(client *api.Client, host, owner, repo, commitSHA string, opts DiscoverOptions) ([]Skill, error) {
|
||||
apiPath := fmt.Sprintf("repos/%s/%s/git/trees/%s?recursive=true", url.PathEscape(owner), url.PathEscape(repo), url.PathEscape(commitSHA))
|
||||
var tree treeResponse
|
||||
if err := client.REST(host, "GET", apiPath, nil, &tree); err != nil {
|
||||
|
|
@ -419,6 +517,9 @@ func DiscoverSkills(client *api.Client, host, owner, repo, commitSHA string) ([]
|
|||
continue
|
||||
}
|
||||
m := matchSkillConventions(entry)
|
||||
if m == nil {
|
||||
m = matchHiddenDirConventions(entry)
|
||||
}
|
||||
if m == nil {
|
||||
continue
|
||||
}
|
||||
|
|
@ -703,9 +804,35 @@ func FetchBlob(client *api.Client, host, owner, repo, sha string) (string, error
|
|||
return string(decoded), nil
|
||||
}
|
||||
|
||||
// DiscoverLocalSkills finds skills in a local directory using the same
|
||||
// conventions as remote discovery.
|
||||
// DiscoverLocalSkills finds non-hidden-dir skills in a local directory using
|
||||
// the same conventions as remote discovery. Hidden-dir skills are excluded; use
|
||||
// DiscoverLocalSkillsWithOptions to retrieve all skills including those in
|
||||
// hidden directories.
|
||||
func DiscoverLocalSkills(dir string) ([]Skill, error) {
|
||||
all, err := DiscoverLocalSkillsWithOptions(dir, DiscoverOptions{})
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
var skills []Skill
|
||||
for _, s := range all {
|
||||
if !s.IsHiddenDirConvention() {
|
||||
skills = append(skills, s)
|
||||
}
|
||||
}
|
||||
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
|
||||
}
|
||||
|
||||
// DiscoverLocalSkillsWithOptions finds skills in a local directory using the
|
||||
// same conventions as remote discovery, with configurable discovery behavior.
|
||||
func DiscoverLocalSkillsWithOptions(dir string, opts DiscoverOptions) ([]Skill, error) {
|
||||
absDir, err := filepath.Abs(dir)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("could not resolve path: %w", err)
|
||||
|
|
@ -751,6 +878,9 @@ func DiscoverLocalSkills(dir string) ([]Skill, error) {
|
|||
|
||||
entry := treeEntry{Path: relPath, Type: "blob"}
|
||||
m := matchSkillConventions(entry)
|
||||
if m == nil {
|
||||
m = matchHiddenDirConventions(entry)
|
||||
}
|
||||
if m == nil {
|
||||
return nil
|
||||
}
|
||||
|
|
|
|||
|
|
@ -116,6 +116,155 @@ func TestMatchSkillConventions(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func TestMatchHiddenDirConventions(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
path string
|
||||
wantNil bool
|
||||
wantName string
|
||||
wantNamespace string
|
||||
wantConvention string
|
||||
}{
|
||||
{
|
||||
name: "claude skills directory",
|
||||
path: ".claude/skills/code-review/SKILL.md",
|
||||
wantName: "code-review",
|
||||
wantConvention: "hidden-dir",
|
||||
},
|
||||
{
|
||||
name: "agents skills directory",
|
||||
path: ".agents/skills/git-commit/SKILL.md",
|
||||
wantName: "git-commit",
|
||||
wantConvention: "hidden-dir",
|
||||
},
|
||||
{
|
||||
name: "github skills directory",
|
||||
path: ".github/skills/issue-triage/SKILL.md",
|
||||
wantName: "issue-triage",
|
||||
wantConvention: "hidden-dir",
|
||||
},
|
||||
{
|
||||
name: "copilot skills directory",
|
||||
path: ".copilot/skills/pr-summary/SKILL.md",
|
||||
wantName: "pr-summary",
|
||||
wantConvention: "hidden-dir",
|
||||
},
|
||||
{
|
||||
name: "namespaced hidden dir skill",
|
||||
path: ".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",
|
||||
wantNil: true,
|
||||
},
|
||||
{
|
||||
name: "too shallow - just hidden dir and SKILL.md",
|
||||
path: ".claude/SKILL.md",
|
||||
wantNil: true,
|
||||
},
|
||||
{
|
||||
name: "no skills subdirectory",
|
||||
path: ".claude/code-review/SKILL.md",
|
||||
wantNil: true,
|
||||
},
|
||||
{
|
||||
name: "non-hidden dir does not match",
|
||||
path: "visible/skills/code-review/SKILL.md",
|
||||
wantNil: true,
|
||||
},
|
||||
{
|
||||
name: "non-hidden-namespaced dir does not match",
|
||||
path: "visible/skills/monalisa/code-review/SKILL.md",
|
||||
wantNil: true,
|
||||
},
|
||||
{
|
||||
name: "too deeply nested hidden dir",
|
||||
path: ".claude/nested/skills/code-review/SKILL.md",
|
||||
wantNil: true,
|
||||
},
|
||||
{
|
||||
name: "invalid skill name",
|
||||
path: ".claude/skills/../SKILL.md",
|
||||
wantNil: true,
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
m := matchHiddenDirConventions(treeEntry{Path: tt.path, Type: "blob"})
|
||||
if tt.wantNil {
|
||||
assert.Nil(t, m)
|
||||
return
|
||||
}
|
||||
require.NotNil(t, m)
|
||||
assert.Equal(t, tt.wantName, m.name)
|
||||
assert.Equal(t, tt.wantNamespace, m.namespace)
|
||||
assert.Equal(t, tt.wantConvention, m.convention)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestHasHiddenDirSkills(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
skills []Skill
|
||||
want bool
|
||||
}{
|
||||
{
|
||||
name: "empty list",
|
||||
skills: nil,
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "only standard skills",
|
||||
skills: []Skill{{Convention: "skills"}, {Convention: "root"}},
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "has hidden-dir skill",
|
||||
skills: []Skill{{Convention: "skills"}, {Convention: "hidden-dir"}},
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "has hidden-dir-namespaced skill",
|
||||
skills: []Skill{{Convention: "hidden-dir-namespaced"}},
|
||||
want: true,
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
assert.Equal(t, tt.want, HasHiddenDirSkills(tt.skills))
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestDisplayNameHiddenDir(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
skill Skill
|
||||
wantName string
|
||||
}{
|
||||
{
|
||||
name: "hidden-dir skill",
|
||||
skill: Skill{Name: "code-review", Convention: "hidden-dir"},
|
||||
wantName: "[hidden-dir] code-review",
|
||||
},
|
||||
{
|
||||
name: "hidden-dir-namespaced skill",
|
||||
skill: Skill{Name: "code-review", Namespace: "monalisa", Convention: "hidden-dir-namespaced"},
|
||||
wantName: "[hidden-dir] monalisa/code-review",
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
assert.Equal(t, tt.wantName, tt.skill.DisplayName())
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestValidateName(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
|
|
@ -740,6 +889,82 @@ func TestDiscoverSkills(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func TestDiscoverSkillsWithOptions(t *testing.T) {
|
||||
hiddenDirTree := map[string]interface{}{
|
||||
"sha": "abc123", "truncated": false,
|
||||
"tree": []map[string]interface{}{
|
||||
{"path": ".claude/skills/code-review", "type": "tree", "sha": "tree-sha-1"},
|
||||
{"path": ".claude/skills/code-review/SKILL.md", "type": "blob", "sha": "blob-1"},
|
||||
{"path": ".agents/skills/git-commit", "type": "tree", "sha": "tree-sha-2"},
|
||||
{"path": ".agents/skills/git-commit/SKILL.md", "type": "blob", "sha": "blob-2"},
|
||||
{"path": "README.md", "type": "blob", "sha": "readme"},
|
||||
},
|
||||
}
|
||||
|
||||
mixedTree := map[string]interface{}{
|
||||
"sha": "abc123", "truncated": false,
|
||||
"tree": []map[string]interface{}{
|
||||
{"path": "skills/standard-skill", "type": "tree", "sha": "tree-sha-1"},
|
||||
{"path": "skills/standard-skill/SKILL.md", "type": "blob", "sha": "blob-1"},
|
||||
{"path": ".claude/skills/hidden-skill", "type": "tree", "sha": "tree-sha-2"},
|
||||
{"path": ".claude/skills/hidden-skill/SKILL.md", "type": "blob", "sha": "blob-2"},
|
||||
},
|
||||
}
|
||||
|
||||
emptyTree := map[string]interface{}{
|
||||
"sha": "abc123", "truncated": false,
|
||||
"tree": []map[string]interface{}{
|
||||
{"path": "README.md", "type": "blob", "sha": "readme"},
|
||||
},
|
||||
}
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
tree map[string]interface{}
|
||||
wantSkills []string
|
||||
wantErr string
|
||||
}{
|
||||
{
|
||||
name: "returns hidden-dir skills",
|
||||
tree: hiddenDirTree,
|
||||
wantSkills: []string{"code-review", "git-commit"},
|
||||
},
|
||||
{
|
||||
name: "mixed tree returns all skills",
|
||||
tree: mixedTree,
|
||||
wantSkills: []string{"hidden-skill", "standard-skill"},
|
||||
},
|
||||
{
|
||||
name: "no skills at all",
|
||||
tree: emptyTree,
|
||||
wantErr: "no skills found",
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
reg := &httpmock.Registry{}
|
||||
defer reg.Verify(t)
|
||||
reg.Register(
|
||||
httpmock.REST("GET", "repos/monalisa/octocat-skills/git/trees/abc123"),
|
||||
httpmock.JSONResponse(tt.tree))
|
||||
client := api.NewClientFromHTTP(&http.Client{Transport: reg})
|
||||
|
||||
skills, err := DiscoverSkillsWithOptions(client, "github.com", "monalisa", "octocat-skills", "abc123", DiscoverOptions{})
|
||||
if tt.wantErr != "" {
|
||||
require.Error(t, err)
|
||||
assert.Contains(t, err.Error(), tt.wantErr)
|
||||
return
|
||||
}
|
||||
require.NoError(t, err)
|
||||
var names []string
|
||||
for _, s := range skills {
|
||||
names = append(names, s.Name)
|
||||
}
|
||||
assert.Equal(t, tt.wantSkills, names)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestDiscoverSkillByPath(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
|
|
@ -984,6 +1209,64 @@ func TestDiscoverLocalSkills(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func TestDiscoverLocalSkillsWithOptions(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
setup func(t *testing.T, dir string)
|
||||
wantSkills []string
|
||||
wantErr string
|
||||
}{
|
||||
{
|
||||
name: "returns hidden dir skills",
|
||||
setup: func(t *testing.T, dir string) {
|
||||
t.Helper()
|
||||
skillDir := filepath.Join(dir, ".claude", "skills", "code-review")
|
||||
require.NoError(t, os.MkdirAll(skillDir, 0o755))
|
||||
require.NoError(t, os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte("# code-review"), 0o644))
|
||||
},
|
||||
wantSkills: []string{"code-review"},
|
||||
},
|
||||
{
|
||||
name: "mixed standard and hidden returns all",
|
||||
setup: func(t *testing.T, dir string) {
|
||||
t.Helper()
|
||||
for _, p := range []string{"skills/standard", ".agents/skills/hidden"} {
|
||||
skillDir := filepath.Join(dir, filepath.FromSlash(p))
|
||||
require.NoError(t, os.MkdirAll(skillDir, 0o755))
|
||||
name := filepath.Base(p)
|
||||
require.NoError(t, os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte("# "+name), 0o644))
|
||||
}
|
||||
},
|
||||
wantSkills: []string{"standard", "hidden"},
|
||||
},
|
||||
{
|
||||
name: "no skills at all",
|
||||
setup: func(t *testing.T, _ string) { t.Helper() },
|
||||
wantErr: "no skills found",
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
dir := filepath.Join(t.TempDir(), "repo")
|
||||
require.NoError(t, os.MkdirAll(dir, 0o755))
|
||||
tt.setup(t, dir)
|
||||
|
||||
skills, err := DiscoverLocalSkillsWithOptions(dir, DiscoverOptions{})
|
||||
if tt.wantErr != "" {
|
||||
require.Error(t, err)
|
||||
assert.Contains(t, err.Error(), tt.wantErr)
|
||||
return
|
||||
}
|
||||
require.NoError(t, err)
|
||||
var names []string
|
||||
for _, s := range skills {
|
||||
names = append(names, s.Name)
|
||||
}
|
||||
assert.ElementsMatch(t, tt.wantSkills, names)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestMatchesSkillPath(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
|
|
|
|||
|
|
@ -47,15 +47,16 @@ type InstallOptions struct {
|
|||
GitClient *git.Client
|
||||
Remotes func() (ghContext.Remotes, error)
|
||||
|
||||
SkillSource string // owner/repo or local path (when --from-local is set)
|
||||
SkillName string // possibly with @version suffix
|
||||
Agent string
|
||||
Scope string
|
||||
ScopeChanged bool // true when --scope was explicitly set
|
||||
Pin string
|
||||
Dir string // overrides --agent and --scope
|
||||
Force bool
|
||||
FromLocal bool // treat SkillSource as a local directory path
|
||||
SkillSource string // owner/repo or local path (when --from-local is set)
|
||||
SkillName string // possibly with @version suffix
|
||||
Agent string
|
||||
Scope string
|
||||
ScopeChanged bool // true when --scope was explicitly set
|
||||
Pin string
|
||||
Dir string // overrides --agent and --scope
|
||||
Force bool
|
||||
FromLocal bool // treat SkillSource as a local directory path
|
||||
AllowHiddenDirs bool // include skills in dot-prefixed directories
|
||||
|
||||
repo ghrepo.Interface // set when SkillSource is a GitHub repository
|
||||
localPath string // set when FromLocal is true
|
||||
|
|
@ -161,6 +162,9 @@ func NewCmdInstall(f *cmdutil.Factory, telemetry ghtelemetry.CommandRecorder, ru
|
|||
|
||||
# Pin to a specific git ref
|
||||
$ gh skill install github/awesome-copilot git-commit --pin v2.0.0
|
||||
|
||||
# Install skills from hidden directories (e.g. .claude/skills/)
|
||||
$ gh skill install owner/repo --allow-hidden-dirs
|
||||
`),
|
||||
Aliases: []string{"add"},
|
||||
Args: cobra.MaximumNArgs(2),
|
||||
|
|
@ -205,6 +209,7 @@ func NewCmdInstall(f *cmdutil.Factory, telemetry ghtelemetry.CommandRecorder, ru
|
|||
cmd.Flags().StringVar(&opts.Dir, "dir", "", "Install to a custom directory (overrides --agent and --scope)")
|
||||
cmd.Flags().BoolVarP(&opts.Force, "force", "f", false, "Overwrite existing skills without prompting")
|
||||
cmd.Flags().BoolVar(&opts.FromLocal, "from-local", false, "Treat the argument as a local directory path instead of a repository")
|
||||
cmd.Flags().BoolVar(&opts.AllowHiddenDirs, "allow-hidden-dirs", false, "Include skills in hidden directories (e.g. .claude/skills/, .agents/skills/)")
|
||||
cmdutil.DisableAuthCheckFlag(cmd.Flags().Lookup("from-local"))
|
||||
|
||||
return cmd
|
||||
|
|
@ -417,12 +422,17 @@ func runLocalInstall(opts *InstallOptions) error {
|
|||
}
|
||||
|
||||
opts.IO.StartProgressIndicatorWithLabel("Discovering skills")
|
||||
skills, err := discovery.DiscoverLocalSkills(absSource)
|
||||
allSkills, err := discovery.DiscoverLocalSkillsWithOptions(absSource, discovery.DiscoverOptions{})
|
||||
opts.IO.StopProgressIndicator()
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
skills, err := filterHiddenDirSkills(opts, allSkills)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
if canPrompt {
|
||||
fmt.Fprintf(opts.IO.ErrOut, "Found %d skill(s)\n", len(skills))
|
||||
}
|
||||
|
|
@ -553,7 +563,7 @@ func resolveVersion(opts *InstallOptions, client *api.Client, hostname string) (
|
|||
|
||||
func discoverSkills(opts *InstallOptions, client *api.Client, 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)
|
||||
allSkills, err := discovery.DiscoverSkillsWithOptions(client, hostname, opts.repo.RepoOwner(), opts.repo.RepoName(), resolved.SHA, discovery.DiscoverOptions{})
|
||||
opts.IO.StopProgressIndicator()
|
||||
if err != nil {
|
||||
var treeTooLarge *discovery.TreeTooLargeError
|
||||
|
|
@ -564,6 +574,10 @@ func discoverSkills(opts *InstallOptions, client *api.Client, hostname string, r
|
|||
}
|
||||
return nil, err
|
||||
}
|
||||
skills, filterErr := filterHiddenDirSkills(opts, allSkills)
|
||||
if filterErr != nil {
|
||||
return nil, filterErr
|
||||
}
|
||||
logConventions(opts.IO, skills)
|
||||
for _, s := range skills {
|
||||
if !discovery.IsSpecCompliant(s.Name) {
|
||||
|
|
@ -1111,3 +1125,42 @@ func kiroResourcePath(installDir, gitRoot string) string {
|
|||
}
|
||||
return filepath.ToSlash(installDir)
|
||||
}
|
||||
|
||||
// filterHiddenDirSkills separates hidden-dir skills from the full list and
|
||||
// applies the --allow-hidden-dirs flag logic. When the flag is set, all skills
|
||||
// are returned and a warning is printed. When the flag is not set, hidden-dir
|
||||
// skills are excluded and an error is returned if no standard skills remain.
|
||||
func filterHiddenDirSkills(opts *InstallOptions, allSkills []discovery.Skill) ([]discovery.Skill, error) {
|
||||
cs := opts.IO.ColorScheme()
|
||||
|
||||
if opts.AllowHiddenDirs {
|
||||
if discovery.HasHiddenDirSkills(allSkills) {
|
||||
fmt.Fprint(opts.IO.ErrOut, heredoc.Docf(`
|
||||
%[1]s Skills in hidden directories (e.g. .claude/, .agents/) may be installed
|
||||
copies from another publisher. Verify the skill's origin and check for a
|
||||
canonical source.
|
||||
`, cs.WarningIcon()))
|
||||
}
|
||||
return allSkills, nil
|
||||
}
|
||||
|
||||
var standard []discovery.Skill
|
||||
var hiddenCount int
|
||||
for _, s := range allSkills {
|
||||
if s.IsHiddenDirConvention() {
|
||||
hiddenCount++
|
||||
} else {
|
||||
standard = append(standard, s)
|
||||
}
|
||||
}
|
||||
|
||||
if len(standard) == 0 && hiddenCount > 0 {
|
||||
return nil, fmt.Errorf(
|
||||
"no standard skills found, but %d skill(s) exist in hidden directories\n"+
|
||||
" Use --allow-hidden-dirs to include them",
|
||||
hiddenCount,
|
||||
)
|
||||
}
|
||||
|
||||
return standard, nil
|
||||
}
|
||||
|
|
|
|||
|
|
@ -119,6 +119,11 @@ func TestNewCmdInstall(t *testing.T) {
|
|||
cli: "--from-local ./local-dir --pin v1.0.0",
|
||||
wantErr: true,
|
||||
},
|
||||
{
|
||||
name: "allow-hidden-dirs flag",
|
||||
cli: "monalisa/skills-repo --allow-hidden-dirs",
|
||||
wantOpts: InstallOptions{SkillSource: "monalisa/skills-repo", Scope: "project", AllowHiddenDirs: true},
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
|
|
@ -157,6 +162,7 @@ func TestNewCmdInstall(t *testing.T) {
|
|||
assert.Equal(t, tt.wantOpts.Dir, gotOpts.Dir)
|
||||
assert.Equal(t, tt.wantOpts.Force, gotOpts.Force)
|
||||
assert.Equal(t, tt.wantOpts.FromLocal, gotOpts.FromLocal)
|
||||
assert.Equal(t, tt.wantOpts.AllowHiddenDirs, gotOpts.AllowHiddenDirs)
|
||||
if tt.wantLocalPath {
|
||||
assert.NotEmpty(t, gotOpts.localPath, "expected localPath to be set")
|
||||
} else {
|
||||
|
|
@ -256,6 +262,14 @@ func singleSkillTreeJSON(name, treeSHA, blobSHA string) string {
|
|||
)
|
||||
}
|
||||
|
||||
// hiddenDirSkillTreeJSON returns tree entries for a hidden-dir skill under .claude/skills/.
|
||||
func hiddenDirSkillTreeJSON(name, treeSHA, blobSHA string) string {
|
||||
return fmt.Sprintf(
|
||||
`{"path": ".claude/skills/%s", "type": "tree", "sha": %q}, {"path": ".claude/skills/%s/SKILL.md", "type": "blob", "sha": %q}`,
|
||||
name, treeSHA, name, blobSHA,
|
||||
)
|
||||
}
|
||||
|
||||
func TestInstallRun(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
|
|
@ -1327,6 +1341,110 @@ func TestInstallRun(t *testing.T) {
|
|||
wantStdout: "Installed git-commit",
|
||||
wantStderr: "Installing to",
|
||||
},
|
||||
{
|
||||
name: "hidden-dir skills excluded without --allow-hidden-dirs",
|
||||
isTTY: false,
|
||||
stubs: func(reg *httpmock.Registry) {
|
||||
stubResolveVersion(reg, "monalisa", "skills-repo", "v1.0.0", "abc123")
|
||||
stubDiscoverTree(reg, "monalisa", "skills-repo", "abc123",
|
||||
hiddenDirSkillTreeJSON("git-commit", "treeSHA", "blobSHA"))
|
||||
},
|
||||
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: "git-commit",
|
||||
Agent: "github-copilot",
|
||||
Scope: "project",
|
||||
ScopeChanged: true,
|
||||
}
|
||||
},
|
||||
wantErr: "no standard skills found, but 1 skill(s) exist in hidden directories",
|
||||
},
|
||||
{
|
||||
name: "hidden-dir skills included with --allow-hidden-dirs",
|
||||
isTTY: true,
|
||||
stubs: func(reg *httpmock.Registry) {
|
||||
stubResolveVersion(reg, "monalisa", "skills-repo", "v1.0.0", "abc123")
|
||||
stubDiscoverTree(reg, "monalisa", "skills-repo", "abc123",
|
||||
hiddenDirSkillTreeJSON("git-commit", "treeSHA", "blobSHA"))
|
||||
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: "git-commit",
|
||||
Agent: "github-copilot",
|
||||
Scope: "project",
|
||||
ScopeChanged: true,
|
||||
Dir: t.TempDir(),
|
||||
AllowHiddenDirs: true,
|
||||
}
|
||||
},
|
||||
wantStdout: "Installed git-commit",
|
||||
wantStderr: "Skills in hidden directories",
|
||||
},
|
||||
{
|
||||
name: "mixed tree without --allow-hidden-dirs returns only standard",
|
||||
isTTY: true,
|
||||
stubs: func(reg *httpmock.Registry) {
|
||||
stubResolveVersion(reg, "monalisa", "skills-repo", "v1.0.0", "abc123")
|
||||
stubDiscoverTree(reg, "monalisa", "skills-repo", "abc123",
|
||||
singleSkillTreeJSON("git-commit", "treeSHA", "blobSHA")+", "+
|
||||
hiddenDirSkillTreeJSON("hidden-skill", "treeSHA2", "blobSHA2"))
|
||||
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: "git-commit",
|
||||
Agent: "github-copilot",
|
||||
Scope: "project",
|
||||
ScopeChanged: true,
|
||||
Dir: t.TempDir(),
|
||||
}
|
||||
},
|
||||
wantStdout: "Installed git-commit",
|
||||
},
|
||||
{
|
||||
name: "mixed tree with --allow-hidden-dirs returns all",
|
||||
isTTY: false,
|
||||
stubs: func(reg *httpmock.Registry) {
|
||||
stubResolveVersion(reg, "monalisa", "skills-repo", "v1.0.0", "abc123")
|
||||
stubDiscoverTree(reg, "monalisa", "skills-repo", "abc123",
|
||||
singleSkillTreeJSON("git-commit", "treeSHA", "blobSHA")+", "+
|
||||
hiddenDirSkillTreeJSON("hidden-skill", "treeSHA2", "blobSHA2"))
|
||||
stubInstallFiles(reg, "monalisa", "skills-repo", "treeSHA2", "blobSHA2", 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: "hidden-skill",
|
||||
Agent: "github-copilot",
|
||||
Scope: "project",
|
||||
ScopeChanged: true,
|
||||
Dir: t.TempDir(),
|
||||
AllowHiddenDirs: true,
|
||||
}
|
||||
},
|
||||
wantStdout: "Installed hidden-skill",
|
||||
wantStderr: "Skills in hidden directories",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
|
|
@ -1853,6 +1971,67 @@ func TestRunLocalInstall(t *testing.T) {
|
|||
},
|
||||
wantErr: "not found in local directory",
|
||||
},
|
||||
{
|
||||
name: "local hidden-dir skills excluded without --allow-hidden-dirs",
|
||||
isTTY: false,
|
||||
setup: func(t *testing.T, sourceDir, _ string) {
|
||||
t.Helper()
|
||||
writeLocalTestSkill(t, sourceDir, filepath.Join(".claude", "skills", "code-review"), heredoc.Doc(`
|
||||
---
|
||||
name: code-review
|
||||
description: Reviews code
|
||||
---
|
||||
# Code Review
|
||||
`))
|
||||
},
|
||||
opts: func(ios *iostreams.IOStreams, sourceDir, targetDir string) *InstallOptions {
|
||||
t.Helper()
|
||||
return &InstallOptions{
|
||||
IO: ios,
|
||||
SkillSource: sourceDir,
|
||||
localPath: sourceDir,
|
||||
SkillName: "code-review",
|
||||
Agent: "github-copilot",
|
||||
Scope: "project",
|
||||
ScopeChanged: true,
|
||||
Dir: targetDir,
|
||||
GitClient: &git.Client{RepoDir: t.TempDir()},
|
||||
}
|
||||
},
|
||||
wantErr: "no standard skills found, but 1 skill(s) exist in hidden directories",
|
||||
},
|
||||
{
|
||||
name: "local hidden-dir skills included with --allow-hidden-dirs",
|
||||
isTTY: false,
|
||||
setup: func(t *testing.T, sourceDir, _ string) {
|
||||
t.Helper()
|
||||
writeLocalTestSkill(t, sourceDir, filepath.Join(".claude", "skills", "code-review"), heredoc.Doc(`
|
||||
---
|
||||
name: code-review
|
||||
description: Reviews code
|
||||
---
|
||||
# Code Review
|
||||
`))
|
||||
},
|
||||
opts: func(ios *iostreams.IOStreams, sourceDir, targetDir string) *InstallOptions {
|
||||
t.Helper()
|
||||
return &InstallOptions{
|
||||
IO: ios,
|
||||
SkillSource: sourceDir,
|
||||
localPath: sourceDir,
|
||||
SkillName: "code-review",
|
||||
Force: true,
|
||||
Agent: "github-copilot",
|
||||
Scope: "project",
|
||||
ScopeChanged: true,
|
||||
Dir: targetDir,
|
||||
AllowHiddenDirs: true,
|
||||
GitClient: &git.Client{RepoDir: t.TempDir()},
|
||||
}
|
||||
},
|
||||
wantStdout: "Installed code-review",
|
||||
wantStderr: "Skills in hidden directories",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue