diff --git a/README.md b/README.md index a48a53d47..4a9e88e69 100644 --- a/README.md +++ b/README.md @@ -513,7 +513,7 @@ picoclaw skills search "web scraping" picoclaw skills install ``` -**Configure ClawHub token** (optional, for higher rate limits): +**Configure skill registries**: Add to your `config.json`: ```json @@ -523,6 +523,11 @@ Add to your `config.json`: "registries": { "clawhub": { "auth_token": "your-clawhub-token" + }, + "github": { + "base_url": "https://github.com", + "auth_token": "your-github-token", + "proxy": "" } } } @@ -530,6 +535,8 @@ Add to your `config.json`: } ``` +`tools.skills.github.*` is deprecated. Use `tools.skills.registries.github.*` instead. + For more details, see [Tools Configuration - Skills](docs/tools_configuration.md#skills-tool). ## 🔗 MCP (Model Context Protocol) diff --git a/README.zh.md b/README.zh.md index 2ba0913fc..bac5deb8e 100644 --- a/README.zh.md +++ b/README.zh.md @@ -507,7 +507,7 @@ picoclaw skills search "web scraping" picoclaw skills install ``` -**配置 ClawHub token**(可选,用于提高速率限制): +**配置 Skills 仓库源**: 在 `config.json` 中添加: ```json @@ -517,6 +517,11 @@ picoclaw skills install "registries": { "clawhub": { "auth_token": "your-clawhub-token" + }, + "github": { + "base_url": "https://github.com", + "auth_token": "your-github-token", + "proxy": "" } } } @@ -524,6 +529,8 @@ picoclaw skills install } ``` +`tools.skills.github.*` 已废弃,请改用 `tools.skills.registries.github.*`。 + 更多详情请参阅 [工具配置 - Skills](docs/zh/tools_configuration.md#skills-tool)。 ## 🔗 MCP (Model Context Protocol) @@ -616,8 +623,3 @@ Discord: WeChat: WeChat group QR code - - - - - diff --git a/cmd/picoclaw/internal/skills/command.go b/cmd/picoclaw/internal/skills/command.go index e8b884977..b62a00351 100644 --- a/cmd/picoclaw/internal/skills/command.go +++ b/cmd/picoclaw/internal/skills/command.go @@ -29,15 +29,6 @@ func NewSkillsCommand() *cobra.Command { } d.workspace = cfg.WorkspacePath() - installer, err := skills.NewSkillInstaller( - d.workspace, - cfg.Tools.Skills.Github.Token.String(), - cfg.Tools.Skills.Github.Proxy, - ) - if err != nil { - return fmt.Errorf("error creating skills installer: %w", err) - } - d.installer = installer // get global config directory and builtin skills directory globalDir := filepath.Dir(internal.GetConfigPath()) @@ -53,6 +44,13 @@ func NewSkillsCommand() *cobra.Command { } installerFn := func() (*skills.SkillInstaller, error) { + if d.installer == nil { + installer, err := skills.NewSkillInstaller(d.workspace, "", "") + if err != nil { + return nil, fmt.Errorf("error creating skills installer: %w", err) + } + d.installer = installer + } if d.installer == nil { return nil, fmt.Errorf("skills installer is not initialized") } @@ -75,7 +73,7 @@ func NewSkillsCommand() *cobra.Command { cmd.AddCommand( newListCommand(loaderFn), - newInstallCommand(installerFn), + newInstallCommand(), newInstallBuiltinCommand(workspaceFn), newListBuiltinCommand(), newRemoveCommand(installerFn), diff --git a/cmd/picoclaw/internal/skills/helpers.go b/cmd/picoclaw/internal/skills/helpers.go index eec2dbb94..c359d40ca 100644 --- a/cmd/picoclaw/internal/skills/helpers.go +++ b/cmd/picoclaw/internal/skills/helpers.go @@ -35,61 +35,32 @@ func skillsListCmd(loader *skills.SkillsLoader) { } } -func skillsInstallCmd(installer *skills.SkillInstaller, repo string) error { - fmt.Printf("Installing skill from %s...\n", repo) - - ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) - defer cancel() - - if err := installer.InstallFromGitHub(ctx, repo); err != nil { - return fmt.Errorf("failed to install skill: %w", err) - } - - fmt.Printf("\u2713 Skill '%s' installed successfully!\n", filepath.Base(repo)) - - return nil -} - // skillsInstallFromRegistry installs a skill from a named registry (e.g. clawhub). -func skillsInstallFromRegistry(cfg *config.Config, registryName, slug string) error { +func skillsInstallFromRegistry(cfg *config.Config, registryName, target string) error { err := utils.ValidateSkillIdentifier(registryName) if err != nil { return fmt.Errorf("✗ invalid registry name: %w", err) } - err = utils.ValidateSkillIdentifier(slug) - if err != nil { - return fmt.Errorf("✗ invalid slug: %w", err) - } - - fmt.Printf("Installing skill '%s' from %s registry...\n", slug, registryName) - - clawHubConfig := cfg.Tools.Skills.Registries.ClawHub - registryMgr := skills.NewRegistryManagerFromConfig(skills.RegistryConfig{ - MaxConcurrentSearches: cfg.Tools.Skills.MaxConcurrentSearches, - ClawHub: skills.ClawHubConfig{ - Enabled: clawHubConfig.Enabled, - BaseURL: clawHubConfig.BaseURL, - AuthToken: clawHubConfig.AuthToken.String(), - SearchPath: clawHubConfig.SearchPath, - SkillsPath: clawHubConfig.SkillsPath, - DownloadPath: clawHubConfig.DownloadPath, - Timeout: clawHubConfig.Timeout, - MaxZipSize: clawHubConfig.MaxZipSize, - MaxResponseSize: clawHubConfig.MaxResponseSize, - }, - }) + registryMgr := skills.NewRegistryManagerFromToolsConfig(cfg.Tools.Skills) registry := registryMgr.GetRegistry(registryName) if registry == nil { return fmt.Errorf("✗ registry '%s' not found or not enabled. check your config.json.", registryName) } + dirName, err := registry.ResolveInstallDirName(target) + if err != nil { + return fmt.Errorf("✗ invalid install target %q: %w", target, err) + } + + fmt.Printf("Installing skill '%s' from %s registry...\n", target, registryName) + workspace := cfg.WorkspacePath() - targetDir := filepath.Join(workspace, "skills", slug) + targetDir := filepath.Join(workspace, "skills", dirName) if _, err = os.Stat(targetDir); err == nil { - return fmt.Errorf("\u2717 skill '%s' already installed at %s", slug, targetDir) + return fmt.Errorf("\u2717 skill '%s' already installed at %s", dirName, targetDir) } ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) @@ -99,7 +70,7 @@ func skillsInstallFromRegistry(cfg *config.Config, registryName, slug string) er return fmt.Errorf("\u2717 failed to create skills directory: %v", err) } - result, err := registry.DownloadAndInstall(ctx, slug, "", targetDir) + result, err := registry.DownloadAndInstall(ctx, target, "", targetDir) if err != nil { rmErr := os.RemoveAll(targetDir) if rmErr != nil { @@ -114,14 +85,14 @@ func skillsInstallFromRegistry(cfg *config.Config, registryName, slug string) er fmt.Printf("\u2717 Failed to remove partial install: %v\n", rmErr) } - return fmt.Errorf("\u2717 Skill '%s' is flagged as malicious and cannot be installed.\n", slug) + return fmt.Errorf("\u2717 Skill '%s' is flagged as malicious and cannot be installed.\n", target) } if result.IsSuspicious { - fmt.Printf("\u26a0\ufe0f Warning: skill '%s' is flagged as suspicious.\n", slug) + fmt.Printf("\u26a0\ufe0f Warning: skill '%s' is flagged as suspicious.\n", target) } - fmt.Printf("\u2713 Skill '%s' v%s installed successfully!\n", slug, result.Version) + fmt.Printf("\u2713 Skill '%s' v%s installed successfully!\n", dirName, result.Version) if result.Summary != "" { fmt.Printf(" %s\n", result.Summary) } @@ -237,21 +208,7 @@ func skillsSearchCmd(query string) { return } - clawHubConfig := cfg.Tools.Skills.Registries.ClawHub - registryMgr := skills.NewRegistryManagerFromConfig(skills.RegistryConfig{ - MaxConcurrentSearches: cfg.Tools.Skills.MaxConcurrentSearches, - ClawHub: skills.ClawHubConfig{ - Enabled: clawHubConfig.Enabled, - BaseURL: clawHubConfig.BaseURL, - AuthToken: clawHubConfig.AuthToken.String(), - SearchPath: clawHubConfig.SearchPath, - SkillsPath: clawHubConfig.SkillsPath, - DownloadPath: clawHubConfig.DownloadPath, - Timeout: clawHubConfig.Timeout, - MaxZipSize: clawHubConfig.MaxZipSize, - MaxResponseSize: clawHubConfig.MaxResponseSize, - }, - }) + registryMgr := skills.NewRegistryManagerFromToolsConfig(cfg.Tools.Skills) ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() diff --git a/cmd/picoclaw/internal/skills/install.go b/cmd/picoclaw/internal/skills/install.go index 78bc421db..9bd9cfd90 100644 --- a/cmd/picoclaw/internal/skills/install.go +++ b/cmd/picoclaw/internal/skills/install.go @@ -6,15 +6,14 @@ import ( "github.com/spf13/cobra" "github.com/sipeed/picoclaw/cmd/picoclaw/internal" - "github.com/sipeed/picoclaw/pkg/skills" ) -func newInstallCommand(installerFn func() (*skills.SkillInstaller, error)) *cobra.Command { +func newInstallCommand() *cobra.Command { var registry string cmd := &cobra.Command{ Use: "install", - Short: "Install skill from GitHub", + Short: "Install skill from configured registry", Example: ` picoclaw skills install sipeed/picoclaw-skills/weather picoclaw skills install --registry clawhub github @@ -34,21 +33,15 @@ picoclaw skills install --registry clawhub github return nil }, RunE: func(_ *cobra.Command, args []string) error { - installer, err := installerFn() + cfg, err := internal.LoadConfig() if err != nil { return err } - if registry != "" { - cfg, err := internal.LoadConfig() - if err != nil { - return err - } - return skillsInstallFromRegistry(cfg, registry, args[0]) } - return skillsInstallCmd(installer, args[0]) + return skillsInstallFromRegistry(cfg, "github", args[0]) }, } diff --git a/cmd/picoclaw/internal/skills/install_test.go b/cmd/picoclaw/internal/skills/install_test.go index 6b362822d..da66f1c80 100644 --- a/cmd/picoclaw/internal/skills/install_test.go +++ b/cmd/picoclaw/internal/skills/install_test.go @@ -8,12 +8,12 @@ import ( ) func TestNewInstallSubcommand(t *testing.T) { - cmd := newInstallCommand(nil) + cmd := newInstallCommand() require.NotNil(t, cmd) assert.Equal(t, "install", cmd.Use) - assert.Equal(t, "Install skill from GitHub", cmd.Short) + assert.Equal(t, "Install skill from configured registry", cmd.Short) assert.Nil(t, cmd.Run) assert.NotNil(t, cmd.RunE) @@ -79,7 +79,7 @@ func TestInstallCommandArgs(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - cmd := newInstallCommand(nil) + cmd := newInstallCommand() if tt.registry != "" { require.NoError(t, cmd.Flags().Set("registry", tt.registry)) diff --git a/config/config.example.json b/config/config.example.json index f0cce6d72..2d2d38496 100644 --- a/config/config.example.json +++ b/config/config.example.json @@ -382,9 +382,16 @@ "timeout": 0, "max_zip_size": 0, "max_response_size": 0 + }, + "github": { + "enabled": true, + "base_url": "https://github.com", + "auth_token": "", + "proxy": "http://127.0.0.1:7891" } }, "github": { + "base_url": "https://github.com", "proxy": "http://127.0.0.1:7891", "token": "" }, diff --git a/docs/tools_configuration.md b/docs/tools_configuration.md index adee9244a..b5b9ba656 100644 --- a/docs/tools_configuration.md +++ b/docs/tools_configuration.md @@ -459,7 +459,7 @@ default (deferred). `aws` explicitly opts in to deferred mode even though it is ## Skills Tool -The skills tool configures skill discovery and installation via registries like ClawHub. +The skills tool configures skill discovery and installation via registries like ClawHub and GitHub. ### Registries @@ -474,13 +474,20 @@ The skills tool configures skill discovery and installation via registries like | `registries.clawhub.timeout` | int | 0 | Request timeout in seconds (0 = default) | | `registries.clawhub.max_zip_size` | int | 0 | Max skill zip size in bytes (0 = default) | | `registries.clawhub.max_response_size` | int | 0 | Max API response size in bytes (0 = default) | +| `registries.github.enabled` | bool | true | Enable GitHub installs via registry config | +| `registries.github.base_url` | string | `https://github.com` | GitHub or GitHub Enterprise base URL | +| `registries.github.auth_token` | string | `""` | GitHub personal access token | +| `registries.github.proxy` | string | `""` | HTTP proxy for GitHub API requests | -### GitHub Integration +### Legacy GitHub Config -| Config | Type | Default | Description | -|------------------|--------|---------|--------------------------------------| -| `github.proxy` | string | `""` | HTTP proxy for GitHub API requests | -| `github.token` | string | `""` | GitHub personal access token | +`github.*` is deprecated. Use `registries.github.*` instead. The legacy fields are still supported for compatibility and will be removed later. + +| Config | Type | Default | Description | +|--------------------|--------|----------------------|--------------------------------| +| `github.base_url` | string | `https://github.com` | Deprecated GitHub base URL | +| `github.proxy` | string | `""` | Deprecated GitHub proxy | +| `github.token` | string | `""` | Deprecated GitHub token | ### Search Settings @@ -500,10 +507,23 @@ The skills tool configures skill discovery and installation via registries like "clawhub": { "enabled": true, "base_url": "https://clawhub.ai", - "auth_token": "" + "auth_token": "", + "search_path": "", + "skills_path": "", + "download_path": "", + "timeout": 0, + "max_zip_size": 0, + "max_response_size": 0 + }, + "github": { + "enabled": true, + "base_url": "https://github.com", + "auth_token": "", + "proxy": "" } }, "github": { + "base_url": "https://github.com", "proxy": "", "token": "" }, diff --git a/docs/zh/tools_configuration.md b/docs/zh/tools_configuration.md index 63ac5000b..db2aa98c6 100644 --- a/docs/zh/tools_configuration.md +++ b/docs/zh/tools_configuration.md @@ -461,3 +461,29 @@ Skills 工具配置通过 ClawHub 等注册表进行技能发现和安装。 - `PICOCLAW_TOOLS_MCP_ENABLED=true` 注意:嵌套的映射式配置(例如 `tools.mcp.servers..*`)在 `config.json` 中配置,而非通过环境变量。 + +## Skills Tool + +Skills 工具用于通过仓库源发现和安装 Skill,支持 ClawHub 与 GitHub。 + +### Registries + +| 配置项 | 类型 | 默认值 | 说明 | +|--------|------|--------|------| +| `registries.clawhub.enabled` | bool | true | 是否启用 ClawHub | +| `registries.clawhub.base_url` | string | `https://clawhub.ai` | ClawHub 基础地址 | +| `registries.clawhub.auth_token` | string | `""` | ClawHub 认证令牌 | +| `registries.github.enabled` | bool | true | 是否启用 GitHub | +| `registries.github.base_url` | string | `https://github.com` | GitHub 或 GitHub Enterprise 基础地址 | +| `registries.github.auth_token` | string | `""` | GitHub 访问令牌 | +| `registries.github.proxy` | string | `""` | GitHub 请求代理 | + +### 旧版 GitHub 配置 + +`github.*` 已废弃,建议迁移到 `registries.github.*`。当前仍保留兼容,后续可移除。 + +| 配置项 | 类型 | 默认值 | 说明 | +|--------|------|--------|------| +| `github.base_url` | string | `https://github.com` | 已废弃 | +| `github.proxy` | string | `""` | 已废弃 | +| `github.token` | string | `""` | 已废弃 | diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index e089e6d3d..fa452b30d 100644 --- a/pkg/agent/loop.go +++ b/pkg/agent/loop.go @@ -306,21 +306,7 @@ func registerSharedTools( find_skills_enable := cfg.Tools.IsToolEnabled("find_skills") install_skills_enable := cfg.Tools.IsToolEnabled("install_skill") if skills_enabled && (find_skills_enable || install_skills_enable) { - clawHubConfig := cfg.Tools.Skills.Registries.ClawHub - registryMgr := skills.NewRegistryManagerFromConfig(skills.RegistryConfig{ - MaxConcurrentSearches: cfg.Tools.Skills.MaxConcurrentSearches, - ClawHub: skills.ClawHubConfig{ - Enabled: clawHubConfig.Enabled, - BaseURL: clawHubConfig.BaseURL, - AuthToken: clawHubConfig.AuthToken.String(), - SearchPath: clawHubConfig.SearchPath, - SkillsPath: clawHubConfig.SkillsPath, - DownloadPath: clawHubConfig.DownloadPath, - Timeout: clawHubConfig.Timeout, - MaxZipSize: clawHubConfig.MaxZipSize, - MaxResponseSize: clawHubConfig.MaxResponseSize, - }, - }) + registryMgr := skills.NewRegistryManagerFromToolsConfig(cfg.Tools.Skills) if find_skills_enable { searchCache := skills.NewSearchCache( diff --git a/pkg/config/config.go b/pkg/config/config.go index fd4466b8c..532adfaf7 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -837,11 +837,12 @@ type ExecConfig struct { } type SkillsToolsConfig struct { - ToolConfig ` yaml:"-" envPrefix:"PICOCLAW_TOOLS_SKILLS_"` - Registries SkillsRegistriesConfig `yaml:",inline,omitempty" json:"registries"` - Github SkillsGithubConfig `yaml:"github,omitempty" json:"github"` - MaxConcurrentSearches int `yaml:"-" json:"max_concurrent_searches" env:"PICOCLAW_TOOLS_SKILLS_MAX_CONCURRENT_SEARCHES"` - SearchCache SearchCacheConfig `yaml:"-" json:"search_cache"` + ToolConfig ` yaml:"-" envPrefix:"PICOCLAW_TOOLS_SKILLS_"` + Registries SkillsRegistriesConfig `yaml:"registries,omitempty" json:"registries"` + // Deprecated: use registries.github instead. + Github SkillsGithubConfig `yaml:"github,omitempty" json:"github"` + MaxConcurrentSearches int `yaml:"-" json:"max_concurrent_searches" env:"PICOCLAW_TOOLS_SKILLS_MAX_CONCURRENT_SEARCHES"` + SearchCache SearchCacheConfig `yaml:"-" json:"search_cache"` } type MediaCleanupConfig struct { @@ -925,25 +926,70 @@ type SearchCacheConfig struct { TTLSeconds int `json:"ttl_seconds" env:"PICOCLAW_SKILLS_SEARCH_CACHE_TTL_SECONDS"` } -type SkillsRegistriesConfig struct { - ClawHub ClawHubRegistryConfig `json:"clawhub" yaml:"clawhub,omitempty"` +type SkillsRegistriesConfig []*SkillRegistryConfig + +func (c *SkillsRegistriesConfig) Get(name string) (SkillRegistryConfig, bool) { + if c == nil { + return SkillRegistryConfig{}, false + } + name = strings.TrimSpace(name) + if name == "" { + return SkillRegistryConfig{}, false + } + for _, registry := range *c { + if registry == nil || registry.Name != name { + continue + } + return *registry, true + } + return SkillRegistryConfig{}, false +} + +func (c *SkillsRegistriesConfig) Set(name string, cfg SkillRegistryConfig) { + if c == nil { + return + } + name = strings.TrimSpace(name) + if name == "" { + return + } + cfg.Name = name + for i, registry := range *c { + if registry == nil || registry.Name != name { + continue + } + (*c)[i] = &cfg + return + } + *c = append(*c, &cfg) } type SkillsGithubConfig struct { - Token SecureString `json:"token,omitzero" yaml:"token,omitempty" env:"PICOCLAW_TOOLS_SKILLS_GITHUB_TOKEN"` - Proxy string `json:"proxy,omitempty" yaml:"-" env:"PICOCLAW_TOOLS_SKILLS_GITHUB_PROXY"` + BaseURL string `json:"base_url,omitempty" yaml:"-" env:"PICOCLAW_TOOLS_SKILLS_GITHUB_BASE_URL"` + Token SecureString `json:"token,omitzero" yaml:"token,omitempty" env:"PICOCLAW_TOOLS_SKILLS_GITHUB_TOKEN"` + Proxy string `json:"proxy,omitempty" yaml:"-" env:"PICOCLAW_TOOLS_SKILLS_GITHUB_PROXY"` } -type ClawHubRegistryConfig struct { - Enabled bool `json:"enabled" yaml:"-" env:"PICOCLAW_SKILLS_REGISTRIES_CLAWHUB_ENABLED"` - BaseURL string `json:"base_url" yaml:"-" env:"PICOCLAW_SKILLS_REGISTRIES_CLAWHUB_BASE_URL"` - AuthToken SecureString `json:"auth_token,omitzero" yaml:"auth_token,omitempty" env:"PICOCLAW_SKILLS_REGISTRIES_CLAWHUB_AUTH_TOKEN"` - SearchPath string `json:"search_path" yaml:"-" env:"PICOCLAW_SKILLS_REGISTRIES_CLAWHUB_SEARCH_PATH"` - SkillsPath string `json:"skills_path" yaml:"-" env:"PICOCLAW_SKILLS_REGISTRIES_CLAWHUB_SKILLS_PATH"` - DownloadPath string `json:"download_path" yaml:"-" env:"PICOCLAW_SKILLS_REGISTRIES_CLAWHUB_DOWNLOAD_PATH"` - Timeout int `json:"timeout" yaml:"-" env:"PICOCLAW_SKILLS_REGISTRIES_CLAWHUB_TIMEOUT"` - MaxZipSize int `json:"max_zip_size" yaml:"-" env:"PICOCLAW_SKILLS_REGISTRIES_CLAWHUB_MAX_ZIP_SIZE"` - MaxResponseSize int `json:"max_response_size" yaml:"-" env:"PICOCLAW_SKILLS_REGISTRIES_CLAWHUB_MAX_RESPONSE_SIZE"` +type SkillRegistryConfig struct { + Name string `json:"name,omitempty" yaml:"-" env:"-"` + Enabled bool `json:"enabled" yaml:"-" env:"-"` + BaseURL string `json:"base_url" yaml:"-" env:"-"` + AuthToken SecureString `json:"auth_token,omitzero" yaml:"auth_token,omitempty" env:"-"` + Param map[string]any `json:"-" yaml:"-" env:"-"` +} + +func (c *SkillRegistryConfig) DecodeParam(target any) error { + if c == nil { + return nil + } + if len(c.Param) == 0 { + return nil + } + data, err := json.Marshal(c.Param) + if err != nil { + return err + } + return json.Unmarshal(data, target) } // MCPServerConfig defines configuration for a single MCP server diff --git a/pkg/config/config_old.go b/pkg/config/config_old.go index 150275aac..b1711d220 100644 --- a/pkg/config/config_old.go +++ b/pkg/config/config_old.go @@ -936,15 +936,13 @@ func (v *webToolsConfigV0) ToWebToolsConfig() WebToolsConfig { } type skillsToolsConfigV0 struct { - ToolConfig ` envPrefix:"PICOCLAW_TOOLS_SKILLS_"` - Registries skillsRegistriesConfigV0 ` json:"registries"` - Github skillsGithubConfigV0 ` json:"github"` - MaxConcurrentSearches int ` json:"max_concurrent_searches" env:"PICOCLAW_TOOLS_SKILLS_MAX_CONCURRENT_SEARCHES"` - SearchCache SearchCacheConfig ` json:"search_cache"` -} - -type skillsRegistriesConfigV0 struct { - ClawHub clawHubRegistryConfigV0 `json:"clawhub"` + ToolConfig `envPrefix:"PICOCLAW_TOOLS_SKILLS_"` + Registries struct { + ClawHub clawHubRegistryConfigV0 `json:"clawhub"` + } ` json:"registries"` + Github skillsGithubConfigV0 ` json:"github"` + MaxConcurrentSearches int ` json:"max_concurrent_searches" env:"PICOCLAW_TOOLS_SKILLS_MAX_CONCURRENT_SEARCHES"` + SearchCache SearchCacheConfig ` json:"search_cache"` } type clawHubRegistryConfigV0 struct { @@ -955,12 +953,15 @@ type clawHubRegistryConfigV0 struct { SkillsPath string `json:"skills_path" env:"PICOCLAW_SKILLS_REGISTRIES_CLAWHUB_SKILLS_PATH"` } -func (v *clawHubRegistryConfigV0) ToClawHubRegistryConfig() ClawHubRegistryConfig { - cfg := ClawHubRegistryConfig{ - Enabled: v.Enabled, - BaseURL: v.BaseURL, - SearchPath: v.SearchPath, - SkillsPath: v.SkillsPath, +func (v *clawHubRegistryConfigV0) ToSkillRegistryConfig() SkillRegistryConfig { + cfg := SkillRegistryConfig{ + Name: "clawhub", + Enabled: v.Enabled, + BaseURL: v.BaseURL, + Param: map[string]any{ + "search_path": v.SearchPath, + "skills_path": v.SkillsPath, + }, } if v.AuthToken != "" { cfg.AuthToken = *NewSecureString(v.AuthToken) @@ -980,20 +981,12 @@ func (v *skillsGithubConfigV0) ToSkillsGithubConfig() SkillsGithubConfig { } } -func (v *skillsRegistriesConfigV0) ToSkillsRegistriesConfig() SkillsRegistriesConfig { - clawHub := v.ClawHub.ToClawHubRegistryConfig() - - return SkillsRegistriesConfig{ - ClawHub: clawHub, - } -} - func (v *skillsToolsConfigV0) ToSkillsToolsConfig() SkillsToolsConfig { - registries := v.Registries.ToSkillsRegistriesConfig() + clawHub := v.Registries.ClawHub.ToSkillRegistryConfig() github := v.Github.ToSkillsGithubConfig() return SkillsToolsConfig{ ToolConfig: v.ToolConfig, - Registries: registries, + Registries: SkillsRegistriesConfig{&clawHub}, Github: github, MaxConcurrentSearches: v.MaxConcurrentSearches, SearchCache: v.SearchCache, diff --git a/pkg/config/config_struct.go b/pkg/config/config_struct.go index 0b8dd85c8..1bb64356b 100644 --- a/pkg/config/config_struct.go +++ b/pkg/config/config_struct.go @@ -5,6 +5,7 @@ import ( "fmt" "path/filepath" "runtime" + "sort" "strings" "sync" @@ -325,3 +326,199 @@ func (v SecureModelList) MarshalYAML() (any, error) { return mm, nil } + +func (v *SkillsRegistriesConfig) UnmarshalJSON(data []byte) error { + var list []*SkillRegistryConfig + if err := json.Unmarshal(data, &list); err == nil { + *v = list + return nil + } + + legacy := map[string]*SkillRegistryConfig{} + if err := json.Unmarshal(data, &legacy); err != nil { + return err + } + + list = make([]*SkillRegistryConfig, 0, len(legacy)) + for name, registry := range legacy { + if registry == nil { + continue + } + registry.Name = name + list = append(list, registry) + } + *v = list + return nil +} + +func (c *SkillRegistryConfig) UnmarshalJSON(data []byte) error { + type alias struct { + Name string `json:"name,omitempty"` + Enabled bool `json:"enabled"` + BaseURL string `json:"base_url"` + AuthToken SecureString `json:"auth_token,omitzero"` + Param map[string]any `json:"param,omitempty"` + } + var raw map[string]json.RawMessage + if err := json.Unmarshal(data, &raw); err != nil { + return err + } + var parsed alias + if err := json.Unmarshal(data, &parsed); err != nil { + return err + } + params := map[string]any{} + for key, value := range parsed.Param { + params[key] = value + } + for key, value := range raw { + switch key { + case "name", "enabled", "base_url", "auth_token", "param": + continue + default: + var decoded any + if err := json.Unmarshal(value, &decoded); err != nil { + return err + } + params[key] = decoded + } + } + c.Name = parsed.Name + c.Enabled = parsed.Enabled + c.BaseURL = parsed.BaseURL + c.AuthToken = parsed.AuthToken + c.Param = params + return nil +} + +func (c SkillRegistryConfig) MarshalJSON() ([]byte, error) { + m := map[string]any{ + "enabled": c.Enabled, + "base_url": c.BaseURL, + } + if c.Name != "" { + m["name"] = c.Name + } + if c.AuthToken.String() != "" { + m["auth_token"] = c.AuthToken + } + for key, value := range c.Param { + if key == "" || key == "param" { + continue + } + if _, exists := m[key]; exists { + continue + } + m[key] = value + } + return json.Marshal(m) +} + +func (c *SkillRegistryConfig) UnmarshalYAML(value *yaml.Node) error { + var raw map[string]any + if err := value.Decode(&raw); err != nil { + return err + } + params := map[string]any{} + if nested, ok := raw["param"].(map[string]any); ok { + for k, v := range nested { + params[k] = v + } + } + for key, v := range raw { + switch key { + case "name": + if s, ok := v.(string); ok { + c.Name = s + } + case "enabled": + if b, ok := v.(bool); ok { + c.Enabled = b + } + case "base_url": + if s, ok := v.(string); ok { + c.BaseURL = s + } + case "auth_token": + data, err := yaml.Marshal(v) + if err != nil { + return err + } + if err := yaml.Unmarshal(data, &c.AuthToken); err != nil { + return err + } + case "param": + continue + default: + params[key] = v + } + } + c.Param = params + return nil +} + +func (c SkillRegistryConfig) MarshalYAML() (any, error) { + m := map[string]any{ + "enabled": c.Enabled, + "base_url": c.BaseURL, + } + if c.Name != "" { + m["name"] = c.Name + } + if c.AuthToken.String() != "" { + m["auth_token"] = c.AuthToken + } + keys := make([]string, 0, len(c.Param)) + for key := range c.Param { + if key == "" || key == "param" { + continue + } + keys = append(keys, key) + } + sort.Strings(keys) + for _, key := range keys { + if _, exists := m[key]; exists { + continue + } + m[key] = c.Param[key] + } + return m, nil +} + +func (v *SkillsRegistriesConfig) UnmarshalYAML(value *yaml.Node) error { + mm := make(map[string]*SkillRegistryConfig) + if err := value.Decode(&mm); err != nil { + logger.Errorf("Decode error: %v", err) + return err + } + for _, registry := range *v { + if registry == nil { + continue + } + sec := mm[registry.Name] + if sec != nil { + registry.AuthToken = sec.AuthToken + } + } + return nil +} + +func (v SkillsRegistriesConfig) MarshalYAML() (any, error) { + type onlySecureRegistryData struct { + AuthToken SecureString `yaml:"auth_token,omitempty"` + } + mm := make(map[string]onlySecureRegistryData) + for _, registry := range v { + if registry == nil || registry.Name == "" { + continue + } + if registry.AuthToken.String() == "" { + continue + } + mm[registry.Name] = onlySecureRegistryData{ + AuthToken: registry.AuthToken, + } + } + + return mm, nil +} diff --git a/pkg/config/config_struct_test.go b/pkg/config/config_struct_test.go index 674b6a064..1d60b795f 100644 --- a/pkg/config/config_struct_test.go +++ b/pkg/config/config_struct_test.go @@ -143,3 +143,63 @@ func TestLoadSecurityValue(t *testing.T) { assert.NotNil(t, v6.Tools.Pico.Token) assert.Equal(t, "newtoken1", v6.Tools.Pico.Token.String()) } + +func TestSkillRegistryConfigDecodeParam(t *testing.T) { + registry := SkillRegistryConfig{ + Name: "github", + Param: map[string]any{ + "proxy": "http://127.0.0.1:7890", + }, + } + + var private struct { + Proxy string `json:"proxy"` + } + err := registry.DecodeParam(&private) + assert.NoError(t, err) + assert.Equal(t, "http://127.0.0.1:7890", private.Proxy) +} + +func TestSkillRegistryConfigJSONFlattensParam(t *testing.T) { + registry := SkillRegistryConfig{ + Name: "github", + Enabled: true, + BaseURL: "https://github.com", + Param: map[string]any{ + "proxy": "http://127.0.0.1:7890", + }, + } + + data, err := json.Marshal(registry) + assert.NoError(t, err) + assert.Contains(t, string(data), `"proxy":"http://127.0.0.1:7890"`) + assert.NotContains(t, string(data), `"param"`) + + var loaded SkillRegistryConfig + err = json.Unmarshal(data, &loaded) + assert.NoError(t, err) + assert.Equal(t, "http://127.0.0.1:7890", loaded.Param["proxy"]) +} + +func TestSkillsRegistriesConfigMarshalYAMLIncludesRegistryToken(t *testing.T) { + registries := SkillsRegistriesConfig{ + &SkillRegistryConfig{ + Name: "github", + AuthToken: *NewSecureString("registry-auth-token"), + }, + } + + data, err := yaml.Marshal(registries) + assert.NoError(t, err) + assert.Contains(t, string(data), "github:") + assert.Contains(t, string(data), "auth_token: registry-auth-token") + + loaded := SkillsRegistriesConfig{ + &SkillRegistryConfig{Name: "github"}, + } + err = yaml.Unmarshal(data, &loaded) + assert.NoError(t, err) + github, ok := loaded.Get("github") + assert.True(t, ok) + assert.Equal(t, "registry-auth-token", github.AuthToken.String()) +} diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index f0449d98f..323f58a93 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -1738,7 +1738,7 @@ func TestFilterSensitiveData_AllTokenTypes(t *testing.T) { Skills: SkillsToolsConfig{ Github: SkillsGithubConfig{Token: *NewSecureString("github-token-xyz")}, Registries: SkillsRegistriesConfig{ - ClawHub: ClawHubRegistryConfig{AuthToken: *NewSecureString("clawhub-auth-token")}, + &SkillRegistryConfig{Name: "clawhub", AuthToken: *NewSecureString("clawhub-auth-token")}, }, }, }, diff --git a/pkg/config/defaults.go b/pkg/config/defaults.go index bb073d436..70da91ec3 100644 --- a/pkg/config/defaults.go +++ b/pkg/config/defaults.go @@ -439,9 +439,17 @@ func DefaultConfig() *Config { Enabled: true, }, Registries: SkillsRegistriesConfig{ - ClawHub: ClawHubRegistryConfig{ + &SkillRegistryConfig{ + Name: "clawhub", Enabled: true, BaseURL: "https://clawhub.ai", + Param: map[string]any{}, + }, + &SkillRegistryConfig{ + Name: "github", + Enabled: true, + BaseURL: "https://github.com", + Param: map[string]any{}, }, }, MaxConcurrentSearches: 2, diff --git a/pkg/config/security_integration_test.go b/pkg/config/security_integration_test.go index 6ca8637f4..821732c5c 100644 --- a/pkg/config/security_integration_test.go +++ b/pkg/config/security_integration_test.go @@ -332,8 +332,9 @@ web: skills: github: token: "file://github_token.txt" - clawhub: - auth_token: "file://clawhub_auth_token.txt" + registries: + clawhub: + auth_token: "file://clawhub_auth_token.txt" ` err = os.WriteFile(securityPath, []byte(securityContent), 0o600) require.NoError(t, err) @@ -431,9 +432,53 @@ skills: assert.Equal(t, "ghp-github-from-file-abc123", cfg.Tools.Skills.Github.Token.String()) t.Logf("Github Token(): %s", cfg.Tools.Skills.Github.Token.String()) - assert.Equal(t, "clawhub-auth-token-from-file", cfg.Tools.Skills.Registries.ClawHub.AuthToken.String()) - t.Logf("ClawHub AuthToken(): %s", cfg.Tools.Skills.Registries.ClawHub.AuthToken.String()) + clawHub, ok := cfg.Tools.Skills.Registries.Get("clawhub") + assert.True(t, ok) + assert.Equal(t, "clawhub-auth-token-from-file", clawHub.AuthToken.String()) + t.Logf("ClawHub AuthToken(): %s", clawHub.AuthToken.String()) t.Log("All security keys are successfully accessible via their respective Key() methods") }) + + t.Run("Github registry token supports security overlay", func(t *testing.T) { + tmpDir := t.TempDir() + + githubTokenFile := filepath.Join(tmpDir, "github_registry_token.txt") + err := os.WriteFile(githubTokenFile, []byte("ghp-github-registry-token-from-file"), 0o600) + require.NoError(t, err) + + configPath := filepath.Join(tmpDir, "config.json") + configContent := `{ + "version": 1, + "tools": { + "skills": { + "registries": { + "github": { + "enabled": true, + "proxy": "http://127.0.0.1:7890" + } + } + } + } +}` + err = os.WriteFile(configPath, []byte(configContent), 0o644) + require.NoError(t, err) + + securityPath := filepath.Join(tmpDir, SecurityConfigFile) + securityContent := `skills: + registries: + github: + auth_token: "file://github_registry_token.txt" +` + err = os.WriteFile(securityPath, []byte(securityContent), 0o600) + require.NoError(t, err) + + cfg, err := LoadConfig(configPath) + require.NoError(t, err) + + githubRegistry, ok := cfg.Tools.Skills.Registries.Get("github") + require.True(t, ok) + assert.Equal(t, "ghp-github-registry-token-from-file", githubRegistry.AuthToken.String()) + assert.Equal(t, "http://127.0.0.1:7890", githubRegistry.Param["proxy"]) + }) } diff --git a/pkg/skills/clawhub_registry.go b/pkg/skills/clawhub_registry.go index bd4bed8fb..9e1e2a2a9 100644 --- a/pkg/skills/clawhub_registry.go +++ b/pkg/skills/clawhub_registry.go @@ -5,11 +5,13 @@ import ( "encoding/json" "fmt" "io" + "log/slog" "net/http" "net/url" "os" "time" + "github.com/sipeed/picoclaw/pkg/config" "github.com/sipeed/picoclaw/pkg/utils" ) @@ -19,6 +21,35 @@ const ( defaultMaxResponseSize = 2 * 1024 * 1024 // 2 MB ) +func init() { + RegisterRegistryProviderBuilder("clawhub", func(_ string, cfg config.SkillRegistryConfig) RegistryProvider { + privateCfg := clawHubRegistryPrivateConfig{} + if err := cfg.DecodeParam(&privateCfg); err != nil { + slog.Warn("invalid clawhub private config", "error", err) + } + return ClawHubConfig{ + Enabled: cfg.Enabled, + BaseURL: cfg.BaseURL, + AuthToken: cfg.AuthToken.String(), + SearchPath: privateCfg.SearchPath, + SkillsPath: privateCfg.SkillsPath, + DownloadPath: privateCfg.DownloadPath, + Timeout: privateCfg.Timeout, + MaxZipSize: privateCfg.MaxZipSize, + MaxResponseSize: privateCfg.MaxResponseSize, + } + }) +} + +type clawHubRegistryPrivateConfig struct { + SearchPath string `json:"search_path"` + SkillsPath string `json:"skills_path"` + DownloadPath string `json:"download_path"` + Timeout int `json:"timeout"` + MaxZipSize int `json:"max_zip_size"` + MaxResponseSize int `json:"max_response_size"` +} + // ClawHubRegistry implements SkillRegistry for the ClawHub platform. type ClawHubRegistry struct { baseURL string @@ -88,6 +119,28 @@ func (c *ClawHubRegistry) Name() string { return "clawhub" } +func (c *ClawHubRegistry) ResolveInstallDirName(target string) (string, error) { + if err := utils.ValidateSkillIdentifier(target); err != nil { + return "", err + } + return target, nil +} + +func (c *ClawHubRegistry) SkillURL(slug string) string { + if slug == "" { + return "" + } + return c.baseURL + "/skills/" + url.PathEscape(slug) +} + +func (c ClawHubConfig) IsEnabled() bool { + return c.Enabled +} + +func (c ClawHubConfig) BuildRegistry() SkillRegistry { + return NewClawHubRegistry(c) +} + // --- Search --- type clawhubSearchResponse struct { diff --git a/pkg/skills/config_bridge.go b/pkg/skills/config_bridge.go new file mode 100644 index 000000000..49febfbd9 --- /dev/null +++ b/pkg/skills/config_bridge.go @@ -0,0 +1,91 @@ +package skills + +import "github.com/sipeed/picoclaw/pkg/config" + +func effectiveRegistryConfigsFromToolsConfig(cfg config.SkillsToolsConfig) []config.SkillRegistryConfig { + effective := make([]config.SkillRegistryConfig, 0, len(cfg.Registries)+1) + seen := map[string]struct{}{} + + for _, registryCfg := range cfg.Registries { + if registryCfg == nil || registryCfg.Name == "" { + continue + } + resolved := *registryCfg + if resolved.Name == "github" { + resolved = applyLegacyGithubRegistryCompatibility(cfg, resolved) + } + effective = append(effective, resolved) + seen[resolved.Name] = struct{}{} + } + + if _, ok := seen["github"]; ok { + return effective + } + + legacyGithubConfigured := cfg.Github.BaseURL != "" || cfg.Github.Token.String() != "" || cfg.Github.Proxy != "" + if !legacyGithubConfigured { + return effective + } + + effective = append(effective, applyLegacyGithubRegistryCompatibility(cfg, config.SkillRegistryConfig{ + Name: "github", + Enabled: true, + })) + return effective +} + +func applyLegacyGithubRegistryCompatibility( + cfg config.SkillsToolsConfig, + registryCfg config.SkillRegistryConfig, +) config.SkillRegistryConfig { + if registryCfg.Name != "github" { + return registryCfg + } + if registryCfg.Param == nil { + registryCfg.Param = map[string]any{} + } + if registryCfg.BaseURL == "" { + registryCfg.BaseURL = cfg.Github.BaseURL + } + if registryCfg.AuthToken.String() == "" { + registryCfg.AuthToken = cfg.Github.Token + } + if _, ok := registryCfg.Param["proxy"]; !ok && cfg.Github.Proxy != "" { + registryCfg.Param["proxy"] = cfg.Github.Proxy + } + return registryCfg +} + +func registryProvidersFromToolsConfig(cfg config.SkillsToolsConfig) []RegistryProvider { + registryConfigs := effectiveRegistryConfigsFromToolsConfig(cfg) + providers := make([]RegistryProvider, 0, len(registryConfigs)) + for _, registryCfg := range registryConfigs { + provider := buildRegistryProvider(registryCfg.Name, registryCfg) + if provider == nil { + continue + } + providers = append(providers, provider) + } + return providers +} + +func NewRegistryManagerFromToolsConfig(cfg config.SkillsToolsConfig) *RegistryManager { + return NewRegistryManagerFromConfig(RegistryConfig{ + Providers: registryProvidersFromToolsConfig(cfg), + MaxConcurrentSearches: cfg.MaxConcurrentSearches, + }) +} + +func LookupRegistryFromToolsConfig(cfg config.SkillsToolsConfig, name string) SkillRegistry { + for _, provider := range registryProvidersFromToolsConfig(cfg) { + if provider == nil { + continue + } + registry := provider.BuildRegistry() + if registry == nil || registry.Name() != name { + continue + } + return registry + } + return nil +} diff --git a/pkg/skills/github_registry.go b/pkg/skills/github_registry.go new file mode 100644 index 000000000..7274e59fa --- /dev/null +++ b/pkg/skills/github_registry.go @@ -0,0 +1,244 @@ +package skills + +import ( + "context" + "encoding/json" + "fmt" + "io" + "log/slog" + "net/http" + "net/url" + "path" + "path/filepath" + "sort" + "strings" + + "github.com/sipeed/picoclaw/pkg/config" +) + +func init() { + RegisterRegistryProviderBuilder("github", func(_ string, cfg config.SkillRegistryConfig) RegistryProvider { + privateCfg := githubRegistryPrivateConfig{} + if err := cfg.DecodeParam(&privateCfg); err != nil { + slog.Warn("invalid github private config", "error", err) + } + return GitHubRegistryConfig{ + Enabled: cfg.Enabled, + BaseURL: cfg.BaseURL, + AuthToken: cfg.AuthToken.String(), + Proxy: privateCfg.Proxy, + } + }) +} + +type githubRegistryPrivateConfig struct { + Proxy string `json:"proxy"` +} + +type GitHubRegistryConfig struct { + Enabled bool + BaseURL string + AuthToken string + Proxy string +} + +type GitHubRegistry struct { + installer *SkillInstaller + webBase string +} + +func (c GitHubRegistryConfig) IsEnabled() bool { + return c.Enabled +} + +func (c GitHubRegistryConfig) BuildRegistry() SkillRegistry { + installer, err := NewSkillInstallerWithBaseURL("", c.BaseURL, c.AuthToken, c.Proxy) + if err != nil { + slog.Warn("failed to create github registry installer", "error", err) + return nil + } + return &GitHubRegistry{ + installer: installer, + webBase: installer.githubBaseURL, + } +} + +func (r *GitHubRegistry) Name() string { + return "github" +} + +func (r *GitHubRegistry) ResolveInstallDirName(target string) (string, error) { + return githubInstallDirName(target) +} + +func (r *GitHubRegistry) SkillURL(target string) string { + ref, err := parseGitHubRef(target) + if err != nil { + return "" + } + base := strings.TrimRight(r.webBase, "/") + urlPath := path.Join(ref.Owner, ref.RepoName) + if ref.SubPath != "" { + return fmt.Sprintf("%s/%s/tree/%s/%s", base, urlPath, url.PathEscape(ref.Ref), ref.SubPath) + } + if ref.Ref != "" && ref.Ref != "main" { + return fmt.Sprintf("%s/%s/tree/%s", base, urlPath, url.PathEscape(ref.Ref)) + } + return fmt.Sprintf("%s/%s", base, urlPath) +} + +type gitHubCodeSearchResponse struct { + Items []gitHubCodeSearchItem `json:"items"` +} + +type gitHubCodeSearchItem struct { + Path string `json:"path"` + HTMLURL string `json:"html_url"` + Score float64 `json:"score"` + Repository struct { + FullName string `json:"full_name"` + Name string `json:"name"` + Description string `json:"description"` + DefaultBranch string `json:"default_branch"` + } `json:"repository"` +} + +func (r *GitHubRegistry) Search(ctx context.Context, query string, limit int) ([]SearchResult, error) { + query = strings.TrimSpace(query) + if query == "" { + return nil, nil + } + if limit <= 0 { + limit = 5 + } + + u, err := url.Parse(strings.TrimRight(r.installer.githubAPIBaseURL, "/") + "/search/code") + if err != nil { + return nil, fmt.Errorf("invalid github api base url: %w", err) + } + q := u.Query() + q.Set("q", fmt.Sprintf("%s filename:SKILL.md", query)) + q.Set("per_page", fmt.Sprintf("%d", limit)) + u.RawQuery = q.Encode() + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, u.String(), nil) + if err != nil { + return nil, err + } + req.Header.Set("Accept", "application/vnd.github+json") + if r.installer.githubToken != "" { + req.Header.Set("Authorization", "Bearer "+r.installer.githubToken) + } + + resp, err := r.installer.client.Do(req) + if err != nil { + return nil, err + } + defer resp.Body.Close() + + body, err := io.ReadAll(io.LimitReader(resp.Body, 2<<20)) + if err != nil { + return nil, fmt.Errorf("failed to read github search response: %w", err) + } + if resp.StatusCode == http.StatusForbidden && r.installer.githubToken == "" && isGitHubRateLimitError(body) { + return nil, nil + } + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + return nil, fmt.Errorf("github search failed: HTTP %d: %s", resp.StatusCode, string(body)) + } + + var parsed gitHubCodeSearchResponse + if err := json.Unmarshal(body, &parsed); err != nil { + return nil, fmt.Errorf("failed to parse github search response: %w", err) + } + + resultsBySlug := map[string]SearchResult{} + for _, item := range parsed.Items { + slug, ok := githubSearchSlug(item) + if !ok { + continue + } + result := SearchResult{ + Score: item.Score, + Slug: slug, + DisplayName: githubSearchDisplayName(item), + Summary: strings.TrimSpace(item.Repository.Description), + Version: strings.TrimSpace(item.Repository.DefaultBranch), + RegistryName: r.Name(), + } + if existing, exists := resultsBySlug[slug]; exists && existing.Score >= result.Score { + continue + } + resultsBySlug[slug] = result + } + + results := make([]SearchResult, 0, len(resultsBySlug)) + for _, result := range resultsBySlug { + results = append(results, result) + } + sort.Slice(results, func(i, j int) bool { + if results[i].Score == results[j].Score { + return results[i].Slug < results[j].Slug + } + return results[i].Score > results[j].Score + }) + if len(results) > limit { + results = results[:limit] + } + return results, nil +} + +func isGitHubRateLimitError(body []byte) bool { + message := strings.ToLower(string(body)) + return strings.Contains(message, "rate limit exceeded") +} + +func githubSearchSlug(item gitHubCodeSearchItem) (string, bool) { + fullName := strings.TrimSpace(item.Repository.FullName) + if fullName == "" { + return "", false + } + cleanPath := strings.Trim(strings.TrimSpace(item.Path), "/") + if cleanPath == "" || filepath.Base(cleanPath) != "SKILL.md" { + return "", false + } + dir := path.Dir(cleanPath) + if dir == "." || dir == "" { + return fullName, true + } + return fullName + "/" + dir, true +} + +func githubSearchDisplayName(item gitHubCodeSearchItem) string { + cleanPath := strings.Trim(strings.TrimSpace(item.Path), "/") + if cleanPath != "" { + dir := path.Dir(cleanPath) + if dir != "." && dir != "" { + return path.Base(dir) + } + } + if name := strings.TrimSpace(item.Repository.Name); name != "" { + return name + } + return strings.TrimSpace(item.Repository.FullName) +} + +func (r *GitHubRegistry) GetSkillMeta(_ context.Context, target string) (*SkillMeta, error) { + ref, err := parseGitHubRef(target) + if err != nil { + return nil, err + } + return &SkillMeta{ + Slug: target, + DisplayName: ref.RepoName, + LatestVersion: ref.Ref, + RegistryName: r.Name(), + }, nil +} + +func (r *GitHubRegistry) DownloadAndInstall( + ctx context.Context, + target, version, targetDir string, +) (*InstallResult, error) { + return r.installer.InstallFromGitHubToDir(ctx, target, version, targetDir) +} diff --git a/pkg/skills/github_registry_test.go b/pkg/skills/github_registry_test.go new file mode 100644 index 000000000..58828fbd9 --- /dev/null +++ b/pkg/skills/github_registry_test.go @@ -0,0 +1,119 @@ +package skills + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/sipeed/picoclaw/pkg/config" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestGitHubRegistrySearch(t *testing.T) { + var server *httptest.Server + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "/api/v3/search/code", r.URL.Path) + assert.Equal(t, "Bearer test-token", r.Header.Get("Authorization")) + assert.Equal(t, "skill search filename:SKILL.md", r.URL.Query().Get("q")) + assert.Equal(t, "2", r.URL.Query().Get("per_page")) + + w.Header().Set("Content-Type", "application/json") + require.NoError(t, json.NewEncoder(w).Encode(gitHubCodeSearchResponse{ + Items: []gitHubCodeSearchItem{ + { + Path: "skills/pr-review/SKILL.md", + Score: 10, + HTMLURL: server.URL + "/foo/bar/blob/main/skills/pr-review/SKILL.md", + Repository: struct { + FullName string `json:"full_name"` + Name string `json:"name"` + Description string `json:"description"` + DefaultBranch string `json:"default_branch"` + }{ + FullName: "foo/bar", + Name: "bar", + Description: "Review pull requests", + DefaultBranch: "main", + }, + }, + { + Path: "SKILL.md", + Score: 5, + HTMLURL: server.URL + "/foo/root/blob/main/SKILL.md", + Repository: struct { + FullName string `json:"full_name"` + Name string `json:"name"` + Description string `json:"description"` + DefaultBranch string `json:"default_branch"` + }{ + FullName: "foo/root", + Name: "root", + Description: "Root skill", + DefaultBranch: "master", + }, + }, + }, + })) + })) + defer server.Close() + + provider := GitHubRegistryConfig{ + Enabled: true, + BaseURL: server.URL, + AuthToken: "test-token", + } + registry := provider.BuildRegistry() + require.NotNil(t, registry) + + results, err := registry.Search(context.Background(), "skill search", 2) + require.NoError(t, err) + require.Len(t, results, 2) + + assert.Equal(t, "foo/bar/skills/pr-review", results[0].Slug) + assert.Equal(t, "pr-review", results[0].DisplayName) + assert.Equal(t, "Review pull requests", results[0].Summary) + assert.Equal(t, "main", results[0].Version) + assert.Equal(t, "github", results[0].RegistryName) + + assert.Equal(t, "foo/root", results[1].Slug) + assert.Equal(t, "root", results[1].DisplayName) + assert.Equal(t, "master", results[1].Version) +} + +func TestGitHubRegistryProviderDecodesProxyParam(t *testing.T) { + builder := buildRegistryProvider("github", config.SkillRegistryConfig{ + Name: "github", + Enabled: true, + BaseURL: "https://github.com", + AuthToken: *config.NewSecureString("test-token"), + Param: map[string]any{ + "proxy": "http://127.0.0.1:7890", + }, + }) + require.NotNil(t, builder) + + registry := builder.BuildRegistry() + require.NotNil(t, registry) + ghRegistry, ok := registry.(*GitHubRegistry) + require.True(t, ok) + assert.Equal(t, "http://127.0.0.1:7890", ghRegistry.installer.proxy) +} + +func TestGitHubRegistrySearchReturnsEmptyOnUnauthenticatedRateLimit(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) + _, _ = w.Write([]byte(`{"message":"API rate limit exceeded for 1.2.3.4"}`)) + })) + defer server.Close() + + registry := GitHubRegistryConfig{Enabled: true, BaseURL: server.URL}.BuildRegistry() + require.NotNil(t, registry) + + results, err := registry.Search(context.Background(), "pr review", 5) + require.NoError(t, err) + assert.Empty(t, results) +} diff --git a/pkg/skills/installer.go b/pkg/skills/installer.go index f6cdee3a6..16a2e55c9 100644 --- a/pkg/skills/installer.go +++ b/pkg/skills/installer.go @@ -12,6 +12,7 @@ import ( "strings" "time" + "github.com/sipeed/picoclaw/pkg/fileutil" "github.com/sipeed/picoclaw/pkg/utils" ) @@ -33,25 +34,106 @@ type GitHubRef struct { } type SkillInstaller struct { - workspace string - client *http.Client - githubToken string - proxy string + workspace string + client *http.Client + githubBaseURL string + githubAPIBaseURL string + githubRawBaseURL string + githubToken string + proxy string } // NewSkillInstaller creates a new skill installer. // proxy is an optional HTTP/HTTPS/SOCKS5 proxy URL for downloading skills. func NewSkillInstaller(workspace, githubToken, proxy string) (*SkillInstaller, error) { + return NewSkillInstallerWithBaseURL(workspace, "", githubToken, proxy) +} + +// NewSkillInstallerWithBaseURL creates a new skill installer with a custom GitHub base URL. +// For github.com this can be left empty. For GitHub Enterprise, set it to the web URL. +func NewSkillInstallerWithBaseURL(workspace, githubBaseURL, githubToken, proxy string) (*SkillInstaller, error) { client, err := utils.CreateHTTPClient(proxy, 15*time.Second) if err != nil { return nil, fmt.Errorf("failed to create HTTP client: %w", err) } + endpoints, err := resolveGitHubEndpoints(githubBaseURL) + if err != nil { + return nil, err + } return &SkillInstaller{ - workspace: workspace, - client: client, - githubToken: githubToken, - proxy: proxy, + workspace: workspace, + client: client, + githubBaseURL: endpoints.WebBaseURL, + githubAPIBaseURL: endpoints.APIBaseURL, + githubRawBaseURL: endpoints.RawBaseURL, + githubToken: githubToken, + proxy: proxy, + }, nil +} + +type gitHubEndpoints struct { + WebBaseURL string + APIBaseURL string + RawBaseURL string +} + +func resolveGitHubEndpoints(baseURL string) (gitHubEndpoints, error) { + trimmed := strings.TrimSpace(baseURL) + if trimmed == "" { + return gitHubEndpoints{ + WebBaseURL: "https://github.com", + APIBaseURL: "https://api.github.com", + RawBaseURL: "https://raw.githubusercontent.com", + }, nil + } + + u, err := url.Parse(trimmed) + if err != nil { + return gitHubEndpoints{}, fmt.Errorf("invalid github base url: %w", err) + } + if u.Scheme == "" || u.Host == "" { + return gitHubEndpoints{}, fmt.Errorf("invalid github base url %q", baseURL) + } + + trimmedPath := strings.TrimSuffix(u.Path, "/") + origin := u.Scheme + "://" + u.Host + + if u.Host == "api.github.com" { + return gitHubEndpoints{ + WebBaseURL: "https://github.com", + APIBaseURL: "https://api.github.com", + RawBaseURL: "https://raw.githubusercontent.com", + }, nil + } + + if strings.HasSuffix(trimmedPath, "/api/v3") { + webBaseURL := origin + strings.TrimSuffix(trimmedPath, "/api/v3") + webBaseURL = strings.TrimSuffix(webBaseURL, "/") + if webBaseURL == origin { + webBaseURL = origin + } + return gitHubEndpoints{ + WebBaseURL: webBaseURL, + APIBaseURL: origin + trimmedPath, + RawBaseURL: webBaseURL + "/raw", + }, nil + } + + webBaseURL := origin + trimmedPath + webBaseURL = strings.TrimSuffix(webBaseURL, "/") + if u.Host == "github.com" { + return gitHubEndpoints{ + WebBaseURL: "https://github.com", + APIBaseURL: "https://api.github.com", + RawBaseURL: "https://raw.githubusercontent.com", + }, nil + } + + return gitHubEndpoints{ + WebBaseURL: webBaseURL, + APIBaseURL: webBaseURL + "/api/v3", + RawBaseURL: webBaseURL + "/raw", }, nil } @@ -104,38 +186,70 @@ func parseGitHubRef(repo string) (GitHubRef, error) { return ref, nil } -func (si *SkillInstaller) InstallFromGitHub(ctx context.Context, repo string) error { +func githubInstallDirName(repo string) (string, error) { + if err := ValidateInstallTarget(repo); err != nil { + return "", err + } ref, err := parseGitHubRef(repo) + if err != nil { + return "", err + } + if ref.SubPath != "" { + return filepath.Base(ref.SubPath), nil + } + return ref.RepoName, nil +} + +func (si *SkillInstaller) InstallFromGitHub(ctx context.Context, repo string) error { + skillName, err := githubInstallDirName(repo) if err != nil { return err } - - skillName := ref.RepoName - if ref.SubPath != "" { - skillName = filepath.Base(ref.SubPath) - } skillDirectory := filepath.Join(si.workspace, "skills", skillName) - if _, err := os.Stat(skillDirectory); err == nil { + if _, statErr := os.Stat(skillDirectory); statErr == nil { return fmt.Errorf("skill '%s' already exists", skillName) } + _, err = si.InstallFromGitHubToDir(ctx, repo, "", skillDirectory) + return err +} + +func (si *SkillInstaller) InstallFromGitHubToDir( + ctx context.Context, + repo, version, skillDirectory string, +) (*InstallResult, error) { + ref, err := parseGitHubRef(repo) + if err != nil { + return nil, err + } + if version != "" { + ref.Ref = version + } // Build GitHub API URL apiPath := path.Join(ref.Owner, ref.RepoName, "contents") if ref.SubPath != "" { apiPath = path.Join(apiPath, ref.SubPath) } - apiURL := fmt.Sprintf("https://api.github.com/repos/%s?ref=%s", apiPath, ref.Ref) + apiURL := fmt.Sprintf("%s/repos/%s?ref=%s", si.githubAPIBaseURL, apiPath, url.QueryEscape(ref.Ref)) if err := si.getGithubDirAllFiles(ctx, apiURL, skillDirectory, true); err != nil { // Fallback to raw download - return si.downloadRaw(ctx, ref.Owner, ref.RepoName, ref.Ref, ref.SubPath, skillDirectory) + if downloadErr := si.downloadRaw( + ctx, + ref.Owner, + ref.RepoName, + ref.Ref, + ref.SubPath, + skillDirectory, + ); downloadErr != nil { + return nil, downloadErr + } + } else if _, err := os.Stat(filepath.Join(skillDirectory, "SKILL.md")); err != nil { + return nil, fmt.Errorf("SKILL.md not found in repository") } - if _, err := os.Stat(filepath.Join(skillDirectory, "SKILL.md")); err != nil { - return fmt.Errorf("SKILL.md not found in repository") - } - return nil + return &InstallResult{Version: ref.Ref}, nil } // downloadDir recursively downloads a directory from GitHub API @@ -193,7 +307,7 @@ func (si *SkillInstaller) downloadRaw(ctx context.Context, owner, repo, ref, sub if subPath != "" { urlPath = path.Join(urlPath, subPath) } - url := fmt.Sprintf("https://raw.githubusercontent.com/%s/SKILL.md", urlPath) + url := fmt.Sprintf("%s/%s/SKILL.md", si.githubRawBaseURL, urlPath) req, err := http.NewRequestWithContext(ctx, "GET", url, nil) if err != nil { @@ -213,12 +327,10 @@ func (si *SkillInstaller) downloadRaw(ctx context.Context, owner, repo, ref, sub localPath := filepath.Join(localDir, "SKILL.md") - // Atomic move from temp to final location. - if err := os.Rename(tmpPath, localPath); err != nil { + if err := fileutil.CopyFile(tmpPath, localPath, 0o600); err != nil { return fmt.Errorf("failed to write skill file: %w", err) } - - return os.Chmod(localPath, 0o600) + return nil } func (si *SkillInstaller) downloadFile(ctx context.Context, url, localPath string) error { @@ -238,12 +350,10 @@ func (si *SkillInstaller) downloadFile(ctx context.Context, url, localPath strin return err } - // Atomic move from temp to final location. - if err := os.Rename(tmpPath, localPath); err != nil { + if err := fileutil.CopyFile(tmpPath, localPath, 0o600); err != nil { return fmt.Errorf("failed to move downloaded file: %w", err) } - - return os.Chmod(localPath, 0o600) + return nil } // shouldDownload determines if a file should be downloaded diff --git a/pkg/skills/installer_test.go b/pkg/skills/installer_test.go index 759cfc489..d292ae525 100644 --- a/pkg/skills/installer_test.go +++ b/pkg/skills/installer_test.go @@ -197,6 +197,16 @@ func TestNewSkillInstaller(t *testing.T) { t.Errorf("githubToken = %v, want 'test-token'", installer.githubToken) } + if installer.githubBaseURL != "https://github.com" { + t.Errorf("githubBaseURL = %v, want https://github.com", installer.githubBaseURL) + } + if installer.githubAPIBaseURL != "https://api.github.com" { + t.Errorf("githubAPIBaseURL = %v, want https://api.github.com", installer.githubAPIBaseURL) + } + if installer.githubRawBaseURL != "https://raw.githubusercontent.com" { + t.Errorf("githubRawBaseURL = %v, want https://raw.githubusercontent.com", installer.githubRawBaseURL) + } + if installer.proxy != "" { t.Errorf("proxy = %v, want empty", installer.proxy) } @@ -234,6 +244,24 @@ func TestNewSkillInstaller_WithProxy(t *testing.T) { } } +func TestNewSkillInstaller_WithBaseURL(t *testing.T) { + tmpDir := t.TempDir() + installer, err := NewSkillInstallerWithBaseURL(tmpDir, "https://github.example.com", "test-token", "") + if err != nil { + t.Fatalf("NewSkillInstallerWithBaseURL() error = %v", err) + } + + if installer.githubBaseURL != "https://github.example.com" { + t.Errorf("githubBaseURL = %v, want https://github.example.com", installer.githubBaseURL) + } + if installer.githubAPIBaseURL != "https://github.example.com/api/v3" { + t.Errorf("githubAPIBaseURL = %v, want https://github.example.com/api/v3", installer.githubAPIBaseURL) + } + if installer.githubRawBaseURL != "https://github.example.com/raw" { + t.Errorf("githubRawBaseURL = %v, want https://github.example.com/raw", installer.githubRawBaseURL) + } +} + func TestNewSkillInstaller_InvalidProxy(t *testing.T) { tmpDir := t.TempDir() installer, err := NewSkillInstaller(tmpDir, "test-token", "://invalid-proxy") diff --git a/pkg/skills/provider_factory.go b/pkg/skills/provider_factory.go new file mode 100644 index 000000000..fe2849e1e --- /dev/null +++ b/pkg/skills/provider_factory.go @@ -0,0 +1,33 @@ +package skills + +import ( + "sync" + + "github.com/sipeed/picoclaw/pkg/config" +) + +type RegistryProviderBuilder func(name string, cfg config.SkillRegistryConfig) RegistryProvider + +var ( + registryProviderBuildersMu sync.RWMutex + registryProviderBuilders = map[string]RegistryProviderBuilder{} +) + +func RegisterRegistryProviderBuilder(name string, builder RegistryProviderBuilder) { + if name == "" || builder == nil { + return + } + registryProviderBuildersMu.Lock() + defer registryProviderBuildersMu.Unlock() + registryProviderBuilders[name] = builder +} + +func buildRegistryProvider(name string, cfg config.SkillRegistryConfig) RegistryProvider { + registryProviderBuildersMu.RLock() + defer registryProviderBuildersMu.RUnlock() + builder := registryProviderBuilders[name] + if builder == nil { + return nil + } + return builder(name, cfg) +} diff --git a/pkg/skills/registry.go b/pkg/skills/registry.go index 45ae72253..e35179175 100644 --- a/pkg/skills/registry.go +++ b/pkg/skills/registry.go @@ -4,6 +4,8 @@ import ( "context" "fmt" "log/slog" + "path" + "strings" "sync" "time" ) @@ -42,11 +44,24 @@ type InstallResult struct { Summary string } +// RegistryProvider creates a registry instance from configuration. +// Different hubs can implement this to plug into the shared manager. +type RegistryProvider interface { + IsEnabled() bool + BuildRegistry() SkillRegistry +} + // SkillRegistry is the interface that all skill registries must implement. // Each registry represents a different source of skills (e.g., clawhub.ai) type SkillRegistry interface { // Name returns the unique name of this registry (e.g., "clawhub"). Name() string + // ResolveInstallDirName returns the directory name to use under workspace/skills + // for a given install target. Different registries can interpret the target + // 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 // 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. @@ -60,7 +75,7 @@ type SkillRegistry interface { // RegistryConfig holds configuration for all skill registries. // This is the input to NewRegistryManagerFromConfig. type RegistryConfig struct { - ClawHub ClawHubConfig + Providers []RegistryProvider MaxConcurrentSearches int } @@ -85,6 +100,29 @@ type RegistryManager struct { mu sync.RWMutex } +func ValidateInstallTarget(target string) error { + target = strings.TrimSpace(target) + if target == "" { + return fmt.Errorf("identifier is required and must be a non-empty string") + } + if strings.Contains(target, "\\") { + return fmt.Errorf("identifier %q contains invalid path separators", target) + } + clean := path.Clean("/" + target) + if clean == "/" || strings.HasPrefix(clean, "/../") || clean == "/.." { + return fmt.Errorf("identifier %q contains invalid path traversal", target) + } + if strings.Contains(target, "//") { + return fmt.Errorf("identifier %q contains empty path segments", target) + } + for _, segment := range strings.Split(strings.Trim(target, "/"), "/") { + if segment == "." || segment == ".." || segment == "" { + return fmt.Errorf("identifier %q contains invalid path segments", target) + } + } + return nil +} + // NewRegistryManager creates an empty RegistryManager. func NewRegistryManager() *RegistryManager { return &RegistryManager{ @@ -100,8 +138,15 @@ func NewRegistryManagerFromConfig(cfg RegistryConfig) *RegistryManager { if cfg.MaxConcurrentSearches > 0 { rm.maxConcurrent = cfg.MaxConcurrentSearches } - if cfg.ClawHub.Enabled { - rm.AddRegistry(NewClawHubRegistry(cfg.ClawHub)) + for _, provider := range cfg.Providers { + if provider == nil || !provider.IsEnabled() { + continue + } + registry := provider.BuildRegistry() + if registry == nil { + continue + } + rm.AddRegistry(registry) } return rm } diff --git a/pkg/skills/registry_test.go b/pkg/skills/registry_test.go index a4694bd43..1c3dfda65 100644 --- a/pkg/skills/registry_test.go +++ b/pkg/skills/registry_test.go @@ -24,6 +24,10 @@ type mockRegistry struct { 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) Search(_ context.Context, _ string, _ int) ([]SearchResult, error) { return m.searchResults, m.searchErr } @@ -170,6 +174,31 @@ func TestSortByScoreDesc(t *testing.T) { assert.Equal(t, "c", results[2].Slug) } +type mockProvider struct { + enabled bool + registry SkillRegistry +} + +func (m mockProvider) IsEnabled() bool { + return m.enabled +} + +func (m mockProvider) BuildRegistry() SkillRegistry { + return m.registry +} + +func TestNewRegistryManagerFromConfigProviders(t *testing.T) { + mgr := NewRegistryManagerFromConfig(RegistryConfig{ + Providers: []RegistryProvider{ + mockProvider{enabled: true, registry: &mockRegistry{name: "alpha"}}, + mockProvider{enabled: false, registry: &mockRegistry{name: "beta"}}, + }, + }) + + assert.NotNil(t, mgr.GetRegistry("alpha")) + assert.Nil(t, mgr.GetRegistry("beta")) +} + func TestIsSafeSlug(t *testing.T) { assert.NoError(t, utils.ValidateSkillIdentifier("github")) assert.NoError(t, utils.ValidateSkillIdentifier("docker-compose")) diff --git a/pkg/tools/skills_install.go b/pkg/tools/skills_install.go index 71bfe730b..aaeef6761 100644 --- a/pkg/tools/skills_install.go +++ b/pkg/tools/skills_install.go @@ -74,11 +74,7 @@ func (t *InstallSkillTool) Execute(ctx context.Context, args map[string]any) *To t.mu.Lock() defer t.mu.Unlock() - // Validate slug slug, _ := args["slug"].(string) - if err := utils.ValidateSkillIdentifier(slug); err != nil { - return ErrorResult(fmt.Sprintf("invalid slug %q: error: %s", slug, err.Error())) - } // Validate registry registryName, _ := args["registry"].(string) @@ -86,15 +82,27 @@ func (t *InstallSkillTool) Execute(ctx context.Context, args map[string]any) *To return ErrorResult(fmt.Sprintf("invalid registry %q: error: %s", registryName, err.Error())) } + // Resolve which registry to use. + registry := t.registryMgr.GetRegistry(registryName) + if registry == nil { + return ErrorResult(fmt.Sprintf("registry %q not found", registryName)) + } + + // Validate target and resolve install directory. + dirName, err := registry.ResolveInstallDirName(slug) + if err != nil { + return ErrorResult(fmt.Sprintf("invalid slug %q: error: %s", slug, err.Error())) + } + version, _ := args["version"].(string) force, _ := args["force"].(bool) // Check if already installed. skillsDir := filepath.Join(t.workspace, "skills") - targetDir := filepath.Join(skillsDir, slug) + targetDir := filepath.Join(skillsDir, dirName) if !force { - if _, err := os.Stat(targetDir); err == nil { + if _, statErr := os.Stat(targetDir); statErr == nil { return ErrorResult( fmt.Sprintf("skill %q already installed at %s. Use force=true to reinstall.", slug, targetDir), ) @@ -104,15 +112,9 @@ func (t *InstallSkillTool) Execute(ctx context.Context, args map[string]any) *To os.RemoveAll(targetDir) } - // Resolve which registry to use. - registry := t.registryMgr.GetRegistry(registryName) - if registry == nil { - return ErrorResult(fmt.Sprintf("registry %q not found", registryName)) - } - // Ensure skills directory exists. - if err := os.MkdirAll(skillsDir, 0o755); err != nil { - return ErrorResult(fmt.Sprintf("failed to create skills directory: %v", err)) + if mkdirErr := os.MkdirAll(skillsDir, 0o755); mkdirErr != nil { + return ErrorResult(fmt.Sprintf("failed to create skills directory: %v", mkdirErr)) } // Download and install (handles metadata, version resolution, extraction). diff --git a/pkg/tools/skills_install_test.go b/pkg/tools/skills_install_test.go index 676fcecc0..f0fcaea0a 100644 --- a/pkg/tools/skills_install_test.go +++ b/pkg/tools/skills_install_test.go @@ -12,6 +12,60 @@ import ( "github.com/sipeed/picoclaw/pkg/skills" ) +type mockInstallRegistry struct{} + +func (m *mockInstallRegistry) Name() string { return "clawhub" } + +func (m *mockInstallRegistry) ResolveInstallDirName(target string) (string, error) { + return target, nil +} + +func (m *mockInstallRegistry) SkillURL(slug string) string { return slug } + +func (m *mockInstallRegistry) Search(context.Context, string, int) ([]skills.SearchResult, error) { + return nil, nil +} + +func (m *mockInstallRegistry) GetSkillMeta(context.Context, string) (*skills.SkillMeta, error) { + return nil, nil +} + +func (m *mockInstallRegistry) DownloadAndInstall( + context.Context, + string, + string, + string, +) (*skills.InstallResult, error) { + return &skills.InstallResult{Version: "test"}, nil +} + +type mockGitHubInstallRegistry struct{} + +func (m *mockGitHubInstallRegistry) Name() string { return "github" } + +func (m *mockGitHubInstallRegistry) ResolveInstallDirName(target string) (string, error) { + return "pr-review", nil +} + +func (m *mockGitHubInstallRegistry) SkillURL(slug string) string { return slug } + +func (m *mockGitHubInstallRegistry) Search(context.Context, string, int) ([]skills.SearchResult, error) { + return nil, nil +} + +func (m *mockGitHubInstallRegistry) GetSkillMeta(context.Context, string) (*skills.SkillMeta, error) { + return nil, nil +} + +func (m *mockGitHubInstallRegistry) DownloadAndInstall( + context.Context, + string, + string, + string, +) (*skills.InstallResult, error) { + return &skills.InstallResult{Version: "main"}, nil +} + func TestInstallSkillToolName(t *testing.T) { tool := NewInstallSkillTool(skills.NewRegistryManager(), t.TempDir()) assert.Equal(t, "install_skill", tool.Name()) @@ -34,7 +88,9 @@ func TestInstallSkillToolEmptySlug(t *testing.T) { } func TestInstallSkillToolUnsafeSlug(t *testing.T) { - tool := NewInstallSkillTool(skills.NewRegistryManager(), t.TempDir()) + registryMgr := skills.NewRegistryManager() + registryMgr.AddRegistry(skills.NewClawHubRegistry(skills.ClawHubConfig{Enabled: true})) + tool := NewInstallSkillTool(registryMgr, t.TempDir()) cases := []string{ "../etc/passwd", @@ -44,7 +100,8 @@ func TestInstallSkillToolUnsafeSlug(t *testing.T) { for _, slug := range cases { result := tool.Execute(context.Background(), map[string]any{ - "slug": slug, + "slug": slug, + "registry": "clawhub", }) assert.True(t, result.IsError, "slug %q should be rejected", slug) assert.Contains(t, result.ForLLM, "invalid slug") @@ -56,7 +113,9 @@ func TestInstallSkillToolAlreadyExists(t *testing.T) { skillDir := filepath.Join(workspace, "skills", "existing-skill") require.NoError(t, os.MkdirAll(skillDir, 0o755)) - tool := NewInstallSkillTool(skills.NewRegistryManager(), workspace) + registryMgr := skills.NewRegistryManager() + registryMgr.AddRegistry(&mockInstallRegistry{}) + tool := NewInstallSkillTool(registryMgr, workspace) result := tool.Execute(context.Background(), map[string]any{ "slug": "existing-skill", "registry": "clawhub", @@ -102,3 +161,17 @@ func TestInstallSkillToolMissingRegistry(t *testing.T) { assert.True(t, result.IsError) assert.Contains(t, result.ForLLM, "invalid registry") } + +func TestInstallSkillToolAllowsGitHubURLSlug(t *testing.T) { + registryMgr := skills.NewRegistryManager() + registryMgr.AddRegistry(&mockGitHubInstallRegistry{}) + tool := NewInstallSkillTool(registryMgr, t.TempDir()) + + result := tool.Execute(context.Background(), map[string]any{ + "slug": "https://github.com/synthetic-lab/octofriend/tree/main/.agents/skills/pr-review", + "registry": "github", + }) + + assert.False(t, result.IsError) + assert.Contains(t, result.ForLLM, `Successfully installed skill`) +} diff --git a/web/backend/api/config.go b/web/backend/api/config.go index 5490b4e18..bdbe079b3 100644 --- a/web/backend/api/config.go +++ b/web/backend/api/config.go @@ -480,13 +480,38 @@ func applyConfigSecretsFromMap(cfg *config.Config, raw map[string]any) { cfg.Tools.Skills.Github.Token.Set(token) } } - registries, hasRegistries := asMapField(skills, "registries") + if registries, hasRegistries := asMapField(skills, "registries"); hasRegistries { + for registryName, rawRegistry := range registries { + registryMap, ok := rawRegistry.(map[string]any) + if !ok { + continue + } + if authToken, hasAuthToken := getSecretString(registryMap, "auth_token"); hasAuthToken { + registryCfg, _ := cfg.Tools.Skills.Registries.Get(registryName) + registryCfg.AuthToken.Set(authToken) + cfg.Tools.Skills.Registries.Set(registryName, registryCfg) + } + } + return + } + + registriesList, hasRegistries := skills["registries"].([]any) if !hasRegistries { return } - if clawHub, hasClawHub := asMapField(registries, "clawhub"); hasClawHub { - if authToken, hasAuthToken := getSecretString(clawHub, "auth_token"); hasAuthToken { - cfg.Tools.Skills.Registries.ClawHub.AuthToken.Set(authToken) + for _, rawRegistry := range registriesList { + registryMap, ok := rawRegistry.(map[string]any) + if !ok { + continue + } + name, _ := registryMap["name"].(string) + if name == "" { + continue + } + if authToken, hasAuthToken := getSecretString(registryMap, "auth_token"); hasAuthToken { + registryCfg, _ := cfg.Tools.Skills.Registries.Get(name) + registryCfg.AuthToken.Set(authToken) + cfg.Tools.Skills.Registries.Set(name, registryCfg) } } } diff --git a/web/backend/api/skills.go b/web/backend/api/skills.go index 2c054c41b..e6e7c0157 100644 --- a/web/backend/api/skills.go +++ b/web/backend/api/skills.go @@ -8,7 +8,6 @@ import ( "io" "io/fs" "net/http" - "net/url" "os" "path/filepath" "regexp" @@ -293,14 +292,6 @@ func (h *Handler) handleInstallSkill(w http.ResponseWriter, r *http.Request) { req.Registry = strings.TrimSpace(req.Registry) req.Version = strings.TrimSpace(req.Version) - if validateErr := utils.ValidateSkillIdentifier(req.Slug); validateErr != nil { - http.Error( - w, - fmt.Sprintf("invalid slug %q: error: %s", req.Slug, validateErr.Error()), - http.StatusBadRequest, - ) - return - } if validateErr := utils.ValidateSkillIdentifier(req.Registry); validateErr != nil { http.Error( w, @@ -316,10 +307,15 @@ func (h *Handler) handleInstallSkill(w http.ResponseWriter, r *http.Request) { http.Error(w, fmt.Sprintf("registry %q not found", req.Registry), http.StatusBadRequest) return } + dirName, err := registry.ResolveInstallDirName(req.Slug) + if err != nil { + http.Error(w, fmt.Sprintf("invalid slug %q: error: %s", req.Slug, err.Error()), http.StatusBadRequest) + return + } workspace := cfg.WorkspacePath() skillsRoot := filepath.Join(workspace, "skills") - targetDir := filepath.Join(workspace, "skills", req.Slug) + targetDir := filepath.Join(workspace, "skills", dirName) workspaceSkillWriteMu.Lock() defer workspaceSkillWriteMu.Unlock() @@ -332,15 +328,15 @@ func (h *Handler) handleInstallSkill(w http.ResponseWriter, r *http.Request) { } if !req.Force && targetExists { - http.Error(w, fmt.Sprintf("skill %q already installed at %s", req.Slug, targetDir), http.StatusConflict) + http.Error(w, fmt.Sprintf("skill %q already installed at %s", dirName, targetDir), http.StatusConflict) return } - if err := os.MkdirAll(skillsRoot, 0o755); err != nil { - http.Error(w, fmt.Sprintf("Failed to create skills directory: %v", err), http.StatusInternalServerError) + if mkdirErr := os.MkdirAll(skillsRoot, 0o755); mkdirErr != nil { + http.Error(w, fmt.Sprintf("Failed to create skills directory: %v", mkdirErr), http.StatusInternalServerError) return } - stagedWorkspaceRoot, stagedTargetDir, err := createStagedSkillInstall(skillsRoot, req.Slug) + stagedWorkspaceRoot, stagedTargetDir, err := createStagedSkillInstall(skillsRoot, dirName) if err != nil { http.Error(w, fmt.Sprintf("Failed to prepare staged install: %v", err), http.StatusInternalServerError) return @@ -361,7 +357,7 @@ func (h *Handler) handleInstallSkill(w http.ResponseWriter, r *http.Request) { return } - if findWorkspaceSkillInfoByDirectory(stagedWorkspaceRoot, req.Slug) == nil { + if findWorkspaceSkillInfoByDirectory(stagedWorkspaceRoot, dirName) == nil { http.Error( w, fmt.Sprintf("Failed to install skill: registry archive for %q is not a valid skill", req.Slug), @@ -394,7 +390,7 @@ func (h *Handler) handleInstallSkill(w http.ResponseWriter, r *http.Request) { return } - validatedSkill := findWorkspaceSkillByDirectory(cfg, req.Slug) + validatedSkill := findWorkspaceSkillByDirectory(cfg, dirName) if validatedSkill == nil { http.Error( w, @@ -511,21 +507,7 @@ func newSkillsLoader(workspace string) *skills.SkillsLoader { } func newSkillsRegistryManager(cfg *config.Config) *skills.RegistryManager { - clawHubConfig := cfg.Tools.Skills.Registries.ClawHub - return skills.NewRegistryManagerFromConfig(skills.RegistryConfig{ - MaxConcurrentSearches: cfg.Tools.Skills.MaxConcurrentSearches, - ClawHub: skills.ClawHubConfig{ - Enabled: clawHubConfig.Enabled, - BaseURL: clawHubConfig.BaseURL, - AuthToken: clawHubConfig.AuthToken.String(), - SearchPath: clawHubConfig.SearchPath, - SkillsPath: clawHubConfig.SkillsPath, - DownloadPath: clawHubConfig.DownloadPath, - Timeout: clawHubConfig.Timeout, - MaxZipSize: clawHubConfig.MaxZipSize, - MaxResponseSize: clawHubConfig.MaxResponseSize, - }, - }) + return skills.NewRegistryManagerFromToolsConfig(cfg.Tools.Skills) } func ensureSkillRegistryToolEnabled(cfg *config.Config, toolName string) error { @@ -740,16 +722,14 @@ func writeSkillOriginMeta(targetDir string, meta installedSkillOriginMeta) error } func registrySkillURL(cfg *config.Config, registryName, slug string) string { - switch registryName { - case "clawhub": - baseURL := strings.TrimRight(cfg.Tools.Skills.Registries.ClawHub.BaseURL, "/") - if baseURL == "" { - baseURL = "https://clawhub.ai" - } - return baseURL + "/skills/" + url.PathEscape(slug) - default: + if cfg == nil || registryName == "" || slug == "" { return "" } + registry := skills.LookupRegistryFromToolsConfig(cfg.Tools.Skills, registryName) + if registry == nil { + return "" + } + return registry.SkillURL(slug) } func registrySkillURLFromMeta(cfg *config.Config, meta *installedSkillOriginMeta) string { diff --git a/web/backend/api/skills_test.go b/web/backend/api/skills_test.go index 17aef485e..85ffbf517 100644 --- a/web/backend/api/skills_test.go +++ b/web/backend/api/skills_test.go @@ -18,6 +18,12 @@ import ( "github.com/sipeed/picoclaw/pkg/config" ) +func setClawHubBaseURL(cfg *config.Config, baseURL string) { + registryCfg, _ := cfg.Tools.Skills.Registries.Get("clawhub") + registryCfg.BaseURL = baseURL + cfg.Tools.Skills.Registries.Set("clawhub", registryCfg) +} + func TestHandleListSkills(t *testing.T) { configPath, cleanup := setupOAuthTestEnv(t) defer cleanup() @@ -583,7 +589,7 @@ func TestHandleSearchSkills(t *testing.T) { })) defer server.Close() - cfg.Tools.Skills.Registries.ClawHub.BaseURL = server.URL + setClawHubBaseURL(cfg, server.URL) if err := config.SaveConfig(configPath, cfg); err != nil { t.Fatalf("SaveConfig() error = %v", err) } @@ -681,7 +687,7 @@ func TestHandleSearchSkillsPagination(t *testing.T) { })) defer server.Close() - cfg.Tools.Skills.Registries.ClawHub.BaseURL = server.URL + setClawHubBaseURL(cfg, server.URL) if err := config.SaveConfig(configPath, cfg); err != nil { t.Fatalf("SaveConfig() error = %v", err) } @@ -755,7 +761,7 @@ func TestHandleSearchSkillsClampsRegistryFanout(t *testing.T) { })) defer server.Close() - cfg.Tools.Skills.Registries.ClawHub.BaseURL = server.URL + setClawHubBaseURL(cfg, server.URL) if err := config.SaveConfig(configPath, cfg); err != nil { t.Fatalf("SaveConfig() error = %v", err) } @@ -838,7 +844,7 @@ func TestHandleInstallSkill(t *testing.T) { })) defer server.Close() - cfg.Tools.Skills.Registries.ClawHub.BaseURL = server.URL + setClawHubBaseURL(cfg, server.URL) if saveErr := config.SaveConfig(configPath, cfg); saveErr != nil { t.Fatalf("SaveConfig() error = %v", saveErr) } @@ -972,7 +978,7 @@ func TestHandleInstallSkillForcePreservesExistingSkillOnFailure(t *testing.T) { })) defer server.Close() - cfg.Tools.Skills.Registries.ClawHub.BaseURL = server.URL + setClawHubBaseURL(cfg, server.URL) if saveErr := config.SaveConfig(configPath, cfg); saveErr != nil { t.Fatalf("SaveConfig() error = %v", saveErr) } @@ -1047,7 +1053,7 @@ func TestHandleInstallSkillRollsBackOnOriginMetadataWriteFailure(t *testing.T) { })) defer server.Close() - cfg.Tools.Skills.Registries.ClawHub.BaseURL = server.URL + setClawHubBaseURL(cfg, server.URL) if saveErr := config.SaveConfig(configPath, cfg); saveErr != nil { t.Fatalf("SaveConfig() error = %v", saveErr) } @@ -1135,7 +1141,7 @@ func TestHandleInstallSkillSerializesConcurrentRequests(t *testing.T) { })) defer server.Close() - cfg.Tools.Skills.Registries.ClawHub.BaseURL = server.URL + setClawHubBaseURL(cfg, server.URL) if saveErr := config.SaveConfig(configPath, cfg); saveErr != nil { t.Fatalf("SaveConfig() error = %v", saveErr) } @@ -1248,7 +1254,7 @@ func TestHandleImportSkillWaitsForConcurrentInstall(t *testing.T) { })) defer server.Close() - cfg.Tools.Skills.Registries.ClawHub.BaseURL = server.URL + setClawHubBaseURL(cfg, server.URL) if saveErr := config.SaveConfig(configPath, cfg); saveErr != nil { t.Fatalf("SaveConfig() error = %v", saveErr) } @@ -1365,7 +1371,7 @@ func TestHandleInstallSkillRejectsInvalidArchive(t *testing.T) { })) defer server.Close() - cfg.Tools.Skills.Registries.ClawHub.BaseURL = server.URL + setClawHubBaseURL(cfg, server.URL) if saveErr := config.SaveConfig(configPath, cfg); saveErr != nil { t.Fatalf("SaveConfig() error = %v", saveErr) }