From d9fab039abdb38a62bd1aa1082ff17018219ef87 Mon Sep 17 00:00:00 2001 From: tommaso-moro Date: Mon, 11 May 2026 10:54:57 +0100 Subject: [PATCH 01/12] add skill list command --- pkg/cmd/skills/list/list.go | 480 +++++++++++++++++++++++++++++++ pkg/cmd/skills/list/list_test.go | 348 ++++++++++++++++++++++ pkg/cmd/skills/skills.go | 5 + 3 files changed, 833 insertions(+) create mode 100644 pkg/cmd/skills/list/list.go create mode 100644 pkg/cmd/skills/list/list_test.go diff --git a/pkg/cmd/skills/list/list.go b/pkg/cmd/skills/list/list.go new file mode 100644 index 00000000000..8e687b2e89a --- /dev/null +++ b/pkg/cmd/skills/list/list.go @@ -0,0 +1,480 @@ +package list + +import ( + "fmt" + "os" + "path/filepath" + "sort" + "strings" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/v2/git" + "github.com/cli/cli/v2/internal/gh/ghtelemetry" + "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/skills/discovery" + "github.com/cli/cli/v2/internal/skills/frontmatter" + "github.com/cli/cli/v2/internal/skills/installer" + "github.com/cli/cli/v2/internal/skills/registry" + "github.com/cli/cli/v2/internal/skills/source" + "github.com/cli/cli/v2/internal/tableprinter" + "github.com/cli/cli/v2/pkg/cmdutil" + "github.com/cli/cli/v2/pkg/iostreams" + "github.com/spf13/cobra" +) + +var skillListFields = []string{ + "skillName", + "hosts", + "scope", + "sourceURL", + "version", + "pinned", + "path", +} + +// ListOptions holds dependencies and user-provided flags for the list command. +type ListOptions struct { + IO *iostreams.IOStreams + Telemetry ghtelemetry.EventRecorder + GitClient *git.Client + Exporter cmdutil.Exporter + + Agent string + Scope string + ScopeChanged bool + Dir string +} + +type agentInfo struct { + id string +} + +type scanTarget struct { + dir string + hosts []agentInfo + scope string +} + +type listedSkill struct { + skillName string + hostIDs []string + scope string + source string + sourceURL string + version string + pinned bool + path string +} + +// ExportData implements cmdutil.exportable for --json output. +func (s listedSkill) ExportData(fields []string) map[string]interface{} { + data := map[string]interface{}{} + for _, f := range fields { + switch f { + case "skillName": + data[f] = s.skillName + case "hosts": + data[f] = s.hostIDs + case "scope": + data[f] = s.scope + case "sourceURL": + data[f] = s.sourceURL + case "version": + data[f] = s.version + case "pinned": + data[f] = s.pinned + case "path": + data[f] = s.path + } + } + return data +} + +// NewCmdList creates the "skills list" command. +func NewCmdList(f *cmdutil.Factory, telemetry ghtelemetry.CommandRecorder, runF func(*ListOptions) error) *cobra.Command { + opts := &ListOptions{ + IO: f.IOStreams, + Telemetry: telemetry, + GitClient: f.GitClient, + } + + cmd := &cobra.Command{ + Use: "list [flags]", + Short: "List installed skills (preview)", + Aliases: []string{"ls"}, + Long: heredoc.Docf(` + List installed agent skills across known agent host directories. + + By default, scans all supported agent hosts in both project and user scope. + Use %[1]s--agent%[1]s to scan one host, %[1]s--scope%[1]s to scan only project or user + scope, or %[1]s--dir%[1]s to scan a custom skills directory. + + Project-scope skills are discovered relative to the current git repository + root. User-scope skills are discovered relative to your home directory. + `, "`"), + Example: heredoc.Doc(` + # List all installed skills + $ gh skill list + + # List skills installed for Claude Code + $ gh skill list --agent claude-code + + # List user-scope skills + $ gh skill list --scope user + + # List skills as JSON + $ gh skill list --json skillName,sourceURL,scope,version,pinned,path + `), + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, args []string) error { + opts.ScopeChanged = cmd.Flags().Changed("scope") + + if opts.Dir != "" && opts.Agent != "" { + return cmdutil.FlagErrorf("--dir and --agent cannot be used together") + } + if opts.Dir != "" && opts.ScopeChanged { + return cmdutil.FlagErrorf("--dir and --scope cannot be used together") + } + + if runF != nil { + return runF(opts) + } + return listRun(opts) + }, + } + + cmdutil.StringEnumFlag(cmd, &opts.Agent, "agent", "", "", registry.AgentIDs(), "Filter by target agent") + cmdutil.StringEnumFlag(cmd, &opts.Scope, "scope", "", "", []string{string(registry.ScopeProject), string(registry.ScopeUser)}, "Filter by installation scope") + cmd.Flags().StringVar(&opts.Dir, "dir", "", "Scan a custom directory for installed skills") + cmdutil.AddJSONFlags(cmd, &opts.Exporter, skillListFields) + + return cmd +} + +func listRun(opts *ListOptions) error { + skills, err := listInstalledSkills(opts) + if err != nil { + return err + } + sortListedSkills(skills) + recordListTelemetry(opts, len(skills)) + + if opts.Exporter != nil { + return opts.Exporter.Write(opts.IO, skills) + } + + if len(skills) == 0 { + return cmdutil.NewNoResultsError("no installed skills found") + } + + return renderTable(opts.IO, skills) +} + +func listInstalledSkills(opts *ListOptions) ([]listedSkill, error) { + targets, err := buildScanTargets(opts) + if err != nil { + return nil, err + } + + var all []listedSkill + for _, target := range targets { + skills, scanErr := scanInstalledSkills(target.dir, target.hosts, target.scope) + if scanErr != nil { + if opts.Dir != "" { + return nil, fmt.Errorf("could not scan directory: %w", scanErr) + } + continue + } + all = append(all, skills...) + } + + return all, nil +} + +func buildScanTargets(opts *ListOptions) ([]scanTarget, error) { + if opts.Dir != "" { + dir, err := filepath.Abs(opts.Dir) + if err != nil { + return nil, fmt.Errorf("could not resolve path: %w", err) + } + return []scanTarget{{dir: dir, scope: "custom"}}, nil + } + + gitRoot := installer.ResolveGitRoot(opts.GitClient) + homeDir := installer.ResolveHomeDir() + + hosts, err := selectedHosts(opts.Agent) + if err != nil { + return nil, err + } + scopes := selectedScopes(opts.Scope) + + byDir := map[string]int{} + var targets []scanTarget + for _, host := range hosts { + for _, scope := range scopes { + dir, installErr := host.InstallDir(scope, gitRoot, homeDir) + if installErr != nil { + continue + } + + if idx, ok := byDir[dir]; ok { + targets[idx].hosts = appendHost(targets[idx].hosts, host) + continue + } + + byDir[dir] = len(targets) + targets = append(targets, scanTarget{ + dir: dir, + hosts: []agentInfo{{id: host.ID}}, + scope: string(scope), + }) + } + } + + return targets, nil +} + +func selectedHosts(agentID string) ([]*registry.AgentHost, error) { + if agentID != "" { + host, err := registry.FindByID(agentID) + if err != nil { + return nil, err + } + return []*registry.AgentHost{host}, nil + } + + hosts := make([]*registry.AgentHost, len(registry.Agents)) + for i := range registry.Agents { + hosts[i] = ®istry.Agents[i] + } + return hosts, nil +} + +func selectedScopes(scope string) []registry.Scope { + if scope != "" { + return []registry.Scope{registry.Scope(scope)} + } + return []registry.Scope{registry.ScopeProject, registry.ScopeUser} +} + +func appendHost(hosts []agentInfo, host *registry.AgentHost) []agentInfo { + for _, existing := range hosts { + if existing.id == host.ID { + return hosts + } + } + return append(hosts, agentInfo{id: host.ID}) +} + +func scanInstalledSkills(skillsDir string, hosts []agentInfo, scope string) ([]listedSkill, error) { + entries, err := os.ReadDir(skillsDir) + if os.IsNotExist(err) { + return nil, nil + } + if err != nil { + return nil, fmt.Errorf("could not read skills directory: %w", err) + } + + var skills []listedSkill + for _, e := range entries { + if !e.IsDir() { + continue + } + + // Flat layout: {dir}/{name}/SKILL.md. + skillDir := filepath.Join(skillsDir, e.Name()) + skillFile := filepath.Join(skillDir, "SKILL.md") + if data, readErr := os.ReadFile(skillFile); readErr == nil { + skills = append(skills, parseInstalledSkill(data, e.Name(), skillDir, hosts, scope)) + continue + } + + // Namespaced layout: {dir}/{namespace}/{name}/SKILL.md. + subEntries, subErr := os.ReadDir(skillDir) + if subErr != nil { + continue + } + for _, sub := range subEntries { + if !sub.IsDir() { + continue + } + subSkillDir := filepath.Join(skillDir, sub.Name()) + subSkillFile := filepath.Join(subSkillDir, "SKILL.md") + if data, readErr := os.ReadFile(subSkillFile); readErr == nil { + installName := e.Name() + "/" + sub.Name() + skills = append(skills, parseInstalledSkill(data, installName, subSkillDir, hosts, scope)) + } + } + } + + return skills, nil +} + +func parseInstalledSkill(data []byte, name, dir string, hosts []agentInfo, scope string) listedSkill { + s := listedSkill{ + skillName: name, + hostIDs: hostIDs(hosts), + scope: scope, + path: dir, + } + + result, err := frontmatter.Parse(string(data)) + if err != nil { + return s + } + + meta := result.Metadata.Meta + if meta == nil { + return s + } + + if sourcePath, _ := meta["github-path"].(string); sourcePath != "" { + if skillName := skillNameFromSourcePath(sourcePath); skillName != "" { + s.skillName = skillName + } + } + + if repoURL, _ := meta["github-repo"].(string); repoURL != "" { + s.sourceURL = repoURL + s.source = repoURL + if repo, parseErr := source.ParseRepoURL(repoURL); parseErr == nil { + s.source = ghrepo.FullName(repo) + s.sourceURL = source.BuildRepoURL(repo.RepoHost(), repo.RepoOwner(), repo.RepoName()) + } + } else if localPath, _ := meta["local-path"].(string); localPath != "" { + s.sourceURL = localPath + s.source = localPath + } + + if ref, _ := meta["github-ref"].(string); ref != "" { + s.version = discovery.ShortRef(ref) + } + if pinnedRef, _ := meta["github-pinned"].(string); pinnedRef != "" { + s.pinned = true + if s.version == "" { + s.version = pinnedRef + } + } + + return s +} + +func skillNameFromSourcePath(sourcePath string) string { + sourcePath = strings.TrimSuffix(sourcePath, "/SKILL.md") + sourcePath = strings.Trim(sourcePath, "/") + if sourcePath == "" { + return "" + } + + parts := strings.Split(sourcePath, "/") + for i := len(parts) - 1; i >= 0; i-- { + if parts[i] != "skills" { + continue + } + + if i >= 2 && parts[i-2] == "plugins" && i+1 < len(parts) { + return parts[i-1] + "/" + parts[len(parts)-1] + } + + afterSkills := len(parts) - i - 1 + switch afterSkills { + case 0: + return "" + case 1: + return parts[i+1] + default: + return parts[i+1] + "/" + parts[len(parts)-1] + } + } + + return parts[len(parts)-1] +} + +func hostIDs(hosts []agentInfo) []string { + ids := make([]string, len(hosts)) + for i, host := range hosts { + ids[i] = host.id + } + return ids +} + +func sortListedSkills(skills []listedSkill) { + sort.Slice(skills, func(i, j int) bool { + if skills[i].skillName != skills[j].skillName { + return skills[i].skillName < skills[j].skillName + } + if skills[i].scope != skills[j].scope { + return skills[i].scope < skills[j].scope + } + if formatHosts(skills[i].hostIDs) != formatHosts(skills[j].hostIDs) { + return formatHosts(skills[i].hostIDs) < formatHosts(skills[j].hostIDs) + } + return skills[i].path < skills[j].path + }) +} + +func renderTable(io *iostreams.IOStreams, skills []listedSkill) error { + table := tableprinter.New(io, tableprinter.WithHeader("Name", "Agent", "Scope", "Source")) + + for _, skill := range skills { + table.AddField(skill.skillName) + table.AddField(formatHosts(skill.hostIDs)) + table.AddField(displayOrDash(skill.scope)) + table.AddField(displayOrDash(skill.source)) + table.EndRow() + } + + return table.Render() +} + +func displayOrDash(value string) string { + if value == "" { + return "-" + } + return value +} + +func formatHosts(hosts []string) string { + if len(hosts) == 0 { + return "-" + } + return strings.Join(hosts, ",") +} + +func recordListTelemetry(opts *ListOptions, skillCount int) { + if opts.Telemetry == nil { + return + } + + agentHosts := opts.Agent + if agentHosts == "" { + agentHosts = "all" + } + scope := opts.Scope + if scope == "" { + scope = "all" + } + customDir := "false" + if opts.Dir != "" { + customDir = "true" + scope = "custom" + } + format := "table" + if opts.Exporter != nil { + format = "json" + } + + opts.Telemetry.Record(ghtelemetry.Event{ + Type: "skill_list", + Dimensions: ghtelemetry.Dimensions{ + "agent_hosts": agentHosts, + "custom_dir": customDir, + "format": format, + "scope": scope, + }, + Measures: ghtelemetry.Measures{ + "skill_count": int64(skillCount), + }, + }) +} diff --git a/pkg/cmd/skills/list/list_test.go b/pkg/cmd/skills/list/list_test.go new file mode 100644 index 00000000000..4b8b397e8e6 --- /dev/null +++ b/pkg/cmd/skills/list/list_test.go @@ -0,0 +1,348 @@ +package list + +import ( + "bytes" + "fmt" + "io" + "os" + "path/filepath" + "testing" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/v2/git" + "github.com/cli/cli/v2/internal/telemetry" + "github.com/cli/cli/v2/pkg/cmdutil" + "github.com/cli/cli/v2/pkg/iostreams" + "github.com/google/shlex" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestNewCmdList(t *testing.T) { + tests := []struct { + name string + cli string + wantOpts ListOptions + wantJSON bool + wantErr string + }{ + { + name: "no flags", + cli: "", + wantOpts: ListOptions{}, + }, + { + name: "agent and scope filters", + cli: "--agent claude-code --scope user", + wantOpts: ListOptions{ + Agent: "claude-code", + Scope: "user", + ScopeChanged: true, + }, + }, + { + name: "custom dir", + cli: "--dir ./skills", + wantOpts: ListOptions{ + Dir: "./skills", + }, + }, + { + name: "json fields", + cli: "--json skillName,sourceURL,scope,version,pinned,path", + wantJSON: true, + }, + { + name: "too many args", + cli: "extra", + wantErr: "unknown command", + }, + { + name: "invalid agent", + cli: "--agent unknown", + wantErr: "invalid argument", + }, + { + name: "invalid scope", + cli: "--scope org", + wantErr: "invalid argument", + }, + { + name: "dir and agent are mutually exclusive", + cli: "--dir ./skills --agent claude-code", + wantErr: "--dir and --agent cannot be used together", + }, + { + name: "dir and scope are mutually exclusive", + cli: "--dir ./skills --scope user", + wantErr: "--dir and --scope cannot be used together", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ios, _, _, _ := iostreams.Test() + f := &cmdutil.Factory{IOStreams: ios, GitClient: &git.Client{}} + + var gotOpts *ListOptions + cmd := NewCmdList(f, &telemetry.NoOpService{}, func(opts *ListOptions) error { + gotOpts = opts + return nil + }) + + args, err := shlex.Split(tt.cli) + require.NoError(t, err) + cmd.SetArgs(args) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + + err = cmd.Execute() + if tt.wantErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + return + } + + require.NoError(t, err) + require.NotNil(t, gotOpts) + assert.Equal(t, tt.wantOpts.Agent, gotOpts.Agent) + assert.Equal(t, tt.wantOpts.Scope, gotOpts.Scope) + assert.Equal(t, tt.wantOpts.ScopeChanged, gotOpts.ScopeChanged) + assert.Equal(t, tt.wantOpts.Dir, gotOpts.Dir) + if tt.wantJSON { + assert.NotNil(t, gotOpts.Exporter) + } + }) + } +} + +func TestNewCmdList_Metadata(t *testing.T) { + ios, _, _, _ := iostreams.Test() + f := &cmdutil.Factory{IOStreams: ios, GitClient: &git.Client{}} + cmd := NewCmdList(f, &telemetry.NoOpService{}, nil) + + assert.Equal(t, "list [flags]", cmd.Use) + assert.NotEmpty(t, cmd.Short) + assert.NotEmpty(t, cmd.Long) + assert.NotEmpty(t, cmd.Example) + assert.Contains(t, cmd.Aliases, "ls") + + for _, flag := range []string{"agent", "scope", "dir", "json"} { + assert.NotNil(t, cmd.Flags().Lookup(flag), "missing flag: --%s", flag) + } +} + +func TestListRun(t *testing.T) { + tests := []struct { + name string + setup func(t *testing.T, repoDir, homeDir string) + opts func(ios *iostreams.IOStreams, repoDir, homeDir string, spy *telemetry.CommandRecorderSpy) *ListOptions + wantStdout string + wantJSON string + wantErr string + verify func(t *testing.T, stdout string, spy *telemetry.CommandRecorderSpy) + }{ + { + name: "lists project skill for selected shared agent", + setup: func(t *testing.T, repoDir, homeDir string) { + writeSkill(t, repoDir, ".agents/skills/git-commit", remoteSkillFrontmatter("git-commit", "skills/git-commit", "refs/tags/v1.0.0", "")) + }, + opts: func(ios *iostreams.IOStreams, repoDir, homeDir string, spy *telemetry.CommandRecorderSpy) *ListOptions { + return &ListOptions{ + IO: ios, + Telemetry: spy, + GitClient: &git.Client{RepoDir: repoDir}, + Agent: "cursor", + Scope: "project", + } + }, + wantStdout: "git-commit\tcursor\tproject\tmonalisa/skills-repo\n", + verify: func(t *testing.T, stdout string, spy *telemetry.CommandRecorderSpy) { + require.Len(t, spy.Events, 1) + event := spy.Events[0] + assert.Equal(t, "skill_list", event.Type) + assert.Equal(t, "cursor", event.Dimensions["agent_hosts"]) + assert.Equal(t, "project", event.Dimensions["scope"]) + assert.Equal(t, int64(1), event.Measures["skill_count"]) + }, + }, + { + name: "lists user skill as json", + setup: func(t *testing.T, repoDir, homeDir string) { + writeSkill(t, homeDir, ".claude/skills/code-review", remoteSkillFrontmatter("code-review", "skills/code-review", "refs/tags/v2.0.0", "v2.0.0")) + }, + opts: func(ios *iostreams.IOStreams, repoDir, homeDir string, spy *telemetry.CommandRecorderSpy) *ListOptions { + exporter := cmdutil.NewJSONExporter() + exporter.SetFields([]string{"skillName", "hosts", "scope", "sourceURL", "version", "pinned", "path"}) + return &ListOptions{ + IO: ios, + Telemetry: spy, + GitClient: &git.Client{RepoDir: repoDir}, + Exporter: exporter, + Agent: "claude-code", + Scope: "user", + } + }, + wantJSON: fmt.Sprintf(`[ + { + "skillName": "code-review", + "hosts": ["claude-code"], + "scope": "user", + "sourceURL": "https://github.com/monalisa/skills-repo", + "version": "v2.0.0", + "pinned": true, + "path": %q + } + ]`, filepath.Join("HOME", ".claude", "skills", "code-review")), + verify: func(t *testing.T, stdout string, spy *telemetry.CommandRecorderSpy) { + assert.Equal(t, "json", spy.Events[0].Dimensions["format"]) + }, + }, + { + name: "custom directory with local metadata", + setup: func(t *testing.T, repoDir, homeDir string) { + customDir := filepath.Join(repoDir, "custom-skills") + writeSkill(t, customDir, "local-helper", heredoc.Doc(` + --- + name: local-helper + metadata: + local-path: /src/local-helper + --- + Body + `)) + }, + opts: func(ios *iostreams.IOStreams, repoDir, homeDir string, spy *telemetry.CommandRecorderSpy) *ListOptions { + return &ListOptions{ + IO: ios, + Telemetry: spy, + GitClient: &git.Client{RepoDir: repoDir}, + Dir: filepath.Join(repoDir, "custom-skills"), + } + }, + wantStdout: "local-helper\t-\tcustom\t/src/local-helper\n", + }, + { + name: "recovers namespaced skill name from source path", + setup: func(t *testing.T, repoDir, homeDir string) { + writeSkill(t, repoDir, ".agents/skills/xlsx-pro", remoteSkillFrontmatter("xlsx-pro", "skills/bob/xlsx-pro", "refs/heads/main", "")) + }, + opts: func(ios *iostreams.IOStreams, repoDir, homeDir string, spy *telemetry.CommandRecorderSpy) *ListOptions { + return &ListOptions{ + IO: ios, + Telemetry: spy, + GitClient: &git.Client{RepoDir: repoDir}, + Agent: "github-copilot", + Scope: "project", + } + }, + wantStdout: "bob/xlsx-pro\tgithub-copilot\tproject\tmonalisa/skills-repo\n", + }, + { + name: "no installed skills returns no results", + opts: func(ios *iostreams.IOStreams, repoDir, homeDir string, spy *telemetry.CommandRecorderSpy) *ListOptions { + return &ListOptions{ + IO: ios, + Telemetry: spy, + GitClient: &git.Client{RepoDir: repoDir}, + Agent: "github-copilot", + Scope: "project", + } + }, + wantErr: "no installed skills found", + }, + { + name: "no installed skills with json returns empty array", + opts: func(ios *iostreams.IOStreams, repoDir, homeDir string, spy *telemetry.CommandRecorderSpy) *ListOptions { + exporter := cmdutil.NewJSONExporter() + exporter.SetFields([]string{"skillName"}) + return &ListOptions{ + IO: ios, + Telemetry: spy, + GitClient: &git.Client{RepoDir: repoDir}, + Exporter: exporter, + Agent: "github-copilot", + Scope: "project", + } + }, + wantJSON: "[]", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + repoDir := t.TempDir() + homeDir := t.TempDir() + t.Setenv("HOME", homeDir) + + if tt.setup != nil { + tt.setup(t, repoDir, homeDir) + } + + ios, _, stdout, _ := iostreams.Test() + ios.SetStdoutTTY(false) + spy := &telemetry.CommandRecorderSpy{} + opts := tt.opts(ios, repoDir, homeDir, spy) + + err := listRun(opts) + if tt.wantErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + return + } + + require.NoError(t, err) + if tt.wantJSON != "" { + expected := tt.wantJSON + expected = string(bytes.ReplaceAll([]byte(expected), []byte(filepath.Join("HOME")), []byte(homeDir))) + assert.JSONEq(t, expected, stdout.String()) + } else { + assert.Equal(t, tt.wantStdout, stdout.String()) + } + if tt.verify != nil { + tt.verify(t, stdout.String(), spy) + } + }) + } +} + +func TestRenderTableUsesAgentHeader(t *testing.T) { + ios, _, stdout, _ := iostreams.Test() + ios.SetStdoutTTY(true) + + err := renderTable(ios, []listedSkill{{ + skillName: "git-commit", + hostIDs: []string{"github-copilot"}, + scope: "project", + source: "monalisa/skills-repo", + version: "v1.0.0", + }}) + + require.NoError(t, err) + assert.Contains(t, stdout.String(), "AGENT") + assert.NotContains(t, stdout.String(), "HOST") +} + +func writeSkill(t *testing.T, baseDir, relDir, content string) { + t.Helper() + skillDir := filepath.Join(baseDir, filepath.FromSlash(relDir)) + require.NoError(t, os.MkdirAll(skillDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte(content), 0o644)) +} + +func remoteSkillFrontmatter(name, sourcePath, ref, pinned string) string { + pinnedLine := "" + if pinned != "" { + pinnedLine = fmt.Sprintf(" github-pinned: %s\n", pinned) + } + return fmt.Sprintf(heredoc.Doc(` + --- + name: %s + metadata: + github-repo: https://github.com/monalisa/skills-repo + github-ref: %s + github-tree-sha: abc123 + github-path: %s + %s--- + Body + `), name, ref, sourcePath, pinnedLine) +} diff --git a/pkg/cmd/skills/skills.go b/pkg/cmd/skills/skills.go index 05a87c38651..1399d049b73 100644 --- a/pkg/cmd/skills/skills.go +++ b/pkg/cmd/skills/skills.go @@ -4,6 +4,7 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/internal/gh/ghtelemetry" "github.com/cli/cli/v2/pkg/cmd/skills/install" + skilllist "github.com/cli/cli/v2/pkg/cmd/skills/list" "github.com/cli/cli/v2/pkg/cmd/skills/preview" "github.com/cli/cli/v2/pkg/cmd/skills/publish" "github.com/cli/cli/v2/pkg/cmd/skills/search" @@ -32,6 +33,9 @@ func NewCmdSkills(f *cmdutil.Factory, telemetry ghtelemetry.CommandRecorder) *co # Install a skill $ gh skill install github/awesome-copilot documentation-writer + # List installed skills + $ gh skill list + # Preview a skill before installing $ gh skill preview github/awesome-copilot documentation-writer @@ -48,6 +52,7 @@ func NewCmdSkills(f *cmdutil.Factory, telemetry ghtelemetry.CommandRecorder) *co } cmd.AddCommand(install.NewCmdInstall(f, telemetry, nil)) + cmd.AddCommand(skilllist.NewCmdList(f, telemetry, nil)) cmd.AddCommand(preview.NewCmdPreview(f, telemetry, nil)) cmd.AddCommand(publish.NewCmdPublish(f, nil)) cmd.AddCommand(search.NewCmdSearch(f, telemetry, nil)) From 216b7cf6689aa2a65a2e4d372ff711ff9934ea1b Mon Sep 17 00:00:00 2001 From: tommaso-moro Date: Wed, 13 May 2026 15:57:56 +0100 Subject: [PATCH 02/12] fix linting --- pkg/cmd/skills/list/list_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/skills/list/list_test.go b/pkg/cmd/skills/list/list_test.go index 4b8b397e8e6..d132eb1111d 100644 --- a/pkg/cmd/skills/list/list_test.go +++ b/pkg/cmd/skills/list/list_test.go @@ -1,11 +1,11 @@ package list import ( - "bytes" "fmt" "io" "os" "path/filepath" + "strings" "testing" "github.com/MakeNowJust/heredoc" @@ -293,7 +293,7 @@ func TestListRun(t *testing.T) { require.NoError(t, err) if tt.wantJSON != "" { expected := tt.wantJSON - expected = string(bytes.ReplaceAll([]byte(expected), []byte(filepath.Join("HOME")), []byte(homeDir))) + expected = strings.ReplaceAll(expected, "HOME", homeDir) assert.JSONEq(t, expected, stdout.String()) } else { assert.Equal(t, tt.wantStdout, stdout.String()) From 1fac1218f39562101cd4ee2f5d99165ccaf254e1 Mon Sep 17 00:00:00 2001 From: tommaso-moro Date: Wed, 13 May 2026 22:34:26 +0100 Subject: [PATCH 03/12] fix tests --- pkg/cmd/skills/list/list_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/skills/list/list_test.go b/pkg/cmd/skills/list/list_test.go index d132eb1111d..01503680fb5 100644 --- a/pkg/cmd/skills/list/list_test.go +++ b/pkg/cmd/skills/list/list_test.go @@ -293,7 +293,7 @@ func TestListRun(t *testing.T) { require.NoError(t, err) if tt.wantJSON != "" { expected := tt.wantJSON - expected = strings.ReplaceAll(expected, "HOME", homeDir) + expected = strings.ReplaceAll(expected, "HOME", strings.ReplaceAll(homeDir, `\`, `\\`)) assert.JSONEq(t, expected, stdout.String()) } else { assert.Equal(t, tt.wantStdout, stdout.String()) From 2a98757f70160596e410acd5e2aa1c74208840e0 Mon Sep 17 00:00:00 2001 From: tommaso-moro Date: Mon, 18 May 2026 10:26:24 +0100 Subject: [PATCH 04/12] fix test --- pkg/cmd/skills/list/list_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/cmd/skills/list/list_test.go b/pkg/cmd/skills/list/list_test.go index 01503680fb5..021b29442fa 100644 --- a/pkg/cmd/skills/list/list_test.go +++ b/pkg/cmd/skills/list/list_test.go @@ -273,6 +273,7 @@ func TestListRun(t *testing.T) { repoDir := t.TempDir() homeDir := t.TempDir() t.Setenv("HOME", homeDir) + t.Setenv("USERPROFILE", homeDir) if tt.setup != nil { tt.setup(t, repoDir, homeDir) From 88b0c5118386513aec81998873125e1c6331fdcf Mon Sep 17 00:00:00 2001 From: tommaso-moro Date: Thu, 21 May 2026 17:58:49 +0100 Subject: [PATCH 05/12] address bagtoad's feedback --- pkg/cmd/skills/list/list.go | 104 ++++++++++++------------- pkg/cmd/skills/list/list_test.go | 126 +++++++++++++++++++++++++------ 2 files changed, 150 insertions(+), 80 deletions(-) diff --git a/pkg/cmd/skills/list/list.go b/pkg/cmd/skills/list/list.go index 8e687b2e89a..05d71285025 100644 --- a/pkg/cmd/skills/list/list.go +++ b/pkg/cmd/skills/list/list.go @@ -32,7 +32,8 @@ var skillListFields = []string{ "path", } -// ListOptions holds dependencies and user-provided flags for the list command. +const scopeCustom = "custom" + type ListOptions struct { IO *iostreams.IOStreams Telemetry ghtelemetry.EventRecorder @@ -45,25 +46,21 @@ type ListOptions struct { Dir string } -type agentInfo struct { - id string -} - type scanTarget struct { - dir string - hosts []agentInfo - scope string + dir string + agentHostIDs []string + scope string } type listedSkill struct { - skillName string - hostIDs []string - scope string - source string - sourceURL string - version string - pinned bool - path string + skillName string + agentHostIDs []string + scope string + source string + sourceURL string + version string + pinned bool + path string } // ExportData implements cmdutil.exportable for --json output. @@ -74,7 +71,7 @@ func (s listedSkill) ExportData(fields []string) map[string]interface{} { case "skillName": data[f] = s.skillName case "hosts": - data[f] = s.hostIDs + data[f] = s.agentHostIDs case "scope": data[f] = s.scope case "sourceURL": @@ -129,11 +126,11 @@ func NewCmdList(f *cmdutil.Factory, telemetry ghtelemetry.CommandRecorder, runF RunE: func(cmd *cobra.Command, args []string) error { opts.ScopeChanged = cmd.Flags().Changed("scope") - if opts.Dir != "" && opts.Agent != "" { - return cmdutil.FlagErrorf("--dir and --agent cannot be used together") + if err := cmdutil.MutuallyExclusive("--dir and --agent cannot be used together", opts.Dir != "", opts.Agent != ""); err != nil { + return err } - if opts.Dir != "" && opts.ScopeChanged { - return cmdutil.FlagErrorf("--dir and --scope cannot be used together") + if err := cmdutil.MutuallyExclusive("--dir and --scope cannot be used together", opts.Dir != "", opts.ScopeChanged); err != nil { + return err } if runF != nil { @@ -178,7 +175,7 @@ func listInstalledSkills(opts *ListOptions) ([]listedSkill, error) { var all []listedSkill for _, target := range targets { - skills, scanErr := scanInstalledSkills(target.dir, target.hosts, target.scope) + skills, scanErr := scanInstalledSkills(target.dir, target.agentHostIDs, target.scope) if scanErr != nil { if opts.Dir != "" { return nil, fmt.Errorf("could not scan directory: %w", scanErr) @@ -197,13 +194,16 @@ func buildScanTargets(opts *ListOptions) ([]scanTarget, error) { if err != nil { return nil, fmt.Errorf("could not resolve path: %w", err) } - return []scanTarget{{dir: dir, scope: "custom"}}, nil + if _, err := os.Stat(dir); err != nil { + return nil, fmt.Errorf("could not access directory: %w", err) + } + return []scanTarget{{dir: dir, scope: scopeCustom}}, nil } gitRoot := installer.ResolveGitRoot(opts.GitClient) homeDir := installer.ResolveHomeDir() - hosts, err := selectedHosts(opts.Agent) + agentHosts, err := selectedHosts(opts.Agent) if err != nil { return nil, err } @@ -211,23 +211,23 @@ func buildScanTargets(opts *ListOptions) ([]scanTarget, error) { byDir := map[string]int{} var targets []scanTarget - for _, host := range hosts { + for _, agentHost := range agentHosts { for _, scope := range scopes { - dir, installErr := host.InstallDir(scope, gitRoot, homeDir) + dir, installErr := agentHost.InstallDir(scope, gitRoot, homeDir) if installErr != nil { continue } if idx, ok := byDir[dir]; ok { - targets[idx].hosts = appendHost(targets[idx].hosts, host) + targets[idx].agentHostIDs = appendAgentHostID(targets[idx].agentHostIDs, agentHost.ID) continue } byDir[dir] = len(targets) targets = append(targets, scanTarget{ - dir: dir, - hosts: []agentInfo{{id: host.ID}}, - scope: string(scope), + dir: dir, + agentHostIDs: []string{agentHost.ID}, + scope: string(scope), }) } } @@ -258,16 +258,16 @@ func selectedScopes(scope string) []registry.Scope { return []registry.Scope{registry.ScopeProject, registry.ScopeUser} } -func appendHost(hosts []agentInfo, host *registry.AgentHost) []agentInfo { - for _, existing := range hosts { - if existing.id == host.ID { - return hosts +func appendAgentHostID(agentHostIDs []string, agentHostID string) []string { + for _, existing := range agentHostIDs { + if existing == agentHostID { + return agentHostIDs } } - return append(hosts, agentInfo{id: host.ID}) + return append(agentHostIDs, agentHostID) } -func scanInstalledSkills(skillsDir string, hosts []agentInfo, scope string) ([]listedSkill, error) { +func scanInstalledSkills(skillsDir string, agentHostIDs []string, scope string) ([]listedSkill, error) { entries, err := os.ReadDir(skillsDir) if os.IsNotExist(err) { return nil, nil @@ -286,7 +286,7 @@ func scanInstalledSkills(skillsDir string, hosts []agentInfo, scope string) ([]l skillDir := filepath.Join(skillsDir, e.Name()) skillFile := filepath.Join(skillDir, "SKILL.md") if data, readErr := os.ReadFile(skillFile); readErr == nil { - skills = append(skills, parseInstalledSkill(data, e.Name(), skillDir, hosts, scope)) + skills = append(skills, parseInstalledSkill(data, e.Name(), skillDir, agentHostIDs, scope)) continue } @@ -303,7 +303,7 @@ func scanInstalledSkills(skillsDir string, hosts []agentInfo, scope string) ([]l subSkillFile := filepath.Join(subSkillDir, "SKILL.md") if data, readErr := os.ReadFile(subSkillFile); readErr == nil { installName := e.Name() + "/" + sub.Name() - skills = append(skills, parseInstalledSkill(data, installName, subSkillDir, hosts, scope)) + skills = append(skills, parseInstalledSkill(data, installName, subSkillDir, agentHostIDs, scope)) } } } @@ -311,12 +311,12 @@ func scanInstalledSkills(skillsDir string, hosts []agentInfo, scope string) ([]l return skills, nil } -func parseInstalledSkill(data []byte, name, dir string, hosts []agentInfo, scope string) listedSkill { +func parseInstalledSkill(data []byte, name, dir string, agentHostIDs []string, scope string) listedSkill { s := listedSkill{ - skillName: name, - hostIDs: hostIDs(hosts), - scope: scope, - path: dir, + skillName: name, + agentHostIDs: agentHostIDs, + scope: scope, + path: dir, } result, err := frontmatter.Parse(string(data)) @@ -391,14 +391,6 @@ func skillNameFromSourcePath(sourcePath string) string { return parts[len(parts)-1] } -func hostIDs(hosts []agentInfo) []string { - ids := make([]string, len(hosts)) - for i, host := range hosts { - ids[i] = host.id - } - return ids -} - func sortListedSkills(skills []listedSkill) { sort.Slice(skills, func(i, j int) bool { if skills[i].skillName != skills[j].skillName { @@ -407,8 +399,8 @@ func sortListedSkills(skills []listedSkill) { if skills[i].scope != skills[j].scope { return skills[i].scope < skills[j].scope } - if formatHosts(skills[i].hostIDs) != formatHosts(skills[j].hostIDs) { - return formatHosts(skills[i].hostIDs) < formatHosts(skills[j].hostIDs) + if formatHosts(skills[i].agentHostIDs) != formatHosts(skills[j].agentHostIDs) { + return formatHosts(skills[i].agentHostIDs) < formatHosts(skills[j].agentHostIDs) } return skills[i].path < skills[j].path }) @@ -419,7 +411,7 @@ func renderTable(io *iostreams.IOStreams, skills []listedSkill) error { for _, skill := range skills { table.AddField(skill.skillName) - table.AddField(formatHosts(skill.hostIDs)) + table.AddField(formatHosts(skill.agentHostIDs)) table.AddField(displayOrDash(skill.scope)) table.AddField(displayOrDash(skill.source)) table.EndRow() @@ -439,7 +431,7 @@ func formatHosts(hosts []string) string { if len(hosts) == 0 { return "-" } - return strings.Join(hosts, ",") + return strings.Join(hosts, ", ") } func recordListTelemetry(opts *ListOptions, skillCount int) { @@ -458,7 +450,7 @@ func recordListTelemetry(opts *ListOptions, skillCount int) { customDir := "false" if opts.Dir != "" { customDir = "true" - scope = "custom" + scope = scopeCustom } format := "table" if opts.Exporter != nil { diff --git a/pkg/cmd/skills/list/list_test.go b/pkg/cmd/skills/list/list_test.go index 021b29442fa..33cd65ce141 100644 --- a/pkg/cmd/skills/list/list_test.go +++ b/pkg/cmd/skills/list/list_test.go @@ -82,7 +82,10 @@ func TestNewCmdList(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { ios, _, _, _ := iostreams.Test() - f := &cmdutil.Factory{IOStreams: ios, GitClient: &git.Client{}} + f := &cmdutil.Factory{ + IOStreams: ios, + GitClient: &git.Client{}, + } var gotOpts *ListOptions cmd := NewCmdList(f, &telemetry.NoOpService{}, func(opts *ListOptions) error { @@ -116,22 +119,6 @@ func TestNewCmdList(t *testing.T) { } } -func TestNewCmdList_Metadata(t *testing.T) { - ios, _, _, _ := iostreams.Test() - f := &cmdutil.Factory{IOStreams: ios, GitClient: &git.Client{}} - cmd := NewCmdList(f, &telemetry.NoOpService{}, nil) - - assert.Equal(t, "list [flags]", cmd.Use) - assert.NotEmpty(t, cmd.Short) - assert.NotEmpty(t, cmd.Long) - assert.NotEmpty(t, cmd.Example) - assert.Contains(t, cmd.Aliases, "ls") - - for _, flag := range []string{"agent", "scope", "dir", "json"} { - assert.NotNil(t, cmd.Flags().Lookup(flag), "missing flag: --%s", flag) - } -} - func TestListRun(t *testing.T) { tests := []struct { name string @@ -198,6 +185,31 @@ func TestListRun(t *testing.T) { assert.Equal(t, "json", spy.Events[0].Dimensions["format"]) }, }, + { + name: "preserves tenant host in json source url", + setup: func(t *testing.T, repoDir, homeDir string) { + writeSkill(t, homeDir, ".copilot/skills/tenant-skill", remoteSkillFrontmatterForRepo("tenant-skill", "https://octocorp.ghe.com/monalisa/skills-repo", "skills/tenant-skill", "refs/heads/main", "")) + }, + opts: func(ios *iostreams.IOStreams, repoDir, homeDir string, spy *telemetry.CommandRecorderSpy) *ListOptions { + exporter := cmdutil.NewJSONExporter() + exporter.SetFields([]string{"skillName", "sourceURL", "path"}) + return &ListOptions{ + IO: ios, + Telemetry: spy, + GitClient: &git.Client{RepoDir: repoDir}, + Exporter: exporter, + Agent: "github-copilot", + Scope: "user", + } + }, + wantJSON: fmt.Sprintf(`[ + { + "skillName": "tenant-skill", + "sourceURL": "https://octocorp.ghe.com/monalisa/skills-repo", + "path": %q + } + ]`, filepath.Join("HOME", ".copilot", "skills", "tenant-skill")), + }, { name: "custom directory with local metadata", setup: func(t *testing.T, repoDir, homeDir string) { @@ -221,6 +233,18 @@ func TestListRun(t *testing.T) { }, wantStdout: "local-helper\t-\tcustom\t/src/local-helper\n", }, + { + name: "custom directory must exist", + opts: func(ios *iostreams.IOStreams, repoDir, homeDir string, spy *telemetry.CommandRecorderSpy) *ListOptions { + return &ListOptions{ + IO: ios, + Telemetry: spy, + GitClient: &git.Client{RepoDir: repoDir}, + Dir: filepath.Join(repoDir, "missing-skills"), + } + }, + wantErr: "could not access directory", + }, { name: "recovers namespaced skill name from source path", setup: func(t *testing.T, repoDir, homeDir string) { @@ -237,6 +261,55 @@ func TestListRun(t *testing.T) { }, wantStdout: "bob/xlsx-pro\tgithub-copilot\tproject\tmonalisa/skills-repo\n", }, + { + name: "recovers plugin skill name from source path", + setup: func(t *testing.T, repoDir, homeDir string) { + writeSkill(t, repoDir, ".agents/skills/foo", remoteSkillFrontmatter("foo", "plugins/myplugin/skills/foo", "refs/heads/main", "")) + }, + opts: func(ios *iostreams.IOStreams, repoDir, homeDir string, spy *telemetry.CommandRecorderSpy) *ListOptions { + return &ListOptions{ + IO: ios, + Telemetry: spy, + GitClient: &git.Client{RepoDir: repoDir}, + Agent: "github-copilot", + Scope: "project", + } + }, + wantStdout: "myplugin/foo\tgithub-copilot\tproject\tmonalisa/skills-repo\n", + }, + { + name: "partial metadata has empty json source url", + setup: func(t *testing.T, repoDir, homeDir string) { + writeSkill(t, repoDir, ".agents/skills/partial", heredoc.Doc(` + --- + name: partial + metadata: + github-ref: refs/heads/main + --- + Body + `)) + }, + opts: func(ios *iostreams.IOStreams, repoDir, homeDir string, spy *telemetry.CommandRecorderSpy) *ListOptions { + exporter := cmdutil.NewJSONExporter() + exporter.SetFields([]string{"skillName", "sourceURL", "version", "pinned"}) + return &ListOptions{ + IO: ios, + Telemetry: spy, + GitClient: &git.Client{RepoDir: repoDir}, + Exporter: exporter, + Agent: "github-copilot", + Scope: "project", + } + }, + wantJSON: `[ + { + "skillName": "partial", + "sourceURL": "", + "version": "main", + "pinned": false + } + ]`, + }, { name: "no installed skills returns no results", opts: func(ios *iostreams.IOStreams, repoDir, homeDir string, spy *telemetry.CommandRecorderSpy) *ListOptions { @@ -311,15 +384,16 @@ func TestRenderTableUsesAgentHeader(t *testing.T) { ios.SetStdoutTTY(true) err := renderTable(ios, []listedSkill{{ - skillName: "git-commit", - hostIDs: []string{"github-copilot"}, - scope: "project", - source: "monalisa/skills-repo", - version: "v1.0.0", + skillName: "git-commit", + agentHostIDs: []string{"github-copilot", "cursor"}, + scope: "project", + source: "monalisa/skills-repo", + version: "v1.0.0", }}) require.NoError(t, err) assert.Contains(t, stdout.String(), "AGENT") + assert.Contains(t, stdout.String(), "github-copilot, cursor") assert.NotContains(t, stdout.String(), "HOST") } @@ -331,6 +405,10 @@ func writeSkill(t *testing.T, baseDir, relDir, content string) { } func remoteSkillFrontmatter(name, sourcePath, ref, pinned string) string { + return remoteSkillFrontmatterForRepo(name, "https://github.com/monalisa/skills-repo", sourcePath, ref, pinned) +} + +func remoteSkillFrontmatterForRepo(name, repoURL, sourcePath, ref, pinned string) string { pinnedLine := "" if pinned != "" { pinnedLine = fmt.Sprintf(" github-pinned: %s\n", pinned) @@ -339,11 +417,11 @@ func remoteSkillFrontmatter(name, sourcePath, ref, pinned string) string { --- name: %s metadata: - github-repo: https://github.com/monalisa/skills-repo + github-repo: %s github-ref: %s github-tree-sha: abc123 github-path: %s %s--- Body - `), name, ref, sourcePath, pinnedLine) + `), name, repoURL, ref, sourcePath, pinnedLine) } From 48ef6eb079a6f655db57241b958a8156146723ba Mon Sep 17 00:00:00 2001 From: tommaso-moro Date: Fri, 22 May 2026 10:20:44 +0100 Subject: [PATCH 06/12] handle skills in skills/ folder when running list command, by marking them as published --- pkg/cmd/skills/list/list.go | 101 ++++++++++++++++++++++++++++--- pkg/cmd/skills/list/list_test.go | 44 ++++++++++++++ 2 files changed, 136 insertions(+), 9 deletions(-) diff --git a/pkg/cmd/skills/list/list.go b/pkg/cmd/skills/list/list.go index 05d71285025..ca227425742 100644 --- a/pkg/cmd/skills/list/list.go +++ b/pkg/cmd/skills/list/list.go @@ -32,7 +32,19 @@ var skillListFields = []string{ "path", } -const scopeCustom = "custom" +const ( + agentHostPublished = "published" + agentHostPublishedDisplay = "n/a (published)" + scopeCustom = "custom" +) + +type scanFilter int + +const ( + scanAllSkills scanFilter = iota + scanInstalledOnly + scanPublishedOnly +) type ListOptions struct { IO *iostreams.IOStreams @@ -50,6 +62,7 @@ type scanTarget struct { dir string agentHostIDs []string scope string + filter scanFilter } type listedSkill struct { @@ -175,7 +188,7 @@ func listInstalledSkills(opts *ListOptions) ([]listedSkill, error) { var all []listedSkill for _, target := range targets { - skills, scanErr := scanInstalledSkills(target.dir, target.agentHostIDs, target.scope) + skills, scanErr := scanInstalledSkills(target.dir, target.agentHostIDs, target.scope, target.filter) if scanErr != nil { if opts.Dir != "" { return nil, fmt.Errorf("could not scan directory: %w", scanErr) @@ -220,6 +233,7 @@ func buildScanTargets(opts *ListOptions) ([]scanTarget, error) { if idx, ok := byDir[dir]; ok { targets[idx].agentHostIDs = appendAgentHostID(targets[idx].agentHostIDs, agentHost.ID) + targets[idx].filter = mergeScanFilters(targets[idx].filter, scanFilterForAgentHost(agentHost, scope)) continue } @@ -228,9 +242,18 @@ func buildScanTargets(opts *ListOptions) ([]scanTarget, error) { dir: dir, agentHostIDs: []string{agentHost.ID}, scope: string(scope), + filter: scanFilterForAgentHost(agentHost, scope), }) } } + if shouldListPublishedProjectSkills(opts.Agent, scopes, gitRoot) { + targets = append(targets, scanTarget{ + dir: filepath.Join(gitRoot, "skills"), + agentHostIDs: []string{agentHostPublished}, + scope: string(registry.ScopeProject), + filter: scanPublishedOnly, + }) + } return targets, nil } @@ -267,7 +290,33 @@ func appendAgentHostID(agentHostIDs []string, agentHostID string) []string { return append(agentHostIDs, agentHostID) } -func scanInstalledSkills(skillsDir string, agentHostIDs []string, scope string) ([]listedSkill, error) { +func scanFilterForAgentHost(agentHost *registry.AgentHost, scope registry.Scope) scanFilter { + if scope == registry.ScopeProject && agentHost.ProjectDir == "skills" { + return scanInstalledOnly + } + return scanAllSkills +} + +func mergeScanFilters(a, b scanFilter) scanFilter { + if a == b { + return a + } + return scanAllSkills +} + +func shouldListPublishedProjectSkills(agentID string, scopes []registry.Scope, gitRoot string) bool { + if agentID != "" || gitRoot == "" { + return false + } + for _, scope := range scopes { + if scope == registry.ScopeProject { + return true + } + } + return false +} + +func scanInstalledSkills(skillsDir string, agentHostIDs []string, scope string, filter scanFilter) ([]listedSkill, error) { entries, err := os.ReadDir(skillsDir) if os.IsNotExist(err) { return nil, nil @@ -286,7 +335,10 @@ func scanInstalledSkills(skillsDir string, agentHostIDs []string, scope string) skillDir := filepath.Join(skillsDir, e.Name()) skillFile := filepath.Join(skillDir, "SKILL.md") if data, readErr := os.ReadFile(skillFile); readErr == nil { - skills = append(skills, parseInstalledSkill(data, e.Name(), skillDir, agentHostIDs, scope)) + skill, hasInstallMetadata := parseInstalledSkill(data, e.Name(), skillDir, agentHostIDs, scope) + if shouldIncludeSkill(filter, hasInstallMetadata) { + skills = append(skills, skill) + } continue } @@ -303,7 +355,10 @@ func scanInstalledSkills(skillsDir string, agentHostIDs []string, scope string) subSkillFile := filepath.Join(subSkillDir, "SKILL.md") if data, readErr := os.ReadFile(subSkillFile); readErr == nil { installName := e.Name() + "/" + sub.Name() - skills = append(skills, parseInstalledSkill(data, installName, subSkillDir, agentHostIDs, scope)) + skill, hasInstallMetadata := parseInstalledSkill(data, installName, subSkillDir, agentHostIDs, scope) + if shouldIncludeSkill(filter, hasInstallMetadata) { + skills = append(skills, skill) + } } } } @@ -311,7 +366,18 @@ func scanInstalledSkills(skillsDir string, agentHostIDs []string, scope string) return skills, nil } -func parseInstalledSkill(data []byte, name, dir string, agentHostIDs []string, scope string) listedSkill { +func shouldIncludeSkill(filter scanFilter, hasInstallMetadata bool) bool { + switch filter { + case scanInstalledOnly: + return hasInstallMetadata + case scanPublishedOnly: + return !hasInstallMetadata + default: + return true + } +} + +func parseInstalledSkill(data []byte, name, dir string, agentHostIDs []string, scope string) (listedSkill, bool) { s := listedSkill{ skillName: name, agentHostIDs: agentHostIDs, @@ -321,13 +387,14 @@ func parseInstalledSkill(data []byte, name, dir string, agentHostIDs []string, s result, err := frontmatter.Parse(string(data)) if err != nil { - return s + return s, false } meta := result.Metadata.Meta if meta == nil { - return s + return s, false } + installMetadata := hasInstallMetadata(meta) if sourcePath, _ := meta["github-path"].(string); sourcePath != "" { if skillName := skillNameFromSourcePath(sourcePath); skillName != "" { @@ -357,7 +424,20 @@ func parseInstalledSkill(data []byte, name, dir string, agentHostIDs []string, s } } - return s + return s, installMetadata +} + +func hasInstallMetadata(meta map[string]interface{}) bool { + for _, key := range []string{"github-repo", "github-ref", "github-tree-sha", "github-path", "github-pinned", "local-path"} { + value, ok := meta[key] + if !ok { + continue + } + if str, ok := value.(string); !ok || strings.TrimSpace(str) != "" { + return true + } + } + return false } func skillNameFromSourcePath(sourcePath string) string { @@ -431,6 +511,9 @@ func formatHosts(hosts []string) string { if len(hosts) == 0 { return "-" } + if len(hosts) == 1 && hosts[0] == agentHostPublished { + return agentHostPublishedDisplay + } return strings.Join(hosts, ", ") } diff --git a/pkg/cmd/skills/list/list_test.go b/pkg/cmd/skills/list/list_test.go index 33cd65ce141..b9eef019f26 100644 --- a/pkg/cmd/skills/list/list_test.go +++ b/pkg/cmd/skills/list/list_test.go @@ -245,6 +245,50 @@ func TestListRun(t *testing.T) { }, wantErr: "could not access directory", }, + { + name: "lists source skills in bare project skills directory as published", + setup: func(t *testing.T, repoDir, homeDir string) { + writeSkill(t, repoDir, "skills/gh", heredoc.Doc(` + --- + name: gh + description: GitHub CLI patterns + --- + Body + `)) + writeSkill(t, repoDir, "skills/gh-skill", heredoc.Doc(` + --- + name: gh-skill + description: GitHub Skill patterns + --- + Body + `)) + }, + opts: func(ios *iostreams.IOStreams, repoDir, homeDir string, spy *telemetry.CommandRecorderSpy) *ListOptions { + return &ListOptions{ + IO: ios, + Telemetry: spy, + GitClient: &git.Client{RepoDir: repoDir}, + Scope: "project", + } + }, + wantStdout: "gh\tn/a (published)\tproject\t-\ngh-skill\tn/a (published)\tproject\t-\n", + }, + { + name: "lists openclaw project skill with install metadata", + setup: func(t *testing.T, repoDir, homeDir string) { + writeSkill(t, repoDir, "skills/openclaw-helper", remoteSkillFrontmatter("openclaw-helper", "skills/openclaw-helper", "refs/heads/main", "")) + }, + opts: func(ios *iostreams.IOStreams, repoDir, homeDir string, spy *telemetry.CommandRecorderSpy) *ListOptions { + return &ListOptions{ + IO: ios, + Telemetry: spy, + GitClient: &git.Client{RepoDir: repoDir}, + Agent: "openclaw", + Scope: "project", + } + }, + wantStdout: "openclaw-helper\topenclaw\tproject\tmonalisa/skills-repo\n", + }, { name: "recovers namespaced skill name from source path", setup: func(t *testing.T, repoDir, homeDir string) { From ecdbd6f9d9c8d7dababd10740355bc06700577df Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Thu, 28 May 2026 12:55:36 -0600 Subject: [PATCH 07/12] Sanitize terminal control characters in skill list output Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/skills/list/list.go | 19 +++++++++++++++++-- pkg/cmd/skills/list/list_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/skills/list/list.go b/pkg/cmd/skills/list/list.go index ca227425742..8047480a176 100644 --- a/pkg/cmd/skills/list/list.go +++ b/pkg/cmd/skills/list/list.go @@ -1,7 +1,9 @@ package list import ( + "bytes" "fmt" + "io" "os" "path/filepath" "sort" @@ -19,7 +21,9 @@ import ( "github.com/cli/cli/v2/internal/tableprinter" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" + "github.com/cli/go-gh/v2/pkg/asciisanitizer" "github.com/spf13/cobra" + "golang.org/x/text/transform" ) var skillListFields = []string{ @@ -490,16 +494,27 @@ func renderTable(io *iostreams.IOStreams, skills []listedSkill) error { table := tableprinter.New(io, tableprinter.WithHeader("Name", "Agent", "Scope", "Source")) for _, skill := range skills { - table.AddField(skill.skillName) + table.AddField(sanitizeForTerminal(skill.skillName)) table.AddField(formatHosts(skill.agentHostIDs)) table.AddField(displayOrDash(skill.scope)) - table.AddField(displayOrDash(skill.source)) + table.AddField(displayOrDash(sanitizeForTerminal(skill.source))) table.EndRow() } return table.Render() } +// sanitizeForTerminal replaces ASCII control characters in s with inert +// caret-style stand-ins so untrusted content cannot inject terminal escapes. +func sanitizeForTerminal(s string) string { + var buf bytes.Buffer + r := transform.NewReader(bytes.NewReader([]byte(s)), &asciisanitizer.Sanitizer{}) + if _, err := io.Copy(&buf, r); err != nil { + return "Unknown" + } + return buf.String() +} + func displayOrDash(value string) string { if value == "" { return "-" diff --git a/pkg/cmd/skills/list/list_test.go b/pkg/cmd/skills/list/list_test.go index b9eef019f26..958f00a0506 100644 --- a/pkg/cmd/skills/list/list_test.go +++ b/pkg/cmd/skills/list/list_test.go @@ -383,6 +383,30 @@ func TestListRun(t *testing.T) { }, wantJSON: "[]", }, + { + name: "sanitizes terminal escapes from skill frontmatter", + setup: func(t *testing.T, repoDir, homeDir string) { + customDir := filepath.Join(repoDir, "custom-skills") + writeSkill(t, customDir, "helper", heredoc.Doc(` + --- + name: helper + metadata: + local-path: "/src/\x1b[33munsanitized-src\x1b[0m" + github-path: "skills/\x1b[31munsanitized-name\x1b[0m/SKILL.md" + --- + Body + `)) + }, + opts: func(ios *iostreams.IOStreams, repoDir, homeDir string, spy *telemetry.CommandRecorderSpy) *ListOptions { + return &ListOptions{ + IO: ios, + Telemetry: spy, + GitClient: &git.Client{RepoDir: repoDir}, + Dir: filepath.Join(repoDir, "custom-skills"), + } + }, + wantStdout: "^[[31munsanitized-name^[[0m\t-\tcustom\t/src/^[[33munsanitized-src^[[0m\n", + }, } for _, tt := range tests { From 989ee0cfe3e439bd41b3d850506501a9a3b0087c Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Thu, 28 May 2026 13:04:27 -0600 Subject: [PATCH 08/12] Stat SKILL.md before reading in skill list Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/skills/list/list.go | 19 ++++++++++++--- pkg/cmd/skills/list/list_test.go | 40 ++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/skills/list/list.go b/pkg/cmd/skills/list/list.go index 8047480a176..42875b147ff 100644 --- a/pkg/cmd/skills/list/list.go +++ b/pkg/cmd/skills/list/list.go @@ -338,7 +338,8 @@ func scanInstalledSkills(skillsDir string, agentHostIDs []string, scope string, // Flat layout: {dir}/{name}/SKILL.md. skillDir := filepath.Join(skillsDir, e.Name()) skillFile := filepath.Join(skillDir, "SKILL.md") - if data, readErr := os.ReadFile(skillFile); readErr == nil { + // TODO: maybe we should surface this error instead of a silent skip + if data, readErr := readSkillFile(skillFile); readErr == nil { skill, hasInstallMetadata := parseInstalledSkill(data, e.Name(), skillDir, agentHostIDs, scope) if shouldIncludeSkill(filter, hasInstallMetadata) { skills = append(skills, skill) @@ -357,7 +358,7 @@ func scanInstalledSkills(skillsDir string, agentHostIDs []string, scope string, } subSkillDir := filepath.Join(skillDir, sub.Name()) subSkillFile := filepath.Join(subSkillDir, "SKILL.md") - if data, readErr := os.ReadFile(subSkillFile); readErr == nil { + if data, readErr := readSkillFile(subSkillFile); readErr == nil { installName := e.Name() + "/" + sub.Name() skill, hasInstallMetadata := parseInstalledSkill(data, installName, subSkillDir, agentHostIDs, scope) if shouldIncludeSkill(filter, hasInstallMetadata) { @@ -370,6 +371,18 @@ func scanInstalledSkills(skillsDir string, agentHostIDs []string, scope string, return skills, nil } +// readSkillFile reads a SKILL.md file only if it resolves to a regular file. +func readSkillFile(path string) ([]byte, error) { + info, err := os.Stat(path) + if err != nil { + return nil, err + } + if !info.Mode().IsRegular() { + return nil, fmt.Errorf("SKILL.md is not a regular file: %s", path) + } + return os.ReadFile(path) +} + func shouldIncludeSkill(filter scanFilter, hasInstallMetadata bool) bool { switch filter { case scanInstalledOnly: @@ -505,7 +518,7 @@ func renderTable(io *iostreams.IOStreams, skills []listedSkill) error { } // sanitizeForTerminal replaces ASCII control characters in s with inert -// caret-style stand-ins so untrusted content cannot inject terminal escapes. +// caret-style stand-ins so frontmatter values cannot inject terminal escapes. func sanitizeForTerminal(s string) string { var buf bytes.Buffer r := transform.NewReader(bytes.NewReader([]byte(s)), &asciisanitizer.Sanitizer{}) diff --git a/pkg/cmd/skills/list/list_test.go b/pkg/cmd/skills/list/list_test.go index 958f00a0506..ed8895aa6e9 100644 --- a/pkg/cmd/skills/list/list_test.go +++ b/pkg/cmd/skills/list/list_test.go @@ -383,6 +383,46 @@ func TestListRun(t *testing.T) { }, wantJSON: "[]", }, + { + name: "lists skill whose SKILL.md is a symlink to a regular file", + setup: func(t *testing.T, repoDir, homeDir string) { + customDir := filepath.Join(repoDir, "custom-skills") + skillDir := filepath.Join(customDir, "linked") + require.NoError(t, os.MkdirAll(skillDir, 0o755)) + target := filepath.Join(repoDir, "target.md") + require.NoError(t, os.WriteFile(target, []byte("---\nname: linked\nmetadata:\n local-path: /src/linked\n---\nBody\n"), 0o644)) + require.NoError(t, os.Symlink(target, filepath.Join(skillDir, "SKILL.md"))) + }, + opts: func(ios *iostreams.IOStreams, repoDir, homeDir string, spy *telemetry.CommandRecorderSpy) *ListOptions { + return &ListOptions{ + IO: ios, + Telemetry: spy, + GitClient: &git.Client{RepoDir: repoDir}, + Dir: filepath.Join(repoDir, "custom-skills"), + } + }, + wantStdout: "linked\t-\tcustom\t/src/linked\n", + }, + { + name: "skips skill whose SKILL.md is not a regular file", + setup: func(t *testing.T, repoDir, homeDir string) { + customDir := filepath.Join(repoDir, "custom-skills") + skillDir := filepath.Join(customDir, "bogus") + require.NoError(t, os.MkdirAll(skillDir, 0o755)) + targetDir := filepath.Join(repoDir, "target-dir") + require.NoError(t, os.MkdirAll(targetDir, 0o755)) + require.NoError(t, os.Symlink(targetDir, filepath.Join(skillDir, "SKILL.md"))) + }, + opts: func(ios *iostreams.IOStreams, repoDir, homeDir string, spy *telemetry.CommandRecorderSpy) *ListOptions { + return &ListOptions{ + IO: ios, + Telemetry: spy, + GitClient: &git.Client{RepoDir: repoDir}, + Dir: filepath.Join(repoDir, "custom-skills"), + } + }, + wantErr: "no installed skills found", + }, { name: "sanitizes terminal escapes from skill frontmatter", setup: func(t *testing.T, repoDir, homeDir string) { From ea42d46f41739af9391edb28fee3cabe782a569d Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Thu, 28 May 2026 14:04:25 -0600 Subject: [PATCH 09/12] Rename hosts field and helpers to agentHosts in skill list Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/skills/list/list.go | 28 ++++++++++++++-------------- pkg/cmd/skills/list/list_test.go | 4 ++-- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/pkg/cmd/skills/list/list.go b/pkg/cmd/skills/list/list.go index 42875b147ff..b2106e719fc 100644 --- a/pkg/cmd/skills/list/list.go +++ b/pkg/cmd/skills/list/list.go @@ -28,7 +28,7 @@ import ( var skillListFields = []string{ "skillName", - "hosts", + "agentHosts", "scope", "sourceURL", "version", @@ -87,7 +87,7 @@ func (s listedSkill) ExportData(fields []string) map[string]interface{} { switch f { case "skillName": data[f] = s.skillName - case "hosts": + case "agentHosts": data[f] = s.agentHostIDs case "scope": data[f] = s.scope @@ -220,7 +220,7 @@ func buildScanTargets(opts *ListOptions) ([]scanTarget, error) { gitRoot := installer.ResolveGitRoot(opts.GitClient) homeDir := installer.ResolveHomeDir() - agentHosts, err := selectedHosts(opts.Agent) + agentHosts, err := selectedAgentHosts(opts.Agent) if err != nil { return nil, err } @@ -262,7 +262,7 @@ func buildScanTargets(opts *ListOptions) ([]scanTarget, error) { return targets, nil } -func selectedHosts(agentID string) ([]*registry.AgentHost, error) { +func selectedAgentHosts(agentID string) ([]*registry.AgentHost, error) { if agentID != "" { host, err := registry.FindByID(agentID) if err != nil { @@ -271,11 +271,11 @@ func selectedHosts(agentID string) ([]*registry.AgentHost, error) { return []*registry.AgentHost{host}, nil } - hosts := make([]*registry.AgentHost, len(registry.Agents)) + agentHosts := make([]*registry.AgentHost, len(registry.Agents)) for i := range registry.Agents { - hosts[i] = ®istry.Agents[i] + agentHosts[i] = ®istry.Agents[i] } - return hosts, nil + return agentHosts, nil } func selectedScopes(scope string) []registry.Scope { @@ -496,8 +496,8 @@ func sortListedSkills(skills []listedSkill) { if skills[i].scope != skills[j].scope { return skills[i].scope < skills[j].scope } - if formatHosts(skills[i].agentHostIDs) != formatHosts(skills[j].agentHostIDs) { - return formatHosts(skills[i].agentHostIDs) < formatHosts(skills[j].agentHostIDs) + if formatAgentHosts(skills[i].agentHostIDs) != formatAgentHosts(skills[j].agentHostIDs) { + return formatAgentHosts(skills[i].agentHostIDs) < formatAgentHosts(skills[j].agentHostIDs) } return skills[i].path < skills[j].path }) @@ -508,7 +508,7 @@ func renderTable(io *iostreams.IOStreams, skills []listedSkill) error { for _, skill := range skills { table.AddField(sanitizeForTerminal(skill.skillName)) - table.AddField(formatHosts(skill.agentHostIDs)) + table.AddField(formatAgentHosts(skill.agentHostIDs)) table.AddField(displayOrDash(skill.scope)) table.AddField(displayOrDash(sanitizeForTerminal(skill.source))) table.EndRow() @@ -535,14 +535,14 @@ func displayOrDash(value string) string { return value } -func formatHosts(hosts []string) string { - if len(hosts) == 0 { +func formatAgentHosts(agentHostIDs []string) string { + if len(agentHostIDs) == 0 { return "-" } - if len(hosts) == 1 && hosts[0] == agentHostPublished { + if len(agentHostIDs) == 1 && agentHostIDs[0] == agentHostPublished { return agentHostPublishedDisplay } - return strings.Join(hosts, ", ") + return strings.Join(agentHostIDs, ", ") } func recordListTelemetry(opts *ListOptions, skillCount int) { diff --git a/pkg/cmd/skills/list/list_test.go b/pkg/cmd/skills/list/list_test.go index ed8895aa6e9..b2b3a79a0e2 100644 --- a/pkg/cmd/skills/list/list_test.go +++ b/pkg/cmd/skills/list/list_test.go @@ -160,7 +160,7 @@ func TestListRun(t *testing.T) { }, opts: func(ios *iostreams.IOStreams, repoDir, homeDir string, spy *telemetry.CommandRecorderSpy) *ListOptions { exporter := cmdutil.NewJSONExporter() - exporter.SetFields([]string{"skillName", "hosts", "scope", "sourceURL", "version", "pinned", "path"}) + exporter.SetFields([]string{"skillName", "agentHosts", "scope", "sourceURL", "version", "pinned", "path"}) return &ListOptions{ IO: ios, Telemetry: spy, @@ -173,7 +173,7 @@ func TestListRun(t *testing.T) { wantJSON: fmt.Sprintf(`[ { "skillName": "code-review", - "hosts": ["claude-code"], + "agentHosts": ["claude-code"], "scope": "user", "sourceURL": "https://github.com/monalisa/skills-repo", "version": "v2.0.0", From 69e6ecc5709f4d4d3a69ca8de9bd920c67a4100c Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Thu, 28 May 2026 14:11:15 -0600 Subject: [PATCH 10/12] Use github-copilot agent ID in skill list tests and help text Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/skills/list/list.go | 4 ++-- pkg/cmd/skills/list/list_test.go | 14 +++++++------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/pkg/cmd/skills/list/list.go b/pkg/cmd/skills/list/list.go index b2106e719fc..c87f9829484 100644 --- a/pkg/cmd/skills/list/list.go +++ b/pkg/cmd/skills/list/list.go @@ -130,8 +130,8 @@ func NewCmdList(f *cmdutil.Factory, telemetry ghtelemetry.CommandRecorder, runF # List all installed skills $ gh skill list - # List skills installed for Claude Code - $ gh skill list --agent claude-code + # List skills installed for GitHub Copilot + $ gh skill list --agent github-copilot # List user-scope skills $ gh skill list --scope user diff --git a/pkg/cmd/skills/list/list_test.go b/pkg/cmd/skills/list/list_test.go index b2b3a79a0e2..94295c7ba6a 100644 --- a/pkg/cmd/skills/list/list_test.go +++ b/pkg/cmd/skills/list/list_test.go @@ -33,9 +33,9 @@ func TestNewCmdList(t *testing.T) { }, { name: "agent and scope filters", - cli: "--agent claude-code --scope user", + cli: "--agent github-copilot --scope user", wantOpts: ListOptions{ - Agent: "claude-code", + Agent: "github-copilot", Scope: "user", ScopeChanged: true, }, @@ -69,7 +69,7 @@ func TestNewCmdList(t *testing.T) { }, { name: "dir and agent are mutually exclusive", - cli: "--dir ./skills --agent claude-code", + cli: "--dir ./skills --agent github-copilot", wantErr: "--dir and --agent cannot be used together", }, { @@ -156,7 +156,7 @@ func TestListRun(t *testing.T) { { name: "lists user skill as json", setup: func(t *testing.T, repoDir, homeDir string) { - writeSkill(t, homeDir, ".claude/skills/code-review", remoteSkillFrontmatter("code-review", "skills/code-review", "refs/tags/v2.0.0", "v2.0.0")) + writeSkill(t, homeDir, ".copilot/skills/code-review", remoteSkillFrontmatter("code-review", "skills/code-review", "refs/tags/v2.0.0", "v2.0.0")) }, opts: func(ios *iostreams.IOStreams, repoDir, homeDir string, spy *telemetry.CommandRecorderSpy) *ListOptions { exporter := cmdutil.NewJSONExporter() @@ -166,21 +166,21 @@ func TestListRun(t *testing.T) { Telemetry: spy, GitClient: &git.Client{RepoDir: repoDir}, Exporter: exporter, - Agent: "claude-code", + Agent: "github-copilot", Scope: "user", } }, wantJSON: fmt.Sprintf(`[ { "skillName": "code-review", - "agentHosts": ["claude-code"], + "agentHosts": ["github-copilot"], "scope": "user", "sourceURL": "https://github.com/monalisa/skills-repo", "version": "v2.0.0", "pinned": true, "path": %q } - ]`, filepath.Join("HOME", ".claude", "skills", "code-review")), + ]`, filepath.Join("HOME", ".copilot", "skills", "code-review")), verify: func(t *testing.T, stdout string, spy *telemetry.CommandRecorderSpy) { assert.Equal(t, "json", spy.Events[0].Dimensions["format"]) }, From 7ef9c54bf87fb17516e93a7ddb932efb7c660a35 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Thu, 4 Jun 2026 10:15:27 -0600 Subject: [PATCH 11/12] Auto-install official extension stubs in CI (#13581) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- ...elemetry-for-official-extension-stub.txtar | 12 ++++-- pkg/cmd/root/official_extension_stub.go | 42 ++++++++++--------- pkg/cmd/root/official_extension_stub_test.go | 27 +++++++++++- 3 files changed, 57 insertions(+), 24 deletions(-) diff --git a/acceptance/testdata/telemetry/telemetry-for-official-extension-stub.txtar b/acceptance/testdata/telemetry/telemetry-for-official-extension-stub.txtar index b200590bf27..603dd2ae183 100644 --- a/acceptance/testdata/telemetry/telemetry-for-official-extension-stub.txtar +++ b/acceptance/testdata/telemetry/telemetry-for-official-extension-stub.txtar @@ -6,10 +6,16 @@ env GH_TELEMETRY=log env GH_TELEMETRY_SAMPLE_RATE=100 +# Ensure CI auto-install behavior does not kick in for this test; +# we want the non-TTY "print install instructions and exit non-zero" path. +env CI='' +env BUILD_NUMBER='' +env RUN_ID='' + # `stack` is registered in extensions.OfficialExtensions. Since no real -# extension is installed, the hidden stub runs and, in a non-TTY session, -# prints install instructions without prompting. -exec gh stack +# extension is installed, the hidden stub runs and, in a non-TTY session +# outside CI, prints install instructions and exits non-zero. +! exec gh stack stderr 'gh extension install github/gh-stack' # The stub invocation records a command_invocation event for the stub's diff --git a/pkg/cmd/root/official_extension_stub.go b/pkg/cmd/root/official_extension_stub.go index af52e43663e..6aa08af936b 100644 --- a/pkg/cmd/root/official_extension_stub.go +++ b/pkg/cmd/root/official_extension_stub.go @@ -4,6 +4,7 @@ import ( "fmt" "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/v2/internal/ci" "github.com/cli/cli/v2/internal/prompter" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/extensions" @@ -38,25 +39,28 @@ func NewCmdOfficialExtensionStub(io *iostreams.IOStreams, p prompter.Prompter, e func officialExtensionStubRun(io *iostreams.IOStreams, p prompter.Prompter, em extensions.ExtensionManager, ext *extensions.OfficialExtension) error { stderr := io.ErrOut - if !io.CanPrompt() { - fmt.Fprint(stderr, heredoc.Docf(` - %[1]s is available as an official extension. - To install it, run: - gh extension install %[2]s/%[3]s - `, fmt.Sprintf("gh %s", ext.Name), ext.Owner, ext.Repo)) - return nil - } - - prompt := heredoc.Docf(` - %[1]s is available as an official extension. - Would you like to install it now? - `, fmt.Sprintf("gh %s", ext.Name)) - confirmed, err := p.Confirm(prompt, true) - if err != nil { - return err - } - if !confirmed { - return nil + // In CI, skip the prompt so agents and CI runners don't block on Y/n. + if !ci.IsCI() { + if io.CanPrompt() { + prompt := heredoc.Docf(` + %[1]s is available as an official extension. + Would you like to install it now? + `, fmt.Sprintf("gh %s", ext.Name)) + confirmed, err := p.Confirm(prompt, true) + if err != nil { + return err + } + if !confirmed { + return nil + } + } else { + fmt.Fprint(stderr, heredoc.Docf(` + %[1]s is available as an official extension. + To install it, run: + gh extension install %[2]s/%[3]s + `, fmt.Sprintf("gh %s", ext.Name), ext.Owner, ext.Repo)) + return cmdutil.SilentError + } } repo := ext.Repository() diff --git a/pkg/cmd/root/official_extension_stub_test.go b/pkg/cmd/root/official_extension_stub_test.go index d2fd4624204..aac50a5c624 100644 --- a/pkg/cmd/root/official_extension_stub_test.go +++ b/pkg/cmd/root/official_extension_stub_test.go @@ -18,6 +18,7 @@ func TestOfficialExtensionStubRun(t *testing.T) { tests := []struct { name string isTTY bool + ciEnv string confirmResult bool confirmErr error installErr error @@ -26,9 +27,17 @@ func TestOfficialExtensionStubRun(t *testing.T) { wantInstalled bool }{ { - name: "non-TTY prints install instructions", + name: "non-TTY in CI auto-installs without prompting", + isTTY: false, + ciEnv: "1", + wantStderr: "Successfully installed github/gh-cool", + wantInstalled: true, + }, + { + name: "non-TTY outside CI prints install instructions and returns silent error", isTTY: false, wantStderr: "gh extension install github/gh-cool", + wantErr: "SilentError", }, { name: "TTY confirmed installs", @@ -37,6 +46,13 @@ func TestOfficialExtensionStubRun(t *testing.T) { wantStderr: "Successfully installed github/gh-cool", wantInstalled: true, }, + { + name: "TTY in CI auto-installs without prompting", + isTTY: true, + ciEnv: "1", + wantStderr: "Successfully installed github/gh-cool", + wantInstalled: true, + }, { name: "TTY declined does not install", isTTY: true, @@ -60,6 +76,13 @@ func TestOfficialExtensionStubRun(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + t.Setenv("CI", "") + t.Setenv("BUILD_NUMBER", "") + t.Setenv("RUN_ID", "") + if tt.ciEnv != "" { + t.Setenv("CI", tt.ciEnv) + } + ios, _, _, stderr := iostreams.Test() if tt.isTTY { ios.SetStdinTTY(true) @@ -97,7 +120,7 @@ func TestOfficialExtensionStubRun(t *testing.T) { assert.Equal(t, "github", repo.RepoOwner()) assert.Equal(t, "gh-cool", repo.RepoName()) assert.Equal(t, "github.com", repo.RepoHost()) - } else if tt.isTTY && !tt.confirmResult && tt.confirmErr == nil { + } else { assert.Empty(t, em.InstallCalls()) } }) From 0faf4b09c16dae7a59ce0794dd98308bba6b01c9 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Thu, 4 Jun 2026 10:54:16 -0600 Subject: [PATCH 12/12] Clean up deferred issue update helper (#13584) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/issue/create/create.go | 8 ++++---- pkg/cmd/issue/edit/edit.go | 20 +++++++++++++------- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/pkg/cmd/issue/create/create.go b/pkg/cmd/issue/create/create.go index fb507cd5c57..23f332d7048 100644 --- a/pkg/cmd/issue/create/create.go +++ b/pkg/cmd/issue/create/create.go @@ -449,7 +449,7 @@ func deferredUpdateIssueOptions(client *api.Client, baseRepo ghrepo.Interface, i var err error typeID, err = issueShared.ResolveIssueTypeName(client, baseRepo, opts.IssueType) if err != nil { - return updateOpts, err + return api.DeferredUpdateIssueOptions{}, err } } updateOpts.IssueTypeID = typeID @@ -458,7 +458,7 @@ func deferredUpdateIssueOptions(client *api.Client, baseRepo ghrepo.Interface, i if opts.Parent != "" { parentID, err := issueShared.ResolveIssueRef(client, baseRepo, opts.Parent) if err != nil { - return updateOpts, fmt.Errorf("resolving --parent reference %q: %w", opts.Parent, err) + return api.DeferredUpdateIssueOptions{}, fmt.Errorf("resolving --parent reference %q: %w", opts.Parent, err) } updateOpts.ParentID = parentID } @@ -466,7 +466,7 @@ func deferredUpdateIssueOptions(client *api.Client, baseRepo ghrepo.Interface, i for _, ref := range opts.BlockedBy { id, err := issueShared.ResolveIssueRef(client, baseRepo, ref) if err != nil { - return updateOpts, fmt.Errorf("resolving --blocked-by reference %q: %w", ref, err) + return api.DeferredUpdateIssueOptions{}, fmt.Errorf("resolving --blocked-by reference %q: %w", ref, err) } updateOpts.AddBlockedByIDs = append(updateOpts.AddBlockedByIDs, id) } @@ -474,7 +474,7 @@ func deferredUpdateIssueOptions(client *api.Client, baseRepo ghrepo.Interface, i for _, ref := range opts.Blocking { id, err := issueShared.ResolveIssueRef(client, baseRepo, ref) if err != nil { - return updateOpts, fmt.Errorf("resolving --blocking reference %q: %w", ref, err) + return api.DeferredUpdateIssueOptions{}, fmt.Errorf("resolving --blocking reference %q: %w", ref, err) } updateOpts.AddBlockingIDs = append(updateOpts.AddBlockingIDs, id) } diff --git a/pkg/cmd/issue/edit/edit.go b/pkg/cmd/issue/edit/edit.go index 668ece3a5ad..5cf52d92e60 100644 --- a/pkg/cmd/issue/edit/edit.go +++ b/pkg/cmd/issue/edit/edit.go @@ -185,6 +185,12 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman opts.Editable.IssueType.Edited = true } + // hasDeferredFlags covers edit flags that flow through the + // deferred update path rather than the prShared.Editable struct, + // so they would otherwise be invisible to Editable.Dirty() below. + // Note that --type (set) is intentionally absent: it lights up + // opts.Editable.IssueType.Edited above, which Editable.Dirty() + // already picks up. Only --remove-type needs to be listed here. hasDeferredFlags := opts.RemoveIssueType || flags.Changed("parent") || opts.RemoveParent || len(opts.AddSubIssues) > 0 || len(opts.RemoveSubIssues) > 0 || @@ -482,7 +488,7 @@ func deferredUpdateIssueOptions(client *api.Client, baseRepo ghrepo.Interface, i } else if editOpts.Parent != "" { parentID, err := issueShared.ResolveIssueRef(client, baseRepo, editOpts.Parent) if err != nil { - return updateOpts, fmt.Errorf("resolving --parent reference %q: %w", editOpts.Parent, err) + return api.DeferredUpdateIssueOptions{}, fmt.Errorf("resolving --parent reference %q: %w", editOpts.Parent, err) } updateOpts.ParentID = parentID } @@ -490,14 +496,14 @@ func deferredUpdateIssueOptions(client *api.Client, baseRepo ghrepo.Interface, i for _, ref := range editOpts.AddSubIssues { id, err := issueShared.ResolveIssueRef(client, baseRepo, ref) if err != nil { - return updateOpts, fmt.Errorf("resolving --add-sub-issue reference %q: %w", ref, err) + return api.DeferredUpdateIssueOptions{}, fmt.Errorf("resolving --add-sub-issue reference %q: %w", ref, err) } updateOpts.AddSubIssueIDs = append(updateOpts.AddSubIssueIDs, id) } for _, ref := range editOpts.RemoveSubIssues { id, err := issueShared.ResolveIssueRef(client, baseRepo, ref) if err != nil { - return updateOpts, fmt.Errorf("resolving --remove-sub-issue reference %q: %w", ref, err) + return api.DeferredUpdateIssueOptions{}, fmt.Errorf("resolving --remove-sub-issue reference %q: %w", ref, err) } updateOpts.RemoveSubIssueIDs = append(updateOpts.RemoveSubIssueIDs, id) } @@ -505,14 +511,14 @@ func deferredUpdateIssueOptions(client *api.Client, baseRepo ghrepo.Interface, i for _, ref := range editOpts.AddBlockedBy { id, err := issueShared.ResolveIssueRef(client, baseRepo, ref) if err != nil { - return updateOpts, fmt.Errorf("resolving --add-blocked-by reference %q: %w", ref, err) + return api.DeferredUpdateIssueOptions{}, fmt.Errorf("resolving --add-blocked-by reference %q: %w", ref, err) } updateOpts.AddBlockedByIDs = append(updateOpts.AddBlockedByIDs, id) } for _, ref := range editOpts.RemoveBlockedBy { id, err := issueShared.ResolveIssueRef(client, baseRepo, ref) if err != nil { - return updateOpts, fmt.Errorf("resolving --remove-blocked-by reference %q: %w", ref, err) + return api.DeferredUpdateIssueOptions{}, fmt.Errorf("resolving --remove-blocked-by reference %q: %w", ref, err) } updateOpts.RemoveBlockedByIDs = append(updateOpts.RemoveBlockedByIDs, id) } @@ -520,14 +526,14 @@ func deferredUpdateIssueOptions(client *api.Client, baseRepo ghrepo.Interface, i for _, ref := range editOpts.AddBlocking { id, err := issueShared.ResolveIssueRef(client, baseRepo, ref) if err != nil { - return updateOpts, fmt.Errorf("resolving --add-blocking reference %q: %w", ref, err) + return api.DeferredUpdateIssueOptions{}, fmt.Errorf("resolving --add-blocking reference %q: %w", ref, err) } updateOpts.AddBlockingIDs = append(updateOpts.AddBlockingIDs, id) } for _, ref := range editOpts.RemoveBlocking { id, err := issueShared.ResolveIssueRef(client, baseRepo, ref) if err != nil { - return updateOpts, fmt.Errorf("resolving --remove-blocking reference %q: %w", ref, err) + return api.DeferredUpdateIssueOptions{}, fmt.Errorf("resolving --remove-blocking reference %q: %w", ref, err) } updateOpts.RemoveBlockingIDs = append(updateOpts.RemoveBlockingIDs, id) }