enhance install skills v2

2026-03-09 17:23:54 +08:00 · 2026-03-09 17:23:54 +08:00 · 9edb454185
commit 9edb454185
parent 2f3e274e26
3 changed files with 211 additions and 87 deletions
--- a/cmd/picoclaw/internal/skills/command.go
+++ b/cmd/picoclaw/internal/skills/command.go
@ -29,7 +29,7 @@ func NewSkillsCommand() *cobra.Command {
 			}

 			d.workspace = cfg.WorkspacePath()
-			d.installer = skills.NewSkillInstaller(d.workspace)
+			d.installer = skills.NewSkillInstaller(d.workspace, cfg.Tools.Skills.Github.Token)

 			// get global config directory and builtin skills directory
 			globalDir := filepath.Dir(internal.GetConfigPath())
--- a/pkg/config/config.go
+++ b/pkg/config/config.go
@ -650,6 +650,7 @@ type ExecConfig struct {
 type SkillsToolsConfig struct {
 	ToolConfig            `                       envPrefix:"PICOCLAW_TOOLS_SKILLS_"`
 	Registries            SkillsRegistriesConfig `                                   json:"registries"`
+	Github                SkillsGithubConfig     `                                   json:"github"`
 	MaxConcurrentSearches int                    `                                   json:"max_concurrent_searches" env:"PICOCLAW_TOOLS_SKILLS_MAX_CONCURRENT_SEARCHES"`
 	SearchCache           SearchCacheConfig      `                                   json:"search_cache"`
 }
@ -694,6 +695,10 @@ type SkillsRegistriesConfig struct {
 	ClawHub ClawHubRegistryConfig `json:"clawhub"`
 }

+type SkillsGithubConfig struct {
+	Token string `json:"token"`
+}
+
 type ClawHubRegistryConfig struct {
 	Enabled         bool   `json:"enabled"           env:"PICOCLAW_SKILLS_REGISTRIES_CLAWHUB_ENABLED"`
 	BaseURL         string `json:"base_url"          env:"PICOCLAW_SKILLS_REGISTRIES_CLAWHUB_BASE_URL"`
@ -961,3 +966,4 @@ func (t *ToolsConfig) IsToolEnabled(name string) bool {
 		return true
 	}
 }
+
--- a/pkg/skills/installer.go
+++ b/pkg/skills/installer.go
@ -8,6 +8,7 @@ import (
 	"net/http"
 	"net/url"
 	"os"
+	"path"
 	"path/filepath"
 	"strings"
 	"time"
@ -16,147 +17,264 @@ import (
 	"github.com/sipeed/picoclaw/pkg/utils"
 )

+// GitHubContent represents a file or directory in GitHub API response
 type GitHubContent struct {
 	Name        string `json:"name"`
 	Path        string `json:"path"`
+	Type        string `json:"type"` // "file" or "dir"
 	DownloadURL string `json:"download_url"`
-	Type        string `json:"type"`
+	URL         string `json:"url"` // API URL for subdirectories
 }

 type SkillInstaller struct {
-	workspace string
+	workspace   string
+	client      *http.Client
+	githubToken string
 }

-func NewSkillInstaller(workspace string) *SkillInstaller {
-	return &SkillInstaller{workspace: workspace}
-}
-
-// parseRepoRef 解析 owner/repo/path 格式，默认 main 分支
-func parseRepoRef(repo string) (owner, repoName, ref, path string, err error) {
-	parts := strings.Split(strings.Trim(repo, "/"), "/")
-	if len(parts) < 3 {
-		return "", "", "", "", fmt.Errorf("invalid format: owner/repo/path")
+func NewSkillInstaller(workspace string, githubToken string) *SkillInstaller {
+	return &SkillInstaller{
+		workspace:   workspace,
+		client:      &http.Client{Timeout: 15 * time.Second},
+		githubToken: githubToken,
 	}
-	return parts[0], parts[1], "main", strings.Join(parts[2:], "/"), nil
 }

-func (si *SkillInstaller) downloadFile(ctx context.Context, fileURL, savePath string) error {
-	dir := filepath.Dir(savePath)
-	if err := os.MkdirAll(dir, 0755); err != nil {
+// 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) (owner, repoName, ref, subPath string, err error) {
+	repo = strings.TrimSpace(repo)
+
+	// Handle full URL
+	if strings.HasPrefix(repo, "http://") || strings.HasPrefix(repo, "https://") {
+		u, err := url.Parse(repo)
+		if err != nil {
+			return "", "", "", "", fmt.Errorf("invalid URL: %w", err)
+		}
+		parts := strings.Split(strings.Trim(u.Path, "/"), "/")
+		if len(parts) < 2 {
+			return "", "", "", "", fmt.Errorf("invalid GitHub URL")
+		}
+		owner, repoName = parts[0], parts[1]
+		ref = "main"
+		// Look for /tree/ or /blob/ in the path
+		for i := 2; i < len(parts); i++ {
+			if parts[i] == "tree" || parts[i] == "blob" {
+				if i+1 < len(parts) {
+					ref = parts[i+1]
+					subPath = strings.Join(parts[i+2:], "/")
+				}
+				break
+			}
+		}
+		return owner, repoName, ref, subPath, nil
+	}
+
+	// Handle shorthand format
+	parts := strings.Split(strings.Trim(repo, "/"), "/")
+	if len(parts) < 2 {
+		return "", "", "", "", fmt.Errorf("invalid format %q: expected 'owner/repo'", repo)
+	}
+	owner, repoName = parts[0], parts[1]
+	ref = "main"
+	if len(parts) > 2 {
+		subPath = strings.Join(parts[2:], "/")
+	}
+	return owner, repoName, ref, subPath, nil
+}
+
+func (si *SkillInstaller) InstallFromGitHub(ctx context.Context, repo string) error {
+	owner, repoName, ref, subPath, err := parseGitHubRef(repo)
+	if err != nil {
 		return err
 	}

-	client := &http.Client{Timeout: 30 * time.Second}
-	req, _ := http.NewRequestWithContext(ctx, "GET", fileURL, nil)
+	skillName := repoName
+	if subPath != "" {
+		skillName = filepath.Base(subPath)
+	}
+	skillDirectory := filepath.Join(si.workspace, "skills", skillName)

-	resp, err := utils.DoRequestWithRetry(client, req)
+	if _, err := os.Stat(skillDirectory); err == nil {
+		return fmt.Errorf("skill '%s' already exists", skillName)
+	}
+
+	// Build GitHub API URL
+	apiPath := path.Join(owner, repoName, "contents")
+	if subPath != "" {
+		apiPath = path.Join(apiPath, subPath)
+	}
+	apiURL := fmt.Sprintf("https://api.github.com/repos/%s?ref=%s", apiPath, ref)
+
+	if err := si.getGithubDirAllFiles(ctx, apiURL, skillDirectory, true); err != nil {
+		// Fallback to raw download
+		return si.downloadRaw(ctx, owner, repoName, ref, subPath, skillDirectory)
+	}
+
+	if _, err := os.Stat(filepath.Join(skillDirectory, "SKILL.md")); err != nil {
+		return fmt.Errorf("SKILL.md not found in repository")
+	}
+	return nil
+}
+
+// downloadDir recursively downloads a directory from GitHub API
+// isRoot: true if this is the skill root directory (only download SKILL.md at root)
+func (si *SkillInstaller) getGithubDirAllFiles(ctx context.Context, apiURL, localDir string, isRoot bool) error {
+	req, err := http.NewRequestWithContext(ctx, "GET", apiURL, nil)
+	if err != nil {
+		return err
+	}
+	if si.githubToken != "" {
+		req.Header.Set("Authorization", "Bearer "+si.githubToken)
+	}
+
+	resp, err := utils.DoRequestWithRetry(si.client, req)
 	if err != nil {
 		return err
 	}
 	defer resp.Body.Close()

-	if resp.StatusCode != http.StatusOK {
+	if resp.StatusCode != 200 {
 		return fmt.Errorf("HTTP %d", resp.StatusCode)
 	}

-	body, _ := io.ReadAll(resp.Body)
-	return fileutil.WriteFileAtomic(savePath, body, 0644)
-}
-
-func (si *SkillInstaller) downloadDir(ctx context.Context, owner, repo, ref, dirPath, localRoot string) error {
-	apiURL := fmt.Sprintf("https://api.github.com/repos/%s/%s/contents/%s?ref=%s",
-		url.PathEscape(owner), url.PathEscape(repo), url.PathEscape(dirPath), url.PathEscape(ref))
-	client := &http.Client{Timeout: 30 * time.Second}
-	req, _ := http.NewRequestWithContext(ctx, "GET", apiURL, nil)
-
-	resp, err := utils.DoRequestWithRetry(client, req)
-	if err != nil {
-		return err
-	}
-	defer resp.Body.Close()
-
-	if resp.StatusCode != http.StatusOK {
-		return fmt.Errorf("GitHub API HTTP %d", resp.StatusCode)
-	}
-
-	var contents []GitHubContent
-	if err := json.NewDecoder(resp.Body).Decode(&contents); err != nil {
+	var items []GitHubContent
+	if err := json.NewDecoder(resp.Body).Decode(&items); err != nil {
 		return err
 	}

-	for _, item := range contents {
-		relPath := strings.TrimPrefix(strings.TrimPrefix(item.Path, dirPath), "/")
-		localPath := filepath.Join(localRoot, relPath)
+	for _, item := range items {
+		localPath := filepath.Join(localDir, item.Name)

 		switch item.Type {
 		case "file":
-			if item.DownloadURL != "" {
-				if err := si.downloadFile(ctx, item.DownloadURL, localPath); err != nil {
-					return fmt.Errorf("download %s: %w", item.Path, err)
-				}
+			if !shouldDownload(item.Name, isRoot) {
+				continue
+			}
+			if err := si.downloadFile(ctx, item.DownloadURL, localPath); err != nil {
+				return fmt.Errorf("download %s: %w", item.Name, err)
 			}
 		case "dir":
-			if err := si.downloadDir(ctx, owner, repo, ref, item.Path, localRoot); err != nil {
-				return fmt.Errorf("download dir %s: %w", item.Path, err)
+			if !isSkillDirectory(item.Name) {
+				continue
+			}
+			if err := si.getGithubDirAllFiles(ctx, item.URL, localPath, false); err != nil {
+				return err
 			}
 		}
 	}
 	return nil
 }

-func (si *SkillInstaller) InstallFromGitHub(ctx context.Context, repo string) error {
-	owner, repoName, ref, path, err := parseRepoRef(repo)
+// downloadRaw is a fallback that downloads just SKILL.md from raw.githubusercontent.com
+func (si *SkillInstaller) downloadRaw(ctx context.Context, owner, repo, ref, subPath, localDir string) error {
+	urlPath := path.Join(owner, repo, ref)
+	if subPath != "" {
+		urlPath = path.Join(urlPath, subPath)
+	}
+	url := fmt.Sprintf("https://raw.githubusercontent.com/%s/SKILL.md", urlPath)
+
+	req, err := http.NewRequestWithContext(ctx, "GET", url, nil)
+	if err != nil {
+		return fmt.Errorf("failed to create request: %w", err)
+	}
+
+	resp, err := utils.DoRequestWithRetry(si.client, req)
+	if err != nil {
+		return fmt.Errorf("failed to fetch skill: %w", err)
+	}
+	defer resp.Body.Close()
+
+	if resp.StatusCode != 200 {
+		return fmt.Errorf("failed to fetch skill: HTTP %d", resp.StatusCode)
+	}
+
+	body, err := io.ReadAll(resp.Body)
+	if err != nil {
+		return fmt.Errorf("failed to read response: %w", err)
+	}
+
+	if err := os.MkdirAll(localDir, 0o755); err != nil {
+		return fmt.Errorf("failed to create skill directory: %w", err)
+	}
+
+	localPath := filepath.Join(localDir, "SKILL.md")
+
+	// Use unified atomic write utility with explicit sync for flash storage reliability.
+	if err := fileutil.WriteFileAtomic(localPath, body, 0o600); err != nil {
+		return fmt.Errorf("failed to write skill file: %w", err)
+	}
+
+	return nil
+}
+
+func (si *SkillInstaller) downloadFile(ctx context.Context, url, localPath string) error {
+	req, err := http.NewRequestWithContext(ctx, "GET", url, nil)
 	if err != nil {
 		return err
 	}

-	skillName := filepath.Base(path)
+	resp, err := utils.DoRequestWithRetry(si.client, req)
+	if err != nil {
+		return err
+	}
+	defer resp.Body.Close()

-	skillDir := filepath.Join(si.workspace, "skills", skillName)
-	if _, err := os.Stat(skillDir); err == nil {
-		return fmt.Errorf("skill '%s' already exists", skillName)
+	if resp.StatusCode != 200 {
+		return fmt.Errorf("HTTP %d", resp.StatusCode)
 	}

-	if err := os.MkdirAll(skillDir, 0755); err != nil {
+	body, err := io.ReadAll(resp.Body)
+	if err != nil {
 		return err
 	}

-	if err := si.downloadDir(ctx, owner, repoName, ref, path, skillDir); err != nil {
-		os.RemoveAll(skillDir)
-		return fmt.Errorf("install skill: %w", err)
+	if err := os.MkdirAll(filepath.Dir(localPath), 0o755); err != nil {
+		return err
 	}
-	return nil
+
+	return fileutil.WriteFileAtomic(localPath, body, 0o600)
+}
+
+// shouldDownload determines if a file should be downloaded
+// root: true if we're at the skill root directory
+func shouldDownload(name string, root bool) bool {
+	if root {
+		return name == "SKILL.md"
+	}
+	return true
+}
+
+// isSkillDir checks if a directory is a standard skill resource directory
+func isSkillDirectory(name string) bool {
+	switch name {
+	case "scripts", "references", "assets", "templates", "docs":
+		return true
+	}
+	return false
 }

 func (si *SkillInstaller) Uninstall(skillName string) error {
-	skillDir := filepath.Join(si.workspace, "skills", skillName)
+	parts := strings.Split(skillName, "/")
+	var finalSkillName string
+	for i := len(parts) - 1; i >= 0; i-- {
+		if parts[i] != "" {
+			finalSkillName = parts[i]
+			break
+		}
+	}
+	if finalSkillName == "" {
+		finalSkillName = skillName
+	}
+
+	skillDir := filepath.Join(si.workspace, "skills", finalSkillName)
+
 	if _, err := os.Stat(skillDir); os.IsNotExist(err) {
-		return fmt.Errorf("skill '%s' not found", skillName)
-	}
-	return os.RemoveAll(skillDir)
-}
-
-// InstallFromRegistry installs a skill from a registry.
-func (si *SkillInstaller) InstallFromRegistry(ctx context.Context, registry SkillRegistry, slug string) error {
-	targetDir := filepath.Join(si.workspace, "skills", slug)
-
-	if _, err := os.Stat(targetDir); err == nil {
-		return fmt.Errorf("skill '%s' already exists", slug)
+		return fmt.Errorf("skill '%s' not found (processed as '%s')", skillName, finalSkillName)
 	}

-	if err := os.MkdirAll(targetDir, 0755); err != nil {
-		return err
-	}
-
-	result, err := registry.DownloadAndInstall(ctx, slug, "", targetDir)
-	if err != nil {
-		os.RemoveAll(targetDir)
-		return err
-	}
-
-	if result.IsMalwareBlocked {
-		os.RemoveAll(targetDir)
-		return fmt.Errorf("skill '%s' is flagged as malicious", slug)
+	if err := os.RemoveAll(skillDir); err != nil {
+		return fmt.Errorf("failed to remove skill '%s': %w", finalSkillName, err)
 	}

 	return nil