fix github registry URL parsing and versioned skill links
This commit is contained in:
parent
f2a5db1349
commit
927093aabf
10 changed files with 200 additions and 23 deletions
|
|
@ -126,7 +126,7 @@ func (c *ClawHubRegistry) ResolveInstallDirName(target string) (string, error) {
|
|||
return target, nil
|
||||
}
|
||||
|
||||
func (c *ClawHubRegistry) SkillURL(slug string) string {
|
||||
func (c *ClawHubRegistry) SkillURL(slug, _ string) string {
|
||||
if slug == "" {
|
||||
return ""
|
||||
}
|
||||
|
|
|
|||
|
|
@ -68,11 +68,15 @@ func (r *GitHubRegistry) Name() string {
|
|||
}
|
||||
|
||||
func (r *GitHubRegistry) ResolveInstallDirName(target string) (string, error) {
|
||||
return githubInstallDirName(target)
|
||||
return githubInstallDirNameWithBaseURL(target, r.webBase)
|
||||
}
|
||||
|
||||
func (r *GitHubRegistry) SkillURL(target string) string {
|
||||
ref, err := parseGitHubRef(target)
|
||||
func (r *GitHubRegistry) SkillURL(target, version string) string {
|
||||
defaultRef := strings.TrimSpace(version)
|
||||
if defaultRef == "" {
|
||||
defaultRef = "main"
|
||||
}
|
||||
ref, err := parseGitHubRefWithBaseURL(target, r.webBase, defaultRef)
|
||||
if err != nil {
|
||||
return ""
|
||||
}
|
||||
|
|
@ -233,7 +237,7 @@ func githubSearchDisplayName(item gitHubCodeSearchItem) string {
|
|||
}
|
||||
|
||||
func (r *GitHubRegistry) GetSkillMeta(_ context.Context, target string) (*SkillMeta, error) {
|
||||
ref, err := parseGitHubRef(target)
|
||||
ref, err := parseGitHubRefWithBaseURL(target, r.webBase, "main")
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
|
|
|||
|
|
@ -136,3 +136,22 @@ func TestGitHubRegistrySearchReturnsEmptyOnUnauthenticatedAuthRequired(t *testin
|
|||
require.NoError(t, err)
|
||||
assert.Empty(t, results)
|
||||
}
|
||||
|
||||
func TestGitHubRegistrySkillURLUsesProvidedVersionAndBasePath(t *testing.T) {
|
||||
registry := GitHubRegistryConfig{
|
||||
Enabled: true,
|
||||
BaseURL: "https://ghe.example.com/git",
|
||||
}.BuildRegistry()
|
||||
require.NotNil(t, registry)
|
||||
|
||||
assert.Equal(
|
||||
t,
|
||||
"https://ghe.example.com/git/org/repo/tree/master/skills/pr-review",
|
||||
registry.SkillURL("org/repo/skills/pr-review", "master"),
|
||||
)
|
||||
assert.Equal(
|
||||
t,
|
||||
"https://ghe.example.com/git/org/repo/tree/dev/skills/pr-review",
|
||||
registry.SkillURL("https://ghe.example.com/git/org/repo/tree/dev/skills/pr-review", ""),
|
||||
)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -137,10 +137,48 @@ func resolveGitHubEndpoints(baseURL string) (gitHubEndpoints, error) {
|
|||
}, nil
|
||||
}
|
||||
|
||||
func parseGitHubRefPathParts(repoURL *url.URL, githubBaseURL string) []string {
|
||||
parts := strings.Split(strings.Trim(repoURL.Path, "/"), "/")
|
||||
if len(parts) == 0 {
|
||||
return parts
|
||||
}
|
||||
if githubBaseURL == "" {
|
||||
return parts
|
||||
}
|
||||
baseURL, err := url.Parse(strings.TrimSpace(githubBaseURL))
|
||||
if err != nil {
|
||||
return parts
|
||||
}
|
||||
if !strings.EqualFold(repoURL.Host, baseURL.Host) || !strings.EqualFold(repoURL.Scheme, baseURL.Scheme) {
|
||||
return parts
|
||||
}
|
||||
baseParts := strings.Split(strings.Trim(baseURL.Path, "/"), "/")
|
||||
if len(baseParts) == 1 && baseParts[0] == "" {
|
||||
baseParts = nil
|
||||
}
|
||||
if len(baseParts) == 0 || len(parts) < len(baseParts)+2 {
|
||||
return parts
|
||||
}
|
||||
for i, part := range baseParts {
|
||||
if parts[i] != part {
|
||||
return parts
|
||||
}
|
||||
}
|
||||
return parts[len(baseParts):]
|
||||
}
|
||||
|
||||
// parseGitHubRef parses a GitHub reference.
|
||||
// Supports: "owner/repo", "owner/repo/path", or full URL like "https://github.com/owner/repo/tree/ref/path"
|
||||
func parseGitHubRef(repo string) (GitHubRef, error) {
|
||||
return parseGitHubRefWithBaseURL(repo, "", "main")
|
||||
}
|
||||
|
||||
func parseGitHubRefWithBaseURL(repo, githubBaseURL, defaultRef string) (GitHubRef, error) {
|
||||
repo = strings.TrimSpace(repo)
|
||||
defaultRef = strings.TrimSpace(defaultRef)
|
||||
if defaultRef == "" {
|
||||
defaultRef = "main"
|
||||
}
|
||||
|
||||
// Handle full URL
|
||||
if strings.HasPrefix(repo, "http://") || strings.HasPrefix(repo, "https://") {
|
||||
|
|
@ -148,14 +186,14 @@ func parseGitHubRef(repo string) (GitHubRef, error) {
|
|||
if err != nil {
|
||||
return GitHubRef{}, fmt.Errorf("invalid URL: %w", err)
|
||||
}
|
||||
parts := strings.Split(strings.Trim(u.Path, "/"), "/")
|
||||
parts := parseGitHubRefPathParts(u, githubBaseURL)
|
||||
if len(parts) < 2 {
|
||||
return GitHubRef{}, fmt.Errorf("invalid GitHub URL")
|
||||
}
|
||||
ref := GitHubRef{
|
||||
Owner: parts[0],
|
||||
RepoName: parts[1],
|
||||
Ref: "main",
|
||||
Ref: defaultRef,
|
||||
}
|
||||
// Look for /tree/ or /blob/ in the path
|
||||
for i := 2; i < len(parts); i++ {
|
||||
|
|
@ -178,7 +216,7 @@ func parseGitHubRef(repo string) (GitHubRef, error) {
|
|||
ref := GitHubRef{
|
||||
Owner: parts[0],
|
||||
RepoName: parts[1],
|
||||
Ref: "main",
|
||||
Ref: defaultRef,
|
||||
}
|
||||
if len(parts) > 2 {
|
||||
ref.SubPath = strings.Join(parts[2:], "/")
|
||||
|
|
@ -187,10 +225,16 @@ func parseGitHubRef(repo string) (GitHubRef, error) {
|
|||
}
|
||||
|
||||
func githubInstallDirName(repo string) (string, error) {
|
||||
if err := ValidateInstallTarget(repo); err != nil {
|
||||
return "", err
|
||||
return githubInstallDirNameWithBaseURL(repo, "")
|
||||
}
|
||||
|
||||
func githubInstallDirNameWithBaseURL(repo, githubBaseURL string) (string, error) {
|
||||
if !strings.HasPrefix(repo, "http://") && !strings.HasPrefix(repo, "https://") {
|
||||
if err := ValidateInstallTarget(repo); err != nil {
|
||||
return "", err
|
||||
}
|
||||
}
|
||||
ref, err := parseGitHubRef(repo)
|
||||
ref, err := parseGitHubRefWithBaseURL(repo, githubBaseURL, "main")
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
|
|
@ -201,7 +245,7 @@ func githubInstallDirName(repo string) (string, error) {
|
|||
}
|
||||
|
||||
func (si *SkillInstaller) InstallFromGitHub(ctx context.Context, repo string) error {
|
||||
skillName, err := githubInstallDirName(repo)
|
||||
skillName, err := githubInstallDirNameWithBaseURL(repo, si.githubBaseURL)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
|
@ -218,7 +262,7 @@ func (si *SkillInstaller) InstallFromGitHubToDir(
|
|||
ctx context.Context,
|
||||
repo, version, skillDirectory string,
|
||||
) (*InstallResult, error) {
|
||||
ref, err := parseGitHubRef(repo)
|
||||
ref, err := parseGitHubRefWithBaseURL(repo, si.githubBaseURL, "main")
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
|
|
|||
|
|
@ -127,6 +127,40 @@ func TestParseGitHubRef(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func TestParseGitHubRefWithBaseURL(t *testing.T) {
|
||||
ref, err := parseGitHubRefWithBaseURL(
|
||||
"https://ghe.example.com/git/org/repo/tree/dev/skills/test",
|
||||
"https://ghe.example.com/git",
|
||||
"main",
|
||||
)
|
||||
if err != nil {
|
||||
t.Fatalf("parseGitHubRefWithBaseURL() unexpected error = %v", err)
|
||||
}
|
||||
if ref.Owner != "org" {
|
||||
t.Fatalf("owner = %q, want org", ref.Owner)
|
||||
}
|
||||
if ref.RepoName != "repo" {
|
||||
t.Fatalf("repo = %q, want repo", ref.RepoName)
|
||||
}
|
||||
if ref.Ref != "dev" {
|
||||
t.Fatalf("ref = %q, want dev", ref.Ref)
|
||||
}
|
||||
if ref.SubPath != "skills/test" {
|
||||
t.Fatalf("subPath = %q, want skills/test", ref.SubPath)
|
||||
}
|
||||
|
||||
dirName, err := githubInstallDirNameWithBaseURL(
|
||||
"https://ghe.example.com/git/org/repo/tree/dev/skills/test",
|
||||
"https://ghe.example.com/git",
|
||||
)
|
||||
if err != nil {
|
||||
t.Fatalf("githubInstallDirNameWithBaseURL() unexpected error = %v", err)
|
||||
}
|
||||
if dirName != "test" {
|
||||
t.Fatalf("dirName = %q, want test", dirName)
|
||||
}
|
||||
}
|
||||
|
||||
func TestShouldDownload(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
|
|
|
|||
|
|
@ -61,7 +61,8 @@ type SkillRegistry interface {
|
|||
// differently (for example, a slug vs owner/repo/path).
|
||||
ResolveInstallDirName(target string) (string, error)
|
||||
// SkillURL returns the web URL for a skill slug if the registry exposes one.
|
||||
SkillURL(slug string) string
|
||||
// version is optional and can be used by registries whose URLs depend on a ref.
|
||||
SkillURL(slug, version string) string
|
||||
// Search searches the registry for skills matching the query.
|
||||
Search(ctx context.Context, query string, limit int) ([]SearchResult, error)
|
||||
// GetSkillMeta retrieves metadata for a specific skill by slug.
|
||||
|
|
|
|||
|
|
@ -26,7 +26,7 @@ func (m *mockRegistry) Name() string { return m.name }
|
|||
|
||||
func (m *mockRegistry) ResolveInstallDirName(target string) (string, error) { return target, nil }
|
||||
|
||||
func (m *mockRegistry) SkillURL(slug string) string { return "https://example.com/skills/" + slug }
|
||||
func (m *mockRegistry) SkillURL(slug, _ string) string { return "https://example.com/skills/" + slug }
|
||||
|
||||
func (m *mockRegistry) Search(_ context.Context, _ string, _ int) ([]SearchResult, error) {
|
||||
return m.searchResults, m.searchErr
|
||||
|
|
|
|||
|
|
@ -20,7 +20,7 @@ func (m *mockInstallRegistry) ResolveInstallDirName(target string) (string, erro
|
|||
return target, nil
|
||||
}
|
||||
|
||||
func (m *mockInstallRegistry) SkillURL(slug string) string { return slug }
|
||||
func (m *mockInstallRegistry) SkillURL(slug, _ string) string { return slug }
|
||||
|
||||
func (m *mockInstallRegistry) Search(context.Context, string, int) ([]skills.SearchResult, error) {
|
||||
return nil, nil
|
||||
|
|
@ -47,7 +47,7 @@ func (m *mockGitHubInstallRegistry) ResolveInstallDirName(target string) (string
|
|||
return "pr-review", nil
|
||||
}
|
||||
|
||||
func (m *mockGitHubInstallRegistry) SkillURL(slug string) string { return slug }
|
||||
func (m *mockGitHubInstallRegistry) SkillURL(slug, _ string) string { return slug }
|
||||
|
||||
func (m *mockGitHubInstallRegistry) Search(context.Context, string, int) ([]skills.SearchResult, error) {
|
||||
return nil, nil
|
||||
|
|
|
|||
|
|
@ -249,7 +249,7 @@ func (h *Handler) handleSearchSkills(w http.ResponseWriter, r *http.Request) {
|
|||
Summary: result.Summary,
|
||||
Version: result.Version,
|
||||
RegistryName: result.RegistryName,
|
||||
URL: registrySkillURL(cfg, result.RegistryName, result.Slug),
|
||||
URL: registrySkillURL(cfg, result.RegistryName, result.Slug, result.Version),
|
||||
Installed: installed,
|
||||
}
|
||||
if installed {
|
||||
|
|
@ -377,7 +377,7 @@ func (h *Handler) handleInstallSkill(w http.ResponseWriter, r *http.Request) {
|
|||
OriginKind: "third_party",
|
||||
Registry: registry.Name(),
|
||||
Slug: req.Slug,
|
||||
RegistryURL: registrySkillURL(cfg, registry.Name(), req.Slug),
|
||||
RegistryURL: registrySkillURL(cfg, registry.Name(), req.Slug, result.Version),
|
||||
InstalledVersion: result.Version,
|
||||
InstalledAt: installedAt,
|
||||
}); err != nil {
|
||||
|
|
@ -412,7 +412,7 @@ func (h *Handler) handleInstallSkill(w http.ResponseWriter, r *http.Request) {
|
|||
Description: validatedSkill.Description,
|
||||
OriginKind: "third_party",
|
||||
RegistryName: registry.Name(),
|
||||
RegistryURL: registrySkillURL(cfg, registry.Name(), req.Slug),
|
||||
RegistryURL: registrySkillURL(cfg, registry.Name(), req.Slug, result.Version),
|
||||
InstalledVersion: result.Version,
|
||||
InstalledAt: installedAt,
|
||||
}
|
||||
|
|
@ -726,7 +726,7 @@ func writeSkillOriginMeta(targetDir string, meta installedSkillOriginMeta) error
|
|||
return fileutil.WriteFileAtomic(filepath.Join(targetDir, ".skill-origin.json"), data, 0o600)
|
||||
}
|
||||
|
||||
func registrySkillURL(cfg *config.Config, registryName, slug string) string {
|
||||
func registrySkillURL(cfg *config.Config, registryName, slug, version string) string {
|
||||
if cfg == nil || registryName == "" || slug == "" {
|
||||
return ""
|
||||
}
|
||||
|
|
@ -734,7 +734,7 @@ func registrySkillURL(cfg *config.Config, registryName, slug string) string {
|
|||
if registry == nil {
|
||||
return ""
|
||||
}
|
||||
return registry.SkillURL(slug)
|
||||
return registry.SkillURL(slug, version)
|
||||
}
|
||||
|
||||
func registrySkillURLFromMeta(cfg *config.Config, meta *installedSkillOriginMeta) string {
|
||||
|
|
@ -747,7 +747,7 @@ func registrySkillURLFromMeta(cfg *config.Config, meta *installedSkillOriginMeta
|
|||
if cfg == nil || meta.Registry == "" {
|
||||
return ""
|
||||
}
|
||||
return registrySkillURL(cfg, meta.Registry, meta.Slug)
|
||||
return registrySkillURL(cfg, meta.Registry, meta.Slug, meta.InstalledVersion)
|
||||
}
|
||||
|
||||
func normalizeImportedSkillName(filename string, content []byte) (string, error) {
|
||||
|
|
|
|||
|
|
@ -26,6 +26,15 @@ func setClawHubBaseURL(cfg *config.Config, baseURL string) {
|
|||
cfg.Tools.Skills.Registries.Set("clawhub", registryCfg)
|
||||
}
|
||||
|
||||
func setGithubBaseURL(cfg *config.Config, baseURL string) {
|
||||
registryCfg, ok := cfg.Tools.Skills.Registries.Get("github")
|
||||
if !ok {
|
||||
return
|
||||
}
|
||||
registryCfg.BaseURL = baseURL
|
||||
cfg.Tools.Skills.Registries.Set("github", registryCfg)
|
||||
}
|
||||
|
||||
func TestHandleListSkills(t *testing.T) {
|
||||
configPath, cleanup := setupOAuthTestEnv(t)
|
||||
defer cleanup()
|
||||
|
|
@ -636,6 +645,72 @@ func TestHandleSearchSkills(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func TestHandleSearchSkillsUsesGitHubResultVersionInURL(t *testing.T) {
|
||||
configPath, cleanup := setupOAuthTestEnv(t)
|
||||
defer cleanup()
|
||||
|
||||
cfg, err := config.LoadConfig(configPath)
|
||||
if err != nil {
|
||||
t.Fatalf("LoadConfig() error = %v", err)
|
||||
}
|
||||
workspace := filepath.Join(t.TempDir(), "workspace")
|
||||
cfg.Agents.Defaults.Workspace = workspace
|
||||
|
||||
var server *httptest.Server
|
||||
server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if r.URL.Path != "/api/v3/search/code" {
|
||||
http.NotFound(w, r)
|
||||
return
|
||||
}
|
||||
json.NewEncoder(w).Encode(map[string]any{
|
||||
"items": []map[string]any{
|
||||
{
|
||||
"path": "skills/pr-review/SKILL.md",
|
||||
"score": 10,
|
||||
"repository": map[string]any{
|
||||
"full_name": "foo/bar",
|
||||
"name": "bar",
|
||||
"description": "Review pull requests",
|
||||
"default_branch": "master",
|
||||
},
|
||||
},
|
||||
},
|
||||
})
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
setGithubBaseURL(cfg, server.URL)
|
||||
clawHubRegistry, _ := cfg.Tools.Skills.Registries.Get("clawhub")
|
||||
clawHubRegistry.Enabled = false
|
||||
cfg.Tools.Skills.Registries.Set("clawhub", clawHubRegistry)
|
||||
if err := config.SaveConfig(configPath, cfg); err != nil {
|
||||
t.Fatalf("SaveConfig() error = %v", err)
|
||||
}
|
||||
|
||||
h := NewHandler(configPath)
|
||||
mux := http.NewServeMux()
|
||||
h.RegisterRoutes(mux)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
req := httptest.NewRequest(http.MethodGet, "/api/skills/search?q=pr+review&limit=5", nil)
|
||||
mux.ServeHTTP(rec, req)
|
||||
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want %d, body=%s", rec.Code, http.StatusOK, rec.Body.String())
|
||||
}
|
||||
|
||||
var resp skillSearchResponse
|
||||
if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil {
|
||||
t.Fatalf("Unmarshal() error = %v", err)
|
||||
}
|
||||
if len(resp.Results) != 1 {
|
||||
t.Fatalf("results count = %d, want 1", len(resp.Results))
|
||||
}
|
||||
if resp.Results[0].URL != server.URL+"/foo/bar/tree/master/skills/pr-review" {
|
||||
t.Fatalf("result URL = %q", resp.Results[0].URL)
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleSearchSkillsPagination(t *testing.T) {
|
||||
configPath, cleanup := setupOAuthTestEnv(t)
|
||||
defer cleanup()
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue