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

Merge pull request #13170 from cli/sammorrowdrums/fix-skills-namespace-dedup

Browse source 
fix: preserve namespace in skills search deduplication
This commit is contained in:
Sam Morrow 2026-04-16 15:55:41 +02:00 • committed by GitHub
commit 963a1438a9
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 253 additions and 34 deletions
Download patch file Download diff file Expand all files Collapse all files

60
acceptance/testdata/skills/skills-install-namespaced.txtar vendored Normal file
View file

@ -0,0 +1,60 @@
# Two namespaced skills with the same base name in the same repo should
# be independently installable using path-based disambiguation.
# Use gh as a credential helper
exec gh auth setup-git
# Create a repo with two namespaced skills that share the name "deploy"
exec gh repo create $ORG/$SCRIPT_NAME-$RANDOM_STRING --public --add-readme
defer gh repo delete --yes $ORG/$SCRIPT_NAME-$RANDOM_STRING
exec gh repo clone $ORG/$SCRIPT_NAME-$RANDOM_STRING
cd $SCRIPT_NAME-$RANDOM_STRING
mkdir -p skills/alice/deploy
mkdir -p skills/bob/deploy
cp $WORK/alice-skill.md skills/alice/deploy/SKILL.md
cp $WORK/bob-skill.md skills/bob/deploy/SKILL.md
exec git add -A
exec git commit -m 'Add namespaced skills'
exec git push origin main
# Publish so the skills are discoverable
exec gh skill publish --tag v1.0.0
# Install alice's deploy skill using the full path to disambiguate
exec gh skill install $ORG/$SCRIPT_NAME-$RANDOM_STRING skills/alice/deploy --scope user --force
stdout 'Installed alice/deploy'
# Install bob's deploy skill using the full path
exec gh skill install $ORG/$SCRIPT_NAME-$RANDOM_STRING skills/bob/deploy --scope user --force
stdout 'Installed bob/deploy'
# Verify both were installed to separate directories
exists $HOME/.copilot/skills/alice/deploy/SKILL.md
exists $HOME/.copilot/skills/bob/deploy/SKILL.md
# Verify each has the correct content
grep 'Alice' $HOME/.copilot/skills/alice/deploy/SKILL.md
grep 'Bob' $HOME/.copilot/skills/bob/deploy/SKILL.md
-- alice-skill.md --
---
name: deploy
description: Alice's deployment skill
---
# Deploy by Alice
Deploys infrastructure using Alice's conventions.
-- bob-skill.md --
---
name: deploy
description: Bob's deployment skill
---
# Deploy by Bob
Deploys infrastructure using Bob's conventions.

12
internal/skills/discovery/discovery.go
View file

@ -316,6 +316,18 @@ func MatchesSkillPath(filePath string) string {
return m.name
}
// MatchSkillPath checks if a file path matches any known skill convention
// and returns the skill name and namespace. Returns empty strings if the
// path doesn't match. The namespace is non-empty for namespaced skills
// (e.g. skills/author/name/SKILL.md) and plugin skills.
func MatchSkillPath(filePath string) (name, namespace string) {
m := matchSkillConventions(treeEntry{Path: filePath})
if m == nil {
return "", ""
}
return m.name, m.namespace
}
// matchSkillConventions checks if a blob path matches any known skill convention.
func matchSkillConventions(entry treeEntry) *skillMatch {
if path.Base(entry.Path) != "SKILL.md" {

24
internal/skills/discovery/discovery_test.go
View file

@ -923,6 +923,30 @@ func TestMatchesSkillPath(t *testing.T) {
}
}
func TestMatchSkillPath(t *testing.T) {
tests := []struct {
testName string
path string
wantName string
wantNamespace string
}{
{testName: "skills convention", path: "skills/code-review/SKILL.md", wantName: "code-review", wantNamespace: ""},
{testName: "namespaced convention", path: "skills/monalisa/issue-triage/SKILL.md", wantName: "issue-triage", wantNamespace: "monalisa"},
{testName: "plugins convention", path: "plugins/hubot/skills/pr-summary/SKILL.md", wantName: "pr-summary", wantNamespace: "hubot"},
{testName: "non-skill file", path: "README.md", wantName: "", wantNamespace: ""},
{testName: "same name different namespace 1", path: "skills/kynan/commit/SKILL.md", wantName: "commit", wantNamespace: "kynan"},
{testName: "same name different namespace 2", path: "skills/will/commit/SKILL.md", wantName: "commit", wantNamespace: "will"},
{testName: "root convention", path: "my-skill/SKILL.md", wantName: "my-skill", wantNamespace: ""},
}
for _, tt := range tests {
t.Run(tt.testName, func(t *testing.T) {
name, namespace := MatchSkillPath(tt.path)
assert.Equal(t, tt.wantName, name)
assert.Equal(t, tt.wantNamespace, namespace)
})
}
}
func TestDiscoverSkillFiles(t *testing.T) {
tests := []struct {
name string

68
pkg/cmd/skills/search/search.go
View file

@ -40,6 +40,7 @@ const (
var SkillSearchFields = []string{
"repo",
"skillName",
"namespace",
"description",
"stars",
"path",
@ -161,12 +162,22 @@ type skillResult struct {
Owner string // parsed from Repo
RepoName string // parsed from Repo
SkillName string
Namespace string // namespace prefix: author/scope for skills/{author}/* or plugin name for plugins/{plugin}/skills/*
Description string
Path string // original file path (e.g. skills/terraform/SKILL.md)
BlobSHA string
Stars int // repository stargazer count
}
// qualifiedName returns the namespace-qualified skill name (e.g. "author/skill")
// or just the skill name if there is no namespace.
func (s skillResult) qualifiedName() string {
if s.Namespace != "" {
return s.Namespace + "/" + s.SkillName
}
return s.SkillName
}
// ExportData implements cmdutil.exportable for --json output.
func (s skillResult) ExportData(fields []string) map[string]interface{} {
data := map[string]interface{}{}
@ -176,6 +187,8 @@ func (s skillResult) ExportData(fields []string) map[string]interface{} {
data[f] = s.Repo
case "skillName":
data[f] = s.SkillName
case "namespace":
data[f] = s.Namespace
case "description":
data[f] = s.Description
case "stars":
@ -411,17 +424,18 @@ func paginate(skills []skillResult, page, limit int) ([]skillResult, int) {
return skills[start:end], totalPages
}
// deduplicateByName caps the number of results with the same skill name.
// Since results are pre-sorted by relevance score, the first occurrences
// deduplicateByName caps the number of results with the same qualified skill
// name. Since results are pre-sorted by relevance score, the first occurrences
// are the best instances. This prevents aggregator repos (which copy
// popular skills verbatim) from flooding results while still showing
// a few alternative sources.
// a few alternative sources. Namespaced skills (e.g. "author/skill") are
// treated as distinct from bare names.
func deduplicateByName(skills []skillResult) []skillResult {
const maxPerName = 3
counts := make(map[string]int)
var result []skillResult
for _, s := range skills {
key := strings.ToLower(s.SkillName)
key := strings.ToLower(s.qualifiedName())
if counts[key] >= maxPerName {
continue
}
@ -484,7 +498,7 @@ func renderTable(io *iostreams.IOStreams, skills []skillResult) error {
table := tableprinter.New(io, tableprinter.WithHeader("REPOSITORY", "SKILL", "DESCRIPTION", "STARS"))
for _, s := range skills {
table.AddField(s.Repo)
table.AddField(s.SkillName)
table.AddField(s.qualifiedName())
desc := s.Description
if isTTY {
desc = text.Truncate(descWidth, desc)
@ -522,7 +536,7 @@ func promptInstall(opts *SearchOptions, skills []skillResult) error {
desc := strings.Join(strings.Fields(s.Description), " ")
descStr = "\n " + cs.Muted(text.Truncate(descWidth, desc))
}
options[i] = s.SkillName + " " + cs.Muted(s.Repo) + starStr + descStr
options[i] = s.qualifiedName() + " " + cs.Muted(s.Repo) + starStr + descStr
}
indices, err := opts.Prompter.MultiSelect(
@ -558,18 +572,27 @@ func promptInstall(opts *SearchOptions, skills []skillResult) error {
for _, idx := range indices {
s := skills[idx]
displayName := s.qualifiedName()
fmt.Fprintf(opts.IO.ErrOut, "\n%s Installing %s from %s...\n",
cs.Blue("::"), s.SkillName, s.Repo)
cs.Blue("::"), displayName, s.Repo)
// Use the repo-relative directory path (e.g. "skills/author/name")
// for disambiguation when installing namespaced skills, so the
// install command can resolve the exact skill without ambiguity.
installArg := s.SkillName
if s.Namespace != "" {
installArg = strings.TrimSuffix(s.Path, "/SKILL.md")
}
//nolint:gosec // arguments are from user-selected search results, not arbitrary input
cmd := exec.Command(opts.Executable, "skills", "install", s.Repo, s.SkillName,
cmd := exec.Command(opts.Executable, "skills", "install", s.Repo, installArg,
"--agent", host.ID, "--scope", scope)
cmd.Stdin = os.Stdin
cmd.Stdout = opts.IO.Out
cmd.Stderr = opts.IO.ErrOut
if err := cmd.Run(); err != nil {
fmt.Fprintf(opts.IO.ErrOut, "%s Failed to install %s from %s: %s\n",
cs.Red("!"), s.SkillName, s.Repo, err)
cs.Red("!"), displayName, s.Repo, err)
}
}
@ -580,6 +603,7 @@ func promptInstall(opts *SearchOptions, skills []skillResult) error {
// Higher scores rank first. Signals (in priority order):
// - Exact skill name match (3 000 points)
// - Partial skill name match (1 000 points)
// - Namespace match (500 points)
// - Description contains query (100 points)
// - Repository stars (sqrt bonus, ~2 400 for 6k stars)
func relevanceScore(s skillResult, query string) int {
@ -596,6 +620,11 @@ func relevanceScore(s skillResult, query string) int {
score += 1_000
}
// Namespace match.
if s.Namespace != "" && strings.Contains(strings.ToLower(s.Namespace), term) {
score += 500
}
// Description match.
if strings.Contains(strings.ToLower(s.Description), term) {
score += 100
@ -612,7 +641,7 @@ func relevanceScore(s skillResult, query string) int {
// filterByRelevance removes results that are not meaningfully related to
// the query. A result is kept if the query term appears in the skill name,
// the YAML description, or the repository owner or name.
// the namespace, the YAML description, or the repository owner or name.
func filterByRelevance(skills []skillResult, query string) []skillResult {
queryTerm := strings.ToLower(query)
termHyphen := strings.ReplaceAll(queryTerm, " ", "-")
@ -620,12 +649,14 @@ func filterByRelevance(skills []skillResult, query string) []skillResult {
filtered := skills[:0] // reuse backing array
for _, s := range skills {
nameLower := strings.ToLower(s.SkillName)
namespaceLower := strings.ToLower(s.Namespace)
descLower := strings.ToLower(s.Description)
ownerLower := strings.ToLower(s.Owner)
repoLower := strings.ToLower(s.RepoName)
if strings.Contains(nameLower, queryTerm) ||
strings.Contains(nameLower, termHyphen) ||
strings.Contains(namespaceLower, queryTerm) ||
strings.Contains(descLower, queryTerm) ||
strings.Contains(ownerLower, queryTerm) ||
strings.Contains(repoLower, queryTerm) {
@ -739,17 +770,17 @@ func fetchPrimaryPages(client *api.Client, host, query string, displayPage, disp
return allItems, totalCount, nil
}
// deduplicateResults extracts unique (repo, skill name) pairs from code search hits.
// deduplicateResults extracts unique (repo, namespace, skill name) triples from code search hits.
func deduplicateResults(items []codeSearchItem) []skillResult {
seen := make(map[string]struct{})
var results []skillResult
for _, item := range items {
skillName := extractSkillName(item.Path)
skillName, namespace := extractSkillInfo(item.Path)
if skillName == "" {
continue
}
key := item.Repository.FullName + "/" + skillName
key := item.Repository.FullName + "/" + namespace + "/" + skillName
if _, ok := seen[key]; ok {
continue
}
@ -761,6 +792,7 @@ func deduplicateResults(items []codeSearchItem) []skillResult {
Owner: owner,
RepoName: repoName,
SkillName: skillName,
Namespace: namespace,
Path: item.Path,
BlobSHA: item.SHA,
})
@ -817,11 +849,11 @@ func fetchDescriptions(client *api.Client, host string, skills []skillResult) ma
return descs
}
// extractSkillName derives the skill name from a SKILL.md path, but only if
// the path matches a known skill convention (skills/*, skills/scope/*, root-level,
// or plugins/*/skills/*). Returns empty string for non-conforming paths.
func extractSkillName(filePath string) string {
return discovery.MatchesSkillPath(filePath)
// extractSkillInfo derives the skill name and namespace from a SKILL.md path,
// but only if the path matches a known skill convention. Returns empty strings
// for non-conforming paths.
func extractSkillInfo(filePath string) (name, namespace string) {
return discovery.MatchSkillPath(filePath)
}
// formatStars formats a star count for display (e.g. 1700 > "1.7k").

123
pkg/cmd/skills/search/search_test.go
View file

@ -190,7 +190,7 @@ func TestSearchRun(t *testing.T) {
httpStubs: func(reg *httpmock.Registry) {
stubKeywordSearch(reg, `{"total_count": 1, "incomplete_results": false, "items": [{"name": "SKILL.md", "path": "skills/author/my-skill/SKILL.md", "repository": {"full_name": "org/repo"}}]}`)
},
wantStdout: "org/repo\tmy-skill\t\t0\n",
wantStdout: "org/repo\tauthor/my-skill\t\t0\n",
},
{
name: "ranks name-matching results first",
@ -225,6 +225,18 @@ func TestSearchRun(t *testing.T) {
},
wantErr: `no skills found on page 999 for query "terraform"`,
},
{
name: "namespaced skills are kept distinct in same repo",
tty: false,
opts: &SearchOptions{Query: "commit", Page: 1, Limit: defaultLimit},
httpStubs: func(reg *httpmock.Registry) {
stubKeywordSearch(reg, `{"total_count": 2, "incomplete_results": false, "items": [
{"name": "SKILL.md", "path": "skills/kynan/commit/SKILL.md", "repository": {"full_name": "org/skills-repo"}},
{"name": "SKILL.md", "path": "skills/will/commit/SKILL.md", "repository": {"full_name": "org/skills-repo"}}
]}`)
},
wantStdout: "org/skills-repo\tkynan/commit\t\t0\norg/skills-repo\twill/commit\t\t0\n",
},
{
name: "json output with selected fields",
tty: false,
@ -398,28 +410,52 @@ func TestDeduplicateResults(t *testing.T) {
assert.Equal(t, "terraform", results[2].SkillName)
}
func TestExtractSkillName(t *testing.T) {
func TestDeduplicateResults_Namespaced(t *testing.T) {
items := []codeSearchItem{
{Path: "skills/kynan/commit/SKILL.md", Repository: codeSearchRepository{FullName: "org/repo"}},
{Path: "skills/will/commit/SKILL.md", Repository: codeSearchRepository{FullName: "org/repo"}},
{Path: "skills/kynan/commit/SKILL.md", Repository: codeSearchRepository{FullName: "org/repo"}}, // duplicate
{Path: "skills/commit/SKILL.md", Repository: codeSearchRepository{FullName: "org/repo"}}, // non-namespaced
}
results := deduplicateResults(items)
require.Equal(t, 3, len(results))
assert.Equal(t, "commit", results[0].SkillName)
assert.Equal(t, "kynan", results[0].Namespace)
assert.Equal(t, "commit", results[1].SkillName)
assert.Equal(t, "will", results[1].Namespace)
assert.Equal(t, "commit", results[2].SkillName)
assert.Equal(t, "", results[2].Namespace)
}
func TestExtractSkillInfo(t *testing.T) {
tests := []struct {
path string
want string
path string
wantName string
wantNamespace string
}{
{"skills/terraform/SKILL.md", "terraform"},
{"skills/author/my-skill/SKILL.md", "my-skill"},
{"SKILL.md", ""},
{"skills/docker/SKILL.md", "docker"},
{"skills/terraform/SKILL.md", "terraform", ""},
{"skills/author/my-skill/SKILL.md", "my-skill", "author"},
{"SKILL.md", "", ""},
{"skills/docker/SKILL.md", "docker", ""},
// Root-level convention
{"my-skill/SKILL.md", "my-skill"},
{"my-skill/SKILL.md", "my-skill", ""},
// Plugins convention
{"plugins/openai/skills/chat/SKILL.md", "chat"},
{"plugins/openai/skills/chat/SKILL.md", "chat", "openai"},
// Non-matching paths should be filtered out
{"random/nested/deep/SKILL.md", ""},
{".hidden/SKILL.md", ""},
{"random/nested/deep/SKILL.md", "", ""},
{".hidden/SKILL.md", "", ""},
// Same-name skills with different namespaces
{"skills/kynan/commit/SKILL.md", "commit", "kynan"},
{"skills/will/commit/SKILL.md", "commit", "will"},
}
for _, tt := range tests {
t.Run(tt.path, func(t *testing.T) {
got := extractSkillName(tt.path)
assert.Equal(t, tt.want, got)
gotName, gotNamespace := extractSkillInfo(tt.path)
assert.Equal(t, tt.wantName, gotName)
assert.Equal(t, tt.wantNamespace, gotNamespace)
})
}
}
@ -432,18 +468,22 @@ func TestFilterByRelevance(t *testing.T) {
{Repo: "acme/terraform-tools", Owner: "acme", RepoName: "terraform-tools", SkillName: "validator"},
{Repo: "x/y", Owner: "x", RepoName: "y", SkillName: "unrelated", Description: "terraform integration"},
{Repo: "x/z", Owner: "x", RepoName: "z", SkillName: "noise"},
{Repo: "org/repo3", Owner: "org", RepoName: "repo3", SkillName: "deploy", Namespace: "terraform"},
}
filtered := filterByRelevance(skills, "terraform")
// Should keep: name match (terraform), owner match (terraform-corp),
// repo name match (terraform-tools), description match (terraform integration).
// repo name match (terraform-tools), description match (terraform integration),
// namespace match (terraform/deploy).
// Should drop: docker, noise.
assert.Equal(t, 4, len(filtered))
assert.Equal(t, 5, len(filtered))
assert.Equal(t, "terraform", filtered[0].SkillName)
assert.Equal(t, "linter", filtered[1].SkillName)
assert.Equal(t, "validator", filtered[2].SkillName)
assert.Equal(t, "unrelated", filtered[3].SkillName)
assert.Equal(t, "deploy", filtered[4].SkillName)
assert.Equal(t, "terraform", filtered[4].Namespace)
}
func TestRankByRelevance(t *testing.T) {
@ -485,3 +525,54 @@ func TestFormatStars(t *testing.T) {
assert.Equal(t, "1.7k", formatStars(1700))
assert.Equal(t, "12.5k", formatStars(12500))
}
func TestQualifiedName(t *testing.T) {
tests := []struct {
name string
skill skillResult
want string
}{
{
name: "no namespace",
skill: skillResult{SkillName: "terraform"},
want: "terraform",
},
{
name: "with namespace",
skill: skillResult{SkillName: "commit", Namespace: "kynan"},
want: "kynan/commit",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equal(t, tt.want, tt.skill.qualifiedName())
})
}
}
func TestDeduplicateByName_Namespaced(t *testing.T) {
// Skills with the same base name but different namespaces should
// be treated as distinct and not collapsed against each other.
skills := []skillResult{
{Repo: "org/repo1", SkillName: "commit", Namespace: "kynan"},
{Repo: "org/repo2", SkillName: "commit", Namespace: "will"},
{Repo: "org/repo3", SkillName: "commit"},
{Repo: "org/repo4", SkillName: "commit", Namespace: "kynan"},
{Repo: "org/repo5", SkillName: "commit", Namespace: "kynan"},
{Repo: "org/repo6", SkillName: "commit", Namespace: "kynan"}, // should be capped (4th kynan/commit)
}
result := deduplicateByName(skills)
// kynan/commit capped at 3, will/commit has 1, bare commit has 1 = 5 total
require.Equal(t, 5, len(result))
assert.Equal(t, "kynan", result[0].Namespace)
assert.Equal(t, "will", result[1].Namespace)
assert.Equal(t, "", result[2].Namespace)
assert.Equal(t, "kynan", result[3].Namespace)
assert.Equal(t, "kynan", result[4].Namespace)
// repo6 should have been dropped
for _, s := range result {
assert.NotEqual(t, "org/repo6", s.Repo)
}
}