fix: preserve namespace in skills search deduplication by SamMorrowDrums · Pull Request #13170 · cli/cli

SamMorrowDrums · 2026-04-15T21:01:57Z

Summary

Skills with the same name but different namespaces (e.g. skills/kynan/commit and skills/will/commit) were being collapsed into a single search result. This PR fixes the deduplication logic to be namespace-aware.

Problem

extractSkillName called MatchesSkillPath which only returned the bare name, discarding the namespace
deduplicateResults used repo/name as the dedup key — two namespaced skills in the same repo with the same base name collapsed into one
deduplicateByName capped by bare name — skills from different namespaces were treated as duplicates
promptInstall passed just the bare skill name to the install subprocess, which could be ambiguous

Changes

internal/skills/discovery/discovery.go

Add MatchSkillPath(filePath) (name, namespace string) — returns both name and namespace

pkg/cmd/skills/search/search.go

Add Namespace field to skillResult and qualifiedName() helper
Replace extractSkillName → extractSkillInfo using new MatchSkillPath
Fix deduplicateResults key: repo/namespace/name
Fix deduplicateByName key: namespace/name
Update table, prompt picker, and JSON output to show qualified names
Pass skill path (not bare name) to install subprocess for namespaced skills
Add namespace to --json fields and relevance scoring/filtering

Tests

Add TestDeduplicateResults_Namespaced — verifies same-name skills with different namespaces are preserved
Add TestDeduplicateByName_Namespaced — verifies cap is per namespace-qualified name
Add TestQualifiedName — verifies qualified name formatting
Add TestMatchSkillPath — verifies new discovery function
Add TestExtractSkillInfo — replaces old TestExtractSkillName with namespace coverage
Update TestFilterByRelevance — includes namespace match case
Add integration test case for namespaced skills in search results
Add acceptance test skills-search-namespaced.txtar

Stacked on

Add agent skills command #13165 (sm/add-skills-command)

Skills with the same name but different namespaces (e.g. skills/kynan/commit and skills/will/commit) were being collapsed into a single search result because extractSkillName discarded the namespace. This also caused deduplicateByName to cap results across different namespaces as if they were the same skill. Changes: - Add MatchSkillPath to discovery package returning both name and namespace (the existing MatchesSkillPath is kept for compat) - Add Namespace field to skillResult in search - Fix deduplicateResults to use repo/namespace/name as the dedup key - Fix deduplicateByName to cap by namespace-qualified name - Update table, prompt, and JSON output to show qualified names - Use skill path for install subprocess when namespace is present, ensuring unambiguous install of namespaced skills - Add namespace to --json fields and relevance scoring/filtering - Add unit tests for namespace dedup, qualified names, and filtering - Add acceptance test for namespaced skill search and install Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Fixes gh skill search collapsing skills that share a base name by making deduplication and display namespace-aware (e.g. kynan/commit vs will/commit), and improves install disambiguation for namespaced skills.

Changes:

Add discovery.MatchSkillPath to extract both skill name and namespace from repo paths.
Update search result modeling/output/deduplication to key on repo/namespace/name, display qualified names, and consider namespace in relevance scoring/filtering.
Add/adjust unit tests and add an acceptance test fixture for namespaced skills.

Show a summary per file

File	Description
`internal/skills/discovery/discovery.go`	Adds `MatchSkillPath` returning `(name, namespace)` for skill path parsing.
`internal/skills/discovery/discovery_test.go`	Adds coverage for `MatchSkillPath` including namespaced and plugin conventions.
`pkg/cmd/skills/search/search.go`	Introduces `Namespace` + `qualifiedName()`, updates dedup keys, output, install arg selection, and relevance logic.
`pkg/cmd/skills/search/search_test.go`	Updates expectations and adds tests for namespaced dedup/qualified name and namespace relevance.
`acceptance/testdata/skills/skills-search-namespaced.txtar`	Adds an acceptance scenario creating/publishing/installing two namespaced skills.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Files reviewed: 5/5 changed files
Comments generated: 4

