diff --git a/internal/skills/registry/registry.go b/internal/skills/registry/registry.go index ecaaaa48d..a8fdc5993 100644 --- a/internal/skills/registry/registry.go +++ b/internal/skills/registry/registry.go @@ -27,6 +27,8 @@ type Scope string const ( ScopeProject Scope = "project" ScopeUser Scope = "user" + + sharedProjectSkillsDir = ".agents/skills" ) // Agents contains all known agent hosts. @@ -34,7 +36,7 @@ var Agents = []AgentHost{ { ID: "github-copilot", Name: "GitHub Copilot", - ProjectDir: ".github/skills", + ProjectDir: sharedProjectSkillsDir, UserDir: ".copilot/skills", }, { @@ -46,25 +48,25 @@ var Agents = []AgentHost{ { ID: "cursor", Name: "Cursor", - ProjectDir: ".cursor/skills", + ProjectDir: sharedProjectSkillsDir, UserDir: ".cursor/skills", }, { ID: "codex", Name: "Codex", - ProjectDir: ".agents/skills", + ProjectDir: sharedProjectSkillsDir, UserDir: ".codex/skills", }, { ID: "gemini", Name: "Gemini CLI", - ProjectDir: ".agent/skills", + ProjectDir: sharedProjectSkillsDir, UserDir: ".gemini/skills", }, { ID: "antigravity", Name: "Antigravity", - ProjectDir: ".agent/skills", + ProjectDir: sharedProjectSkillsDir, UserDir: ".gemini/antigravity/skills", }, } diff --git a/internal/skills/registry/registry_test.go b/internal/skills/registry/registry_test.go index e17668b87..003a28afa 100644 --- a/internal/skills/registry/registry_test.go +++ b/internal/skills/registry/registry_test.go @@ -38,11 +38,9 @@ func TestFindByID(t *testing.T) { } func TestInstallDir(t *testing.T) { - host, err := FindByID("github-copilot") - require.NoError(t, err) - tests := []struct { name string + hostID string scope Scope gitRoot string homeDir string @@ -50,21 +48,64 @@ func TestInstallDir(t *testing.T) { wantErr bool }{ { - name: "project scope", + name: "github copilot project scope", + hostID: "github-copilot", scope: ScopeProject, gitRoot: "/tmp/monalisa-repo", homeDir: "/home/monalisa", - wantDir: filepath.Join("/tmp/monalisa-repo", ".github", "skills"), + wantDir: filepath.Join("/tmp/monalisa-repo", ".agents", "skills"), }, { - name: "user scope", + name: "github copilot user scope", + hostID: "github-copilot", scope: ScopeUser, gitRoot: "/tmp/monalisa-repo", homeDir: "/home/monalisa", wantDir: filepath.Join("/home/monalisa", ".copilot", "skills"), }, + { + name: "claude code project scope", + hostID: "claude-code", + scope: ScopeProject, + gitRoot: "/tmp/monalisa-repo", + homeDir: "/home/monalisa", + wantDir: filepath.Join("/tmp/monalisa-repo", ".claude", "skills"), + }, + { + name: "cursor project scope", + hostID: "cursor", + scope: ScopeProject, + gitRoot: "/tmp/monalisa-repo", + homeDir: "/home/monalisa", + wantDir: filepath.Join("/tmp/monalisa-repo", ".agents", "skills"), + }, + { + name: "codex project scope", + hostID: "codex", + scope: ScopeProject, + gitRoot: "/tmp/monalisa-repo", + homeDir: "/home/monalisa", + wantDir: filepath.Join("/tmp/monalisa-repo", ".agents", "skills"), + }, + { + name: "gemini project scope", + hostID: "gemini", + scope: ScopeProject, + gitRoot: "/tmp/monalisa-repo", + homeDir: "/home/monalisa", + wantDir: filepath.Join("/tmp/monalisa-repo", ".agents", "skills"), + }, + { + name: "antigravity project scope", + hostID: "antigravity", + scope: ScopeProject, + gitRoot: "/tmp/monalisa-repo", + homeDir: "/home/monalisa", + wantDir: filepath.Join("/tmp/monalisa-repo", ".agents", "skills"), + }, { name: "project scope without git root", + hostID: "github-copilot", scope: ScopeProject, gitRoot: "", homeDir: "/home/monalisa", @@ -72,6 +113,7 @@ func TestInstallDir(t *testing.T) { }, { name: "user scope without home dir", + hostID: "github-copilot", scope: ScopeUser, gitRoot: "/tmp/monalisa-repo", homeDir: "", @@ -79,6 +121,7 @@ func TestInstallDir(t *testing.T) { }, { name: "invalid scope", + hostID: "github-copilot", scope: "bogus", gitRoot: "/tmp/monalisa-repo", homeDir: "/home/monalisa", @@ -87,6 +130,9 @@ func TestInstallDir(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + host, err := FindByID(tt.hostID) + require.NoError(t, err) + dir, err := host.InstallDir(tt.scope, tt.gitRoot, tt.homeDir) if tt.wantErr { assert.Error(t, err) @@ -121,16 +167,7 @@ func TestRepoNameFromRemote(t *testing.T) { func TestUniqueProjectDirs(t *testing.T) { dirs := UniqueProjectDirs() - require.NotEmpty(t, dirs) - - // Should deduplicate — e.g. gemini and antigravity share .agent/skills - seen := map[string]int{} - for _, d := range dirs { - seen[d]++ - } - for dir, count := range seen { - assert.Equalf(t, 1, count, "directory %q appears %d times, expected 1", dir, count) - } + assert.Equal(t, []string{".agents/skills", ".claude/skills"}, dirs) } func TestScopeLabels(t *testing.T) { diff --git a/pkg/cmd/skills/install/install.go b/pkg/cmd/skills/install/install.go index 149d682eb..b0ca4bf4a 100644 --- a/pkg/cmd/skills/install/install.go +++ b/pkg/cmd/skills/install/install.go @@ -82,17 +82,22 @@ func NewCmdInstall(f *cmdutil.Factory, runF func(*installOptions) error) *cobra. scope (in your home directory, available everywhere): Host Project User - GitHub Copilot .github/skills ~/.copilot/skills + GitHub Copilot .agents/skills ~/.copilot/skills Claude Code .claude/skills ~/.claude/skills - Cursor .cursor/skills ~/.cursor/skills + Cursor .agents/skills ~/.cursor/skills Codex .agents/skills ~/.codex/skills - Gemini CLI .agent/skills ~/.gemini/skills - Antigravity .agent/skills ~/.gemini/antigravity/skills + Gemini CLI .agents/skills ~/.gemini/skills + Antigravity .agents/skills ~/.gemini/antigravity/skills Use %[1]s--agent%[1]s and %[1]s--scope%[1]s to control placement, or %[1]s--dir%[1]s for a custom directory. The default scope is %[1]sproject%[1]s, and the default agent is %[1]sgithub-copilot%[1]s (when running non-interactively). + At project scope, GitHub Copilot, Cursor, Codex, Gemini CLI, and + Antigravity all use the shared %[1]s.agents/skills%[1]s directory. If you + select multiple hosts that resolve to the same destination, each skill is + installed there only once. + The first argument can be a GitHub repository in %[1]sOWNER/REPO%[1]s format or a local directory path (e.g. %[1]s.%[1]s, %[1]s./my-skills%[1]s, %[1]s~/skills%[1]s). For local directories, skills are auto-discovered using the same @@ -287,26 +292,14 @@ func installRun(opts *installOptions) error { homeDir := installer.ResolveHomeDir() source = ghrepo.FullName(opts.repo) - type hostPlan struct { - host *registry.AgentHost - skills []discovery.Skill - } - var plans []hostPlan - for _, host := range selectedHosts { - installSkills, err := checkOverwrite(opts, selectedSkills, host, scope, gitRoot, homeDir, canPrompt) - if err != nil { - return err - } - if len(installSkills) == 0 { - fmt.Fprintf(opts.IO.ErrOut, "No skills to install for %s.\n", host.Name) - continue - } - plans = append(plans, hostPlan{host: host, skills: installSkills}) + plans, err := buildInstallPlans(opts, selectedSkills, selectedHosts, scope, gitRoot, homeDir, canPrompt) + if err != nil { + return err } for _, plan := range plans { if len(plans) > 1 { - fmt.Fprintf(opts.IO.ErrOut, "\nInstalling to %s...\n", plan.host.Name) + fmt.Fprintf(opts.IO.ErrOut, "\nInstalling to %s for %s...\n", friendlyDir(plan.dir), formatPlanHosts(plan.hosts)) } result, err := installer.Install(&installer.Options{ @@ -317,11 +310,7 @@ func installRun(opts *installOptions) error { SHA: resolved.SHA, PinnedRef: opts.Pin, Skills: plan.skills, - AgentHost: plan.host, - Scope: scope, - Dir: opts.Dir, - GitRoot: gitRoot, - HomeDir: homeDir, + Dir: plan.dir, Client: apiClient, OnProgress: installProgress(opts.IO, len(plan.skills)), }) @@ -425,36 +414,20 @@ func runLocalInstall(opts *installOptions) error { gitRoot := installer.ResolveGitRoot(opts.GitClient) homeDir := installer.ResolveHomeDir() - type hostPlan struct { - host *registry.AgentHost - skills []discovery.Skill - } - var plans []hostPlan - for _, host := range selectedHosts { - installSkills, err := checkOverwrite(opts, selectedSkills, host, scope, gitRoot, homeDir, canPrompt) - if err != nil { - return err - } - if len(installSkills) == 0 { - fmt.Fprintf(opts.IO.ErrOut, "No skills to install for %s.\n", host.Name) - continue - } - plans = append(plans, hostPlan{host: host, skills: installSkills}) + plans, err := buildInstallPlans(opts, selectedSkills, selectedHosts, scope, gitRoot, homeDir, canPrompt) + if err != nil { + return err } for _, plan := range plans { if len(plans) > 1 { - fmt.Fprintf(opts.IO.ErrOut, "\nInstalling to %s...\n", plan.host.Name) + fmt.Fprintf(opts.IO.ErrOut, "\nInstalling to %s for %s...\n", friendlyDir(plan.dir), formatPlanHosts(plan.hosts)) } result, err := installer.InstallLocal(&installer.LocalOptions{ SourceDir: absSource, Skills: plan.skills, - AgentHost: plan.host, - Scope: scope, - Dir: opts.Dir, - GitRoot: gitRoot, - HomeDir: homeDir, + Dir: plan.dir, }) if err != nil { return err @@ -586,6 +559,12 @@ type skillSelector struct { fetchDescriptions func() } +type installPlan struct { + dir string + hosts []*registry.AgentHost + skills []discovery.Skill +} + func selectSkillsWithSelector(opts *installOptions, skills []discovery.Skill, canPrompt bool, sel skillSelector) ([]discovery.Skill, error) { checkCollisions := func(ss []discovery.Skill) error { return collisionError(ss, sel.sourceHint) @@ -823,20 +802,63 @@ func resolveScope(opts *installOptions, canPrompt bool) (registry.Scope, error) return registry.ScopeUser, nil } +func buildInstallPlans(opts *installOptions, selectedSkills []discovery.Skill, selectedHosts []*registry.AgentHost, scope registry.Scope, gitRoot, homeDir string, canPrompt bool) ([]installPlan, error) { + byDir := make(map[string]*installPlan) + orderedDirs := make([]string, 0, len(selectedHosts)) + + for _, host := range selectedHosts { + targetDir, err := resolveInstallDir(opts, host, scope, gitRoot, homeDir) + if err != nil { + return nil, err + } + + plan, ok := byDir[targetDir] + if !ok { + plan = &installPlan{dir: targetDir} + byDir[targetDir] = plan + orderedDirs = append(orderedDirs, targetDir) + } + plan.hosts = append(plan.hosts, host) + } + + plans := make([]installPlan, 0, len(orderedDirs)) + for _, dir := range orderedDirs { + plan := byDir[dir] + installSkills, err := checkOverwrite(opts, selectedSkills, plan.dir, canPrompt) + if err != nil { + return nil, err + } + if len(installSkills) == 0 { + fmt.Fprintf(opts.IO.ErrOut, "No skills to install in %s for %s.\n", friendlyDir(plan.dir), formatPlanHosts(plan.hosts)) + continue + } + plan.skills = installSkills + plans = append(plans, *plan) + } + + return plans, nil +} + +func resolveInstallDir(opts *installOptions, host *registry.AgentHost, scope registry.Scope, gitRoot, homeDir string) (string, error) { + if opts.Dir != "" { + return opts.Dir, nil + } + return host.InstallDir(scope, gitRoot, homeDir) +} + +func formatPlanHosts(hosts []*registry.AgentHost) string { + names := make([]string, len(hosts)) + for i, host := range hosts { + names[i] = host.Name + } + return strings.Join(names, ", ") +} + func truncateDescription(s string, maxWidth int) string { return text.Truncate(maxWidth, text.RemoveExcessiveWhitespace(s)) } -func checkOverwrite(opts *installOptions, skills []discovery.Skill, host *registry.AgentHost, scope registry.Scope, gitRoot, homeDir string, canPrompt bool) ([]discovery.Skill, error) { - targetDir := opts.Dir - if targetDir == "" { - var err error - targetDir, err = host.InstallDir(scope, gitRoot, homeDir) - if err != nil { - return nil, err - } - } - +func checkOverwrite(opts *installOptions, skills []discovery.Skill, targetDir string, canPrompt bool) ([]discovery.Skill, error) { var existing, fresh []discovery.Skill for _, s := range skills { dir := filepath.Join(targetDir, filepath.FromSlash(s.InstallName())) diff --git a/pkg/cmd/skills/install/install_test.go b/pkg/cmd/skills/install/install_test.go index 84fb24b7a..c753ae51c 100644 --- a/pkg/cmd/skills/install/install_test.go +++ b/pkg/cmd/skills/install/install_test.go @@ -1332,6 +1332,47 @@ func TestInstallProgress(t *testing.T) { assert.NotNil(t, installProgress(ios, 2)) } +func TestInstallRun_DeduplicatesSharedProjectDirAcrossHosts(t *testing.T) { + homeDir := t.TempDir() + t.Setenv("HOME", homeDir) + t.Setenv("USERPROFILE", homeDir) + + reg := &httpmock.Registry{} + defer reg.Verify(t) + + stubResolveVersion(reg, "monalisa", "octocat-skills", "v1.0.0", "abc123") + stubDiscoverTree(reg, "monalisa", "octocat-skills", "abc123", + singleSkillTreeJSON("git-commit", "tree-gc", "blob-gc")) + stubInstallFiles(reg, "monalisa", "octocat-skills", "tree-gc", "blob-gc", gitCommitContent) + + ios, _, stdout, stderr := iostreams.Test() + ios.SetStdoutTTY(true) + ios.SetStdinTTY(true) + ios.SetStderrTTY(true) + + pm := &prompter.PrompterMock{ + MultiSelectFunc: func(prompt string, defaults []string, options []string) ([]int, error) { + return []int{0, 2}, nil // GitHub Copilot + Cursor share .agents/skills + }, + SelectFunc: func(prompt, defaultValue string, options []string) (int, error) { + return 0, nil // project scope + }, + } + + err := installRun(&installOptions{ + IO: ios, + HttpClient: func() (*http.Client, error) { return &http.Client{Transport: reg}, nil }, + Prompter: pm, + GitClient: &git.Client{RepoDir: t.TempDir()}, + SkillSource: "monalisa/octocat-skills", + SkillName: "git-commit", + Force: true, + }) + require.NoError(t, err) + assert.Equal(t, 1, strings.Count(stdout.String(), "Installed git-commit")) + assert.NotContains(t, stderr.String(), "Installing to") +} + func TestRunLocalInstall(t *testing.T) { tests := []struct { name string diff --git a/pkg/cmd/skills/publish/publish_test.go b/pkg/cmd/skills/publish/publish_test.go index e4c368b70..862dbc9f7 100644 --- a/pkg/cmd/skills/publish/publish_test.go +++ b/pkg/cmd/skills/publish/publish_test.go @@ -661,7 +661,7 @@ func TestPublishRun(t *testing.T) { }, opts: func(ios *iostreams.IOStreams, dir string, _ *httpmock.Registry) *publishOptions { t.Helper() - require.NoError(t, os.MkdirAll(filepath.Join(dir, ".github", "skills", "installed"), 0o755)) + require.NoError(t, os.MkdirAll(filepath.Join(dir, ".agents", "skills", "installed"), 0o755)) runGitInDir(t, dir, "init", "--initial-branch=main") runGitInDir(t, dir, "config", "user.email", "monalisa@github.com") runGitInDir(t, dir, "config", "user.name", "Monalisa Octocat") @@ -686,12 +686,12 @@ func TestPublishRun(t *testing.T) { --- Body. `)) - require.NoError(t, os.MkdirAll(filepath.Join(dir, ".github", "skills", "installed"), 0o755)) + require.NoError(t, os.MkdirAll(filepath.Join(dir, ".agents", "skills", "installed"), 0o755)) runGitInDir(t, dir, "init", "--initial-branch=main") runGitInDir(t, dir, "config", "user.email", "monalisa@github.com") runGitInDir(t, dir, "config", "user.name", "Monalisa Octocat") - require.NoError(t, os.WriteFile(filepath.Join(dir, ".gitignore"), []byte(".github/skills\n"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(dir, ".gitignore"), []byte(".agents/skills\n"), 0o644)) runGitInDir(t, dir, "add", ".gitignore") runGitInDir(t, dir, "commit", "-m", "init") }, diff --git a/pkg/cmd/skills/update/update.go b/pkg/cmd/skills/update/update.go index 1dfe76007..afb8377e3 100644 --- a/pkg/cmd/skills/update/update.go +++ b/pkg/cmd/skills/update/update.go @@ -415,9 +415,9 @@ func updateRun(opts *updateOptions) error { } // scanAllAgents walks every registered agent's skill directory (project + user scope) and -// collects installed skills. Skills are deduplicated by directory path. +// collects installed skills. Shared install roots are scanned only once. func scanAllAgents(gitRoot, homeDir string) []installedSkill { - seen := make(map[string]bool) + scannedDirs := make(map[string]bool) var all []installedSkill for i := range registry.Agents { @@ -427,17 +427,15 @@ func scanAllAgents(gitRoot, homeDir string) []installedSkill { if err != nil { continue } + if scannedDirs[dir] { + continue + } + scannedDirs[dir] = true skills, err := scanInstalledSkills(dir, host, scope) if err != nil { continue } - for _, s := range skills { - if seen[s.dir] { - continue - } - seen[s.dir] = true - all = append(all, s) - } + all = append(all, skills...) } } diff --git a/pkg/cmd/skills/update/update_test.go b/pkg/cmd/skills/update/update_test.go index 81fc87efe..0c7953f66 100644 --- a/pkg/cmd/skills/update/update_test.go +++ b/pkg/cmd/skills/update/update_test.go @@ -12,6 +12,7 @@ import ( "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/prompter" + "github.com/cli/cli/v2/internal/skills/registry" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/httpmock" @@ -243,6 +244,48 @@ func TestPromptForSkillOrigin(t *testing.T) { } } +func TestScanAllAgentsDeduplicatesSharedProjectDirs(t *testing.T) { + repoDir := t.TempDir() + homeDir := t.TempDir() + + sharedSkillDir := filepath.Join(repoDir, ".agents", "skills", "git-commit") + require.NoError(t, os.MkdirAll(sharedSkillDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(sharedSkillDir, "SKILL.md"), []byte(heredoc.Doc(` + --- + name: git-commit + metadata: + github-owner: monalisa + github-repo: octocat-skills + github-tree-sha: abc123 + --- + Body + `)), 0o644)) + + claudeSkillDir := filepath.Join(repoDir, ".claude", "skills", "code-review") + require.NoError(t, os.MkdirAll(claudeSkillDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(claudeSkillDir, "SKILL.md"), []byte(heredoc.Doc(` + --- + name: code-review + metadata: + github-owner: monalisa + github-repo: octocat-skills + github-tree-sha: def456 + --- + Body + `)), 0o644)) + + skills := scanAllAgents(repoDir, homeDir) + require.Len(t, skills, 2) + + byName := make(map[string]installedSkill) + for _, skill := range skills { + byName[skill.name] = skill + } + + assert.Equal(t, registry.ScopeProject, byName["git-commit"].scope) + assert.Equal(t, registry.ScopeProject, byName["code-review"].scope) +} + func TestUpdateRun(t *testing.T) { tests := []struct { name string @@ -260,7 +303,7 @@ func TestUpdateRun(t *testing.T) { t.Helper() t.Setenv("HOME", dir) t.Setenv("USERPROFILE", dir) - skillDir := filepath.Join(dir, ".github", "skills", "code-review") + skillDir := filepath.Join(dir, ".agents", "skills", "code-review") require.NoError(t, os.MkdirAll(skillDir, 0o755)) require.NoError(t, os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte(heredoc.Doc(` ---