STACKITGit/cli
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge pull request #13265 from SamMorrowDrums/sammorrowdrums/preview-allow-hidden-dirs-flag

Browse source 
feat(skills): add --allow-hidden-dirs flag to preview command
This commit is contained in:
Sam Morrow 2026-04-24 11:39:24 +02:00 • committed by GitHub
commit 5a121bf331
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 364 additions and 39 deletions
Download patch file Download diff file Expand all files Collapse all files

42
pkg/cmd/skills/install/install.go
View file

@ -391,7 +391,7 @@ func installRun(opts *InstallOptions) error {
}
printFileTree(opts.IO.ErrOut, cs, result.Dir, result.Installed)
printReviewHint(opts.IO.ErrOut, cs, repoSource, resolved.SHA, result.Installed)
printReviewHint(opts.IO.ErrOut, cs, repoSource, resolved.SHA, result.Installed, opts.AllowHiddenDirs)
printHostHints(opts.IO.ErrOut, cs, plan.hosts, result.Installed, result.Dir, gitRoot)
}
@ -536,7 +536,7 @@ func runLocalInstall(opts *InstallOptions) error {
}
printFileTree(opts.IO.ErrOut, cs, result.Dir, result.Installed)
printReviewHint(opts.IO.ErrOut, cs, "", "", result.Installed)
printReviewHint(opts.IO.ErrOut, cs, "", "", result.Installed, false)
printHostHints(opts.IO.ErrOut, cs, plan.hosts, result.Installed, result.Dir, gitRoot)
}
@ -1118,8 +1118,10 @@ func printPreInstallDisclaimer(w io.Writer, cs *iostreams.ColorScheme) {
// printReviewHint warns the user to review installed skills and suggests preview commands.
// When sha is non-empty the suggested commands include @SHA so the user previews
// exactly the version that was installed.
func printReviewHint(w io.Writer, cs *iostreams.ColorScheme, repo, sha string, skillNames []string) {
// exactly the version that was installed. When allowHiddenDirs is true, the
// suggested commands include --allow-hidden-dirs so previewing hidden-dir
// skills works without an extra manual step.
func printReviewHint(w io.Writer, cs *iostreams.ColorScheme, repo, sha string, skillNames []string, allowHiddenDirs bool) {
if len(skillNames) == 0 {
return
}
@ -1130,11 +1132,15 @@ func printReviewHint(w io.Writer, cs *iostreams.ColorScheme, repo, sha string, s
}
fmt.Fprintln(w, " Review installed content before use:")
fmt.Fprintln(w)
hiddenFlag := ""
if allowHiddenDirs {
hiddenFlag = " --allow-hidden-dirs"
}
for _, name := range skillNames {
if sha != "" {
fmt.Fprintf(w, " gh skill preview %s %s@%s\n", repo, name, sha)
fmt.Fprintf(w, " gh skill preview %s %s@%s%s\n", repo, name, sha, hiddenFlag)
} else {
fmt.Fprintf(w, " gh skill preview %s %s\n", repo, name)
fmt.Fprintf(w, " gh skill preview %s %s%s\n", repo, name, hiddenFlag)
}
}
fmt.Fprintln(w)
@ -1180,10 +1186,9 @@ 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.
// filterHiddenDirSkills applies the --allow-hidden-dirs flag logic. When the
// flag is set, all skills are returned with a warning. Otherwise, hidden-dir
// skills are excluded with an error if no standard skills remain.
func filterHiddenDirSkills(opts *InstallOptions, allSkills []discovery.Skill) ([]discovery.Skill, error) {
cs := opts.IO.ColorScheme()
@ -1198,25 +1203,16 @@ func filterHiddenDirSkills(opts *InstallOptions, allSkills []discovery.Skill) ([
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 {
r := discovery.PartitionHiddenDirSkills(allSkills)
if len(r.Standard) == 0 && r.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,
r.HiddenCount,
)
}
return standard, nil
return r.Standard, nil
}
// checkUpstreamProvenance fetches the skill's SKILL.md via the contents API

29
pkg/cmd/skills/install/install_test.go
View file

@ -2141,11 +2141,12 @@ func Test_isSkillPath(t *testing.T) {
func Test_printReviewHint(t *testing.T) {
tests := []struct {
name string
repo string
sha string
skillNames []string
wantOutput string
name string
repo string
sha string
skillNames []string
allowHiddenDirs bool
wantOutput string
}{
{
name: "remote install with SHA includes SHA in preview command",
@ -2182,6 +2183,22 @@ func Test_printReviewHint(t *testing.T) {
skillNames: []string{},
wantOutput: "",
},
{
name: "allow-hidden-dirs appends flag to preview command",
repo: "owner/repo",
sha: "abc123",
skillNames: []string{"hidden-skill"},
allowHiddenDirs: true,
wantOutput: "gh skill preview owner/repo hidden-skill@abc123 --allow-hidden-dirs",
},
{
name: "allow-hidden-dirs without SHA",
repo: "owner/repo",
sha: "",
skillNames: []string{"hidden-skill"},
allowHiddenDirs: true,
wantOutput: "gh skill preview owner/repo hidden-skill --allow-hidden-dirs",
},
}
for _, tt := range tests {
@ -2189,7 +2206,7 @@ func Test_printReviewHint(t *testing.T) {
ios, _, _, _ := iostreams.Test()
cs := ios.ColorScheme()
var buf strings.Builder
printReviewHint(&buf, cs, tt.repo, tt.sha, tt.skillNames)
printReviewHint(&buf, cs, tt.repo, tt.sha, tt.skillNames, tt.allowHiddenDirs)
if tt.wantOutput == "" {
assert.Empty(t, buf.String())
} else {

49
pkg/cmd/skills/preview/preview.go
View file

@ -32,9 +32,10 @@ type PreviewOptions struct {
ExecutablePath string
RenderFile func(string, string) string
RepoArg string
SkillName string
Version string // resolved from @suffix on SkillName
RepoArg string
SkillName string
Version string // resolved from @suffix on SkillName
AllowHiddenDirs bool // include skills in dot-prefixed directories
repo ghrepo.Interface
}
@ -110,6 +111,8 @@ func NewCmdPreview(f *cmdutil.Factory, telemetry ghtelemetry.CommandRecorder, ru
},
}
cmd.Flags().BoolVar(&opts.AllowHiddenDirs, "allow-hidden-dirs", false, "Include skills in hidden directories (e.g. .claude/skills/, .agents/skills/)")
return cmd
}
@ -151,12 +154,17 @@ func previewRun(opts *PreviewOptions) error {
}
opts.IO.StartProgressIndicatorWithLabel("Discovering skills")
skills, err := discovery.DiscoverSkills(apiClient, hostname, owner, repoName, resolved.SHA)
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
}
sort.Slice(skills, func(i, j int) bool {
return skills[i].DisplayName() < skills[j].DisplayName()
})
@ -388,6 +396,39 @@ func isMarkdownFile(filePath string) bool {
}
}
// filterHiddenDirSkills applies the --allow-hidden-dirs flag logic. When the
// flag is set, all skills are returned with a warning. Otherwise, hidden-dir
// skills are excluded with a hint or error.
func filterHiddenDirSkills(opts *PreviewOptions, 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
}
r := discovery.PartitionHiddenDirSkills(allSkills)
if r.HiddenCount > 0 {
if len(r.Standard) == 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",
r.HiddenCount,
)
}
fmt.Fprintf(opts.IO.ErrOut, "%s %d skill(s) in hidden directories were excluded, use --%s to include them\n",
cs.Yellow("!"), r.HiddenCount, "allow-hidden-dirs")
}
return r.Standard, nil
}
func selectSkill(opts *PreviewOptions, skills []discovery.Skill) (discovery.Skill, error) {
if opts.SkillName != "" {
for _, s := range skills {

263
pkg/cmd/skills/preview/preview_test.go
View file

@ -11,6 +11,7 @@ import (
"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/internal/prompter"
"github.com/cli/cli/v2/internal/skills/discovery"
"github.com/cli/cli/v2/internal/telemetry"
"github.com/cli/cli/v2/pkg/cmdutil"
"github.com/cli/cli/v2/pkg/httpmock"
@ -22,12 +23,13 @@ import (
func TestNewCmdPreview(t *testing.T) {
tests := []struct {
name string
input string
wantRepo string
wantSkillName string
wantVersion string
wantErr bool
name string
input string
wantRepo string
wantSkillName string
wantVersion string
wantAllowHiddenDirs bool
wantErr bool
}{
{
name: "repo and skill",
@ -64,6 +66,13 @@ func TestNewCmdPreview(t *testing.T) {
input: "a b c",
wantErr: true,
},
{
name: "allow-hidden-dirs flag",
input: "github/awesome-copilot my-skill --allow-hidden-dirs",
wantRepo: "github/awesome-copilot",
wantSkillName: "my-skill",
wantAllowHiddenDirs: true,
},
}
for _, tt := range tests {
@ -95,6 +104,7 @@ func TestNewCmdPreview(t *testing.T) {
assert.Equal(t, tt.wantRepo, gotOpts.RepoArg)
assert.Equal(t, tt.wantSkillName, gotOpts.SkillName)
assert.Equal(t, tt.wantVersion, gotOpts.Version)
assert.Equal(t, tt.wantAllowHiddenDirs, gotOpts.AllowHiddenDirs)
})
}
}
@ -1068,3 +1078,244 @@ func TestPreviewRun_TelemetryVisibility(t *testing.T) {
})
}
}
func TestFilterHiddenDirSkills(t *testing.T) {
standardSkill := discovery.Skill{Name: "my-skill", Convention: "standard"}
hiddenSkill := discovery.Skill{Name: "hidden-skill", Convention: "hidden-dir"}
hiddenNS := discovery.Skill{Name: "ns-skill", Convention: "hidden-dir-namespaced"}
tests := []struct {
name string
allowHiddenDirs bool
skills []discovery.Skill
wantCount int
wantErr string
wantStderr string
}{
{
name: "no hidden skills returns all",
skills: []discovery.Skill{standardSkill},
wantCount: 1,
},
{
name: "hidden skills excluded by default",
skills: []discovery.Skill{standardSkill, hiddenSkill},
wantCount: 1,
wantStderr: "1 skill(s) in hidden directories were excluded",
},
{
name: "multiple hidden skills excluded with hint",
skills: []discovery.Skill{standardSkill, hiddenSkill, hiddenNS},
wantCount: 1,
wantStderr: "2 skill(s) in hidden directories were excluded",
},
{
name: "only hidden skills returns error",
skills: []discovery.Skill{hiddenSkill, hiddenNS},
wantErr: "no standard skills found, but 2 skill(s) exist in hidden directories",
},
{
name: "allow-hidden-dirs includes all skills",
allowHiddenDirs: true,
skills: []discovery.Skill{standardSkill, hiddenSkill},
wantCount: 2,
wantStderr: "Skills in hidden directories",
},
{
name: "allow-hidden-dirs with no hidden skills",
allowHiddenDirs: true,
skills: []discovery.Skill{standardSkill},
wantCount: 1,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ios, _, _, stderr := iostreams.Test()
opts := &PreviewOptions{
IO: ios,
AllowHiddenDirs: tt.allowHiddenDirs,
}
result, err := filterHiddenDirSkills(opts, tt.skills)
if tt.wantErr != "" {
require.Error(t, err)
assert.Contains(t, err.Error(), tt.wantErr)
return
}
require.NoError(t, err)
assert.Len(t, result, tt.wantCount)
if tt.wantStderr != "" {
assert.Contains(t, stderr.String(), tt.wantStderr)
}
})
}
}
func TestPreviewRun_HiddenDirSkillsExcluded(t *testing.T) {
skillContent := heredoc.Doc(`
---
name: my-skill
description: A test skill
---
# My Skill
This is the skill content.
`)
encodedContent := base64.StdEncoding.EncodeToString([]byte(skillContent))
// Tree contains both a standard skill and a hidden-dir skill
treeJSON := `{
"sha": "abc123",
"truncated": false,
"tree": [
{"path": "skills/my-skill", "type": "tree", "sha": "treeSHA"},
{"path": "skills/my-skill/SKILL.md", "type": "blob", "sha": "blob123"},
{"path": ".claude/skills/hidden-skill", "type": "tree", "sha": "treeHidden"},
{"path": ".claude/skills/hidden-skill/SKILL.md", "type": "blob", "sha": "blobHidden"}
]
}`
t.Run("hidden skills excluded by default with hint", func(t *testing.T) {
reg := &httpmock.Registry{}
defer reg.Verify(t)
reg.Register(
httpmock.REST("GET", "repos/owner/repo/releases/latest"),
httpmock.StringResponse(`{"tag_name": "v1.0.0"}`),
)
reg.Register(
httpmock.REST("GET", "repos/owner/repo/git/ref/tags/v1.0.0"),
httpmock.StringResponse(`{"object": {"sha": "abc123", "type": "commit"}}`),
)
reg.Register(
httpmock.REST("GET", "repos/owner/repo/git/trees/abc123"),
httpmock.StringResponse(treeJSON),
)
reg.Register(
httpmock.REST("GET", "repos/owner/repo/git/trees/treeSHA"),
httpmock.StringResponse(`{
"tree": [
{"path": "SKILL.md", "type": "blob", "sha": "blob123", "size": 50}
]
}`),
)
reg.Register(
httpmock.REST("GET", "repos/owner/repo/git/blobs/blob123"),
httpmock.StringResponse(`{"sha": "blob123", "content": "`+encodedContent+`", "encoding": "base64"}`),
)
ios, _, stdout, stderr := iostreams.Test()
ios.SetStdoutTTY(false)
ios.SetStdinTTY(false)
opts := &PreviewOptions{
IO: ios,
HttpClient: func() (*http.Client, error) { return &http.Client{Transport: reg}, nil },
Prompter: &prompter.PrompterMock{},
repo: ghrepo.New("owner", "repo"),
SkillName: "my-skill",
Telemetry: &telemetry.NoOpService{},
}
err := previewRun(opts)
require.NoError(t, err)
assert.Contains(t, stdout.String(), "My Skill")
assert.Contains(t, stderr.String(), "skill(s) in hidden directories were excluded")
assert.Contains(t, stderr.String(), "allow-hidden-dirs")
})
t.Run("allow-hidden-dirs includes hidden skills", func(t *testing.T) {
reg := &httpmock.Registry{}
defer reg.Verify(t)
reg.Register(
httpmock.REST("GET", "repos/owner/repo/releases/latest"),
httpmock.StringResponse(`{"tag_name": "v1.0.0"}`),
)
reg.Register(
httpmock.REST("GET", "repos/owner/repo/git/ref/tags/v1.0.0"),
httpmock.StringResponse(`{"object": {"sha": "abc123", "type": "commit"}}`),
)
reg.Register(
httpmock.REST("GET", "repos/owner/repo/git/trees/abc123"),
httpmock.StringResponse(treeJSON),
)
reg.Register(
httpmock.REST("GET", "repos/owner/repo/git/trees/treeHidden"),
httpmock.StringResponse(`{
"tree": [
{"path": "SKILL.md", "type": "blob", "sha": "blobHidden", "size": 50}
]
}`),
)
reg.Register(
httpmock.REST("GET", "repos/owner/repo/git/blobs/blobHidden"),
httpmock.StringResponse(`{"sha": "blobHidden", "content": "`+encodedContent+`", "encoding": "base64"}`),
)
ios, _, stdout, stderr := iostreams.Test()
ios.SetStdoutTTY(false)
ios.SetStdinTTY(false)
opts := &PreviewOptions{
IO: ios,
HttpClient: func() (*http.Client, error) { return &http.Client{Transport: reg}, nil },
Prompter: &prompter.PrompterMock{},
repo: ghrepo.New("owner", "repo"),
SkillName: "hidden-skill",
AllowHiddenDirs: true,
Telemetry: &telemetry.NoOpService{},
}
err := previewRun(opts)
require.NoError(t, err)
assert.Contains(t, stdout.String(), "My Skill")
assert.Contains(t, stderr.String(), "Skills in hidden directories")
assert.NotContains(t, stderr.String(), "were excluded")
})
t.Run("only hidden skills without flag returns error", func(t *testing.T) {
onlyHiddenTree := `{
"sha": "abc123",
"truncated": false,
"tree": [
{"path": ".claude/skills/hidden-skill", "type": "tree", "sha": "treeHidden"},
{"path": ".claude/skills/hidden-skill/SKILL.md", "type": "blob", "sha": "blobHidden"}
]
}`
reg := &httpmock.Registry{}
defer reg.Verify(t)
reg.Register(
httpmock.REST("GET", "repos/owner/repo/releases/latest"),
httpmock.StringResponse(`{"tag_name": "v1.0.0"}`),
)
reg.Register(
httpmock.REST("GET", "repos/owner/repo/git/ref/tags/v1.0.0"),
httpmock.StringResponse(`{"object": {"sha": "abc123", "type": "commit"}}`),
)
reg.Register(
httpmock.REST("GET", "repos/owner/repo/git/trees/abc123"),
httpmock.StringResponse(onlyHiddenTree),
)
ios, _, _, _ := iostreams.Test()
ios.SetStdoutTTY(false)
ios.SetStdinTTY(false)
opts := &PreviewOptions{
IO: ios,
HttpClient: func() (*http.Client, error) { return &http.Client{Transport: reg}, nil },
Prompter: &prompter.PrompterMock{},
repo: ghrepo.New("owner", "repo"),
SkillName: "hidden-skill",
Telemetry: &telemetry.NoOpService{},
}
err := previewRun(opts)
require.Error(t, err)
assert.Contains(t, err.Error(), "no standard skills found")
assert.Contains(t, err.Error(), "--allow-hidden-dirs")
})
}