- Keep skillName as bare name in JSON output for backward compat; namespace is available as a separate --json field - Fix Namespace field comment to cover plugin namespaces too - Trim /SKILL.md from install path arg to match comment - Rename acceptance test to skills-install-namespaced since it tests install disambiguation, not search Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

babakks

LGTM

babakks · 2026-04-16T09:12:30Z


+func TestMatchSkillPath(t *testing.T) {
+	tests := []struct {
+		testName      string


nitpick: I'd rather change testName to either name or about.

babakks · 2026-04-16T09:17:02Z

 	var result []skillResult
 	for _, s := range skills {
-		key := strings.ToLower(s.SkillName)
+		key := strings.ToLower(s.qualifiedName())


question: what if two skill names only differ in casing?

babakks · 2026-04-16T09:19:45Z

+		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)


nitpick: indented with whitespaces.

babakks · 2026-04-16T09:26:59Z

+		key := item.Repository.FullName + "/" + namespace + "/" + skillName
 		if _, ok := seen[key]; ok {
 			continue
 		}
 		seen[key] = struct{}{}


nitpick: the nicer/idiomatic way is to avoid string keys (and string-based collisions) and use typed map keys:

func deduplicateResults(items []codeSearchItem) []skillResult { type key struct { repo string skill string namespace string } seen := make(map[key]struct{}{}) // ... k := key{item.Repository.FullName, namespace, skillName} if _, ok := seen[k]; ok { continue } seen[k] = struct{}{} }

- Remove direct opts.client injection in publish; use HttpClient factory pattern (PR #13168 feedback) - Rename testName to name in discovery test struct (PR #13170 feedback) - Use typed struct keys for dedup map with case-insensitive comparison in deduplicateResults (PR #13170 feedback) - Simplify remote selection to use Remotes() ordering instead of manual origin-first logic (PR #13171 feedback) - Fix push icon timing: show no icon before push, SuccessIcon after success (PR #13171 feedback) Closes #13184 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

SamMorrowDrums requested a review from a team as a code owner April 15, 2026 21:01

SamMorrowDrums requested review from BagToad and Copilot April 15, 2026 21:01

SamMorrowDrums changed the base branch from trunk to sm/add-skills-command April 15, 2026 21:02

Copilot started reviewing on behalf of SamMorrowDrums April 15, 2026 21:02 View session

Copilot AI reviewed Apr 15, 2026

View reviewed changes

Comment thread pkg/cmd/skills/search/search.go

Comment thread pkg/cmd/skills/search/search.go Outdated

Comment thread pkg/cmd/skills/search/search.go Outdated

Comment thread acceptance/testdata/skills/skills-install-namespaced.txtar

babakks approved these changes Apr 16, 2026

View reviewed changes

SamMorrowDrums mentioned this pull request Apr 16, 2026

Skills: post-merge review feedback follow-ups #13184

Closed

8 tasks

SamMorrowDrums merged commit 963a143 into sm/add-skills-command Apr 16, 2026
8 checks passed

SamMorrowDrums deleted the sammorrowdrums/fix-skills-namespace-dedup branch April 16, 2026 13:55

SamMorrowDrums mentioned this pull request Apr 16, 2026

Address post-merge review feedback for skills commands #13185

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: preserve namespace in skills search deduplication#13170

fix: preserve namespace in skills search deduplication#13170
SamMorrowDrums merged 2 commits intosm/add-skills-commandfrom
sammorrowdrums/fix-skills-namespace-dedup

SamMorrowDrums commented Apr 15, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

babakks left a comment

Uh oh!

babakks Apr 16, 2026

Uh oh!

babakks Apr 16, 2026

Uh oh!

babakks Apr 16, 2026

Uh oh!

babakks Apr 16, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

SamMorrowDrums commented Apr 15, 2026

Summary

Problem

Changes

Stacked on

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Copilot's findings

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

babakks left a comment

Choose a reason for hiding this comment

Uh oh!

babakks Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

babakks Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

babakks Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

babakks Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants