refactor: use shared discovery logic in publish command
Replace the hardcoded skills/ directory requirement in the publish
command with the shared DiscoverLocalSkills() function used by install
and other commands. This removes the opinionated restriction that skills
must live under a skills/ directory and supports all discovery
conventions:
- skills/*/SKILL.md
- skills/{scope}/*/SKILL.md
- */SKILL.md (root-level)
- plugins/{scope}/skills/*/SKILL.md
All per-skill validation (frontmatter, spec-compliant naming, metadata
stripping, etc.) is preserved.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
parent
c5cff0f298
commit
b63f5bfd9a
2 changed files with 140 additions and 67 deletions
|
|
@ -7,6 +7,7 @@ import (
|
|||
"fmt"
|
||||
"net/http"
|
||||
"os"
|
||||
"path"
|
||||
"path/filepath"
|
||||
"regexp"
|
||||
"sort"
|
||||
|
|
@ -102,9 +103,15 @@ func NewCmdPublish(f *cmdutil.Factory, runF func(*PublishOptions) error) *cobra.
|
|||
Validate a local repository's skills against the Agent Skills specification
|
||||
and publish them by creating a GitHub release.
|
||||
|
||||
Skills are discovered using the same conventions as install:
|
||||
|
||||
- %[1]sskills/*/SKILL.md%[1]s
|
||||
- %[1]sskills/{scope}/*/SKILL.md%[1]s
|
||||
- %[1]s*/SKILL.md%[1]s (root-level)
|
||||
- %[1]splugins/{scope}/skills/*/SKILL.md%[1]s
|
||||
|
||||
Validation checks include:
|
||||
|
||||
- Skills follow the %[1]sskills/*/SKILL.md%[1]s directory convention
|
||||
- Skill names match the strict agentskills.io naming rules
|
||||
- Each skill name matches its directory name
|
||||
- Required frontmatter fields (name, description) are present
|
||||
|
|
@ -176,36 +183,18 @@ func publishRun(opts *PublishOptions) error {
|
|||
|
||||
var diagnostics []publishDiagnostic
|
||||
|
||||
// Check for skills directory
|
||||
skillsDir := filepath.Join(dir, "skills")
|
||||
info, err := os.Stat(skillsDir)
|
||||
if err != nil || !info.IsDir() {
|
||||
return fmt.Errorf("no skills/ directory found in %s; run this command from a repository root containing a skills/ directory", dir)
|
||||
}
|
||||
|
||||
// Discover skill directories
|
||||
entries, err := os.ReadDir(skillsDir)
|
||||
skills, err := discovery.DiscoverLocalSkills(dir)
|
||||
if err != nil {
|
||||
return fmt.Errorf("could not read skills/ directory: %w", err)
|
||||
return err
|
||||
}
|
||||
|
||||
var skillDirs []string
|
||||
for _, e := range entries {
|
||||
if e.IsDir() {
|
||||
skillDirs = append(skillDirs, e.Name())
|
||||
}
|
||||
}
|
||||
|
||||
if len(skillDirs) == 0 {
|
||||
return fmt.Errorf("no skill directories found in %s/skills/", dir)
|
||||
}
|
||||
|
||||
for _, dirName := range skillDirs {
|
||||
skillPath := filepath.Join(skillsDir, dirName, "SKILL.md")
|
||||
for _, skill := range skills {
|
||||
dirName := path.Base(skill.Path)
|
||||
skillPath := filepath.Join(dir, filepath.FromSlash(skill.Path), "SKILL.md")
|
||||
content, err := os.ReadFile(skillPath)
|
||||
if err != nil {
|
||||
diagnostics = append(diagnostics, publishDiagnostic{
|
||||
skill: dirName,
|
||||
skill: skill.DisplayName(),
|
||||
severity: "error",
|
||||
message: "missing SKILL.md file",
|
||||
})
|
||||
|
|
@ -215,7 +204,7 @@ func publishRun(opts *PublishOptions) error {
|
|||
result, err := frontmatter.Parse(string(content))
|
||||
if err != nil {
|
||||
diagnostics = append(diagnostics, publishDiagnostic{
|
||||
skill: dirName,
|
||||
skill: skill.DisplayName(),
|
||||
severity: "error",
|
||||
message: fmt.Sprintf("invalid frontmatter YAML: %s", err),
|
||||
})
|
||||
|
|
@ -225,7 +214,7 @@ func publishRun(opts *PublishOptions) error {
|
|||
// Validate name field exists
|
||||
if result.Metadata.Name == "" {
|
||||
diagnostics = append(diagnostics, publishDiagnostic{
|
||||
skill: dirName,
|
||||
skill: skill.DisplayName(),
|
||||
severity: "error",
|
||||
message: "missing required field: name",
|
||||
})
|
||||
|
|
@ -233,7 +222,7 @@ func publishRun(opts *PublishOptions) error {
|
|||
// Validate name matches directory
|
||||
if result.Metadata.Name != dirName {
|
||||
diagnostics = append(diagnostics, publishDiagnostic{
|
||||
skill: dirName,
|
||||
skill: skill.DisplayName(),
|
||||
severity: "error",
|
||||
message: fmt.Sprintf("name %q does not match directory name %q", result.Metadata.Name, dirName),
|
||||
})
|
||||
|
|
@ -242,7 +231,7 @@ func publishRun(opts *PublishOptions) error {
|
|||
// Validate name is spec-compliant
|
||||
if !discovery.IsSpecCompliant(result.Metadata.Name) {
|
||||
diagnostics = append(diagnostics, publishDiagnostic{
|
||||
skill: dirName,
|
||||
skill: skill.DisplayName(),
|
||||
severity: "error",
|
||||
message: fmt.Sprintf("name %q does not follow agentskills.io naming convention (lowercase alphanumeric + hyphens)", result.Metadata.Name),
|
||||
})
|
||||
|
|
@ -252,13 +241,13 @@ func publishRun(opts *PublishOptions) error {
|
|||
// Validate description field exists
|
||||
if result.Metadata.Description == "" {
|
||||
diagnostics = append(diagnostics, publishDiagnostic{
|
||||
skill: dirName,
|
||||
skill: skill.DisplayName(),
|
||||
severity: "error",
|
||||
message: "missing required field: description",
|
||||
})
|
||||
} else if len(result.Metadata.Description) > 1024 {
|
||||
diagnostics = append(diagnostics, publishDiagnostic{
|
||||
skill: dirName,
|
||||
skill: skill.DisplayName(),
|
||||
severity: "warning",
|
||||
message: fmt.Sprintf("description is %d chars (recommended max: 1024)", len(result.Metadata.Description)),
|
||||
})
|
||||
|
|
@ -268,7 +257,7 @@ func publishRun(opts *PublishOptions) error {
|
|||
if raw, ok := result.RawYAML["allowed-tools"]; ok {
|
||||
if _, isSlice := raw.([]interface{}); isSlice {
|
||||
diagnostics = append(diagnostics, publishDiagnostic{
|
||||
skill: dirName,
|
||||
skill: skill.DisplayName(),
|
||||
severity: "error",
|
||||
message: "allowed-tools must be a string (space-delimited), not an array",
|
||||
})
|
||||
|
|
@ -283,26 +272,26 @@ func publishRun(opts *PublishOptions) error {
|
|||
fixed, fixErr := stripGitHubMetadata(string(content))
|
||||
if fixErr != nil {
|
||||
diagnostics = append(diagnostics, publishDiagnostic{
|
||||
skill: dirName,
|
||||
skill: skill.DisplayName(),
|
||||
severity: "error",
|
||||
message: fmt.Sprintf("could not strip install metadata: %s", fixErr),
|
||||
})
|
||||
} else if writeErr := os.WriteFile(skillPath, []byte(fixed), 0o644); writeErr != nil {
|
||||
diagnostics = append(diagnostics, publishDiagnostic{
|
||||
skill: dirName,
|
||||
skill: skill.DisplayName(),
|
||||
severity: "error",
|
||||
message: fmt.Sprintf("could not write fixed SKILL.md: %s", writeErr),
|
||||
})
|
||||
} else {
|
||||
diagnostics = append(diagnostics, publishDiagnostic{
|
||||
skill: dirName,
|
||||
skill: skill.DisplayName(),
|
||||
severity: "fixed",
|
||||
message: fmt.Sprintf("stripped install metadata: %s", strings.Join(githubKeys, ", ")),
|
||||
})
|
||||
}
|
||||
} else {
|
||||
diagnostics = append(diagnostics, publishDiagnostic{
|
||||
skill: dirName,
|
||||
skill: skill.DisplayName(),
|
||||
severity: "error",
|
||||
message: fmt.Sprintf("contains install metadata that must be stripped: %s (use --fix)", strings.Join(githubKeys, ", ")),
|
||||
})
|
||||
|
|
@ -314,7 +303,7 @@ func publishRun(opts *PublishOptions) error {
|
|||
if result.Metadata.License == "" {
|
||||
if _, ok := result.RawYAML["license"]; !ok {
|
||||
diagnostics = append(diagnostics, publishDiagnostic{
|
||||
skill: dirName,
|
||||
skill: skill.DisplayName(),
|
||||
severity: "warning",
|
||||
message: "recommended field missing: license",
|
||||
})
|
||||
|
|
@ -325,7 +314,7 @@ func publishRun(opts *PublishOptions) error {
|
|||
bodyLines := strings.Count(result.Body, "\n") + 1
|
||||
if bodyLines > 500 {
|
||||
diagnostics = append(diagnostics, publishDiagnostic{
|
||||
skill: dirName,
|
||||
skill: skill.DisplayName(),
|
||||
severity: "warning",
|
||||
message: fmt.Sprintf("skill body is %d lines (recommended max: 500 for efficient context)", bodyLines),
|
||||
})
|
||||
|
|
@ -376,7 +365,11 @@ func publishRun(opts *PublishOptions) error {
|
|||
|
||||
if client != nil {
|
||||
// Security and ruleset checks (advisory, always shown)
|
||||
securityDiags := checkSecuritySettings(client, host, owner, repo, skillsDir)
|
||||
var skillAbsDirs []string
|
||||
for _, skill := range skills {
|
||||
skillAbsDirs = append(skillAbsDirs, filepath.Join(dir, filepath.FromSlash(skill.Path)))
|
||||
}
|
||||
securityDiags := checkSecuritySettings(client, host, owner, repo, skillAbsDirs)
|
||||
diagnostics = append(diagnostics, securityDiags...)
|
||||
|
||||
rulesetDiags := checkTagProtection(client, host, owner, repo)
|
||||
|
|
@ -406,7 +399,7 @@ func publishRun(opts *PublishOptions) error {
|
|||
}
|
||||
|
||||
if canPrompt {
|
||||
renderDiagnosticsTTY(opts, skillDirs, diagnostics, errors, warnings, fixes, owner, repo)
|
||||
renderDiagnosticsTTY(opts, len(skills), diagnostics, errors, warnings, fixes, owner, repo)
|
||||
} else {
|
||||
renderDiagnosticsPlain(opts, diagnostics, errors, warnings)
|
||||
}
|
||||
|
|
@ -707,7 +700,7 @@ func checkTagProtection(client *api.Client, host, owner, repo string) []publishD
|
|||
}
|
||||
|
||||
// checkSecuritySettings checks whether recommended security features are enabled.
|
||||
func checkSecuritySettings(client *api.Client, host, owner, repo, skillsDir string) []publishDiagnostic {
|
||||
func checkSecuritySettings(client *api.Client, host, owner, repo string, skillDirs []string) []publishDiagnostic {
|
||||
if client == nil {
|
||||
return nil
|
||||
}
|
||||
|
|
@ -738,7 +731,7 @@ func checkSecuritySettings(client *api.Client, host, owner, repo, skillsDir stri
|
|||
})
|
||||
}
|
||||
|
||||
hasCode, hasManifests := detectCodeAndManifests(skillsDir)
|
||||
hasCode, hasManifests := detectCodeAndManifests(skillDirs)
|
||||
|
||||
if hasCode {
|
||||
alertsPath := fmt.Sprintf("repos/%s/%s/code-scanning/alerts?per_page=1&state=open", owner, repo)
|
||||
|
|
@ -781,28 +774,35 @@ var manifestFiles = map[string]bool{
|
|||
"composer.json": true, "composer.lock": true,
|
||||
}
|
||||
|
||||
// detectCodeAndManifests walks the skills directory looking for code files
|
||||
// detectCodeAndManifests walks the skill directories looking for code files
|
||||
// and dependency manifests.
|
||||
func detectCodeAndManifests(skillsDir string) (hasCode, hasManifests bool) {
|
||||
_ = filepath.Walk(skillsDir, func(path string, info os.FileInfo, err error) error {
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if info.IsDir() {
|
||||
func detectCodeAndManifests(skillDirs []string) (hasCode, hasManifests bool) {
|
||||
for _, dir := range skillDirs {
|
||||
_ = filepath.Walk(dir, func(path string, info os.FileInfo, err error) error {
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if info.IsDir() {
|
||||
return nil
|
||||
}
|
||||
ext := filepath.Ext(info.Name())
|
||||
if codeExtensions[ext] {
|
||||
hasCode = true
|
||||
}
|
||||
if manifestFiles[info.Name()] {
|
||||
hasManifests = true
|
||||
}
|
||||
if hasCode && hasManifests {
|
||||
// Stop walking this skill directory early; the outer loop
|
||||
// continues to process remaining skill directories.
|
||||
return filepath.SkipAll
|
||||
}
|
||||
return nil
|
||||
}
|
||||
ext := filepath.Ext(info.Name())
|
||||
if codeExtensions[ext] {
|
||||
hasCode = true
|
||||
}
|
||||
if manifestFiles[info.Name()] {
|
||||
hasManifests = true
|
||||
}
|
||||
})
|
||||
if hasCode && hasManifests {
|
||||
return filepath.SkipAll
|
||||
return
|
||||
}
|
||||
return nil
|
||||
})
|
||||
}
|
||||
return
|
||||
}
|
||||
|
||||
|
|
@ -961,7 +961,7 @@ func detectMissingRepoDiagnostic(gitClient *git.Client, dir string) []publishDia
|
|||
}}
|
||||
}
|
||||
|
||||
func renderDiagnosticsTTY(opts *PublishOptions, skillDirs []string, diagnostics []publishDiagnostic, errors, warnings, fixes int, owner, repo string) {
|
||||
func renderDiagnosticsTTY(opts *PublishOptions, skillCount int, diagnostics []publishDiagnostic, errors, warnings, fixes int, owner, repo string) {
|
||||
cs := opts.IO.ColorScheme()
|
||||
|
||||
// Separate info messages from errors/warnings for cleaner output
|
||||
|
|
@ -975,7 +975,7 @@ func renderDiagnosticsTTY(opts *PublishOptions, skillDirs []string, diagnostics
|
|||
}
|
||||
|
||||
if len(issues) == 0 && fixes == 0 {
|
||||
fmt.Fprintf(opts.IO.Out, "%s %d skill(s) validated successfully\n", cs.SuccessIcon(), len(skillDirs))
|
||||
fmt.Fprintf(opts.IO.Out, "%s %d skill(s) validated successfully\n", cs.SuccessIcon(), skillCount)
|
||||
} else {
|
||||
for _, d := range issues {
|
||||
var prefix string
|
||||
|
|
|
|||
|
|
@ -168,16 +168,16 @@ func TestPublishRun(t *testing.T) {
|
|||
wantStderr string
|
||||
}{
|
||||
{
|
||||
name: "no skills directory",
|
||||
name: "no skills found",
|
||||
setup: func(_ *testing.T, _ string) {},
|
||||
opts: func(ios *iostreams.IOStreams, dir string, _ *httpmock.Registry) *PublishOptions {
|
||||
t.Helper()
|
||||
return &PublishOptions{IO: ios, Dir: dir}
|
||||
},
|
||||
wantErr: "no skills/ directory",
|
||||
wantErr: "no skills found",
|
||||
},
|
||||
{
|
||||
name: "missing SKILL.md",
|
||||
name: "empty skills directory has no discoverable skills",
|
||||
setup: func(t *testing.T, dir string) {
|
||||
t.Helper()
|
||||
require.NoError(t, os.MkdirAll(filepath.Join(dir, "skills", "empty-skill"), 0o755))
|
||||
|
|
@ -186,8 +186,7 @@ func TestPublishRun(t *testing.T) {
|
|||
t.Helper()
|
||||
return &PublishOptions{IO: ios, Dir: dir}
|
||||
},
|
||||
wantErr: "validation failed",
|
||||
wantStdout: "missing SKILL.md",
|
||||
wantErr: "no skills found",
|
||||
},
|
||||
{
|
||||
name: "missing name in frontmatter",
|
||||
|
|
@ -245,6 +244,80 @@ func TestPublishRun(t *testing.T) {
|
|||
wantErr: "validation failed",
|
||||
wantStdout: "naming convention",
|
||||
},
|
||||
{
|
||||
name: "root-level skill discovered and validated",
|
||||
isTTY: true,
|
||||
setup: func(t *testing.T, dir string) {
|
||||
t.Helper()
|
||||
// Create a root-level skill (*/SKILL.md convention)
|
||||
skillDir := filepath.Join(dir, "my-root-skill")
|
||||
require.NoError(t, os.MkdirAll(skillDir, 0o755))
|
||||
require.NoError(t, os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte(heredoc.Doc(`
|
||||
---
|
||||
name: my-root-skill
|
||||
description: A root-level skill
|
||||
license: MIT
|
||||
---
|
||||
Body.
|
||||
`)), 0o644))
|
||||
initGitRepo(t, dir, map[string]string{
|
||||
"origin": "https://github.com/monalisa/skills-repo.git",
|
||||
})
|
||||
},
|
||||
stubs: func(reg *httpmock.Registry) {
|
||||
stubAllSecureRemote(reg, "monalisa", "skills-repo")
|
||||
},
|
||||
opts: func(ios *iostreams.IOStreams, dir string, reg *httpmock.Registry) *PublishOptions {
|
||||
t.Helper()
|
||||
return &PublishOptions{
|
||||
IO: ios,
|
||||
Dir: dir,
|
||||
DryRun: true,
|
||||
GitClient: &git.Client{},
|
||||
client: api.NewClientFromHTTP(&http.Client{Transport: reg}),
|
||||
host: "github.com",
|
||||
}
|
||||
},
|
||||
wantStdout: "1 skill(s) validated successfully",
|
||||
wantStderr: "Dry run complete",
|
||||
},
|
||||
{
|
||||
name: "namespaced skill discovered and validated",
|
||||
isTTY: true,
|
||||
setup: func(t *testing.T, dir string) {
|
||||
t.Helper()
|
||||
// Create a namespaced skill (skills/{scope}/*/SKILL.md convention)
|
||||
skillDir := filepath.Join(dir, "skills", "monalisa", "scoped-skill")
|
||||
require.NoError(t, os.MkdirAll(skillDir, 0o755))
|
||||
require.NoError(t, os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte(heredoc.Doc(`
|
||||
---
|
||||
name: scoped-skill
|
||||
description: A namespaced skill
|
||||
license: MIT
|
||||
---
|
||||
Body.
|
||||
`)), 0o644))
|
||||
initGitRepo(t, dir, map[string]string{
|
||||
"origin": "https://github.com/monalisa/skills-repo.git",
|
||||
})
|
||||
},
|
||||
stubs: func(reg *httpmock.Registry) {
|
||||
stubAllSecureRemote(reg, "monalisa", "skills-repo")
|
||||
},
|
||||
opts: func(ios *iostreams.IOStreams, dir string, reg *httpmock.Registry) *PublishOptions {
|
||||
t.Helper()
|
||||
return &PublishOptions{
|
||||
IO: ios,
|
||||
Dir: dir,
|
||||
DryRun: true,
|
||||
GitClient: &git.Client{},
|
||||
client: api.NewClientFromHTTP(&http.Client{Transport: reg}),
|
||||
host: "github.com",
|
||||
}
|
||||
},
|
||||
wantStdout: "1 skill(s) validated successfully",
|
||||
wantStderr: "Dry run complete",
|
||||
},
|
||||
{
|
||||
name: "valid skill dry-run passes validation",
|
||||
isTTY: true,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue