Conversation
Cover the three primary discovery entry points with httpmock-based tests. DiscoverSkills: happy path, truncated tree, no skills, API error, dedup. DiscoverSkillByPath: path resolution, namespaces, invalid name, missing directory, missing SKILL.md. DiscoverLocalSkills: convention matching, root skill, no skills, nonexistent directory. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Test InstallLocal public API instead of private installLocalSkill Replace tests that called installLocalSkill directly with tests through InstallLocal. Adds coverage for AgentHost+Scope resolution path, multiple skills, and missing Dir/AgentHost error. Fixes symlink test to require.NoError on os.Symlink. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Test partial failure in concurrent Install Add test where one of two skills fails (500 on tree fetch). Asserts that result.Installed contains the successful skill and err wraps the failed skill name. Fixes test loop to not clear Dir for partial failure cases. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Refactor update tests to table-driven pattern Consolidate 16 individual test functions into 3 standalone + 3 table tests matching cli/cli conventions. Fix ArgsPassedToOptions to use iostreams.Test() instead of os.Stdout/os.Stderr. Use GitHub-branded test data. No coverage lost. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Add update execution test that verifies SKILL.md is rewritten All prior update tests used DryRun or hit early exits. New test exercises the full fetch-and-rewrite path: stale treeSHA triggers re-download, SKILL.md is overwritten with new content and metadata. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Use heredoc.Doc for multiline SKILL.md strings in update tests Replace escaped newline strings with heredoc.Doc backtick literals for readability, matching cli/cli conventions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Add interactive update path tests Cover confirm-and-apply, confirm-cancelled, and no-metadata prompt paths in TestUpdateRun. These interactive branches were previously untested since all prior tests used non-TTY or DryRun. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Test no-metadata prompt enrichment through full update path Add test where a skill with no GitHub metadata is prompted for origin, user provides owner/repo, skill gets enriched and proceeds through version resolution and file rewrite. Covers lines 222-224 in update.go. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Replace deprecated cs.Gray with cs.Muted Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Test namespaced skill update with --dir base resolution Cover the filepath.Dir double-up path for namespaced skills (name contains '/') when using --dir. Verifies the install base is resolved correctly so the update writes to the right directory. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Test install failure during update reports error and preserves file Cover the path where version resolution succeeds but blob fetch fails during the actual install. Verifies stderr error message, SilentError return, and that the original SKILL.md is not modified. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Dedupe resolveGitRoot/resolveHomeDir into installer, rename scanAllHosts Move ResolveGitRoot and ResolveHomeDir to the installer package to eliminate duplication between install and update commands. Fix ResolveGitRoot to check RepoDir before calling ToplevelDir. Rename scanAllHosts to scanAllAgents to match registry naming. Add test exercising scanAllAgents via updateRun without --dir. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Use heredoc.Doc for multiline YAML strings across all test files Convert 13 escaped-newline frontmatter strings to heredoc.Doc for readability. Applies to discovery, frontmatter, install, update, publish, and preview test files. Preserves edge-case test strings and fmt.Sprintf interpolations as-is. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Use git.Client.Copy() instead of struct copy to avoid mutex copy Fixes go vet 'copies lock value' warnings in publish command where *git.Client was copied by value to set a different RepoDir. Rename terse variable names (bc/ic/dc) to branchGit/ignoreGit/dirGit. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Rewrite publish tests: table-driven through publishRun Consolidate 35 test functions into 2: TestNewCmdPublish (4 cases for CLI arg parsing) and TestPublishRun (22 cases exercising all behavior through the command's run function). No individual helper function tests — every codepath tested through publishRun scenarios. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Remove .gitkeep from acceptance/testdata/skills Delete the placeholder .gitkeep file from acceptance/testdata/skills. The directory no longer needs a placeholder file to be tracked in the repository. Rename testPublishGitClient to newTestGitClient Rename the test helper function testPublishGitClient to newTestGitClient in pkg/cmd/skills/publish/publish_test.go and update all call sites accordingly. This is a purely refactor/name-change with no behavioral changes to tests. Fix Windows CI: set USERPROFILE alongside HOME in tests os.UserHomeDir() uses USERPROFILE on Windows, not HOME. All tests that redirect HOME for lockfile isolation now also set USERPROFILE to the same temp directory. Co-Authored-By: Copilot <223556219+Copilot@users.noreply.github.com> Use range-over-int in acquireLock retry loop Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Test lock acquisition edge cases through RecordInstall Make lockRetries and lockRetryInterval configurable (package-level vars) so tests can avoid the 3s retry wait. Add two RecordInstall cases: - Stale lock (>30s old) is broken and install succeeds - Fresh lock exhausts retries, proceeds best-effort without lock Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Rename test helpers for lockfile tests Rename setupHome to setupTestHome and readLockfile to readTestLockfile in internal/skills/lockfile tests, and update all call sites and comments accordingly. This is a refactor-only change to clarify test helper names with no behavior change. Co-Authored-By: Copilot <223556219+Copilot@users.noreply.github.com> Test read() degradation through RecordInstall, delete TestRead Move corrupt JSON and wrong version cases into TestRecordInstall table. RecordInstall calls read() internally, so these exercise the same degradation paths through the public API. Verifies the lockfile is rewritten with correct version and new data after recovery. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Fix InstalledAt preservation test to actually prove preservation Move the update-preserves-InstalledAt case out of the table into a standalone subtest that reads InstalledAt between two RecordInstall calls and asserts exact equality. The table version only checked NotEmpty which couldn't detect if InstalledAt was overwritten. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Merge duplicate plugin test into TestMatchSkillConventions table The standalone TestDuplicatePluginSkills_DifferentAuthors re-implemented dedup logic that belongs in DiscoverSkills. Replace with a table case that tests convention matching only. Dedup is already covered by TestDiscoverSkills. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Fix broken validateName max-length test case Replace make([]byte, N) (which produces null bytes) with strings.Repeat to actually test the 64-character boundary. Add positive test for valid 64-char name. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Replace name-matching hack with createDir field in TestDiscoverLocalSkills Use a struct field instead of comparing tt.name to control whether the test directory is created. Prevents silent breakage if someone renames the test case. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Improve collisions tests: table-driven FormatCollisions, exercise DisplayName Convert TestFormatCollisions to table test with nil-input case. Update single collision case to use different conventions (plugins vs skills) so DisplayName() logic is actually exercised in the assertion. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Add tests for MatchesSkillPath, DiscoverSkillFiles, ListSkillFiles, FetchDescriptionsConcurrent Also cover previously untested branches: root convention matching, annotated tag dereference failure, empty tag_name/default_branch fallbacks, recursive walkTree with subtrees, and skill directory deduplication. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Test full GitHub key stripping in InjectLocalMetadata Add all 7 github-* keys to the input metadata and assert all are absent after injection. Previously only tested github-owner and github-repo removal. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Test Serialize trailing-newline addition for body without newline Add case where body doesn't end in newline and assert the output has one appended. Previously this branch was uncovered. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Test InjectGitHubMetadata with no existing frontmatter Add case where content has no --- delimiters, exercising the RawYAML == nil branch that creates frontmatter from scratch. Also fix test data to use GitHub-branded names. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Convert TestInjectLocalMetadata to table-driven with no-metadata case Add case for content with no frontmatter, exercising the meta == nil branch. Aligns with table-driven pattern used throughout. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Replace name-matching hack with useAgentHost field in TestInstallLocal Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Add tests for ResolveGitRoot Cover RepoDir shortcut, nil client fallback, and empty RepoDir fallback. Skip ResolveHomeDir — it's a thin os.UserHomeDir wrapper with no logic to test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Test OnProgress callback in both single and multi-skill Install paths Cover the progress reporting branches in Install for both the single-skill fast path (len==1) and the concurrent multi-skill path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Cover missing InstallDir error branches and malformed URL in registry Add user-scope-without-homeDir and invalid-scope cases to TestInstallDir. Add malformed URL case to TestRepoNameFromRemote. Coverage 80.5% → 87.8%. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Rewrite install tests: table-driven through installRun and runLocalInstall Consolidate 48 individual test functions into 6: TestNewCmdInstall (10 cases for CLI parsing), TestInstallRun (21 cases for remote install flow), TestRunLocalInstall (10 cases for local install flow), plus TestIsLocalPath, TestIsSkillPath, TestFriendlyDir for pure input classification. Delete zero-value Help test. All behavior tested through public functions instead of calling internal helpers directly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Fix data race in OnProgress test with atomic counter The OnProgress callback was appending to a shared slice from concurrent goroutines. Replace with sync/atomic counter to avoid the race. Co-Authored-By: Copilot <223556219+Copilot@users.noreply.github.com> Add interactive install tests for skill selection, scope, host, and overwrite Exercise the interactive TTY paths in installRun: MultiSelectWithSearch for skill selection, Select for scope prompt, MultiSelect for host selection, and Confirm for overwrite declined. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Exercise skillSearchFunc fully through interactive mock Update the interactive skill selection test to use 31 skills (exceeding maxSearchResults cap), include a skill without a description, and have the mock call searchFunc with both empty and filtered queries. Verifies the MoreResults count, label formatting, and truncation branches. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Fill remaining install coverage gaps Add local path detection cases to TestNewCmdInstall. Add interactive repo prompt, user scope selection, overwrite without metadata, and single exact match cases to TestInstallRun. Add bare tilde expansion to TestRunLocalInstall. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Move HOME/USERPROFILE setenv to test loops, remove per-case duplication Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Add isTTY field to install test tables, centralize TTY setup Move TTY configuration from individual opts funcs into the test loops. Each table case declares isTTY: true/false and the loop sets all three streams accordingly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Remove INSTALL_TARGET env var hack from install test Metadata injection is already proven by installer package tests. This test only needs to verify installRun orchestrates correctly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Add ScopeChanged: true to all install tests with explicit Scope Ensures tests simulate the same state cobra produces when --scope is explicitly provided, preventing silent codepath divergence if the default scope behavior changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Fix assert.Error → require.Error in TestNewCmdSearch Prevents nil panic on err.Error() if the command unexpectedly returns nil. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Improve preview test quality and coverage - Fix assert.Error/assert.NoError → require.Error/require.NoError to prevent nil panics in TestNewCmdPreview and TestPreviewRun - Add renderAllFiles edge case tests: maxFiles cap (20 files), maxBytes cap (512KB), and FetchBlob error fallback message - Replace custom discardWriter with io.Discard - Use GitHub-branded names (monalisa) in new tests Co-Authored-By: Copilot <223556219+Copilot@users.noreply.github.com> Add search test coverage: rate limits, owner scope, blob enrichment - Add HTTP 429 and 403+Retry-After rate limit test cases - Add owner-scoped no-results test (exercises noResultsMessage branch) - Add blob description enrichment test (exercises fetchDescriptions path) - Replace custom splitOnSpaces with strings.Fields - Replace custom discardWriter with io.Discard Co-Authored-By: Copilot <223556219+Copilot@users.noreply.github.com> Remove low-value alias test for preview command The test only asserts a string literal matches another string literal. Alias presence is already visible in the command definition. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Replace local pluralize with text.Pluralize The internal/text package already provides this function via go-gh. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Inline collapseWhitespace — just strings.Fields + Join Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Doc: suggest using go-humanize for star formatting Co-Authored-By: Copilot <223556219+Copilot@users.noreply.github.com> Return cmdutil.CancelError on user cancellation in publish and update Both commands returned nil (success exit) when the user declined confirmation. The core CLI pattern is to return CancelError so the process exits with a non-zero status. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Add interactive publish prompt tests and isTTY field Cover all prompt branches in runPublishRelease: - Topic confirm + semver tag selection + final confirm (happy path) - Custom tag input path (select idx=1) - Final confirm declined (CancelError) - Immutable releases prompt (enable via PATCH) Add isTTY field to test table struct for centralized TTY setup, matching the pattern used in install tests. Add auto-confirm prompters to existing TTY tests that now need them. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Remove duplicate giturl import alias in publish The git package was imported twice — once as 'git' and again as 'giturl'. Use git.ParseURL directly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Fix data race in search enrichment fetchDescriptions and fetchRepoStars run concurrently but both wrote to fields of the same skillResult slice elements, triggering the race detector. Refactor both functions to return index-keyed maps instead of mutating the slice directly. enrichSkills merges the maps into the slice after both goroutines complete. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> refactor: remove Claude plugin branding, align with Open Plugin Spec Replace all 'Claude plugin' references with generic 'plugin' terminology to align with the vendor-neutral Open Plugin Spec (https://github.com/vercel-labs/open-plugin-spec). Changes: - Rename .claude-plugin/ to .plugin/ (spec §5.1 vendor-neutral manifest) - Rename claudePluginJSON/claudeAuthor types to pluginJSON/pluginAuthor - Rename claudeMarketplaceJSON to marketplaceJSON - Rename generateClaudePlugin to generatePlugin - Remove 'Claude Code' from plugin-related comments, help text, and flags - Update install.go plugins/ convention message Factual host references (Claude Code as an agent name, .claude/skills directories) are intentionally preserved — those are product names, not plugin branding. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Remove --plugins flag from publish command Remove the --plugins flag and all associated plugin generation code from the publish flow. This was scope creep — the publish command should focus on validating and publishing skills, not generating plugin manifests. Removed: - --plugins flag and Plugins option field - generatePlugin, generateMarketplace, buildPluginDescription functions - pluginJSON, marketplaceJSON, marketplacePlugin types - Related tests and help text The install command's ability to discover and pluck skills from plugin- structured repositories (plugins/ convention) is preserved. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> don't fall back on default branch if you can't fetch latest release improve search algo by using square rot instead of log for stars, and reduce weight for exact name match add support for --unpin flag when updating a skill
…t it: cursor, codex, gemini CLI, github copilot, antigravity. excluded: claude code
remove git sha because we only need git tree sha remove github-owner from frontmatter, and make github-repo support full url. Only support github.com as host, error out otherwise
support specifying a sha in gh skills preview command
Co-authored-by: Sam Morrow <info@sam-morrow.com>
There was a problem hiding this comment.
Pull request overview
Adds a new preview “gh skill/skills” command group to GitHub CLI for discovering, previewing, installing, publishing, and updating markdown-based agent skills (SKILL.md) from GitHub repositories, including host/scope-aware install locations and metadata/lockfile support.
Changes:
- Introduces
gh skillroot command and wires it intoghroot. - Implements core skill plumbing: discovery conventions, frontmatter metadata injection, host registry, lockfile + cross-process locking, and installer.
- Adds
skill search,skill preview, andskill updatecommands plus unit + acceptance test coverage and supporting testscript fixtures.
Show a summary per file
| File | Description |
|---|---|
| pkg/cmd/skills/update/update.go | Implements gh skill update including scanning installed skills, prompting for missing metadata, and reinstalling updated skills. |
| pkg/cmd/skills/skills.go | Adds top-level gh skill command group and subcommand registration. |
| pkg/cmd/skills/search/search.go | Implements gh skill search via GitHub Code Search with ranking, enrichment, paging, JSON output, and interactive install flow. |
| pkg/cmd/skills/search/search_test.go | Unit tests for search flags/validation, ranking/deduping, and API behaviors (including rate limiting). |
| pkg/cmd/skills/preview/preview.go | Implements gh skill preview to resolve refs, discover skills, render a file tree, and page rendered content (interactive + non-interactive). |
| pkg/cmd/skills/preview/preview_test.go | Unit tests for preview resolution, interactive selection, file browsing, and rendering limits. |
| pkg/cmd/root/root.go | Wires the new skill command into the CLI root. |
| internal/skills/source/source.go | Adds supported-host enforcement and repo URL parsing/building for metadata. |
| internal/skills/source/source_test.go | Tests for repo URL building/parsing and supported-host validation. |
| internal/skills/registry/registry.go | Defines supported agent hosts and install-dir resolution for project/user scopes. |
| internal/skills/registry/registry_test.go | Tests agent lookup, directory resolution, and helper utilities. |
| internal/skills/lockfile/lockfile.go | Adds .skill-lock.json read/repair/write logic with file locking and install recording. |
| internal/skills/lockfile/lockfile_test.go | Tests lockfile creation, updates, corruption recovery, and lock contention handling. |
| internal/skills/installer/installer.go | Implements remote/local install flows, metadata injection, safe path handling, and lockfile recording. |
| internal/skills/installer/installer_test.go | Tests install behaviors (local/remote, metadata injection, traversal defense, progress, lockfile writing). |
| internal/skills/frontmatter/frontmatter.go | Frontmatter parsing + serialization and GitHub/local metadata injection helpers. |
| internal/skills/frontmatter/frontmatter_test.go | Tests parsing/serialization and metadata injection semantics. |
| internal/skills/discovery/discovery.go | Repo/local skill discovery (multiple conventions), ref resolution, file listing, blob fetching, and name validation. |
| internal/skills/discovery/collisions.go | Detects install-name collisions across discovered skills. |
| internal/skills/discovery/collisions_test.go | Tests collision detection and formatting. |
| internal/flock/flock.go | Adds OS-agnostic sentinel error for lock contention. |
| internal/flock/flock_unix.go | Implements non-blocking exclusive file locks on Unix via flock. |
| internal/flock/flock_windows.go | Implements non-blocking exclusive file locks on Windows via LockFileEx. |
| internal/flock/flock_test.go | Tests lock acquisition, contention behavior, and error cases. |
| go.mod | Promotes golang.org/x/sys to a direct dependency (needed for Windows locking). |
| git/client.go | Adds RemoteURL, IsIgnored, and ShortSHA helpers used by skills UX and other flows. |
| git/client_test.go | Tests new git client helpers (RemoteURL, IsIgnored, ShortSHA). |
| acceptance/acceptance_test.go | Adds a TestSkills testscript suite entrypoint and import ordering adjustment. |
| acceptance/testdata/skills/skills-update.txtar | Acceptance coverage for update flows (dry-run + force reinstall) against a local skills dir. |
| acceptance/testdata/skills/skills-update-noinstalled.txtar | Acceptance coverage for update with no installed skills. |
| acceptance/testdata/skills/skills-search.txtar | Acceptance coverage for search, JSON output, and short-query validation. |
| acceptance/testdata/skills/skills-search-page.txtar | Acceptance coverage for pagination behavior. |
| acceptance/testdata/skills/skills-search-noresults.txtar | Acceptance coverage for no-results behavior in non-TTY. |
| acceptance/testdata/skills/skills-publish-lifecycle.txtar | End-to-end publish/install/preview/update lifecycle acceptance script. |
| acceptance/testdata/skills/skills-publish-dry-run.txtar | Acceptance coverage for publish validation dry-run paths and aliases. |
| 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 requiring a skill name. |
| acceptance/testdata/skills/skills-install.txtar | Acceptance coverage for install into user scope and custom --dir with metadata + lockfile checks. |
| 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 pinned vs unpinned install behavior. |
| acceptance/testdata/skills/skills-install-nonexistent-skill.txtar | Acceptance coverage for installing a missing skill from a valid repo. |
| acceptance/testdata/skills/skills-install-nested-files.txtar | Acceptance coverage for installing skills with nested directories. |
| acceptance/testdata/skills/skills-install-invalid-repo.txtar | Acceptance coverage for invalid/nonexistent repo errors. |
| acceptance/testdata/skills/skills-install-invalid-agent.txtar | Acceptance coverage for invalid agent ID flag validation output. |
| acceptance/testdata/skills/skills-install-from-local.txtar | Acceptance coverage for local directory installs with local metadata injection. |
| acceptance/testdata/skills/skills-install-force.txtar | Acceptance coverage for overwrite semantics with --force. |
| .gitignore | Ignores gh binary/artifact. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 52/53 changed files
- Comments generated: 5
| result, err := frontmatter.Parse(string(data)) | ||
| if err != nil { | ||
| return installedSkill{}, false | ||
| } |
There was a problem hiding this comment.
parseInstalledSkill returns ok=false when frontmatter.Parse errors (e.g. malformed YAML frontmatter). That causes scanInstalledSkills to silently skip the skill entirely, so gh skill update won’t report or skip it with a warning (despite later logic intending to skip invalid metadata per-skill). Consider returning ok=true with metadataErr set (or a dedicated parse error field) so the caller can warn and continue, instead of dropping the skill from the scan.
- Update comment in relevanceScore() from '10 000 points' to '3 000 points' to match the actual implementation value of 3_000 - Update corresponding test comment for consistency - Fix typo: 'swaped' → 'swapped' in formatStars comment Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…, fail on path traversal - preview: remove 'fetched > 0' guard so the 512KB size cap applies uniformly to all files including the first - update: return skills with corrupted YAML frontmatter with metadataErr set instead of silently dropping them from scan results - installer: fail installation when a path traversal is detected in remote or local skill files instead of silently skipping Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When a directory argument is provided to `gh skill publish`, the remote detection now correctly uses the target directory's git remotes instead of the current working directory's remotes. Previously, `detectGitHubRemote` used the factory-provided git client which pointed to the CWD. This meant that running `gh skill publish /path/to/repo-bar` from inside repo-foo would detect repo-foo's remotes and potentially create the release on the wrong repo. The fix copies the git client and sets `RepoDir` to the target directory, matching the pattern already used by `detectMissingRepoDiagnostic` and `checkInstalledSkillDirs`. Co-authored-by: BagToad <47394200+BagToad@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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>
Remove the now-unused newTestGitClient helper that was left behind after refactoring tests to use initGitRepo directly. This fixes the golangci-lint 'unused' error in CI. Co-authored-by: BagToad <47394200+BagToad@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- 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>
Add cmdutil.DisableAuthCheckFlag for --from-local on install so that installing from a local directory does not require authentication. This follows the same pattern used by attestation verify for its --bundle flag. The --dry-run flag on publish is intentionally left with auth enabled because dry-run validation includes remote repository checks (security settings, tag protection, topics). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…local-flags Disable auth check for local-only skill flags
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>
…nt-path URL-encode parentPath in skills discovery API call
Co-authored-by: Kynan Ware <47394200+BagToad@users.noreply.github.com>
…remote-detection
Signed-off-by: Babak K. Shandiz <babakks@github.com>
Signed-off-by: Babak K. Shandiz <babakks@github.com>
docs(skill): polish skill commadset docs
Replace the hardcoded skills/ directory requirement in the publish
command with the shared DiscoverLocalSkills() function used by install
and other commands. This removes the opinionated restriction that skills
must live under a skills/ directory and supports all discovery
conventions:
- skills/*/SKILL.md
- skills/{scope}/*/SKILL.md
- */SKILL.md (root-level)
- plugins/{scope}/skills/*/SKILL.md
All per-skill validation (frontmatter, spec-compliant naming, metadata
stripping, etc.) is preserved.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Like gh pr create, skill publish now automatically pushes unpushed
local commits before creating a release. This prevents the footgun
where a release is created against stale remote state when the user
has local commits that haven't been pushed yet.
The ensurePushed function checks for unpushed commits using
git rev-list @{push}..HEAD. If commits exist or the branch has
never been pushed, it pushes automatically and prints a status
message. This matches the CLI's opinionated-defaults philosophy
of doing the right thing by default.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…hed-check feat(skills): auto-push unpushed commits before publish
…discovery Publish: use shared discovery logic instead of requiring skills/ directory
) Remove the dead code block that silently swallowed errors from opts.HttpClient() and opts.Config() when creating an API client. Instead, create the client with proper error propagation inside the remote-checks block where it is actually needed. Changes: - Remove the error-swallowing client == nil fallback (lines 363-376) - Create the API client inside the remote repo checks block with proper error returns from HttpClient() and Config() - Resolve host from repoInfo first, then fall back to config - Remove the now-unreachable 'client == nil' early exit before publish Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…e-dedup fix: preserve namespace in skills search deduplication
The skills-publish-dry-run acceptance test expected 'no skills/ directory found' on stderr, but the actual error message from discovery is 'no skills found in <dir>'. Update the stderr matcher accordingly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ce-test Fix skills-publish-dry-run acceptance test error message mismatch
Summary
Adds
gh skill(aliased asgh skills) a new command group for installing and managing agent skills from GitHub repositories. Skills are markdown-based instruction files (SKILL.md) that extend AI coding agents with reusable capabilities.This is a preview feature and subject to change.
Commands
gh skill install- Install skills from a GitHub repository or local directory. Supports multiple agent hosts (Copilot, Claude Code, Cursor, Codex, Gemini CLI, Antigravity) with project-scope and user-scope placement. Includes--pinfor version pinning,--agent/--scopefor placement control, and interactive host selection in TTY mode.gh skill preview- Render a skill'sSKILL.mdin the terminal via the configured pager without installing. Includes a file tree and interactive file picker for multi-file skills.gh skill search- Search public repositories for skills using the GitHub Code Search API. Results are ranked by relevance with interactive install support. Supports--json,--jq, and--templateoutput flags.gh skill update- Check installed skills for updates by comparing local tree SHAs against remote. Scans all known host directories automatically. Supports--force,--all, and--unpin.gh skill publish- Validate skills against the Agent Skills specification and publish via GitHub releases. Checks naming rules, frontmatter fields, and directory conventions. Supports--dry-runand--tagfor non-interactive use.