URL-encode parentPath in skills discovery API call#13172
Merged
SamMorrowDrums merged 1 commit intoApr 15, 2026
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a new gh skill command group and the underlying “skills” infrastructure (discovery, install/update, metadata handling, locking), and includes an update to how repository content paths are constructed for the skills discovery API.
Changes:
- Add top-level
gh skillcommand group with subcommands (search/preview/update) and wire it into the root CLI. - Implement internal skills primitives: discovery, frontmatter metadata injection, install/update workflows, agent registry, and lockfile + cross-platform file locking.
- Add/extend unit tests and acceptance test scripts for the new skills functionality.
Show a summary per file
| File | Description |
|---|---|
| pkg/cmd/skills/update/update.go | Implements gh skill update workflow: scan installed skills, compare SHAs, and reinstall updates. |
| pkg/cmd/skills/skills.go | Adds the skill top-level command and registers subcommands. |
| pkg/cmd/skills/search/search.go | Implements gh skill search using GitHub Code Search, ranking, enrichment, and interactive install. |
| pkg/cmd/skills/search/search_test.go | Unit tests for gh skill search behaviors (ranking, pagination, rate limiting, JSON output). |
| pkg/cmd/skills/preview/preview.go | Implements gh skill preview including interactive file tree browsing and markdown rendering. |
| pkg/cmd/skills/preview/preview_test.go | Unit tests for preview flows (version resolution, interactive picker, file rendering limits). |
| pkg/cmd/root/root.go | Wires the new skill command into the root command tree. |
| internal/skills/source/source.go | Adds helpers for parsing/validating source repo metadata and supported host validation. |
| internal/skills/source/source_test.go | Unit tests for repo URL parsing and supported host validation. |
| internal/skills/registry/registry.go | Defines supported agent hosts and resolves install directories + scope labels. |
| internal/skills/registry/registry_test.go | Unit tests for agent lookup, install dir resolution, and remote parsing. |
| internal/skills/lockfile/lockfile.go | Adds .skill-lock.json read/modify/write with inter-process locking. |
| internal/skills/lockfile/lockfile_test.go | Unit tests for lockfile creation, updates, corruption recovery, and contention handling. |
| internal/skills/installer/installer.go | Implements local and remote skill installation, metadata injection, and lockfile recording. |
| internal/skills/installer/installer_test.go | Unit tests for local/remote installs, traversal defense, and progress callbacks. |
| internal/skills/frontmatter/frontmatter.go | Parses frontmatter and injects GitHub/local metadata into SKILL.md. |
| internal/skills/frontmatter/frontmatter_test.go | Unit tests for parsing and metadata injection/serialization. |
| internal/skills/discovery/discovery.go | Implements ref resolution, skill discovery, blob fetching, and path-based lookup. |
| internal/skills/discovery/collisions.go | Detects install-name collisions across discovered skills. |
| internal/skills/discovery/collisions_test.go | Unit tests for collision detection and formatting. |
| internal/flock/flock.go | Defines a platform-agnostic lock contention sentinel error. |
| internal/flock/flock_unix.go | Unix flock-based non-blocking lock implementation. |
| internal/flock/flock_windows.go | Windows LockFileEx-based non-blocking lock implementation. |
| internal/flock/flock_test.go | Cross-platform tests for acquiring/releasing/contending locks. |
| git/client.go | Adds git helper methods RemoteURL, IsIgnored, and ShortSHA. |
| git/client_test.go | Tests for RemoteURL, IsIgnored, and ShortSHA. |
| go.mod | Promotes golang.org/x/sys to a direct dependency (for Windows lock implementation). |
| acceptance/acceptance_test.go | Adds a dedicated acceptance test runner for the skills testscript suite. |
| acceptance/testdata/skills/skills-update.txtar | Acceptance coverage for update (dry-run, force update, and content rewrite assertions). |
| acceptance/testdata/skills/skills-update-noinstalled.txtar | Acceptance coverage for update when no skills are installed. |
| acceptance/testdata/skills/skills-search.txtar | Acceptance coverage for search and JSON output. |
| acceptance/testdata/skills/skills-search-page.txtar | Acceptance coverage for pagination. |
| acceptance/testdata/skills/skills-search-noresults.txtar | Acceptance coverage for no-results behavior. |
| acceptance/testdata/skills/skills-publish-lifecycle.txtar | End-to-end acceptance flow for publish/install/preview/update. |
| acceptance/testdata/skills/skills-publish-dry-run.txtar | Acceptance coverage for publish/validate dry-run behaviors. |
| acceptance/testdata/skills/skills-preview.txtar | Acceptance coverage for preview rendering and missing-skill errors. |
| acceptance/testdata/skills/skills-preview-noninteractive.txtar | Acceptance coverage for non-interactive preview constraints. |
| acceptance/testdata/skills/skills-install.txtar | Acceptance coverage for install, metadata injection, lockfile writing, and --dir. |
| acceptance/testdata/skills/skills-install-scope.txtar | Acceptance coverage for project vs user scope install paths. |
| acceptance/testdata/skills/skills-install-pin.txtar | Acceptance coverage for --pin installs. |
| acceptance/testdata/skills/skills-install-nonexistent-skill.txtar | Acceptance coverage for installing a nonexistent skill. |
| acceptance/testdata/skills/skills-install-nested-files.txtar | Acceptance coverage for nested-file skills install. |
| acceptance/testdata/skills/skills-install-invalid-repo.txtar | Acceptance coverage for invalid/nonexistent repo handling. |
| acceptance/testdata/skills/skills-install-invalid-agent.txtar | Acceptance coverage for invalid --agent selection. |
| acceptance/testdata/skills/skills-install-from-local.txtar | Acceptance coverage for --from-local installs and local metadata injection. |
| acceptance/testdata/skills/skills-install-force.txtar | Acceptance coverage for --force overwrite behavior. |
| .gitignore | Ignores the built gh binary. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
internal/skills/discovery/discovery.go:495
- In
DiscoverSkillByPath,url.PathEscape(parentPath)will also escape path separators (/), turning e.g.skills/nsintoskills%2Fns, which breaks the GitHub Contents API path parameter. Instead, escape each path segment and re-join with/(preserving slashes), or use an existing helper for encoding repo paths if one exists. (Also consider usingurl.QueryEscapeforref=since it’s a query parameter.)
parentPath := path.Dir(skillPath)
apiPath := fmt.Sprintf("repos/%s/%s/contents/%s?ref=%s", url.PathEscape(owner), url.PathEscape(repo), url.PathEscape(parentPath), url.PathEscape(commitSHA))
- Files reviewed: 52/53 changed files
- Comments generated: 3
The parentPath parameter in the contents API path was not URL-encoded, which would cause failures when paths contain spaces or other special characters. Apply url.PathEscape() to parentPath, consistent with the rest of the file. commitSHA is left unescaped since SHAs are hex-only and never need encoding. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
aa9914b to
9f9b93a
Compare
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 [@​SamMorrowDrums](https://github.com/SamMorrowDrums) in [#​13165](cli/cli#13165) - Suggest and install official extensions for unknown commands by [@​BagToad](https://github.com/BagToad) in [#​13175](cli/cli#13175) - `gh skill publish`: auto-push unpushed commits before publish by [@​SamMorrowDrums](https://github.com/SamMorrowDrums) in [#​13171](cli/cli#13171) - Disable auth check for `gh extension install` by [@​BagToad](https://github.com/BagToad) in [#​13176](cli/cli#13176) ##### 🐛 Fixes - Fix infinite loop in `gh release list --limit 0` by [@​Bahtya](https://github.com/Bahtya) in [#​13097](cli/cli#13097) - Ensure `api` and `auth` commands record agentic invocations by [@​williammartin](https://github.com/williammartin) in [#​13046](cli/cli#13046) - Disable auth check for local-only skill flags by [@​SamMorrowDrums](https://github.com/SamMorrowDrums) in [#​13173](cli/cli#13173) - URL-encode parentPath in skills discovery API call by [@​SamMorrowDrums](https://github.com/SamMorrowDrums) in [#​13172](cli/cli#13172) - Fix: use target directory remotes in skills publish by [@​SamMorrowDrums](https://github.com/SamMorrowDrums) in [#​13169](cli/cli#13169) - Fix: preserve namespace in skills search deduplication by [@​SamMorrowDrums](https://github.com/SamMorrowDrums) in [#​13170](cli/cli#13170) ##### 📚 Docs & Chores - docs: include PGP key fingerprints by [@​babakks](https://github.com/babakks) in [#​13112](cli/cli#13112) - docs: add sha/md5 checksums of keyring files by [@​babakks](https://github.com/babakks) in [#​13150](cli/cli#13150) - docs: fix SHA512 checksum for GPG key by [@​timsu92](https://github.com/timsu92) in [#​13157](cli/cli#13157) - docs(skill): polish skill commandset docs by [@​babakks](https://github.com/babakks) in [#​13183](cli/cli#13183) - Document dependency CVE policy in SECURITY.md by [@​BagToad](https://github.com/BagToad) in [#​13119](cli/cli#13119) - Replace github.com/golang/snappy with klauspost/compress/snappy by [@​thaJeztah](https://github.com/thaJeztah) in [#​13048](cli/cli#13048) - chore: bump to go1.26.2 by [@​babakks](https://github.com/babakks) in [#​13116](cli/cli#13116) - chore: delete experimental script/debian-devel by [@​babakks](https://github.com/babakks) in [#​13127](cli/cli#13127) - Suggest first party extensions by [@​williammartin](https://github.com/williammartin) in [#​13182](cli/cli#13182) - Add cli/skill-reviewers as CODEOWNERS for skills packages by [@​BagToad](https://github.com/BagToad) in [#​13189](cli/cli#13189) - Add [@​cli/code-reviewers](https://github.com/cli/code-reviewers) to all CODEOWNERS rules by [@​BagToad](https://github.com/BagToad) in [#​13190](cli/cli#13190) - Address post-merge review feedback for skills commands by [@​SamMorrowDrums](https://github.com/SamMorrowDrums) in [#​13185](cli/cli#13185) - Fix skills-publish-dry-run acceptance test error message mismatch by [@​SamMorrowDrums](https://github.com/SamMorrowDrums) in [#​13187](cli/cli#13187) - Skills: replace real git in publish tests with CommandStubber by [@​SamMorrowDrums](https://github.com/SamMorrowDrums) in [#​13188](cli/cli#13188) - Remove redundant nil-client fallback in skills publish by [@​SamMorrowDrums](https://github.com/SamMorrowDrums) in [#​13168](cli/cli#13168) - Publish: use shared discovery logic instead of requiring skills/ directory by [@​SamMorrowDrums](https://github.com/SamMorrowDrums) in [#​13167](cli/cli#13167) #####Dependencies - chore(deps): bump github.com/klauspost/compress from 1.18.4 to 1.18.5 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​13071](cli/cli#13071) - chore(deps): bump github.com/yuin/goldmark from 1.7.16 to 1.8.2 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​13045](cli/cli#13045) - chore(deps): bump charm.land/bubbles/v2 from 2.0.0 to 2.1.0 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​13051](cli/cli#13051) - chore(deps): bump github.com/sigstore/timestamp-authority/v2 from 2.0.3 to 2.0.6 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​13152](cli/cli#13152) - chore(deps): bump github.com/google/go-containerregistry from 0.21.3 to 0.21.4 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​13129](cli/cli#13129) - chore(deps): bump github.com/sigstore/protobuf-specs from 0.5.0 to 0.5.1 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​13128](cli/cli#13128) - chore(deps): bump github.com/in-toto/attestation from 1.1.2 to 1.2.0 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​13044](cli/cli#13044) - chore(deps): bump advanced-security/filter-sarif from 1.0.1 to 1.1 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​12918](cli/cli#12918) - chore(deps): bump google.golang.org/grpc from 1.79.3 to 1.80.0 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​13076](cli/cli#13076) - chore(deps): bump github.com/hashicorp/go-version from 1.8.0 to 1.9.0 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​13065](cli/cli#13065) #### New Contributors - [@​thaJeztah](https://github.com/thaJeztah) made their first contribution in [#​13048](cli/cli#13048) - [@​Bahtya](https://github.com/Bahtya) made their first contribution in [#​13097](cli/cli#13097) - [@​timsu92](https://github.com/timsu92) made their first contribution in [#​13157](cli/cli#13157) - [@​SamMorrowDrums](https://github.com/SamMorrowDrums) made their first contribution in [#​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=-->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #13165.
The
parentPathparameter in the contents API path (DiscoverSkillByPath) was not URL-encoded, which would cause failures when paths contain spaces or other special characters.Changes
url.PathEscape()toparentPathin thefmt.Sprintfcall indiscovery.go, consistent with all other API path construction in the filecommitSHAis left unescaped since SHAs are hex-only and never need encodingparent path with spaces is URL encoded) that exercises a path with spacesFixes the review comment from #13165.