Expand test coverage and fix invariants/bugs

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
This commit is contained in:
Kynan Ware 2026-04-03 12:37:47 -06:00 committed by Sam Morrow
parent 40b2a784e3
commit 8ea84d0dee
No known key found for this signature in database
19 changed files with 5464 additions and 2649 deletions

View file

View file

@ -21,13 +21,13 @@ func TestFindNameCollisions(t *testing.T) {
want: nil,
},
{
name: "single collision",
name: "single collision with different conventions",
skills: []Skill{
{Name: "pr-summary", Path: "skills/pr-summary"},
{Name: "pr-summary", Path: "skills/monalisa/pr-summary"},
{Name: "pr-summary", Path: "plugins/hubot/skills/pr-summary", Convention: "plugins"},
},
want: []NameCollision{
{Name: "pr-summary", DisplayNames: []string{"pr-summary", "pr-summary"}},
{Name: "pr-summary", DisplayNames: []string{"pr-summary", "[plugins] pr-summary"}},
},
},
{
@ -53,10 +53,28 @@ func TestFindNameCollisions(t *testing.T) {
}
func TestFormatCollisions(t *testing.T) {
collisions := []NameCollision{
{Name: "pr-summary", DisplayNames: []string{"skills/pr-summary", "plugins/hubot/pr-summary"}},
{Name: "code-review", DisplayNames: []string{"skills/code-review", "skills/monalisa/code-review"}},
tests := []struct {
name string
collisions []NameCollision
want string
}{
{
name: "formats multiple collisions",
collisions: []NameCollision{
{Name: "pr-summary", DisplayNames: []string{"skills/pr-summary", "plugins/hubot/pr-summary"}},
{Name: "code-review", DisplayNames: []string{"skills/code-review", "skills/monalisa/code-review"}},
},
want: "pr-summary: skills/pr-summary, plugins/hubot/pr-summary\n code-review: skills/code-review, skills/monalisa/code-review",
},
{
name: "nil input returns empty string",
collisions: nil,
want: "",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equal(t, tt.want, FormatCollisions(tt.collisions))
})
}
got := FormatCollisions(collisions)
assert.Equal(t, "pr-summary: skills/pr-summary, plugins/hubot/pr-summary\n code-review: skills/code-review, skills/monalisa/code-review", got)
}

View file

@ -2,8 +2,12 @@ package discovery
import (
"net/http"
"os"
"path/filepath"
"strings"
"testing"
"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/v2/api"
"github.com/cli/cli/v2/pkg/httpmock"
"github.com/stretchr/testify/assert"
@ -73,6 +77,29 @@ func TestMatchSkillConventions(t *testing.T) {
path: "skills/code-review/README.md",
wantNil: true,
},
{
name: "plugin skill from different author",
path: "plugins/monalisa/skills/code-review/SKILL.md",
wantName: "code-review",
wantNamespace: "monalisa",
wantConvention: "plugins",
},
{
name: "root convention single-skill repo",
path: "code-review/SKILL.md",
wantName: "code-review",
wantConvention: "root",
},
{
name: "root convention excludes skills dir",
path: "skills/SKILL.md",
wantNil: true,
},
{
name: "root convention excludes dot-prefixed",
path: ".hidden/SKILL.md",
wantNil: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
@ -89,32 +116,6 @@ func TestMatchSkillConventions(t *testing.T) {
}
}
func TestDuplicatePluginSkills_DifferentAuthors(t *testing.T) {
entries := []treeEntry{
{Path: "plugins/monalisa/skills/code-review/SKILL.md", Type: "blob"},
{Path: "plugins/hubot/skills/code-review/SKILL.md", Type: "blob"},
}
seen := make(map[string]bool)
var matches []skillMatch
for _, e := range entries {
m := matchSkillConventions(e)
if m == nil || seen[m.skillDir] {
continue
}
seen[m.skillDir] = true
matches = append(matches, *m)
}
require.Len(t, matches, 2)
assert.Equal(t, "monalisa", matches[0].namespace)
assert.Equal(t, "hubot", matches[1].namespace)
assert.NotEqual(t,
Skill{Name: matches[0].name, Namespace: matches[0].namespace}.InstallName(),
Skill{Name: matches[1].name, Namespace: matches[1].namespace}.InstallName(),
)
}
func TestValidateName(t *testing.T) {
tests := []struct {
name string
@ -122,8 +123,8 @@ func TestValidateName(t *testing.T) {
want bool
}{
{name: "empty", input: "", want: false},
{name: "too long", input: string(make([]byte, 65)), want: false},
{name: "max length", input: "a" + string(make([]byte, 63)), want: false}, // 64 'a's would be valid but []byte gives null bytes
{name: "too long", input: strings.Repeat("a", 65), want: false},
{name: "max length is valid", input: strings.Repeat("a", 64), want: true},
{name: "contains slash", input: "foo/bar", want: false},
{name: "contains dotdot", input: "foo..bar", want: false},
{name: "starts with dot", input: ".hidden", want: false},
@ -261,6 +262,57 @@ func TestResolveRef(t *testing.T) {
wantRef: "main",
wantSHA: "branch-sha",
},
{
name: "annotated tag dereference failure",
version: "v4.0",
stubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/monalisa/octocat-skills/git/ref/tags/v4.0"),
httpmock.JSONResponse(map[string]interface{}{
"object": map[string]interface{}{"sha": "tag-obj-sha", "type": "tag"},
}))
reg.Register(
httpmock.REST("GET", "repos/monalisa/octocat-skills/git/tags/tag-obj-sha"),
httpmock.StatusStringResponse(500, "server error"))
},
wantErr: "could not dereference annotated tag",
},
{
name: "empty tag_name in latest release falls back to default branch",
stubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/monalisa/octocat-skills/releases/latest"),
httpmock.JSONResponse(map[string]interface{}{"tag_name": ""}))
reg.Register(
httpmock.REST("GET", "repos/monalisa/octocat-skills"),
httpmock.JSONResponse(map[string]interface{}{"default_branch": "main"}))
reg.Register(
httpmock.REST("GET", "repos/monalisa/octocat-skills/git/ref/heads/main"),
httpmock.JSONResponse(map[string]interface{}{
"object": map[string]interface{}{"sha": "fallback-sha"},
}))
},
wantRef: "main",
wantSHA: "fallback-sha",
},
{
name: "empty default_branch falls back to main",
stubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/monalisa/octocat-skills/releases/latest"),
httpmock.StatusStringResponse(404, "not found"))
reg.Register(
httpmock.REST("GET", "repos/monalisa/octocat-skills"),
httpmock.JSONResponse(map[string]interface{}{"default_branch": ""}))
reg.Register(
httpmock.REST("GET", "repos/monalisa/octocat-skills/git/ref/heads/main"),
httpmock.JSONResponse(map[string]interface{}{
"object": map[string]interface{}{"sha": "main-sha"},
}))
},
wantRef: "main",
wantSHA: "main-sha",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
@ -339,3 +391,549 @@ func TestFetchBlob(t *testing.T) {
})
}
}
func TestDiscoverSkills(t *testing.T) {
tests := []struct {
name string
stubs func(*httpmock.Registry)
wantSkills []string
wantErr string
}{
{
name: "discovers skills from tree",
stubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/monalisa/octocat-skills/git/trees/abc123"),
httpmock.JSONResponse(map[string]interface{}{
"sha": "abc123", "truncated": false,
"tree": []map[string]interface{}{
{"path": "skills/code-review", "type": "tree", "sha": "tree-sha-1"},
{"path": "skills/code-review/SKILL.md", "type": "blob", "sha": "blob-1"},
{"path": "skills/issue-triage", "type": "tree", "sha": "tree-sha-2"},
{"path": "skills/issue-triage/SKILL.md", "type": "blob", "sha": "blob-2"},
{"path": "README.md", "type": "blob", "sha": "readme"},
},
}))
},
wantSkills: []string{"code-review", "issue-triage"},
},
{
name: "truncated tree returns error",
stubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/monalisa/octocat-skills/git/trees/abc123"),
httpmock.JSONResponse(map[string]interface{}{
"sha": "abc123", "truncated": true, "tree": []map[string]interface{}{},
}))
},
wantErr: "too large",
},
{
name: "no skills found",
stubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/monalisa/octocat-skills/git/trees/abc123"),
httpmock.JSONResponse(map[string]interface{}{
"sha": "abc123", "truncated": false,
"tree": []map[string]interface{}{
{"path": "README.md", "type": "blob", "sha": "readme"},
},
}))
},
wantErr: "no skills found",
},
{
name: "API error",
stubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/monalisa/octocat-skills/git/trees/abc123"),
httpmock.StatusStringResponse(500, "server error"))
},
wantErr: "could not fetch repository tree",
},
{
name: "deduplicates skills from same directory",
stubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/monalisa/octocat-skills/git/trees/abc123"),
httpmock.JSONResponse(map[string]interface{}{
"sha": "abc123", "truncated": false,
"tree": []map[string]interface{}{
{"path": "skills/code-review", "type": "tree", "sha": "tree-sha"},
{"path": "skills/code-review/SKILL.md", "type": "blob", "sha": "blob-1"},
{"path": "skills/code-review/SKILL.md", "type": "blob", "sha": "blob-2"},
},
}))
},
wantSkills: []string{"code-review"},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
reg := &httpmock.Registry{}
defer reg.Verify(t)
tt.stubs(reg)
client := api.NewClientFromHTTP(&http.Client{Transport: reg})
skills, err := DiscoverSkills(client, "github.com", "monalisa", "octocat-skills", "abc123")
if tt.wantErr != "" {
require.Error(t, err)
assert.Contains(t, err.Error(), tt.wantErr)
return
}
require.NoError(t, err)
var names []string
for _, s := range skills {
names = append(names, s.Name)
}
assert.Equal(t, tt.wantSkills, names)
})
}
}
func TestDiscoverSkillByPath(t *testing.T) {
tests := []struct {
name string
skillPath string
stubs func(*httpmock.Registry)
wantName string
wantNS string
wantErr string
}{
{
name: "discovers skill by path",
skillPath: "skills/code-review",
stubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/monalisa/octocat-skills/contents/skills"),
httpmock.JSONResponse([]map[string]interface{}{
{"name": "code-review", "path": "skills/code-review", "sha": "tree-sha", "type": "dir"},
}))
reg.Register(
httpmock.REST("GET", "repos/monalisa/octocat-skills/git/trees/tree-sha"),
httpmock.JSONResponse(map[string]interface{}{
"sha": "tree-sha", "truncated": false,
"tree": []map[string]interface{}{
{"path": "SKILL.md", "type": "blob", "sha": "blob-sha"},
},
}))
reg.Register(
httpmock.REST("GET", "repos/monalisa/octocat-skills/git/blobs/blob-sha"),
httpmock.JSONResponse(map[string]interface{}{
"sha": "blob-sha", "encoding": "base64", "content": "IyBTa2lsbA==",
}))
},
wantName: "code-review",
},
{
name: "namespaced path sets namespace",
skillPath: "skills/monalisa/issue-triage",
stubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/monalisa/octocat-skills/contents/skills/monalisa"),
httpmock.JSONResponse([]map[string]interface{}{
{"name": "issue-triage", "path": "skills/monalisa/issue-triage", "sha": "tree-sha", "type": "dir"},
}))
reg.Register(
httpmock.REST("GET", "repos/monalisa/octocat-skills/git/trees/tree-sha"),
httpmock.JSONResponse(map[string]interface{}{
"sha": "tree-sha", "truncated": false,
"tree": []map[string]interface{}{
{"path": "SKILL.md", "type": "blob", "sha": "blob-sha"},
},
}))
reg.Register(
httpmock.REST("GET", "repos/monalisa/octocat-skills/git/blobs/blob-sha"),
httpmock.JSONResponse(map[string]interface{}{
"sha": "blob-sha", "encoding": "base64", "content": "IyBTa2lsbA==",
}))
},
wantName: "issue-triage",
wantNS: "monalisa",
},
{
name: "strips trailing SKILL.md from path",
skillPath: "skills/code-review/SKILL.md",
stubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/monalisa/octocat-skills/contents/skills"),
httpmock.JSONResponse([]map[string]interface{}{
{"name": "code-review", "path": "skills/code-review", "sha": "tree-sha", "type": "dir"},
}))
reg.Register(
httpmock.REST("GET", "repos/monalisa/octocat-skills/git/trees/tree-sha"),
httpmock.JSONResponse(map[string]interface{}{
"sha": "tree-sha", "truncated": false,
"tree": []map[string]interface{}{
{"path": "SKILL.md", "type": "blob", "sha": "blob-sha"},
},
}))
reg.Register(
httpmock.REST("GET", "repos/monalisa/octocat-skills/git/blobs/blob-sha"),
httpmock.JSONResponse(map[string]interface{}{
"sha": "blob-sha", "encoding": "base64", "content": "IyBTa2lsbA==",
}))
},
wantName: "code-review",
},
{
name: "invalid skill name",
skillPath: "skills/.hidden-skill",
wantErr: "invalid skill name",
},
{
name: "skill directory not found",
skillPath: "skills/nonexistent",
stubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/monalisa/octocat-skills/contents/skills"),
httpmock.JSONResponse([]map[string]interface{}{
{"name": "other-skill", "path": "skills/other-skill", "sha": "tree-sha", "type": "dir"},
}))
},
wantErr: "skill directory",
},
{
name: "no SKILL.md in directory",
skillPath: "skills/code-review",
stubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/monalisa/octocat-skills/contents/skills"),
httpmock.JSONResponse([]map[string]interface{}{
{"name": "code-review", "path": "skills/code-review", "sha": "tree-sha", "type": "dir"},
}))
reg.Register(
httpmock.REST("GET", "repos/monalisa/octocat-skills/git/trees/tree-sha"),
httpmock.JSONResponse(map[string]interface{}{
"sha": "tree-sha", "truncated": false,
"tree": []map[string]interface{}{
{"path": "README.md", "type": "blob", "sha": "readme"},
},
}))
},
wantErr: "no SKILL.md found",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
reg := &httpmock.Registry{}
defer reg.Verify(t)
if tt.stubs != nil {
tt.stubs(reg)
}
client := api.NewClientFromHTTP(&http.Client{Transport: reg})
skill, err := DiscoverSkillByPath(client, "github.com", "monalisa", "octocat-skills", "abc123", tt.skillPath)
if tt.wantErr != "" {
require.Error(t, err)
assert.Contains(t, err.Error(), tt.wantErr)
return
}
require.NoError(t, err)
assert.Equal(t, tt.wantName, skill.Name)
assert.Equal(t, tt.wantNS, skill.Namespace)
})
}
}
func TestDiscoverLocalSkills(t *testing.T) {
tests := []struct {
name string
createDir bool
setup func(t *testing.T, dir string)
wantSkills []string
wantErr string
}{
{
name: "discovers skills in skills/ directory",
createDir: true,
setup: func(t *testing.T, dir string) {
t.Helper()
for _, name := range []string{"code-review", "issue-triage"} {
skillDir := filepath.Join(dir, "skills", name)
require.NoError(t, os.MkdirAll(skillDir, 0o755))
require.NoError(t, os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte("# "+name), 0o644))
}
},
wantSkills: []string{"code-review", "issue-triage"},
},
{
name: "single skill at root",
createDir: true,
setup: func(t *testing.T, dir string) {
t.Helper()
require.NoError(t, os.WriteFile(filepath.Join(dir, "SKILL.md"), []byte(heredoc.Doc(`
---
name: root-skill
---
# Root
`)), 0o644))
},
wantSkills: []string{"root-skill"},
},
{
name: "no skills found",
createDir: true,
setup: func(t *testing.T, dir string) {
t.Helper()
require.NoError(t, os.WriteFile(filepath.Join(dir, "README.md"), []byte("# Not a skill"), 0o644))
},
wantErr: "no skills found",
},
{
name: "nonexistent directory",
setup: func(t *testing.T, dir string) {},
wantErr: "could not access",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
dir := filepath.Join(t.TempDir(), "repo")
if tt.createDir {
require.NoError(t, os.MkdirAll(dir, 0o755))
}
tt.setup(t, dir)
skills, err := DiscoverLocalSkills(dir)
if tt.wantErr != "" {
require.Error(t, err)
assert.Contains(t, err.Error(), tt.wantErr)
return
}
require.NoError(t, err)
var names []string
for _, s := range skills {
names = append(names, s.Name)
}
assert.ElementsMatch(t, tt.wantSkills, names)
})
}
}
func TestMatchesSkillPath(t *testing.T) {
tests := []struct {
name string
path string
wantName string
}{
{name: "skills convention", path: "skills/code-review/SKILL.md", wantName: "code-review"},
{name: "namespaced convention", path: "skills/monalisa/issue-triage/SKILL.md", wantName: "issue-triage"},
{name: "plugins convention", path: "plugins/hubot/skills/pr-summary/SKILL.md", wantName: "pr-summary"},
{name: "non-skill file", path: "README.md", wantName: ""},
{name: "non-SKILL.md in skill dir", path: "skills/code-review/prompt.txt", wantName: ""},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equal(t, tt.wantName, MatchesSkillPath(tt.path))
})
}
}
func TestDiscoverSkillFiles(t *testing.T) {
tests := []struct {
name string
stubs func(*httpmock.Registry)
wantPaths []string
wantErr string
}{
{
name: "returns files with skill path prefix",
stubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/monalisa/octocat-skills/git/trees/tree123"),
httpmock.JSONResponse(map[string]interface{}{
"sha": "tree123", "truncated": false,
"tree": []map[string]interface{}{
{"path": "SKILL.md", "type": "blob", "sha": "sha1", "size": 10},
{"path": "scripts/setup.sh", "type": "blob", "sha": "sha2", "size": 50},
{"path": "scripts", "type": "tree", "sha": "treesub"},
},
}))
},
wantPaths: []string{"skills/code-review/SKILL.md", "skills/code-review/scripts/setup.sh"},
},
{
name: "truncated tree falls back to walkTree",
stubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/monalisa/octocat-skills/git/trees/tree123"),
httpmock.JSONResponse(map[string]interface{}{
"sha": "tree123", "truncated": true, "tree": []map[string]interface{}{},
}))
reg.Register(
httpmock.REST("GET", "repos/monalisa/octocat-skills/git/trees/tree123"),
httpmock.JSONResponse(map[string]interface{}{
"sha": "tree123",
"tree": []map[string]interface{}{
{"path": "SKILL.md", "type": "blob", "sha": "sha1", "size": 10},
},
}))
},
wantPaths: []string{"skills/code-review/SKILL.md"},
},
{
name: "API error",
stubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/monalisa/octocat-skills/git/trees/tree123"),
httpmock.StatusStringResponse(500, "server error"))
},
wantErr: "could not fetch skill tree",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
reg := &httpmock.Registry{}
defer reg.Verify(t)
tt.stubs(reg)
client := api.NewClientFromHTTP(&http.Client{Transport: reg})
files, err := DiscoverSkillFiles(client, "github.com", "monalisa", "octocat-skills", "tree123", "skills/code-review")
if tt.wantErr != "" {
require.Error(t, err)
assert.Contains(t, err.Error(), tt.wantErr)
return
}
require.NoError(t, err)
var paths []string
for _, f := range files {
paths = append(paths, f.Path)
}
assert.Equal(t, tt.wantPaths, paths)
})
}
}
func TestListSkillFiles(t *testing.T) {
tests := []struct {
name string
stubs func(*httpmock.Registry)
wantPaths []string
wantErr string
}{
{
name: "returns relative paths",
stubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/monalisa/octocat-skills/git/trees/tree123"),
httpmock.JSONResponse(map[string]interface{}{
"sha": "tree123", "truncated": false,
"tree": []map[string]interface{}{
{"path": "SKILL.md", "type": "blob", "sha": "sha1", "size": 10},
{"path": "prompt.txt", "type": "blob", "sha": "sha2", "size": 20},
},
}))
},
wantPaths: []string{"SKILL.md", "prompt.txt"},
},
{
name: "truncated tree falls back to walkTree with nested subtree",
stubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/monalisa/octocat-skills/git/trees/tree123"),
httpmock.JSONResponse(map[string]interface{}{
"sha": "tree123", "truncated": true, "tree": []map[string]interface{}{},
}))
// walkTree fetches the top-level tree non-recursively
reg.Register(
httpmock.REST("GET", "repos/monalisa/octocat-skills/git/trees/tree123"),
httpmock.JSONResponse(map[string]interface{}{
"sha": "tree123",
"tree": []map[string]interface{}{
{"path": "SKILL.md", "type": "blob", "sha": "sha1", "size": 10},
{"path": "scripts", "type": "tree", "sha": "subtree1"},
},
}))
// walkTree recurses into the "scripts" subtree
reg.Register(
httpmock.REST("GET", "repos/monalisa/octocat-skills/git/trees/subtree1"),
httpmock.JSONResponse(map[string]interface{}{
"sha": "subtree1",
"tree": []map[string]interface{}{
{"path": "setup.sh", "type": "blob", "sha": "sha2", "size": 50},
},
}))
},
wantPaths: []string{"SKILL.md", "scripts/setup.sh"},
},
{
name: "API error",
stubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/monalisa/octocat-skills/git/trees/tree123"),
httpmock.StatusStringResponse(500, "server error"))
},
wantErr: "could not fetch skill tree",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
reg := &httpmock.Registry{}
defer reg.Verify(t)
tt.stubs(reg)
client := api.NewClientFromHTTP(&http.Client{Transport: reg})
files, err := ListSkillFiles(client, "github.com", "monalisa", "octocat-skills", "tree123")
if tt.wantErr != "" {
require.Error(t, err)
assert.Contains(t, err.Error(), tt.wantErr)
return
}
require.NoError(t, err)
var paths []string
for _, f := range files {
paths = append(paths, f.Path)
}
assert.Equal(t, tt.wantPaths, paths)
})
}
}
func TestFetchDescriptionsConcurrent(t *testing.T) {
tests := []struct {
name string
skills []Skill
stubs func(*httpmock.Registry)
wantDescs []string
}{
{
name: "fetches descriptions for skills without one",
skills: []Skill{
{Name: "code-review", BlobSHA: "blob1"},
{Name: "issue-triage", Description: "already set"},
},
stubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/monalisa/octocat-skills/git/blobs/blob1"),
httpmock.JSONResponse(map[string]interface{}{
"sha": "blob1", "encoding": "base64",
"content": "LS0tCm5hbWU6IGNvZGUtcmV2aWV3CmRlc2NyaXB0aW9uOiBSZXZpZXdzIFBScwotLS0KIyBUZXN0",
}))
},
wantDescs: []string{"Reviews PRs", "already set"},
},
{
name: "no-op when all descriptions set",
skills: []Skill{
{Name: "code-review", Description: "set"},
},
stubs: func(reg *httpmock.Registry) {},
wantDescs: []string{"set"},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
reg := &httpmock.Registry{}
defer reg.Verify(t)
tt.stubs(reg)
client := api.NewClientFromHTTP(&http.Client{Transport: reg})
FetchDescriptionsConcurrent(client, "github.com", "monalisa", "octocat-skills", tt.skills, nil)
var descs []string
for _, s := range tt.skills {
descs = append(descs, s.Description)
}
assert.Equal(t, tt.wantDescs, descs)
})
}
}

View file

@ -4,6 +4,7 @@ import (
"strings"
"testing"
"github.com/MakeNowJust/heredoc"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
@ -18,8 +19,14 @@ func TestParse(t *testing.T) {
wantErr bool
}{
{
name: "valid frontmatter",
content: "---\nname: test-skill\ndescription: A test skill\n---\n# Body\n",
name: "valid frontmatter",
content: heredoc.Doc(`
---
name: test-skill
description: A test skill
---
# Body
`),
wantName: "test-skill",
wantDesc: "A test skill",
wantBody: "# Body\n",
@ -71,18 +78,24 @@ func TestInjectGitHubMetadata(t *testing.T) {
wantNotContain []string
}{
{
name: "injects metadata without pin",
content: "---\nname: my-skill\ndescription: desc\n---\n# Body\n",
owner: "owner",
repo: "repo",
name: "injects metadata without pin",
content: heredoc.Doc(`
---
name: my-skill
description: desc
---
# Body
`),
owner: "monalisa",
repo: "octocat-skills",
ref: "v1.0.0",
sha: "abc123",
treeSHA: "tree456",
pinnedRef: "",
skillPath: "skills/my-skill",
wantContains: []string{
"github-owner: owner",
"github-repo: repo",
"github-owner: monalisa",
"github-repo: octocat-skills",
"github-ref: v1.0.0",
"github-sha: abc123",
"github-tree-sha: tree456",
@ -94,10 +107,15 @@ func TestInjectGitHubMetadata(t *testing.T) {
},
},
{
name: "injects pinned ref",
content: "---\nname: my-skill\n---\n# Body\n",
owner: "owner",
repo: "repo",
name: "injects pinned ref",
content: heredoc.Doc(`
---
name: my-skill
---
# Body
`),
owner: "monalisa",
repo: "octocat-skills",
ref: "v1.0.0",
sha: "abc",
treeSHA: "tree",
@ -107,6 +125,22 @@ func TestInjectGitHubMetadata(t *testing.T) {
"github-pinned: v1.0.0",
},
},
{
name: "injects metadata into content with no frontmatter",
content: "# Body only\n",
owner: "monalisa",
repo: "octocat-skills",
ref: "v1.0.0",
sha: "abc123",
treeSHA: "tree456",
pinnedRef: "",
skillPath: "skills/my-skill",
wantContains: []string{
"github-owner: monalisa",
"github-repo: octocat-skills",
"# Body only",
},
},
}
for _, tt := range tests {
@ -124,13 +158,49 @@ func TestInjectGitHubMetadata(t *testing.T) {
}
func TestInjectLocalMetadata(t *testing.T) {
content := "---\nname: my-skill\nmetadata:\n github-owner: old\n github-repo: old\n---\n# Body\n"
got, err := InjectLocalMetadata(content, "/home/user/skills/my-skill")
require.NoError(t, err)
assert.Contains(t, got, "local-path: /home/user/skills/my-skill")
assert.NotContains(t, got, "github-owner")
assert.NotContains(t, got, "github-repo")
tests := []struct {
name string
content string
wantContains []string
wantNotContain []string
}{
{
name: "strips all github keys and injects local-path",
content: heredoc.Doc(`
---
name: my-skill
metadata:
github-owner: old
github-repo: old
github-ref: v1.0.0
github-sha: abc123
github-tree-sha: tree456
github-pinned: v1.0.0
github-path: skills/my-skill
---
# Body
`),
wantContains: []string{"local-path: /home/monalisa/skills/my-skill"},
wantNotContain: []string{"github-owner", "github-repo", "github-ref", "github-sha", "github-tree-sha", "github-pinned", "github-path"},
},
{
name: "injects into content with no existing metadata",
content: "# Body only\n",
wantContains: []string{"local-path: /home/monalisa/skills/my-skill"},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := InjectLocalMetadata(tt.content, "/home/monalisa/skills/my-skill")
require.NoError(t, err)
for _, s := range tt.wantContains {
assert.Contains(t, got, s)
}
for _, s := range tt.wantNotContain {
assert.NotContains(t, got, s)
}
})
}
}
func TestSerialize(t *testing.T) {
@ -158,6 +228,12 @@ func TestSerialize(t *testing.T) {
body: "",
wantSuffix: "---\n",
},
{
name: "body without trailing newline gets one added",
frontmatter: map[string]interface{}{"name": "test"},
body: "# No trailing newline",
wantSuffix: "# No trailing newline\n",
},
}
for _, tt := range tests {

View file

@ -1,6 +1,7 @@
package installer
import (
"context"
"errors"
"fmt"
"os"
@ -9,6 +10,7 @@ import (
"sync"
"github.com/cli/cli/v2/api"
"github.com/cli/cli/v2/git"
"github.com/cli/cli/v2/internal/safepaths"
"github.com/cli/cli/v2/internal/skills/discovery"
"github.com/cli/cli/v2/internal/skills/frontmatter"
@ -295,3 +297,29 @@ func installSkill(opts *Options, skill discovery.Skill, baseDir string) error {
return nil
}
// ResolveGitRoot returns the git repository root using the provided client,
// falling back to the current working directory on error.
func ResolveGitRoot(gc *git.Client) string {
if gc != nil && gc.RepoDir != "" {
return gc.RepoDir
}
if gc != nil {
if root, err := gc.ToplevelDir(context.Background()); err == nil {
return root
}
}
if cwd, err := os.Getwd(); err == nil {
return cwd
}
return ""
}
// ResolveHomeDir returns the user's home directory, or "" on error.
func ResolveHomeDir() string {
home, err := os.UserHomeDir()
if err != nil {
return ""
}
return home
}

View file

@ -6,26 +6,30 @@ import (
"net/http"
"os"
"path/filepath"
"strings"
"sync/atomic"
"testing"
"github.com/cli/cli/v2/api"
"github.com/cli/cli/v2/git"
"github.com/cli/cli/v2/internal/skills/discovery"
"github.com/cli/cli/v2/internal/skills/registry"
"github.com/cli/cli/v2/pkg/httpmock"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestInstallLocalSkill(t *testing.T) {
func TestInstallLocal(t *testing.T) {
tests := []struct {
name string
skill discovery.Skill
setup func(t *testing.T, srcDir string)
verify func(t *testing.T, destDir string)
name string
skills []discovery.Skill
useAgentHost bool
setup func(t *testing.T, srcDir string)
verify func(t *testing.T, destDir string)
wantErr string
}{
{
name: "copies files",
skill: discovery.Skill{Name: "code-review", Path: "skills/code-review"},
name: "copies files via Dir",
skills: []discovery.Skill{{Name: "code-review", Path: "skills/code-review"}},
setup: func(t *testing.T, srcDir string) {
t.Helper()
skillSrc := filepath.Join(srcDir, "skills", "code-review")
@ -44,8 +48,8 @@ func TestInstallLocalSkill(t *testing.T) {
},
},
{
name: "nested directories",
skill: discovery.Skill{Name: "issue-triage", Path: "skills/issue-triage"},
name: "nested directories",
skills: []discovery.Skill{{Name: "issue-triage", Path: "skills/issue-triage"}},
setup: func(t *testing.T, srcDir string) {
t.Helper()
deep := filepath.Join(srcDir, "skills", "issue-triage", "prompts", "templates")
@ -62,15 +66,15 @@ func TestInstallLocalSkill(t *testing.T) {
},
},
{
name: "skips symlinks",
skill: discovery.Skill{Name: "pr-summary", Path: "skills/pr-summary"},
name: "skips symlinks",
skills: []discovery.Skill{{Name: "pr-summary", Path: "skills/pr-summary"}},
setup: func(t *testing.T, srcDir string) {
t.Helper()
skillSrc := filepath.Join(srcDir, "skills", "pr-summary")
require.NoError(t, os.MkdirAll(skillSrc, 0o755))
require.NoError(t, os.WriteFile(filepath.Join(skillSrc, "SKILL.md"), []byte("# PR Summary"), 0o644))
require.NoError(t, os.WriteFile(filepath.Join(skillSrc, "prompt.txt"), []byte("summarize"), 0o644))
os.Symlink(filepath.Join(skillSrc, "prompt.txt"), filepath.Join(skillSrc, "link.txt"))
require.NoError(t, os.Symlink(filepath.Join(skillSrc, "prompt.txt"), filepath.Join(skillSrc, "link.txt")))
},
verify: func(t *testing.T, destDir string) {
t.Helper()
@ -81,8 +85,8 @@ func TestInstallLocalSkill(t *testing.T) {
},
},
{
name: "injects metadata into SKILL.md",
skill: discovery.Skill{Name: "copilot-helper", Path: "skills/copilot-helper"},
name: "injects metadata into SKILL.md",
skills: []discovery.Skill{{Name: "copilot-helper", Path: "skills/copilot-helper"}},
setup: func(t *testing.T, srcDir string) {
t.Helper()
skillSrc := filepath.Join(srcDir, "skills", "copilot-helper")
@ -93,10 +97,53 @@ func TestInstallLocalSkill(t *testing.T) {
t.Helper()
content, err := os.ReadFile(filepath.Join(destDir, "copilot-helper", "SKILL.md"))
require.NoError(t, err)
assert.True(t, strings.Contains(string(content), "local-path"),
"expected SKILL.md to contain local-path metadata, got: %s", string(content))
assert.Contains(t, string(content), "local-path")
},
},
{
name: "multiple skills",
skills: []discovery.Skill{
{Name: "code-review", Path: "skills/code-review"},
{Name: "issue-triage", Path: "skills/issue-triage"},
},
setup: func(t *testing.T, srcDir string) {
t.Helper()
for _, name := range []string{"code-review", "issue-triage"} {
skillSrc := filepath.Join(srcDir, "skills", name)
require.NoError(t, os.MkdirAll(skillSrc, 0o755))
require.NoError(t, os.WriteFile(filepath.Join(skillSrc, "SKILL.md"), []byte("# "+name), 0o644))
}
},
verify: func(t *testing.T, destDir string) {
t.Helper()
_, err := os.Stat(filepath.Join(destDir, "code-review", "SKILL.md"))
assert.NoError(t, err)
_, err = os.Stat(filepath.Join(destDir, "issue-triage", "SKILL.md"))
assert.NoError(t, err)
},
},
{
name: "resolves install dir from AgentHost and Scope",
skills: []discovery.Skill{{Name: "code-review", Path: "skills/code-review"}},
useAgentHost: true,
setup: func(t *testing.T, srcDir string) {
t.Helper()
skillSrc := filepath.Join(srcDir, "skills", "code-review")
require.NoError(t, os.MkdirAll(skillSrc, 0o755))
require.NoError(t, os.WriteFile(filepath.Join(skillSrc, "SKILL.md"), []byte("# Code Review"), 0o644))
},
verify: func(t *testing.T, destDir string) {
t.Helper()
_, err := os.Stat(filepath.Join(destDir, ".github", "skills", "code-review", "SKILL.md"))
assert.NoError(t, err)
},
},
{
name: "no dir or agent host",
skills: []discovery.Skill{{Name: "code-review"}},
setup: func(t *testing.T, srcDir string) {},
wantErr: "either Dir or AgentHost must be specified",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
@ -104,8 +151,32 @@ func TestInstallLocalSkill(t *testing.T) {
destDir := t.TempDir()
tt.setup(t, srcDir)
err := installLocalSkill(srcDir, tt.skill, destDir)
opts := &LocalOptions{
SourceDir: srcDir,
Skills: tt.skills,
Dir: destDir,
}
if tt.useAgentHost {
host, err := registry.FindByID("github-copilot")
require.NoError(t, err)
opts.Dir = ""
opts.AgentHost = host
opts.Scope = registry.ScopeProject
opts.GitRoot = destDir
}
if tt.wantErr != "" {
opts.Dir = ""
}
result, err := InstallLocal(opts)
if tt.wantErr != "" {
require.Error(t, err)
assert.Contains(t, err.Error(), tt.wantErr)
return
}
require.NoError(t, err)
assert.NotEmpty(t, result.Dir)
assert.Len(t, result.Installed, len(tt.skills))
tt.verify(t, destDir)
})
}
@ -258,23 +329,31 @@ func stubTreeAndBlob(reg *httpmock.Registry, treeSHA string) {
}
func TestInstall(t *testing.T) {
var progressCount atomic.Int32
tests := []struct {
name string
skills []discovery.Skill
stubs func(*httpmock.Registry)
onProgress func(done, total int)
wantInstalled []string
wantErr string
}{
{
name: "single skill",
name: "single skill calls OnProgress",
skills: []discovery.Skill{
{Name: "code-review", Path: "skills/code-review", TreeSHA: "tree-cr"},
},
stubs: func(reg *httpmock.Registry) { stubTreeAndBlob(reg, "tree-cr") },
stubs: func(reg *httpmock.Registry) { stubTreeAndBlob(reg, "tree-cr") },
onProgress: func(done, total int) {
progressCount.Add(1)
},
wantInstalled: []string{"code-review"},
},
{
name: "multiple skills concurrently",
name: "multiple skills concurrently with progress",
skills: []discovery.Skill{
{Name: "code-review", Path: "skills/code-review", TreeSHA: "tree-cr"},
{Name: "issue-triage", Path: "skills/issue-triage", TreeSHA: "tree-it"},
@ -283,8 +362,28 @@ func TestInstall(t *testing.T) {
stubTreeAndBlob(reg, "tree-cr")
stubTreeAndBlob(reg, "tree-it")
},
onProgress: func(done, total int) {
progressCount.Add(1)
},
wantInstalled: []string{"code-review", "issue-triage"},
},
{
name: "partial failure returns successful installs and error",
skills: []discovery.Skill{
{Name: "code-review", Path: "skills/code-review", TreeSHA: "tree-cr"},
{Name: "issue-triage", Path: "skills/issue-triage", TreeSHA: "tree-fail"},
},
stubs: func(reg *httpmock.Registry) {
stubTreeAndBlob(reg, "tree-cr")
reg.Register(
httpmock.REST("GET", "repos/monalisa/octocat-skills/git/trees/tree-fail"),
httpmock.StatusStringResponse(500, "server error"))
},
wantInstalled: []string{"code-review"},
wantErr: "failed to install skill",
},
{
name: "no dir or agent host",
skills: []discovery.Skill{{Name: "code-review"}},
@ -294,7 +393,10 @@ func TestInstall(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Setenv("HOME", t.TempDir())
progressCount.Store(0)
homeDir := t.TempDir()
t.Setenv("HOME", homeDir)
t.Setenv("USERPROFILE", homeDir)
destDir := t.TempDir()
reg := &httpmock.Registry{}
@ -303,16 +405,17 @@ func TestInstall(t *testing.T) {
client := api.NewClientFromHTTP(&http.Client{Transport: reg})
opts := &Options{
Host: "github.com",
Owner: "monalisa",
Repo: "octocat-skills",
Ref: "v1.0",
SHA: "commit123",
Client: client,
Skills: tt.skills,
Dir: destDir,
Host: "github.com",
Owner: "monalisa",
Repo: "octocat-skills",
Ref: "v1.0",
SHA: "commit123",
Client: client,
Skills: tt.skills,
Dir: destDir,
OnProgress: tt.onProgress,
}
if tt.wantErr != "" {
if tt.wantErr != "" && len(tt.wantInstalled) == 0 {
opts.Dir = ""
}
@ -320,19 +423,58 @@ func TestInstall(t *testing.T) {
if tt.wantErr != "" {
require.Error(t, err)
assert.Contains(t, err.Error(), tt.wantErr)
if len(tt.wantInstalled) > 0 {
require.NotNil(t, result, "partial failure should return non-nil result")
assert.ElementsMatch(t, tt.wantInstalled, result.Installed)
}
return
}
require.NoError(t, err)
assert.ElementsMatch(t, tt.wantInstalled, result.Installed)
assert.Equal(t, destDir, result.Dir)
homeDir, _ := os.UserHomeDir()
homeDir, _ = os.UserHomeDir()
lockPath := filepath.Join(homeDir, ".agents", ".skill-lock.json")
lockData, err := os.ReadFile(lockPath)
require.NoError(t, err, "lockfile should have been written")
for _, name := range tt.wantInstalled {
assert.Contains(t, string(lockData), name)
}
if tt.onProgress != nil {
assert.True(t, progressCount.Load() > 0, "OnProgress should have been called")
}
})
}
}
func TestResolveGitRoot(t *testing.T) {
tests := []struct {
name string
client *git.Client
wantDir string
}{
{
name: "returns RepoDir when set",
client: &git.Client{RepoDir: "/monalisa/repo"},
wantDir: "/monalisa/repo",
},
{
name: "nil client falls back to cwd",
client: nil,
},
{
name: "empty RepoDir falls back to ToplevelDir or cwd",
client: &git.Client{},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := ResolveGitRoot(tt.client)
if tt.wantDir != "" {
assert.Equal(t, tt.wantDir, got)
} else {
assert.NotEmpty(t, got, "should fall back to ToplevelDir or cwd")
}
})
}
}

View file

@ -131,6 +131,11 @@ func newFile() *file {
}
}
var (
lockRetries = 30
lockRetryInterval = 100 * time.Millisecond
)
// acquireLock creates an exclusive lock file to serialize concurrent access.
// Returns an unlock function. If locking fails after retries, it proceeds
// unlocked rather than blocking the user indefinitely.
@ -146,7 +151,7 @@ func acquireLock() (unlock func()) {
return func() {}
}
for i := 0; i < 30; i++ {
for range lockRetries {
f, createErr := os.OpenFile(lkPath, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0o644)
if createErr == nil {
f.Close()
@ -162,7 +167,7 @@ func acquireLock() (unlock func()) {
os.Remove(lkPath)
continue
}
time.Sleep(100 * time.Millisecond)
time.Sleep(lockRetryInterval)
}
// Best-effort: proceed without lock.

View file

@ -5,16 +5,18 @@ import (
"os"
"path/filepath"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// setupHome redirects HOME to a temp dir and returns the expected lockfile path.
func setupHome(t *testing.T) string {
// setupTestHome redirects HOME to a temp dir and returns the expected lockfile path.
func setupTestHome(t *testing.T) string {
t.Helper()
home := t.TempDir()
t.Setenv("HOME", home)
t.Setenv("USERPROFILE", home)
return filepath.Join(home, agentsDir, lockFile)
}
@ -39,7 +41,7 @@ func TestRecordInstall(t *testing.T) {
treeSHA: "abc123",
verify: func(t *testing.T, lockPath string) {
t.Helper()
f := readLockfile(t, lockPath)
f := readTestLockfile(t, lockPath)
require.Contains(t, f.Skills, "code-review")
e := f.Skills["code-review"]
assert.Equal(t, "monalisa/octocat-skills", e.Source)
@ -62,30 +64,10 @@ func TestRecordInstall(t *testing.T) {
pinnedRef: "v1.0.0",
verify: func(t *testing.T, lockPath string) {
t.Helper()
f := readLockfile(t, lockPath)
f := readTestLockfile(t, lockPath)
assert.Equal(t, "v1.0.0", f.Skills["pr-summary"].PinnedRef)
},
},
{
name: "update preserves InstalledAt and updates treeSHA",
setup: func(t *testing.T) {
t.Helper()
require.NoError(t, RecordInstall("code-review", "monalisa", "octocat-skills", "skills/code-review/SKILL.md", "old-sha", ""))
},
skill: "code-review",
owner: "monalisa",
repo: "octocat-skills",
skillPath: "skills/code-review/SKILL.md",
treeSHA: "new-sha",
verify: func(t *testing.T, lockPath string) {
t.Helper()
f := readLockfile(t, lockPath)
e := f.Skills["code-review"]
assert.Equal(t, "new-sha", e.SkillFolderHash, "treeSHA should be updated")
// InstalledAt should be preserved (not empty proves it wasn't clobbered)
assert.NotEmpty(t, e.InstalledAt, "InstalledAt should be preserved from first install")
},
},
{
name: "multiple skills coexist",
setup: func(t *testing.T) {
@ -99,15 +81,118 @@ func TestRecordInstall(t *testing.T) {
treeSHA: "sha2",
verify: func(t *testing.T, lockPath string) {
t.Helper()
f := readLockfile(t, lockPath)
f := readTestLockfile(t, lockPath)
assert.Contains(t, f.Skills, "code-review")
assert.Contains(t, f.Skills, "issue-triage")
},
},
{
name: "succeeds despite stale lock file",
setup: func(t *testing.T) {
t.Helper()
lockPath, err := lockfilePath()
require.NoError(t, err)
require.NoError(t, os.MkdirAll(filepath.Dir(lockPath), 0o755))
lkPath := lockPath + ".lk"
f, err := os.Create(lkPath)
require.NoError(t, err)
f.Close()
staleTime := time.Now().Add(-60 * time.Second)
require.NoError(t, os.Chtimes(lkPath, staleTime, staleTime))
},
skill: "code-review",
owner: "monalisa",
repo: "octocat-skills",
skillPath: "skills/code-review/SKILL.md",
treeSHA: "abc123",
verify: func(t *testing.T, lockPath string) {
t.Helper()
f := readTestLockfile(t, lockPath)
require.Contains(t, f.Skills, "code-review")
_, err := os.Stat(lockPath + ".lk")
assert.True(t, os.IsNotExist(err), "stale lock should be removed after RecordInstall")
},
},
{
name: "proceeds without lock after retries exhausted",
setup: func(t *testing.T) {
t.Helper()
// Reduce retries to avoid 3s wait in tests.
origRetries := lockRetries
origInterval := lockRetryInterval
lockRetries = 1
lockRetryInterval = 0
t.Cleanup(func() {
lockRetries = origRetries
lockRetryInterval = origInterval
})
// Create a fresh (non-stale) lock file that won't be broken.
lockPath, err := lockfilePath()
require.NoError(t, err)
require.NoError(t, os.MkdirAll(filepath.Dir(lockPath), 0o755))
f, err := os.Create(lockPath + ".lk")
require.NoError(t, err)
f.Close()
},
skill: "code-review",
owner: "monalisa",
repo: "octocat-skills",
skillPath: "skills/code-review/SKILL.md",
treeSHA: "abc123",
verify: func(t *testing.T, lockPath string) {
t.Helper()
f := readTestLockfile(t, lockPath)
require.Contains(t, f.Skills, "code-review", "should succeed best-effort without lock")
},
},
{
name: "recovers from corrupt lockfile",
setup: func(t *testing.T) {
t.Helper()
lockPath, err := lockfilePath()
require.NoError(t, err)
require.NoError(t, os.MkdirAll(filepath.Dir(lockPath), 0o755))
require.NoError(t, os.WriteFile(lockPath, []byte("{invalid json"), 0o644))
},
skill: "code-review",
owner: "monalisa",
repo: "octocat-skills",
skillPath: "skills/code-review/SKILL.md",
treeSHA: "abc123",
verify: func(t *testing.T, lockPath string) {
t.Helper()
f := readTestLockfile(t, lockPath)
assert.Equal(t, lockVersion, f.Version)
require.Contains(t, f.Skills, "code-review")
},
},
{
name: "recovers from wrong version lockfile",
setup: func(t *testing.T) {
t.Helper()
lockPath, err := lockfilePath()
require.NoError(t, err)
require.NoError(t, os.MkdirAll(filepath.Dir(lockPath), 0o755))
data, _ := json.Marshal(file{Version: 999, Skills: map[string]entry{"old-skill": {}}})
require.NoError(t, os.WriteFile(lockPath, data, 0o644))
},
skill: "code-review",
owner: "monalisa",
repo: "octocat-skills",
skillPath: "skills/code-review/SKILL.md",
treeSHA: "abc123",
verify: func(t *testing.T, lockPath string) {
t.Helper()
f := readTestLockfile(t, lockPath)
assert.Equal(t, lockVersion, f.Version)
require.Contains(t, f.Skills, "code-review")
assert.NotContains(t, f.Skills, "old-skill", "wrong-version data should be discarded")
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
lockPath := setupHome(t)
lockPath := setupTestHome(t)
if tt.setup != nil {
tt.setup(t)
}
@ -117,73 +202,25 @@ func TestRecordInstall(t *testing.T) {
tt.verify(t, lockPath)
})
}
// This case lives outside the table because it needs to read the lockfile
// between two RecordInstall calls to capture the first InstalledAt value.
t.Run("update preserves InstalledAt and updates treeSHA", func(t *testing.T) {
lockPath := setupTestHome(t)
require.NoError(t, RecordInstall("code-review", "monalisa", "octocat-skills", "skills/code-review/SKILL.md", "old-sha", ""))
firstInstalledAt := readTestLockfile(t, lockPath).Skills["code-review"].InstalledAt
require.NoError(t, RecordInstall("code-review", "monalisa", "octocat-skills", "skills/code-review/SKILL.md", "new-sha", ""))
entry := readTestLockfile(t, lockPath).Skills["code-review"]
assert.Equal(t, "new-sha", entry.SkillFolderHash, "treeSHA should be updated")
assert.Equal(t, firstInstalledAt, entry.InstalledAt, "InstalledAt should be preserved from first install")
})
}
func TestRead(t *testing.T) {
tests := []struct {
name string
setup func(t *testing.T, lockPath string)
wantSkill bool
}{
{
name: "missing file returns fresh state",
setup: func(t *testing.T, lockPath string) {},
},
{
name: "corrupt JSON returns fresh state",
setup: func(t *testing.T, lockPath string) {
t.Helper()
require.NoError(t, os.MkdirAll(filepath.Dir(lockPath), 0o755))
require.NoError(t, os.WriteFile(lockPath, []byte("{invalid json"), 0o644))
},
},
{
name: "wrong version returns fresh state",
setup: func(t *testing.T, lockPath string) {
t.Helper()
require.NoError(t, os.MkdirAll(filepath.Dir(lockPath), 0o755))
data, _ := json.Marshal(file{Version: 999, Skills: map[string]entry{"x": {}}})
require.NoError(t, os.WriteFile(lockPath, data, 0o644))
},
},
{
name: "valid lockfile",
setup: func(t *testing.T, lockPath string) {
t.Helper()
require.NoError(t, os.MkdirAll(filepath.Dir(lockPath), 0o755))
f := &file{
Version: lockVersion,
Skills: map[string]entry{
"code-review": {Source: "monalisa/octocat-skills", SourceType: "github"},
},
}
data, err := json.MarshalIndent(f, "", " ")
require.NoError(t, err)
require.NoError(t, os.WriteFile(lockPath, data, 0o644))
},
wantSkill: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
lockPath := setupHome(t)
tt.setup(t, lockPath)
loaded, err := read()
require.NoError(t, err)
assert.Equal(t, lockVersion, loaded.Version)
if tt.wantSkill {
assert.Contains(t, loaded.Skills, "code-review")
} else {
assert.Empty(t, loaded.Skills)
}
})
}
}
// readLockfile is a test helper that reads and parses the lockfile from disk.
func readLockfile(t *testing.T, path string) *file {
// readTestLockfile is a test helper that reads and parses the lockfile from disk.
func readTestLockfile(t *testing.T, path string) *file {
t.Helper()
data, err := os.ReadFile(path)
require.NoError(t, err, "lockfile should exist at %s", path)

View file

@ -70,6 +70,20 @@ func TestInstallDir(t *testing.T) {
homeDir: "/home/monalisa",
wantErr: true,
},
{
name: "user scope without home dir",
scope: ScopeUser,
gitRoot: "/tmp/monalisa-repo",
homeDir: "",
wantErr: true,
},
{
name: "invalid scope",
scope: "bogus",
gitRoot: "/tmp/monalisa-repo",
homeDir: "/home/monalisa",
wantErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
@ -95,6 +109,7 @@ func TestRepoNameFromRemote(t *testing.T) {
{"git@github.com:monalisa/octocat-skills", "monalisa/octocat-skills"},
{"ssh://git@github.com/monalisa/octocat-skills.git", "monalisa/octocat-skills"},
{"ssh://git@github.com/monalisa/octocat-skills", "monalisa/octocat-skills"},
{"not-a-url", ""},
{"", ""},
}
for _, tt := range tests {

View file

@ -1,7 +1,6 @@
package install
import (
"context"
"errors"
"fmt"
"io"
@ -284,8 +283,8 @@ func installRun(opts *installOptions) error {
return err
}
gitRoot := resolveGitRoot(opts.GitClient)
homeDir := resolveHomeDir()
gitRoot := installer.ResolveGitRoot(opts.GitClient)
homeDir := installer.ResolveHomeDir()
source = ghrepo.FullName(opts.repo)
type hostPlan struct {
@ -423,8 +422,8 @@ func runLocalInstall(opts *installOptions) error {
return err
}
gitRoot := resolveGitRoot(opts.GitClient)
homeDir := resolveHomeDir()
gitRoot := installer.ResolveGitRoot(opts.GitClient)
homeDir := installer.ResolveHomeDir()
type hostPlan struct {
host *registry.AgentHost
@ -570,7 +569,7 @@ func logConventions(io *iostreams.IOStreams, skills []discovery.Skill) {
fmt.Fprintf(io.ErrOut, "Note: found %d namespaced skill(s) in skills/{author}/ directories\n", n)
}
if n, ok := conventions["plugins"]; ok {
fmt.Fprintf(io.ErrOut, "Note: found %d skill(s) using the Claude Code plugins/ convention\n", n)
fmt.Fprintf(io.ErrOut, "Note: found %d skill(s) using the plugins/ convention\n", n)
}
if n, ok := conventions["root"]; ok {
fmt.Fprintf(io.ErrOut, "Note: found %d skill(s) at the repository root\n", n)
@ -952,7 +951,7 @@ func printFileTree(w io.Writer, cs *iostreams.ColorScheme, dir string, skillName
func printTreeDir(w io.Writer, cs *iostreams.ColorScheme, dir, indent string) {
entries, err := os.ReadDir(dir)
if err != nil {
fmt.Fprintf(w, "%s%s\n", indent, cs.Gray("(could not read directory)"))
fmt.Fprintf(w, "%s%s\n", indent, cs.Muted("(could not read directory)"))
return
}
for i, entry := range entries {
@ -965,10 +964,10 @@ func printTreeDir(w io.Writer, cs *iostreams.ColorScheme, dir, indent string) {
}
name := entry.Name()
if entry.IsDir() {
fmt.Fprintf(w, "%s%s%s\n", indent, cs.Gray(connector), cs.Bold(name+"/"))
printTreeDir(w, cs, filepath.Join(dir, name), indent+cs.Gray(childIndent))
fmt.Fprintf(w, "%s%s%s\n", indent, cs.Muted(connector), cs.Bold(name+"/"))
printTreeDir(w, cs, filepath.Join(dir, name), indent+cs.Muted(childIndent))
} else {
fmt.Fprintf(w, "%s%s%s\n", indent, cs.Gray(connector), name)
fmt.Fprintf(w, "%s%s%s\n", indent, cs.Muted(connector), name)
}
}
}
@ -990,28 +989,3 @@ func printReviewHint(w io.Writer, cs *iostreams.ColorScheme, repo string, skillN
}
fmt.Fprintln(w)
}
func resolveGitRoot(gc *git.Client) string {
if gc == nil {
if cwd, err := os.Getwd(); err == nil {
return cwd
}
return ""
}
root, err := gc.ToplevelDir(context.Background())
if err != nil {
if cwd, err := os.Getwd(); err == nil {
return cwd
}
return ""
}
return root
}
func resolveHomeDir() string {
home, err := os.UserHomeDir()
if err != nil {
return ""
}
return home
}

File diff suppressed because it is too large Load diff

View file

@ -199,16 +199,16 @@ func renderAllFiles(opts *previewOptions, cs *iostreams.ColorScheme, skill disco
totalBytes := 0
for _, f := range extraFiles {
if fetched >= maxFiles {
fmt.Fprintf(out, "\n%s\n", cs.Gray(fmt.Sprintf("(skipped remaining files — showing first %d)", maxFiles)))
fmt.Fprintf(out, "\n%s\n", cs.Muted(fmt.Sprintf("(skipped remaining files — showing first %d)", maxFiles)))
break
}
if totalBytes+f.Size > maxTotalBytes && fetched > 0 {
fmt.Fprintf(out, "\n%s\n", cs.Gray("(skipped remaining files — size limit reached)"))
fmt.Fprintf(out, "\n%s\n", cs.Muted("(skipped remaining files — size limit reached)"))
break
}
fileContent, fetchErr := discovery.FetchBlob(apiClient, hostname, owner, repo, f.SHA)
if fetchErr != nil {
fmt.Fprintf(out, "\n%s\n\n%s\n", cs.Bold("── "+f.Path+" ──"), cs.Gray("(could not fetch file)"))
fmt.Fprintf(out, "\n%s\n\n%s\n", cs.Bold("── "+f.Path+" ──"), cs.Muted("(could not fetch file)"))
continue
}
fetched++
@ -373,10 +373,10 @@ func printTree(w io.Writer, cs *iostreams.ColorScheme, nodes []*treeNode, indent
childIndent = " "
}
if node.isDir {
fmt.Fprintf(w, "%s%s%s\n", indent, cs.Gray(connector), cs.Bold(node.name+"/"))
printTree(w, cs, node.children, indent+cs.Gray(childIndent))
fmt.Fprintf(w, "%s%s%s\n", indent, cs.Muted(connector), cs.Bold(node.name+"/"))
printTree(w, cs, node.children, indent+cs.Muted(childIndent))
} else {
fmt.Fprintf(w, "%s%s%s\n", indent, cs.Gray(connector), node.name)
fmt.Fprintf(w, "%s%s%s\n", indent, cs.Muted(connector), node.name)
}
}
}

View file

@ -3,9 +3,12 @@ package preview
import (
"encoding/base64"
"fmt"
"io"
"net/http"
"strings"
"testing"
"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/internal/prompter"
"github.com/cli/cli/v2/pkg/cmdutil"
@ -13,6 +16,7 @@ import (
"github.com/cli/cli/v2/pkg/iostreams"
"github.com/google/shlex"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestNewCmdPreview(t *testing.T) {
@ -62,31 +66,32 @@ func TestNewCmdPreview(t *testing.T) {
args, _ := shlex.Split(tt.input)
cmd.SetArgs(args)
cmd.SetOut(&discardWriter{})
cmd.SetErr(&discardWriter{})
cmd.SetOut(io.Discard)
cmd.SetErr(io.Discard)
err := cmd.Execute()
if tt.wantErr {
assert.Error(t, err)
require.Error(t, err)
return
}
assert.NoError(t, err)
require.NoError(t, err)
assert.Equal(t, tt.wantRepo, gotOpts.RepoArg)
assert.Equal(t, tt.wantSkillName, gotOpts.SkillName)
})
}
}
func TestNewCmdPreview_Alias(t *testing.T) {
ios, _, _, _ := iostreams.Test()
f := &cmdutil.Factory{IOStreams: ios, Prompter: &prompter.PrompterMock{}}
cmd := NewCmdPreview(f, func(_ *previewOptions) error { return nil })
assert.Contains(t, cmd.Aliases, "show")
}
func TestPreviewRun(t *testing.T) {
skillContent := "---\nname: my-skill\ndescription: A test skill\n---\n# My Skill\n\nThis is the skill content."
skillContent := heredoc.Doc(`
---
name: my-skill
description: A test skill
---
# My Skill
This is the skill content.
`)
encodedContent := base64.StdEncoding.EncodeToString([]byte(skillContent))
tests := []struct {
@ -266,11 +271,11 @@ func TestPreviewRun(t *testing.T) {
err := previewRun(tt.opts)
if tt.wantErr != "" {
assert.EqualError(t, err, tt.wantErr)
require.EqualError(t, err, tt.wantErr)
return
}
assert.NoError(t, err)
require.NoError(t, err)
if tt.wantStdout != "" {
assert.Contains(t, stdout.String(), tt.wantStdout)
}
@ -338,12 +343,19 @@ func TestPreviewRun_Interactive(t *testing.T) {
}
err := previewRun(opts)
assert.NoError(t, err)
require.NoError(t, err)
assert.Contains(t, stdout.String(), "Selected Skill")
}
func TestPreviewRun_ShowsFileTree(t *testing.T) {
skillContent := "---\nname: my-skill\ndescription: test\n---\n# My Skill\nBody."
skillContent := heredoc.Doc(`
---
name: my-skill
description: test
---
# My Skill
Body.
`)
encodedContent := base64.StdEncoding.EncodeToString([]byte(skillContent))
scriptContent := "#!/bin/bash\necho hello"
@ -426,7 +438,7 @@ func TestPreviewRun_ShowsFileTree(t *testing.T) {
}
err := previewRun(opts)
assert.NoError(t, err)
require.NoError(t, err)
out := stdout.String()
assert.Contains(t, out, "echo hello")
@ -450,7 +462,7 @@ func TestPreviewRun_ShowsFileTree(t *testing.T) {
}
err := previewRun(opts)
assert.NoError(t, err)
require.NoError(t, err)
out := stdout.String()
assert.Contains(t, out, "my-skill/")
@ -460,7 +472,174 @@ func TestPreviewRun_ShowsFileTree(t *testing.T) {
})
}
// discardWriter is a no-op writer for suppressing cobra output in tests.
type discardWriter struct{}
func TestPreviewRun_RenderLimits(t *testing.T) {
skillContent := heredoc.Doc(`
---
name: my-skill
description: test
---
# My Skill
`)
encodedSkill := base64.StdEncoding.EncodeToString([]byte(skillContent))
func (d *discardWriter) Write(p []byte) (int, error) { return len(p), nil }
// Helper: build a tree JSON with N extra files (beyond SKILL.md)
buildTree := func(n int) string {
entries := []string{
`{"path": "skills/my-skill", "type": "tree", "sha": "treeSHA"}`,
`{"path": "skills/my-skill/SKILL.md", "type": "blob", "sha": "blobSKILL"}`,
}
for i := range n {
entries = append(entries, fmt.Sprintf(
`{"path": "skills/my-skill/file%03d.txt", "type": "blob", "sha": "blob%03d"}`, i, i))
}
return fmt.Sprintf(`{"sha":"abc123","truncated":false,"tree":[%s]}`,
strings.Join(entries, ","))
}
// Helper: build subtree JSON with N extra files
buildSubtree := func(n int, sizes []int) string {
entries := []string{
`{"path": "SKILL.md", "type": "blob", "sha": "blobSKILL", "size": 50}`,
}
for i := range n {
sz := 10
if i < len(sizes) {
sz = sizes[i]
}
entries = append(entries, fmt.Sprintf(
`{"path": "file%03d.txt", "type": "blob", "sha": "blob%03d", "size": %d}`, i, i, sz))
}
return fmt.Sprintf(`{"tree":[%s]}`, strings.Join(entries, ","))
}
// Common stubs for resolve + discover
registerBase := func(reg *httpmock.Registry, treeJSON, subtreeJSON string) {
reg.Register(
httpmock.REST("GET", "repos/monalisa/skills-repo/releases/latest"),
httpmock.StringResponse(`{"tag_name": "v1.0.0"}`),
)
reg.Register(
httpmock.REST("GET", "repos/monalisa/skills-repo/git/ref/tags/v1.0.0"),
httpmock.StringResponse(`{"object": {"sha": "abc123", "type": "commit"}}`),
)
reg.Register(
httpmock.REST("GET", "repos/monalisa/skills-repo/git/trees/abc123"),
httpmock.StringResponse(treeJSON),
)
reg.Register(
httpmock.REST("GET", "repos/monalisa/skills-repo/git/trees/treeSHA"),
httpmock.StringResponse(subtreeJSON),
)
reg.Register(
httpmock.REST("GET", "repos/monalisa/skills-repo/git/blobs/blobSKILL"),
httpmock.StringResponse(`{"sha": "blobSKILL", "content": "`+encodedSkill+`", "encoding": "base64"}`),
)
}
t.Run("maxFiles cap truncates at 20", func(t *testing.T) {
reg := &httpmock.Registry{}
defer reg.Verify(t)
n := 22
treeJSON := buildTree(n)
subtreeJSON := buildSubtree(n, nil)
registerBase(reg, treeJSON, subtreeJSON)
// Register blob stubs for files 0-19 (first 20 get fetched)
tinyContent := base64.StdEncoding.EncodeToString([]byte("tiny"))
for i := range 20 {
reg.Register(
httpmock.REST("GET", fmt.Sprintf("repos/monalisa/skills-repo/git/blobs/blob%03d", i)),
httpmock.StringResponse(fmt.Sprintf(`{"sha": "blob%03d", "content": "%s", "encoding": "base64"}`, i, tinyContent)),
)
}
// Files 20 and 21 should NOT be fetched
ios, _, stdout, _ := iostreams.Test()
ios.SetStdoutTTY(false)
ios.SetStdinTTY(false)
opts := &previewOptions{
IO: ios,
HttpClient: func() (*http.Client, error) { return &http.Client{Transport: reg}, nil },
Prompter: &prompter.PrompterMock{},
repo: ghrepo.New("monalisa", "skills-repo"),
SkillName: "my-skill",
}
err := previewRun(opts)
require.NoError(t, err)
out := stdout.String()
assert.Contains(t, out, "showing first 20")
assert.Contains(t, out, "file019.txt") // last fetched
})
t.Run("maxBytes cap stops fetching", func(t *testing.T) {
reg := &httpmock.Registry{}
defer reg.Verify(t)
// Two files: first is 500KB, second would exceed 512KB cap
sizes := []int{500 * 1024, 100 * 1024}
treeJSON := buildTree(2)
subtreeJSON := buildSubtree(2, sizes)
registerBase(reg, treeJSON, subtreeJSON)
bigContent := base64.StdEncoding.EncodeToString(make([]byte, 500*1024))
reg.Register(
httpmock.REST("GET", "repos/monalisa/skills-repo/git/blobs/blob000"),
httpmock.StringResponse(fmt.Sprintf(`{"sha": "blob000", "content": "%s", "encoding": "base64"}`, bigContent)),
)
// blob001 should NOT be fetched — size limit reached
ios, _, stdout, _ := iostreams.Test()
ios.SetStdoutTTY(false)
ios.SetStdinTTY(false)
opts := &previewOptions{
IO: ios,
HttpClient: func() (*http.Client, error) { return &http.Client{Transport: reg}, nil },
Prompter: &prompter.PrompterMock{},
repo: ghrepo.New("monalisa", "skills-repo"),
SkillName: "my-skill",
}
err := previewRun(opts)
require.NoError(t, err)
out := stdout.String()
assert.Contains(t, out, "size limit reached")
})
t.Run("blob fetch error shows fallback message", func(t *testing.T) {
reg := &httpmock.Registry{}
defer reg.Verify(t)
treeJSON := buildTree(1)
subtreeJSON := buildSubtree(1, nil)
registerBase(reg, treeJSON, subtreeJSON)
reg.Register(
httpmock.REST("GET", "repos/monalisa/skills-repo/git/blobs/blob000"),
httpmock.StatusStringResponse(500, "server error"),
)
ios, _, stdout, _ := iostreams.Test()
ios.SetStdoutTTY(false)
ios.SetStdinTTY(false)
opts := &previewOptions{
IO: ios,
HttpClient: func() (*http.Client, error) { return &http.Client{Transport: reg}, nil },
Prompter: &prompter.PrompterMock{},
repo: ghrepo.New("monalisa", "skills-repo"),
SkillName: "my-skill",
}
err := previewRun(opts)
require.NoError(t, err)
out := stdout.String()
assert.Contains(t, out, "could not fetch file")
})
}

View file

@ -15,7 +15,6 @@ import (
"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/v2/api"
"github.com/cli/cli/v2/git"
giturl "github.com/cli/cli/v2/git"
"github.com/cli/cli/v2/internal/gh"
"github.com/cli/cli/v2/internal/ghinstance"
"github.com/cli/cli/v2/internal/ghrepo"
@ -40,10 +39,9 @@ type publishOptions struct {
Dir string // directory to validate (default: cwd)
// Flags
Fix bool // --fix flag: auto-fix issues where possible
Plugins bool // --plugins flag: generate .claude-plugin/ manifest
DryRun bool // --dry-run flag: validate only, don't publish
Tag string // --tag flag: release tag to create
Fix bool // --fix flag: auto-fix issues where possible
DryRun bool // --dry-run flag: validate only, don't publish
Tag string // --tag flag: release tag to create
// Testing overrides
client *api.Client // injectable for tests; nil means use factory HttpClient
@ -126,8 +124,6 @@ func NewCmdPublish(f *cmdutil.Factory, runF func(*publishOptions) error) *cobra.
Use --dry-run to validate without publishing.
Use --tag to publish non-interactively with a specific tag.
Use --fix to automatically strip install metadata from committed files.
Use --plugins to generate a .claude-plugin/plugin.json manifest for
Claude Code plugin discovery.
`),
Example: heredoc.Doc(`
# Validate and publish interactively
@ -141,9 +137,6 @@ func NewCmdPublish(f *cmdutil.Factory, runF func(*publishOptions) error) *cobra.
# Validate and strip install metadata
$ gh skills publish --fix
# Generate Claude Code plugin manifest
$ gh skills publish --plugins
`),
Aliases: []string{"validate"},
Args: cobra.MaximumNArgs(1),
@ -159,7 +152,6 @@ func NewCmdPublish(f *cmdutil.Factory, runF func(*publishOptions) error) *cobra.
}
cmd.Flags().BoolVar(&opts.Fix, "fix", false, "Auto-fix issues where possible (e.g. strip install metadata)")
cmd.Flags().BoolVar(&opts.Plugins, "plugins", false, "Generate .claude-plugin/ manifest for Claude Code plugin discovery")
cmd.Flags().BoolVar(&opts.DryRun, "dry-run", false, "Validate without publishing")
cmd.Flags().StringVar(&opts.Tag, "tag", "", "Version tag for the release (e.g. v1.0.0)")
@ -181,7 +173,6 @@ func publishRun(opts *publishOptions) error {
return fmt.Errorf("could not resolve path: %w", err)
}
cs := opts.IO.ColorScheme()
canPrompt := opts.IO.CanPrompt()
// Use injected client or create one from the factory HttpClient
@ -405,19 +396,6 @@ func publishRun(opts *publishOptions) error {
renderDiagnosticsPlain(opts, diagnostics, errors, warnings)
}
// Generate Claude Code plugin manifest if requested
if opts.Plugins {
pluginDiags := generateClaudePlugin(dir, skillDirs, owner, repo)
for _, d := range pluginDiags {
switch d.severity {
case "error":
fmt.Fprintf(opts.IO.ErrOut, "%s %s\n", cs.FailureIcon(), d.message)
default:
fmt.Fprintf(opts.IO.Out, "%s %s\n", cs.SuccessIcon(), d.message)
}
}
}
if errors > 0 {
return fmt.Errorf("validation failed with %d error(s)", errors)
}
@ -577,9 +555,9 @@ func runPublishRelease(opts *publishOptions, client *api.Client, host, owner, re
// 4. Inform if not on default branch
var currentBranch string
if opts.GitClient != nil {
bc := *opts.GitClient
bc.RepoDir = dir
if b, err := bc.CurrentBranch(context.Background()); err == nil {
branchGitClient := opts.GitClient.Copy()
branchGitClient.RepoDir = dir
if b, err := branchGitClient.CurrentBranch(context.Background()); err == nil {
currentBranch = b
}
}
@ -597,7 +575,7 @@ func runPublishRelease(opts *publishOptions, client *api.Client, host, owner, re
}
if !confirmed {
fmt.Fprintf(opts.IO.ErrOut, "Publish cancelled.\n")
return nil
return cmdutil.CancelError
}
}
@ -825,9 +803,9 @@ func checkInstalledSkillDirs(gitClient *git.Client, repoDir string) []publishDia
}
if gitClient != nil {
ic := *gitClient
ic.RepoDir = repoDir
if ic.IsIgnored(context.Background(), relPath) {
ignoreGitClient := gitClient.Copy()
ignoreGitClient.RepoDir = repoDir
if ignoreGitClient.IsIgnored(context.Background(), relPath) {
continue
}
}
@ -899,7 +877,7 @@ func detectGitHubRemote(gitClient *git.Client) (owner, repo string) {
// parseGitHubURL extracts owner/repo from a GitHub remote URL.
// Only GitHub.com URLs are recognized.
func parseGitHubURL(rawURL string) (owner, repo string) {
u, err := giturl.ParseURL(rawURL)
u, err := git.ParseURL(rawURL)
if err != nil {
return "", ""
}
@ -921,16 +899,16 @@ func detectMissingRepoDiagnostic(gitClient *git.Client, dir string) []publishDia
return nil
}
dc := *gitClient
dc.RepoDir = dir
if _, err := dc.GitDir(context.Background()); err != nil {
dirGitClient := gitClient.Copy()
dirGitClient.RepoDir = dir
if _, err := dirGitClient.GitDir(context.Background()); err != nil {
return []publishDiagnostic{{
severity: "warning",
message: "not a git repository — initialize with: git init && gh repo create",
}}
}
remotes, err := dc.Remotes(context.Background())
remotes, err := dirGitClient.Remotes(context.Background())
if err != nil || len(remotes) == 0 {
return []publishDiagnostic{{
severity: "warning",
@ -940,7 +918,7 @@ func detectMissingRepoDiagnostic(gitClient *git.Client, dir string) []publishDia
var urls []string
for _, r := range remotes {
if url, err := dc.RemoteURL(context.Background(), r.Name); err == nil {
if url, err := dirGitClient.RemoteURL(context.Background(), r.Name); err == nil {
urls = append(urls, url)
}
}
@ -1062,185 +1040,3 @@ func stripGitHubMetadata(content string) (string, error) {
return frontmatter.Serialize(result.RawYAML, result.Body)
}
// claudePluginJSON is the .claude-plugin/plugin.json structure.
type claudePluginJSON struct {
Name string `json:"name"`
Description string `json:"description,omitempty"`
Version string `json:"version,omitempty"`
Author *claudeAuthor `json:"author,omitempty"`
Homepage string `json:"homepage,omitempty"`
Repository string `json:"repository,omitempty"`
License string `json:"license,omitempty"`
Keywords []string `json:"keywords,omitempty"`
}
type claudeAuthor struct {
Name string `json:"name"`
}
// claudeMarketplaceJSON is the .claude-plugin/marketplace.json structure.
type claudeMarketplaceJSON struct {
Name string `json:"name"`
Owner claudeAuthor `json:"owner"`
Plugins []claudeMarketplacePlugin `json:"plugins"`
}
type claudeMarketplacePlugin struct {
Name string `json:"name"`
Source string `json:"source"`
Description string `json:"description,omitempty"`
}
// generateClaudePlugin creates .claude-plugin/plugin.json (and optionally
// marketplace.json for multi-skill repos).
func generateClaudePlugin(dir string, skillDirs []string, owner, repo string) []publishDiagnostic {
var diags []publishDiagnostic
pluginDir := filepath.Join(dir, ".claude-plugin")
pluginPath := filepath.Join(pluginDir, "plugin.json")
// Don't overwrite existing plugin.json
if _, err := os.Stat(pluginPath); err == nil {
diags = append(diags, publishDiagnostic{
severity: "info",
message: ".claude-plugin/plugin.json already exists (skipped)",
})
return diags
}
pluginName := filepath.Base(dir)
if repo != "" {
pluginName = repo
}
description := buildPluginDescription(dir, skillDirs)
plugin := claudePluginJSON{
Name: pluginName,
Description: description,
Version: "1.0.0",
Keywords: []string{"agent-skills"},
}
if owner != "" && repo != "" {
plugin.Repository = fmt.Sprintf("https://github.com/%s/%s", owner, repo)
plugin.Homepage = fmt.Sprintf("https://github.com/%s/%s", owner, repo)
plugin.Author = &claudeAuthor{Name: owner}
}
// Collect license from any skill
for _, skillName := range skillDirs {
skillPath := filepath.Join(dir, "skills", skillName, "SKILL.md")
content, err := os.ReadFile(skillPath)
if err != nil {
continue
}
result, err := frontmatter.Parse(string(content))
if err != nil {
continue
}
if result.Metadata.License != "" {
plugin.License = result.Metadata.License
break
}
}
if err := os.MkdirAll(pluginDir, 0o755); err != nil {
diags = append(diags, publishDiagnostic{
severity: "error",
message: fmt.Sprintf("could not create .claude-plugin/: %v", err),
})
return diags
}
data, err := json.MarshalIndent(plugin, "", " ")
if err != nil {
diags = append(diags, publishDiagnostic{
severity: "error",
message: fmt.Sprintf("could not serialize plugin.json: %v", err),
})
return diags
}
if err := os.WriteFile(pluginPath, append(data, '\n'), 0o644); err != nil {
diags = append(diags, publishDiagnostic{
severity: "error",
message: fmt.Sprintf("could not write plugin.json: %v", err),
})
return diags
}
diags = append(diags, publishDiagnostic{
severity: "info",
message: fmt.Sprintf("generated .claude-plugin/plugin.json for %q with %d skill(s)", pluginName, len(skillDirs)),
})
// Generate marketplace.json for multi-skill repos with a GitHub remote
if len(skillDirs) > 1 && owner != "" && repo != "" {
marketplacePath := filepath.Join(pluginDir, "marketplace.json")
if _, err := os.Stat(marketplacePath); err != nil {
mDiags := generateMarketplace(marketplacePath, pluginName, owner, skillDirs, dir)
diags = append(diags, mDiags...)
}
}
return diags
}
// generateMarketplace creates a marketplace.json for plugin marketplace discovery.
func generateMarketplace(path, pluginName, owner string, skillDirs []string, dir string) []publishDiagnostic {
desc := buildPluginDescription(dir, skillDirs)
plugins := []claudeMarketplacePlugin{{
Name: pluginName,
Source: ".",
Description: desc,
}}
marketplace := claudeMarketplaceJSON{
Name: pluginName,
Owner: claudeAuthor{Name: owner},
Plugins: plugins,
}
data, err := json.MarshalIndent(marketplace, "", " ")
if err != nil {
return []publishDiagnostic{{
severity: "error",
message: fmt.Sprintf("could not serialize marketplace.json: %v", err),
}}
}
if err := os.WriteFile(path, append(data, '\n'), 0o644); err != nil {
return []publishDiagnostic{{
severity: "error",
message: fmt.Sprintf("could not write marketplace.json: %v", err),
}}
}
return []publishDiagnostic{{
severity: "info",
message: "generated .claude-plugin/marketplace.json for plugin marketplace discovery",
}}
}
// buildPluginDescription creates a description from skill names and descriptions.
func buildPluginDescription(dir string, skillDirs []string) string {
if len(skillDirs) == 1 {
skillPath := filepath.Join(dir, "skills", skillDirs[0], "SKILL.md")
if content, err := os.ReadFile(skillPath); err == nil {
if result, err := frontmatter.Parse(string(content)); err == nil && result.Metadata.Description != "" {
return result.Metadata.Description
}
}
}
var names []string
for _, name := range skillDirs {
names = append(names, name)
}
if len(names) <= 5 {
return fmt.Sprintf("Agent skills: %s", strings.Join(names, ", "))
}
return fmt.Sprintf("Agent skills collection with %d skills", len(names))
}

File diff suppressed because it is too large Load diff

View file

@ -365,18 +365,32 @@ func truncateForProcessing(skills []skillResult, page, limit int) []skillResult
}
// enrichSkills fetches descriptions and star counts concurrently.
// Each function collects results into a map; merges happen after both complete
// to avoid concurrent writes to the shared skills slice.
func enrichSkills(client *api.Client, host string, skills []skillResult) {
var descMap map[int]string
var starsMap map[int]int
var wg sync.WaitGroup
wg.Add(2)
go func() {
defer wg.Done()
fetchDescriptions(client, host, skills)
descMap = fetchDescriptions(client, host, skills)
}()
go func() {
defer wg.Done()
fetchRepoStars(client, host, skills)
starsMap = fetchRepoStars(client, host, skills)
}()
wg.Wait()
for i := range skills {
if desc, ok := descMap[i]; ok {
skills[i].Description = desc
}
if stars, ok := starsMap[i]; ok {
skills[i].Stars = stars
}
}
}
// paginate slices results to the requested page window.
@ -423,7 +437,7 @@ func renderResults(opts *searchOptions, skills []skillResult, totalPages int) er
cs := opts.IO.ColorScheme()
header := fmt.Sprintf("\n%s Showing %s matching %q",
cs.SuccessIcon(),
pluralize(len(skills), "skill"),
text.Pluralize(len(skills), "skill"),
opts.Query,
)
if totalPages > 1 {
@ -498,14 +512,14 @@ func promptInstall(opts *searchOptions, skills []skillResult) error {
for i, s := range skills {
starStr := ""
if s.Stars > 0 {
starStr = " " + cs.Gray("★ "+formatStars(s.Stars))
starStr = " " + cs.Muted("★ "+formatStars(s.Stars))
}
descStr := ""
if s.Description != "" {
desc := collapseWhitespace(s.Description)
descStr = "\n " + cs.Gray(text.Truncate(descWidth, desc))
desc := strings.Join(strings.Fields(s.Description), " ")
descStr = "\n " + cs.Muted(text.Truncate(descWidth, desc))
}
options[i] = s.SkillName + " " + cs.Gray(s.Repo) + starStr + descStr
options[i] = s.SkillName + " " + cs.Muted(s.Repo) + starStr + descStr
}
indices, err := opts.Prompter.MultiSelect(
@ -564,7 +578,7 @@ func promptInstall(opts *searchOptions, skills []skillResult) error {
// - Exact skill name match (10 000 points)
// - Partial skill name match (1 000 points)
// - Description contains query (100 points)
// - Repository stars (logarithmic bonus, up to ~700 points)
// - Repository stars (sqrt bonus, ~2 400 for 6k stars)
func relevanceScore(s skillResult, query string) int {
term := strings.ToLower(query)
termHyphen := strings.ReplaceAll(term, " ", "-")
@ -574,7 +588,7 @@ func relevanceScore(s skillResult, query string) int {
// use hyphens as word separators (e.g. query "mcp apps" → "mcp-apps").
skillLower := strings.ToLower(s.SkillName)
if skillLower == term || skillLower == termHyphen {
score += 10_000
score += 3_000
} else if strings.Contains(skillLower, term) || strings.Contains(skillLower, termHyphen) {
score += 1_000
}
@ -584,10 +598,10 @@ func relevanceScore(s skillResult, query string) int {
score += 100
}
// Stars bonus: use log₁₀ scaling so popular repos rank higher without
// completely drowning out less-popular but more relevant results.
// Stars bonus: use √n scaling so popular repos rank meaningfully higher
// without completely drowning out less-popular but more relevant results.
if s.Stars > 0 {
score += int(math.Log10(float64(s.Stars)) * 150)
score += int(math.Sqrt(float64(s.Stars)) * 30)
}
return score
@ -763,12 +777,14 @@ func splitRepo(fullName string) (string, string) {
// fetchDescriptions fetches SKILL.md frontmatter descriptions concurrently
// for all search results. Each result may come from a different repo.
func fetchDescriptions(client *api.Client, host string, skills []skillResult) {
func fetchDescriptions(client *api.Client, host string, skills []skillResult) map[int]string {
const maxWorkers = 10
sem := make(chan struct{}, maxWorkers)
var wg sync.WaitGroup
var mu sync.Mutex
descs := make(map[int]string)
for i := range skills {
if skills[i].BlobSHA == "" {
continue
@ -789,11 +805,13 @@ func fetchDescriptions(client *api.Client, host string, skills []skillResult) {
}
mu.Lock()
skills[idx].Description = result.Metadata.Description
descs[idx] = result.Metadata.Description
mu.Unlock()
}(i)
}
wg.Wait()
return descs
}
// extractSkillName derives the skill name from a SKILL.md path, but only if
@ -803,21 +821,8 @@ func extractSkillName(filePath string) string {
return discovery.MatchesSkillPath(filePath)
}
func pluralize(count int, singular string) string {
if count == 1 {
return fmt.Sprintf("%d %s", count, singular)
}
return fmt.Sprintf("%d %ss", count, singular)
}
// collapseWhitespace replaces runs of whitespace (newlines, tabs, etc.)
// with a single space.
func collapseWhitespace(s string) string {
fields := strings.Fields(s)
return strings.Join(fields, " ")
}
// formatStars formats a star count for display (e.g. 1700 → "1.7k").
// TODO kw: Could be swaped for go-humanize.
func formatStars(n int) string {
if n >= 1000 {
return fmt.Sprintf("%.1fk", float64(n)/1000)
@ -832,7 +837,7 @@ type repoInfo struct {
// fetchRepoStars fetches stargazer counts for each unique repository in
// the result set, using bounded concurrency.
func fetchRepoStars(client *api.Client, host string, skills []skillResult) {
func fetchRepoStars(client *api.Client, host string, skills []skillResult) map[int]int {
const maxWorkers = 10
sem := make(chan struct{}, maxWorkers)
var wg sync.WaitGroup
@ -865,9 +870,11 @@ func fetchRepoStars(client *api.Client, host string, skills []skillResult) {
}
wg.Wait()
for i := range skills {
if stars, ok := repoStars[skills[i].Repo]; ok {
skills[i].Stars = stars
result := make(map[int]int, len(skills))
for i, s := range skills {
if stars, ok := repoStars[s.Repo]; ok {
result[i] = stars
}
}
return result
}

View file

@ -1,7 +1,9 @@
package search
import (
"io"
"net/http"
"strings"
"testing"
"github.com/cli/cli/v2/internal/config"
@ -88,19 +90,15 @@ func TestNewCmdSearch(t *testing.T) {
argv := []string{}
if tt.args != "" {
for _, part := range splitOnSpaces(tt.args) {
if part != "" {
argv = append(argv, part)
}
}
argv = strings.Fields(tt.args)
}
cmd.SetArgs(argv)
cmd.SetOut(&discardWriter{})
cmd.SetErr(&discardWriter{})
cmd.SetOut(io.Discard)
cmd.SetErr(io.Discard)
_, err := cmd.ExecuteC()
if tt.wantErr != "" {
assert.Error(t, err)
require.Error(t, err)
assert.Contains(t, err.Error(), tt.wantErr)
return
}
@ -252,6 +250,78 @@ func TestSearchRun(t *testing.T) {
},
wantErr: rateLimitErrorMessage,
},
{
name: "HTTP 429 returns rate limit error",
tty: false,
opts: &searchOptions{Query: "terraform", Page: 1, Limit: defaultLimit},
httpStubs: func(reg *httpmock.Registry) {
for range 3 {
reg.Register(
httpmock.REST("GET", "search/code"),
httpmock.StatusStringResponse(429, `{"message": "Too Many Requests"}`),
)
}
},
wantErr: rateLimitErrorMessage,
},
{
name: "HTTP 403 with Retry-After returns rate limit error",
tty: false,
opts: &searchOptions{Query: "terraform", Page: 1, Limit: defaultLimit},
httpStubs: func(reg *httpmock.Registry) {
for range 3 {
reg.Register(
httpmock.REST("GET", "search/code"),
httpmock.WithHeader(
httpmock.StatusJSONResponse(403, map[string]string{"message": "secondary rate limit"}),
"Retry-After", "60",
),
)
}
},
wantErr: rateLimitErrorMessage,
},
{
name: "no results with owner scope",
tty: true,
opts: &searchOptions{Query: "nonexistent", Owner: "monalisa", Page: 1, Limit: defaultLimit},
httpStubs: func(reg *httpmock.Registry) {
// With --owner set, only path + primary searches fire (no owner search).
for range 2 {
reg.Register(
httpmock.REST("GET", "search/code"),
httpmock.StringResponse(emptyCodeResponse),
)
}
},
wantErr: `no skills found matching "nonexistent" from owner "monalisa"`,
},
{
name: "enriches results with blob descriptions",
tty: false,
opts: &searchOptions{Query: "terraform", Page: 1, Limit: defaultLimit},
httpStubs: func(reg *httpmock.Registry) {
codeResponse := `{"total_count": 1, "incomplete_results": false, "items": [
{"name": "SKILL.md", "path": "skills/terraform/SKILL.md", "sha": "abc123",
"repository": {"full_name": "org/repo"}}
]}`
stubKeywordSearch(reg, codeResponse)
// Blob fetch for description enrichment
reg.Register(
httpmock.REST("GET", "repos/org/repo/git/blobs/abc123"),
httpmock.JSONResponse(map[string]string{
"content": "LS0tCmRlc2NyaXB0aW9uOiBBdXRvbWF0ZXMgVGVycmFmb3JtIGluZnJhc3RydWN0dXJlCi0tLQojIFRlcnJhZm9ybSBTa2lsbAo=",
"encoding": "base64",
}),
)
// Repo stars fetch
reg.Register(
httpmock.REST("GET", "repos/org/repo"),
httpmock.JSONResponse(map[string]int{"stargazers_count": 42}),
)
},
wantStdout: "org/repo\tterraform\tAutomates Terraform infrastructure\t42\n",
},
}
for _, tt := range tests {
@ -396,28 +466,3 @@ func TestFormatStars(t *testing.T) {
assert.Equal(t, "1.7k", formatStars(1700))
assert.Equal(t, "12.5k", formatStars(12500))
}
func splitOnSpaces(s string) []string {
var parts []string
current := ""
for _, c := range s {
if c == ' ' {
if current != "" {
parts = append(parts, current)
current = ""
}
} else {
current += string(c)
}
}
if current != "" {
parts = append(parts, current)
}
return parts
}
type discardWriter struct{}
func (d *discardWriter) Write(p []byte) (n int, err error) {
return len(p), nil
}

View file

@ -1,7 +1,6 @@
package update
import (
"context"
"fmt"
"net/http"
"os"
@ -38,6 +37,7 @@ type updateOptions struct {
All bool // --all flag (update without prompting)
Force bool // --force flag (re-download even if SHAs match)
DryRun bool // --dry-run flag (report only, no changes)
Unpin bool // --unpin flag (clear pinned ref and include in update)
Dir string // --dir flag (scan a custom directory)
}
@ -86,7 +86,8 @@ func NewCmdUpdate(f *cmdutil.Factory, runF func(*updateOptions) error) *cobra.Co
checks only those specific skills.
Pinned skills (installed with --pin) are skipped with a notice.
Use "gh skills install --pin <new-ref>" to change the pinned version.
Use --unpin to clear the pinned version and include those skills
in the update.
Skills without GitHub metadata (e.g. installed manually or by another
tool) are prompted for their source repository in interactive mode.
@ -116,6 +117,9 @@ func NewCmdUpdate(f *cmdutil.Factory, runF func(*updateOptions) error) *cobra.Co
# Check for updates without applying (read-only)
$ gh skills update --dry-run
# Unpin skills and update them to latest
$ gh skills update --unpin
`),
RunE: func(cmd *cobra.Command, args []string) error {
opts.Skills = args
@ -129,6 +133,7 @@ func NewCmdUpdate(f *cmdutil.Factory, runF func(*updateOptions) error) *cobra.Co
cmd.Flags().BoolVar(&opts.All, "all", false, "Update all skills without prompting")
cmd.Flags().BoolVar(&opts.Force, "force", false, "Re-download even if already up to date")
cmd.Flags().BoolVar(&opts.DryRun, "dry-run", false, "Report available updates without modifying files")
cmd.Flags().BoolVar(&opts.Unpin, "unpin", false, "Clear pinned version and include pinned skills in update")
cmd.Flags().StringVar(&opts.Dir, "dir", "", "Scan a custom directory for installed skills")
return cmd
@ -150,8 +155,8 @@ func updateRun(opts *updateOptions) error {
}
hostname, _ := cfg.Authentication().DefaultHost()
gitRoot := resolveGitRoot(opts.GitClient)
homeDir := resolveHomeDir()
gitRoot := installer.ResolveGitRoot(opts.GitClient)
homeDir := installer.ResolveHomeDir()
// Scan for installed skills
var installed []installedSkill
@ -162,7 +167,7 @@ func updateRun(opts *updateOptions) error {
}
installed = skills
} else {
installed = scanAllHosts(gitRoot, homeDir)
installed = scanAllAgents(gitRoot, homeDir)
}
if len(installed) == 0 {
@ -238,7 +243,7 @@ func updateRun(opts *updateOptions) error {
if s.owner == "" || s.repo == "" {
continue
}
if s.pinned != "" {
if s.pinned != "" && !opts.Unpin {
pinned = append(pinned, s)
continue
}
@ -315,7 +320,7 @@ func updateRun(opts *updateOptions) error {
}
for _, s := range pinned {
fmt.Fprintf(opts.IO.ErrOut, "%s %s is pinned to %s (skipped)\n", cs.Gray("⊘"), s.name, s.pinned)
fmt.Fprintf(opts.IO.ErrOut, "%s %s is pinned to %s (skipped)\n", cs.Muted("⊘"), s.name, s.pinned)
}
for _, name := range noMeta {
fmt.Fprintf(opts.IO.ErrOut, "%s %s has no GitHub metadata — reinstall to enable updates\n", cs.WarningIcon(), name)
@ -339,7 +344,7 @@ func updateRun(opts *updateOptions) error {
} else {
fmt.Fprintf(opts.IO.Out, " %s %s (%s/%s) %s → %s [%s]\n",
cs.Cyan("•"), u.local.name, u.local.owner, u.local.repo,
cs.Gray(git.ShortSHA(u.local.treeSHA)), git.ShortSHA(u.newSHA),
cs.Muted(git.ShortSHA(u.local.treeSHA)), git.ShortSHA(u.newSHA),
u.resolved.Ref)
}
}
@ -359,7 +364,7 @@ func updateRun(opts *updateOptions) error {
}
if !confirmed {
fmt.Fprintf(opts.IO.ErrOut, "Update cancelled.\n")
return nil
return cmdutil.CancelError
}
}
@ -409,9 +414,9 @@ func updateRun(opts *updateOptions) error {
return nil
}
// scanAllHosts walks every known host directory (project + user scope) and
// scanAllAgents walks every registered agent's skill directory (project + user scope) and
// collects installed skills. Skills are deduplicated by directory path.
func scanAllHosts(gitRoot, homeDir string) []installedSkill {
func scanAllAgents(gitRoot, homeDir string) []installedSkill {
seen := make(map[string]bool)
var all []installedSkill
@ -533,28 +538,3 @@ func promptForSkillOrigin(p prompter.Prompter, skillName string) (owner, repo, r
}
return r.RepoOwner(), r.RepoName(), "", true, nil
}
func resolveGitRoot(gc *git.Client) string {
if gc == nil {
if cwd, err := os.Getwd(); err == nil {
return cwd
}
return ""
}
root, err := gc.ToplevelDir(context.Background())
if err != nil {
if cwd, err := os.Getwd(); err == nil {
return cwd
}
return ""
}
return root
}
func resolveHomeDir() string {
home, err := os.UserHomeDir()
if err != nil {
return ""
}
return home
}

File diff suppressed because it is too large Load diff