From 9d0954b5c5eb2aaa0702efc44adfdbd96c34f939 Mon Sep 17 00:00:00 2001 From: lxowalle Date: Thu, 9 Apr 2026 20:41:40 +0800 Subject: [PATCH] fix command error --- cmd/picoclaw/internal/skills/command.go | 17 +---------------- cmd/picoclaw/internal/skills/helpers.go | 25 +++++++++++++++++-------- cmd/picoclaw/internal/skills/remove.go | 9 +++------ pkg/skills/github_registry.go | 9 +++++++++ pkg/skills/github_registry_test.go | 18 ++++++++++++++++++ 5 files changed, 48 insertions(+), 30 deletions(-) diff --git a/cmd/picoclaw/internal/skills/command.go b/cmd/picoclaw/internal/skills/command.go index b62a00351..678031c0a 100644 --- a/cmd/picoclaw/internal/skills/command.go +++ b/cmd/picoclaw/internal/skills/command.go @@ -12,7 +12,6 @@ import ( type deps struct { workspace string - installer *skills.SkillInstaller skillsLoader *skills.SkillsLoader } @@ -43,20 +42,6 @@ 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") - } - return d.installer, nil - } - loaderFn := func() (*skills.SkillsLoader, error) { if d.skillsLoader == nil { return nil, fmt.Errorf("skills loader is not initialized") @@ -76,7 +61,7 @@ func NewSkillsCommand() *cobra.Command { newInstallCommand(), newInstallBuiltinCommand(workspaceFn), newListBuiltinCommand(), - newRemoveCommand(installerFn), + newRemoveCommand(workspaceFn), newSearchCommand(), newShowCommand(loaderFn), ) diff --git a/cmd/picoclaw/internal/skills/helpers.go b/cmd/picoclaw/internal/skills/helpers.go index c359d40ca..d115bd384 100644 --- a/cmd/picoclaw/internal/skills/helpers.go +++ b/cmd/picoclaw/internal/skills/helpers.go @@ -100,15 +100,24 @@ func skillsInstallFromRegistry(cfg *config.Config, registryName, target string) return nil } -func skillsRemoveCmd(installer *skills.SkillInstaller, skillName string) { - fmt.Printf("Removing skill '%s'...\n", skillName) - - if err := installer.Uninstall(skillName); err != nil { - fmt.Printf("✗ Failed to remove skill: %v\n", err) - os.Exit(1) +func skillsRemoveFromWorkspace(workspace, skillName string) error { + name := strings.TrimSpace(skillName) + name = strings.Trim(name, "/") + if name == "" { + return fmt.Errorf("skill name is required") } - - fmt.Printf("✓ Skill '%s' removed successfully!\n", skillName) + if strings.Contains(name, "/") { + parts := strings.Split(name, "/") + name = parts[len(parts)-1] + } + skillDir := filepath.Join(workspace, "skills", name) + if _, err := os.Stat(skillDir); os.IsNotExist(err) { + return fmt.Errorf("skill '%s' not found", name) + } + if err := os.RemoveAll(skillDir); err != nil { + return fmt.Errorf("failed to remove skill '%s': %w", name, err) + } + return nil } func skillsInstallBuiltinCmd(workspace string) { diff --git a/cmd/picoclaw/internal/skills/remove.go b/cmd/picoclaw/internal/skills/remove.go index cd7d3a8b4..2b30e94a2 100644 --- a/cmd/picoclaw/internal/skills/remove.go +++ b/cmd/picoclaw/internal/skills/remove.go @@ -2,11 +2,9 @@ package skills import ( "github.com/spf13/cobra" - - "github.com/sipeed/picoclaw/pkg/skills" ) -func newRemoveCommand(installerFn func() (*skills.SkillInstaller, error)) *cobra.Command { +func newRemoveCommand(workspaceFn func() (string, error)) *cobra.Command { cmd := &cobra.Command{ Use: "remove", Aliases: []string{"rm", "uninstall"}, @@ -14,12 +12,11 @@ func newRemoveCommand(installerFn func() (*skills.SkillInstaller, error)) *cobra Args: cobra.ExactArgs(1), Example: `picoclaw skills remove weather`, RunE: func(_ *cobra.Command, args []string) error { - installer, err := installerFn() + workspace, err := workspaceFn() if err != nil { return err } - skillsRemoveCmd(installer, args[0]) - return nil + return skillsRemoveFromWorkspace(workspace, args[0]) }, } diff --git a/pkg/skills/github_registry.go b/pkg/skills/github_registry.go index 7274e59fa..6ff865910 100644 --- a/pkg/skills/github_registry.go +++ b/pkg/skills/github_registry.go @@ -140,6 +140,9 @@ func (r *GitHubRegistry) Search(ctx context.Context, query string, limit int) ([ if err != nil { return nil, fmt.Errorf("failed to read github search response: %w", err) } + if resp.StatusCode == http.StatusUnauthorized && r.installer.githubToken == "" && isGitHubAuthRequiredError(body) { + return nil, nil + } if resp.StatusCode == http.StatusForbidden && r.installer.githubToken == "" && isGitHubRateLimitError(body) { return nil, nil } @@ -193,6 +196,12 @@ func isGitHubRateLimitError(body []byte) bool { return strings.Contains(message, "rate limit exceeded") } +func isGitHubAuthRequiredError(body []byte) bool { + message := strings.ToLower(string(body)) + return strings.Contains(message, "requires authentication") || + strings.Contains(message, "must be authenticated to access the code search api") +} + func githubSearchSlug(item gitHubCodeSearchItem) (string, bool) { fullName := strings.TrimSpace(item.Repository.FullName) if fullName == "" { diff --git a/pkg/skills/github_registry_test.go b/pkg/skills/github_registry_test.go index b82b23976..911b4ac4c 100644 --- a/pkg/skills/github_registry_test.go +++ b/pkg/skills/github_registry_test.go @@ -118,3 +118,21 @@ func TestGitHubRegistrySearchReturnsEmptyOnUnauthenticatedRateLimit(t *testing.T require.NoError(t, err) assert.Empty(t, results) } + +func TestGitHubRegistrySearchReturnsEmptyOnUnauthenticatedAuthRequired(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) + _, _ = w.Write([]byte( + `{"message":"Requires authentication","errors":[{"message":"Must be authenticated to access the code search API"}]}`, + )) + })) + 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) +}