Skip to content

fix: preserve namespace in skills search deduplication#13170

Merged
SamMorrowDrums merged 2 commits into
sm/add-skills-commandfrom
sammorrowdrums/fix-skills-namespace-dedup
Apr 16, 2026
Merged

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

Conversation

@SamMorrowDrums

Copy link
Copy Markdown
Contributor

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

  1. extractSkillName called MatchesSkillPath which only returned the bare name, discarding the namespace
  2. deduplicateResults used repo/name as the dedup key — two namespaced skills in the same repo with the same base name collapsed into one
  3. deduplicateByName capped by bare name — skills from different namespaces were treated as duplicates
  4. 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 extractSkillNameextractSkillInfo 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

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>
@SamMorrowDrums SamMorrowDrums requested a review from a team as a code owner April 15, 2026 21:01
@SamMorrowDrums SamMorrowDrums changed the base branch from trunk to sm/add-skills-command April 15, 2026 21:02

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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
- 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 babakks left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM


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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: indented with whitespaces.

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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{}{}
}

@SamMorrowDrums SamMorrowDrums merged commit 963a143 into sm/add-skills-command Apr 16, 2026
8 checks passed
@SamMorrowDrums SamMorrowDrums deleted the sammorrowdrums/fix-skills-namespace-dedup branch April 16, 2026 13:55
SamMorrowDrums added a commit that referenced this pull request Apr 16, 2026
- 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>
tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request May 7, 2026
This MR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [cli/cli](https://github.com/cli/cli) | minor | `v2.89.0` → `v2.90.0` |

MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot).

**Proposed changes to behavior should be submitted there as MRs.**

---

### Release Notes

<details>
<summary>cli/cli (cli/cli)</summary>

### [`v2.90.0`](https://github.com/cli/cli/releases/tag/v2.90.0): GitHub CLI 2.90.0

[Compare Source](cli/cli@v2.89.0...v2.90.0)

#### Manage agent skills with `gh skill` (Public Preview)

[Agent skills](https://agentskills.io) are portable sets of instructions, scripts, and resources that teach AI coding agents how to perform specific tasks. The new `gh skill` command makes it easy to discover, install, manage, and publish agent skills from GitHub repositories - right from the CLI.

```

# Discover skills
gh skill search copilot

# Preview a skill without installing
gh skill preview github/awesome-copilot documentation-writer

# Install a skill
gh skill install github/awesome-copilot documentation-writer

# Pin to a specific version
gh skill install github/awesome-copilot documentation-writer --pin v1.2.0

# Check installed skills for updates
gh skill update --all

# Validate and publish your own skills
gh skill publish --dry-run
```

Skills are automatically installed to the correct directory for your agent host. `gh skill` supports GitHub Copilot, Claude Code, Cursor, Codex, Gemini CLI, and Antigravity. Target a specific agent and scope with `--agent` and `--scope` flags.

`gh skill publish` validates skills against the [Agent Skills specification](https://agentskills.io/specification) and checks remote settings like tag protection and immutable releases to improve supply chain security.

Read the full announcement on the [GitHub Blog](https://github.blog/changelog/2026-04-16-manage-agent-skills-with-github-cli/).

`gh skill` is launching in public preview and is subject to change without notice.

#### Official extension suggestions

When you run a command that matches a known official extension that isn't installed (e.g. `gh stack`), the CLI now offers to install it instead of showing a generic "unknown command" error.

This feature is available for [github/gh-aw](https://github.com/github/gh-aw) and [github/gh-stack](https://github.com/github/gh-stack).

When possible, you'll be prompted to install immediately. When prompting isn't possible, the CLI prints the `gh extension install` command to run.

#### `gh extension install` no longer requires authentication

`gh extension install` previously required a valid auth token even though it only needs to download a public release asset. The auth check has been removed, so you can install extensions without being logged in.

#### What's Changed

##### ✨ Features

- Add `gh skill` command group: install, preview, search, update, publish by [@&#8203;SamMorrowDrums](https://github.com/SamMorrowDrums) in [#&#8203;13165](cli/cli#13165)
- Suggest and install official extensions for unknown commands by [@&#8203;BagToad](https://github.com/BagToad) in [#&#8203;13175](cli/cli#13175)
- `gh skill publish`: auto-push unpushed commits before publish by [@&#8203;SamMorrowDrums](https://github.com/SamMorrowDrums) in [#&#8203;13171](cli/cli#13171)
- Disable auth check for `gh extension install` by [@&#8203;BagToad](https://github.com/BagToad) in [#&#8203;13176](cli/cli#13176)

##### 🐛 Fixes

- Fix infinite loop in `gh release list --limit 0` by [@&#8203;Bahtya](https://github.com/Bahtya) in [#&#8203;13097](cli/cli#13097)
- Ensure `api` and `auth` commands record agentic invocations by [@&#8203;williammartin](https://github.com/williammartin) in [#&#8203;13046](cli/cli#13046)
- Disable auth check for local-only skill flags by [@&#8203;SamMorrowDrums](https://github.com/SamMorrowDrums) in [#&#8203;13173](cli/cli#13173)
- URL-encode parentPath in skills discovery API call by [@&#8203;SamMorrowDrums](https://github.com/SamMorrowDrums) in [#&#8203;13172](cli/cli#13172)
- Fix: use target directory remotes in skills publish by [@&#8203;SamMorrowDrums](https://github.com/SamMorrowDrums) in [#&#8203;13169](cli/cli#13169)
- Fix: preserve namespace in skills search deduplication by [@&#8203;SamMorrowDrums](https://github.com/SamMorrowDrums) in [#&#8203;13170](cli/cli#13170)

##### 📚 Docs & Chores

- docs: include PGP key fingerprints by [@&#8203;babakks](https://github.com/babakks) in [#&#8203;13112](cli/cli#13112)
- docs: add sha/md5 checksums of keyring files by [@&#8203;babakks](https://github.com/babakks) in [#&#8203;13150](cli/cli#13150)
- docs: fix SHA512 checksum for GPG key by [@&#8203;timsu92](https://github.com/timsu92) in [#&#8203;13157](cli/cli#13157)
- docs(skill): polish skill commandset docs by [@&#8203;babakks](https://github.com/babakks) in [#&#8203;13183](cli/cli#13183)
- Document dependency CVE policy in SECURITY.md by [@&#8203;BagToad](https://github.com/BagToad) in [#&#8203;13119](cli/cli#13119)
- Replace github.com/golang/snappy with klauspost/compress/snappy by [@&#8203;thaJeztah](https://github.com/thaJeztah) in [#&#8203;13048](cli/cli#13048)
- chore: bump to go1.26.2 by [@&#8203;babakks](https://github.com/babakks) in [#&#8203;13116](cli/cli#13116)
- chore: delete experimental script/debian-devel by [@&#8203;babakks](https://github.com/babakks) in [#&#8203;13127](cli/cli#13127)
- Suggest first party extensions by [@&#8203;williammartin](https://github.com/williammartin) in [#&#8203;13182](cli/cli#13182)
- Add cli/skill-reviewers as CODEOWNERS for skills packages by [@&#8203;BagToad](https://github.com/BagToad) in [#&#8203;13189](cli/cli#13189)
- Add [@&#8203;cli/code-reviewers](https://github.com/cli/code-reviewers) to all CODEOWNERS rules by [@&#8203;BagToad](https://github.com/BagToad) in [#&#8203;13190](cli/cli#13190)
- Address post-merge review feedback for skills commands by [@&#8203;SamMorrowDrums](https://github.com/SamMorrowDrums) in [#&#8203;13185](cli/cli#13185)
- Fix skills-publish-dry-run acceptance test error message mismatch by [@&#8203;SamMorrowDrums](https://github.com/SamMorrowDrums) in [#&#8203;13187](cli/cli#13187)
- Skills: replace real git in publish tests with CommandStubber by [@&#8203;SamMorrowDrums](https://github.com/SamMorrowDrums) in [#&#8203;13188](cli/cli#13188)
- Remove redundant nil-client fallback in skills publish by [@&#8203;SamMorrowDrums](https://github.com/SamMorrowDrums) in [#&#8203;13168](cli/cli#13168)
- Publish: use shared discovery logic instead of requiring skills/ directory by [@&#8203;SamMorrowDrums](https://github.com/SamMorrowDrums) in [#&#8203;13167](cli/cli#13167)

##### :dependabot: Dependencies

- chore(deps): bump github.com/klauspost/compress from 1.18.4 to 1.18.5 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;13071](cli/cli#13071)
- chore(deps): bump github.com/yuin/goldmark from 1.7.16 to 1.8.2 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;13045](cli/cli#13045)
- chore(deps): bump charm.land/bubbles/v2 from 2.0.0 to 2.1.0 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;13051](cli/cli#13051)
- chore(deps): bump github.com/sigstore/timestamp-authority/v2 from 2.0.3 to 2.0.6 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;13152](cli/cli#13152)
- chore(deps): bump github.com/google/go-containerregistry from 0.21.3 to 0.21.4 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;13129](cli/cli#13129)
- chore(deps): bump github.com/sigstore/protobuf-specs from 0.5.0 to 0.5.1 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;13128](cli/cli#13128)
- chore(deps): bump github.com/in-toto/attestation from 1.1.2 to 1.2.0 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;13044](cli/cli#13044)
- chore(deps): bump advanced-security/filter-sarif from 1.0.1 to 1.1 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;12918](cli/cli#12918)
- chore(deps): bump google.golang.org/grpc from 1.79.3 to 1.80.0 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;13076](cli/cli#13076)
- chore(deps): bump github.com/hashicorp/go-version from 1.8.0 to 1.9.0 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;13065](cli/cli#13065)

#### New Contributors

- [@&#8203;thaJeztah](https://github.com/thaJeztah) made their first contribution in [#&#8203;13048](cli/cli#13048)
- [@&#8203;Bahtya](https://github.com/Bahtya) made their first contribution in [#&#8203;13097](cli/cli#13097)
- [@&#8203;timsu92](https://github.com/timsu92) made their first contribution in [#&#8203;13157](cli/cli#13157)
- [@&#8203;SamMorrowDrums](https://github.com/SamMorrowDrums) made their first contribution in [#&#8203;13173](cli/cli#13173)

**Full Changelog**: <cli/cli@v2.89.0...v2.90.0>

</details>

---

### Configuration

📅 **Schedule**: (UTC)

- Branch creation
  - At any time (no schedule defined)
- Automerge
  - At any time (no schedule defined)

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4xMjMuNiIsInVwZGF0ZWRJblZlciI6IjQzLjEyMy42IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiLCJhdXRvbWF0aW9uOmJvdC1hdXRob3JlZCIsImRlcGVuZGVuY3ktdHlwZTo6bWlub3IiXX0=-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants