fix github skills registry fallback and install metadata
This commit is contained in:
parent
61a43b173a
commit
eac37366fa
8 changed files with 249 additions and 19 deletions
|
|
@ -970,12 +970,14 @@ func (v *clawHubRegistryConfigV0) ToSkillRegistryConfig() SkillRegistryConfig {
|
|||
}
|
||||
|
||||
type skillsGithubConfigV0 struct {
|
||||
BaseURL string `json:"base_url,omitempty" env:"PICOCLAW_TOOLS_SKILLS_GITHUB_BASE_URL"`
|
||||
Token string `json:"token" env:"PICOCLAW_TOOLS_SKILLS_GITHUB_TOKEN"`
|
||||
Proxy string `json:"proxy,omitempty" env:"PICOCLAW_TOOLS_SKILLS_GITHUB_PROXY"`
|
||||
}
|
||||
|
||||
func (v *skillsGithubConfigV0) ToSkillsGithubConfig() SkillsGithubConfig {
|
||||
return SkillsGithubConfig{
|
||||
BaseURL: v.BaseURL,
|
||||
Token: *NewSecureString(v.Token),
|
||||
Proxy: v.Proxy,
|
||||
}
|
||||
|
|
|
|||
|
|
@ -262,3 +262,16 @@ func TestSkillsRegistriesConfigMarshalJSONPreservesObjectShape(t *testing.T) {
|
|||
assert.True(t, ok)
|
||||
assert.Equal(t, "https://clawhub.ai", clawhub.BaseURL)
|
||||
}
|
||||
|
||||
func TestSkillsGithubConfigV0ToSkillsGithubConfigPreservesBaseURL(t *testing.T) {
|
||||
legacy := skillsGithubConfigV0{
|
||||
BaseURL: "https://ghe.example.com/git",
|
||||
Token: "ghp-test-token",
|
||||
Proxy: "http://127.0.0.1:7890",
|
||||
}
|
||||
|
||||
converted := legacy.ToSkillsGithubConfig()
|
||||
assert.Equal(t, "https://ghe.example.com/git", converted.BaseURL)
|
||||
assert.Equal(t, "ghp-test-token", converted.Token.String())
|
||||
assert.Equal(t, "http://127.0.0.1:7890", converted.Proxy)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -94,3 +94,22 @@ func LookupRegistryFromToolsConfig(cfg config.SkillsToolsConfig, name string) Sk
|
|||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func NormalizeInstallTargetForRegistry(cfg config.SkillsToolsConfig, registryName, target string) string {
|
||||
if registryName == "" || target == "" {
|
||||
return target
|
||||
}
|
||||
registry := LookupRegistryFromToolsConfig(cfg, registryName)
|
||||
if registry == nil {
|
||||
return target
|
||||
}
|
||||
ghRegistry, ok := registry.(*GitHubRegistry)
|
||||
if !ok {
|
||||
return target
|
||||
}
|
||||
normalized, err := canonicalGitHubRegistrySlugWithBaseURL(target, ghRegistry.webBase)
|
||||
if err != nil || normalized == "" {
|
||||
return target
|
||||
}
|
||||
return normalized
|
||||
}
|
||||
|
|
|
|||
|
|
@ -147,10 +147,12 @@ func (r *GitHubRegistry) Search(ctx context.Context, query string, limit int) ([
|
|||
return nil, fmt.Errorf("failed to read github search response: %w", err)
|
||||
}
|
||||
if resp.StatusCode == http.StatusUnauthorized && r.installer.githubToken == "" && isGitHubAuthRequiredError(body) {
|
||||
return nil, fmt.Errorf("github search requires authentication; %s", githubAuthTokenHelp)
|
||||
slog.Warn("github search requires authentication; returning no results", "help", githubAuthTokenHelp)
|
||||
return []SearchResult{}, nil
|
||||
}
|
||||
if resp.StatusCode == http.StatusForbidden && r.installer.githubToken == "" && isGitHubRateLimitError(body) {
|
||||
return nil, fmt.Errorf("github search hit the unauthenticated rate limit; %s", githubAuthTokenHelp)
|
||||
slog.Warn("github search hit unauthenticated rate limit; returning no results", "help", githubAuthTokenHelp)
|
||||
return []SearchResult{}, nil
|
||||
}
|
||||
if resp.StatusCode < 200 || resp.StatusCode >= 300 {
|
||||
return nil, fmt.Errorf("github search failed: HTTP %d: %s", resp.StatusCode, string(body))
|
||||
|
|
@ -238,7 +240,23 @@ func githubSearchDisplayName(item gitHubCodeSearchItem) string {
|
|||
return strings.TrimSpace(item.Repository.FullName)
|
||||
}
|
||||
|
||||
func canonicalGitHubRegistrySlugWithBaseURL(target, githubBaseURL string) (string, error) {
|
||||
ref, err := parseGitHubRefWithBaseURL(target, githubBaseURL, "")
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
slug := path.Join(ref.Owner, ref.RepoName)
|
||||
if ref.SubPath != "" {
|
||||
slug = path.Join(slug, ref.SubPath)
|
||||
}
|
||||
return slug, nil
|
||||
}
|
||||
|
||||
func (r *GitHubRegistry) GetSkillMeta(ctx context.Context, target string) (*SkillMeta, error) {
|
||||
slug, err := canonicalGitHubRegistrySlugWithBaseURL(target, r.webBase)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
ref, err := parseGitHubRefWithBaseURL(target, r.webBase, "")
|
||||
if err != nil {
|
||||
return nil, err
|
||||
|
|
@ -250,7 +268,7 @@ func (r *GitHubRegistry) GetSkillMeta(ctx context.Context, target string) (*Skil
|
|||
}
|
||||
}
|
||||
return &SkillMeta{
|
||||
Slug: target,
|
||||
Slug: slug,
|
||||
DisplayName: ref.RepoName,
|
||||
LatestVersion: ref.Ref,
|
||||
RegistryName: r.Name(),
|
||||
|
|
|
|||
|
|
@ -103,7 +103,7 @@ func TestGitHubRegistryProviderDecodesProxyParam(t *testing.T) {
|
|||
assert.Equal(t, "http://127.0.0.1:7890", ghRegistry.installer.proxy)
|
||||
}
|
||||
|
||||
func TestGitHubRegistrySearchReturnsHelpfulErrorOnUnauthenticatedRateLimit(t *testing.T) {
|
||||
func TestGitHubRegistrySearchReturnsNoResultsOnUnauthenticatedRateLimit(t *testing.T) {
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
assert.Empty(t, r.Header.Get("Authorization"))
|
||||
w.WriteHeader(http.StatusForbidden)
|
||||
|
|
@ -115,12 +115,11 @@ func TestGitHubRegistrySearchReturnsHelpfulErrorOnUnauthenticatedRateLimit(t *te
|
|||
require.NotNil(t, registry)
|
||||
|
||||
results, err := registry.Search(context.Background(), "pr review", 5)
|
||||
require.Error(t, err)
|
||||
assert.Nil(t, results)
|
||||
assert.Contains(t, err.Error(), "registries.github.auth_token")
|
||||
require.NoError(t, err)
|
||||
assert.Empty(t, results)
|
||||
}
|
||||
|
||||
func TestGitHubRegistrySearchReturnsHelpfulErrorOnUnauthenticatedAuthRequired(t *testing.T) {
|
||||
func TestGitHubRegistrySearchReturnsNoResultsOnUnauthenticatedAuthRequired(t *testing.T) {
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
assert.Empty(t, r.Header.Get("Authorization"))
|
||||
w.WriteHeader(http.StatusUnauthorized)
|
||||
|
|
@ -134,9 +133,25 @@ func TestGitHubRegistrySearchReturnsHelpfulErrorOnUnauthenticatedAuthRequired(t
|
|||
require.NotNil(t, registry)
|
||||
|
||||
results, err := registry.Search(context.Background(), "pr review", 5)
|
||||
require.Error(t, err)
|
||||
assert.Nil(t, results)
|
||||
assert.Contains(t, err.Error(), "registries.github.auth_token")
|
||||
require.NoError(t, err)
|
||||
assert.Empty(t, results)
|
||||
}
|
||||
|
||||
func TestGitHubRegistryGetSkillMetaCanonicalizesURLSlug(t *testing.T) {
|
||||
registry := GitHubRegistryConfig{
|
||||
Enabled: true,
|
||||
BaseURL: "https://ghe.example.com/git",
|
||||
}.BuildRegistry()
|
||||
require.NotNil(t, registry)
|
||||
|
||||
meta, err := registry.GetSkillMeta(
|
||||
context.Background(),
|
||||
"https://ghe.example.com/git/org/repo/tree/dev/skills/pr-review",
|
||||
)
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, meta)
|
||||
assert.Equal(t, "org/repo/skills/pr-review", meta.Slug)
|
||||
assert.Equal(t, "dev", meta.LatestVersion)
|
||||
}
|
||||
|
||||
func TestGitHubRegistrySkillURLUsesProvidedVersionAndBasePath(t *testing.T) {
|
||||
|
|
|
|||
|
|
@ -238,3 +238,20 @@ func TestExplicitGithubRegistryBaseURLBeatsLegacyCompat(t *testing.T) {
|
|||
assert.True(t, ok)
|
||||
assert.Equal(t, "https://ghe-explicit.example.com/scm", ghRegistry.webBase)
|
||||
}
|
||||
|
||||
func TestNormalizeInstallTargetForRegistryCanonicalizesGitHubURLs(t *testing.T) {
|
||||
cfg := config.DefaultConfig().Tools.Skills
|
||||
cfg.Registries.Set("github", config.SkillRegistryConfig{
|
||||
Name: "github",
|
||||
Enabled: true,
|
||||
BaseURL: "https://ghe.example.com/git",
|
||||
Param: map[string]any{},
|
||||
})
|
||||
|
||||
got := NormalizeInstallTargetForRegistry(
|
||||
cfg,
|
||||
"github",
|
||||
"https://ghe.example.com/git/org/repo/tree/dev/skills/pr-review",
|
||||
)
|
||||
assert.Equal(t, "org/repo/skills/pr-review", got)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -372,12 +372,13 @@ func (h *Handler) handleInstallSkill(w http.ResponseWriter, r *http.Request) {
|
|||
}
|
||||
|
||||
installedAt := time.Now().UnixMilli()
|
||||
normalizedSlug := skills.NormalizeInstallTargetForRegistry(cfg.Tools.Skills, registry.Name(), req.Slug)
|
||||
if err := persistSkillOriginMeta(stagedTargetDir, installedSkillOriginMeta{
|
||||
Version: 1,
|
||||
OriginKind: "third_party",
|
||||
Registry: registry.Name(),
|
||||
Slug: req.Slug,
|
||||
RegistryURL: registrySkillURL(cfg, registry.Name(), req.Slug, result.Version),
|
||||
Slug: normalizedSlug,
|
||||
RegistryURL: registrySkillURL(cfg, registry.Name(), normalizedSlug, result.Version),
|
||||
InstalledVersion: result.Version,
|
||||
InstalledAt: installedAt,
|
||||
}); err != nil {
|
||||
|
|
@ -412,7 +413,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, result.Version),
|
||||
RegistryURL: registrySkillURL(cfg, registry.Name(), normalizedSlug, result.Version),
|
||||
InstalledVersion: result.Version,
|
||||
InstalledAt: installedAt,
|
||||
}
|
||||
|
|
@ -570,8 +571,11 @@ func buildOccupiedWorkspaceSkillsByDirectory(cfg *config.Config) (map[string]ski
|
|||
|
||||
key := filepath.Base(filepath.Dir(skill.Path))
|
||||
if meta, err := readInstalledSkillOriginMeta(skill.Path); err == nil && meta != nil && meta.Slug != "" {
|
||||
key = skills.NormalizeInstallTargetForRegistry(cfg.Tools.Skills, meta.Registry, meta.Slug)
|
||||
if key == "" {
|
||||
key = meta.Slug
|
||||
}
|
||||
}
|
||||
if key == "" {
|
||||
continue
|
||||
}
|
||||
|
|
|
|||
|
|
@ -711,6 +711,56 @@ func TestHandleSearchSkillsUsesGitHubResultVersionInURL(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func TestHandleSearchSkillsGitHubRateLimitDegradesGracefully(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
|
||||
|
||||
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
|
||||
}
|
||||
w.WriteHeader(http.StatusForbidden)
|
||||
_, _ = w.Write([]byte(`{"message":"API rate limit exceeded for 1.2.3.4"}`))
|
||||
}))
|
||||
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) != 0 {
|
||||
t.Fatalf("results count = %d, want 0", len(resp.Results))
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleSearchSkillsPagination(t *testing.T) {
|
||||
configPath, cleanup := setupOAuthTestEnv(t)
|
||||
defer cleanup()
|
||||
|
|
@ -1169,6 +1219,98 @@ func TestHandleInstallSkillDefaultsRegistryToGitHub(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func TestHandleInstallSkillTracksGitHubURLInstallsAsInstalled(t *testing.T) {
|
||||
configPath, cleanup := setupOAuthTestEnv(t)
|
||||
defer cleanup()
|
||||
|
||||
cfg, loadErr := config.LoadConfig(configPath)
|
||||
if loadErr != nil {
|
||||
t.Fatalf("LoadConfig() error = %v", loadErr)
|
||||
}
|
||||
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) {
|
||||
switch r.URL.Path {
|
||||
case "/api/v3/repos/foo/bar":
|
||||
json.NewEncoder(w).Encode(map[string]any{"default_branch": "master"})
|
||||
case "/api/v3/repos/foo/bar/contents/.agents/skills/pr-review":
|
||||
assert.Equal(t, "ref=master", r.URL.RawQuery)
|
||||
json.NewEncoder(w).Encode([]map[string]any{{
|
||||
"type": "file",
|
||||
"name": "SKILL.md",
|
||||
"download_url": server.URL + "/raw/foo/bar/master/.agents/skills/pr-review/SKILL.md",
|
||||
}})
|
||||
case "/api/v3/search/code":
|
||||
json.NewEncoder(w).Encode(map[string]any{
|
||||
"items": []map[string]any{{
|
||||
"path": ".agents/skills/pr-review/SKILL.md",
|
||||
"score": 10,
|
||||
"repository": map[string]any{
|
||||
"full_name": "foo/bar",
|
||||
"name": "bar",
|
||||
"description": "PR review skill",
|
||||
"default_branch": "master",
|
||||
},
|
||||
}},
|
||||
})
|
||||
case "/raw/foo/bar/master/.agents/skills/pr-review/SKILL.md":
|
||||
_, _ = w.Write([]byte("---\nname: pr-review\ndescription: PR review skill\n---\n# PR Review\n"))
|
||||
default:
|
||||
http.NotFound(w, r)
|
||||
}
|
||||
}))
|
||||
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 saveErr := config.SaveConfig(configPath, cfg); saveErr != nil {
|
||||
t.Fatalf("SaveConfig() error = %v", saveErr)
|
||||
}
|
||||
|
||||
h := NewHandler(configPath)
|
||||
mux := http.NewServeMux()
|
||||
h.RegisterRoutes(mux)
|
||||
|
||||
installBody, err := json.Marshal(installSkillRequest{
|
||||
Slug: server.URL + "/foo/bar/tree/master/.agents/skills/pr-review",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("Marshal() error = %v", err)
|
||||
}
|
||||
|
||||
installRec := httptest.NewRecorder()
|
||||
installReq := httptest.NewRequest(http.MethodPost, "/api/skills/install", bytes.NewReader(installBody))
|
||||
installReq.Header.Set("Content-Type", "application/json")
|
||||
mux.ServeHTTP(installRec, installReq)
|
||||
|
||||
if installRec.Code != http.StatusOK {
|
||||
t.Fatalf("install status = %d, want %d, body=%s", installRec.Code, http.StatusOK, installRec.Body.String())
|
||||
}
|
||||
|
||||
searchRec := httptest.NewRecorder()
|
||||
searchReq := httptest.NewRequest(http.MethodGet, "/api/skills/search?q=pr+review&limit=5", nil)
|
||||
mux.ServeHTTP(searchRec, searchReq)
|
||||
|
||||
if searchRec.Code != http.StatusOK {
|
||||
t.Fatalf("search status = %d, want %d, body=%s", searchRec.Code, http.StatusOK, searchRec.Body.String())
|
||||
}
|
||||
|
||||
var searchResp skillSearchResponse
|
||||
if err := json.Unmarshal(searchRec.Body.Bytes(), &searchResp); err != nil {
|
||||
t.Fatalf("Unmarshal(search response) error = %v", err)
|
||||
}
|
||||
if len(searchResp.Results) != 1 {
|
||||
t.Fatalf("search results count = %d, want 1", len(searchResp.Results))
|
||||
}
|
||||
if !searchResp.Results[0].Installed || searchResp.Results[0].InstalledName != "pr-review" {
|
||||
t.Fatalf("search result should be treated as installed after URL install, got %#v", searchResp.Results[0])
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleInstallSkillRollsBackOnOriginMetadataWriteFailure(t *testing.T) {
|
||||
configPath, cleanup := setupOAuthTestEnv(t)
|
||||
defer cleanup()
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue