fix(skills): register subcommands during command construction

- Move subcommand registration out of PersistentPreRunE
- Ensure `picoclaw skills <subcommand>` resolves correctly
- Minor install command and test cleanups

cmd/picoclaw/internal/skills/command.go

@@ -10,46 +10,70 @@ import (
 	"github.com/sipeed/picoclaw/pkg/skills"
 )
 
+type deps struct {
+	workspace    string
+	installer    *skills.SkillInstaller
+	skillsLoader *skills.SkillsLoader
+}
+
 func NewSkillsCommand() *cobra.Command {
+	var d deps
+
 	cmd := &cobra.Command{
 		Use:   "skills",
 		Short: "Manage skills",
-		RunE: func(cmd *cobra.Command, _ []string) error {
+		PersistentPreRunE: func(cmd *cobra.Command, _ []string) error {
-			return cmd.Help()
-		},
-	}
-
-	var loaded bool
-
-	cmd.PersistentPreRunE = func(cmd *cobra.Command, _ []string) error {
 			cfg, err := internal2.LoadConfig()
 			if err != nil {
 				return fmt.Errorf("error loading config: %w", err)
 			}
 
-		workspace := cfg.WorkspacePath()
+			d.workspace = cfg.WorkspacePath()
-		installer := skills.NewSkillInstaller(workspace)
+			d.installer = skills.NewSkillInstaller(d.workspace)
 
 			// get global config directory and builtin skills directory
 			globalDir := filepath.Dir(internal2.GetConfigPath())
 			globalSkillsDir := filepath.Join(globalDir, "skills")
 			builtinSkillsDir := filepath.Join(globalDir, "picoclaw", "skills")
-		skillsLoader := skills.NewSkillsLoader(workspace, globalSkillsDir, builtinSkillsDir)
+			d.skillsLoader = skills.NewSkillsLoader(d.workspace, globalSkillsDir, builtinSkillsDir)
 
-		if !loaded {
-			cmd.AddCommand(
-				newListCommand(skillsLoader),
-				newInstallCommand(installer),
-				newInstallBuiltinCommand(workspace),
-				newListBuiltinCommand(),
-				newRemoveCommand(installer),
-				newSearchCommand(installer),
-				newShowCommand(skillsLoader),
-			)
-			loaded = true
-		}
 			return nil
+		},
+		RunE: func(cmd *cobra.Command, _ []string) error {
+			return cmd.Help()
+		},
 	}
 
+	installerFn := func() (*skills.SkillInstaller, error) {
+		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")
+		}
+		return d.skillsLoader, nil
+	}
+
+	workspaceFn := func() (string, error) {
+		if d.workspace == "" {
+			return "", fmt.Errorf("workspace is not initialized")
+		}
+		return d.workspace, nil
+	}
+
+	cmd.AddCommand(
+		newListCommand(loaderFn),
+		newInstallCommand(installerFn),
+		newInstallBuiltinCommand(workspaceFn),
+		newListBuiltinCommand(),
+		newRemoveCommand(installerFn),
+		newSearchCommand(installerFn),
+		newShowCommand(loaderFn),
+	)
+
 	return cmd
 }

cmd/picoclaw/internal/skills/install.go

@@ -9,7 +9,7 @@ import (
 	"github.com/sipeed/picoclaw/pkg/skills"
 )
 
-func newInstallCommand(installer *skills.SkillInstaller) *cobra.Command {
+func newInstallCommand(installerFn func() (*skills.SkillInstaller, error)) *cobra.Command {
 	var registry string
 
 	cmd := &cobra.Command{
@@ -20,9 +20,7 @@ picoclaw skills install sipeed/picoclaw-skills/weather
 picoclaw skills install --registry clawhub github
 `,
 		Args: func(cmd *cobra.Command, args []string) error {
-			reg, _ := cmd.Flags().GetString("registry")
+			if registry != "" {
-
-			if reg != "" {
 				if len(args) != 2 {
 					return fmt.Errorf("when --registry is set, exactly 2 arguments are required: <name> <slug>")
 				}
@@ -36,6 +34,11 @@ picoclaw skills install --registry clawhub github
 			return nil
 		},
 		RunE: func(_ *cobra.Command, args []string) error {
+			installer, err := installerFn()
+			if err != nil {
+				return err
+			}
+
 			if registry != "" {
 				cfg, err := internal.LoadConfig()
 				if err != nil {
@@ -49,7 +52,7 @@ picoclaw skills install --registry clawhub github
 		},
 	}
 
-	cmd.Flags().StringVar(&registry, "registry", "", "--registry <name> <slug>")
+	cmd.Flags().StringVar(&registry, "registry", "", "Install from registry: --registry <name> <slug>")
 
 	return cmd
 }

cmd/picoclaw/internal/skills/installbuiltin.go

@@ -2,13 +2,18 @@ package skills
 
 import "github.com/spf13/cobra"
 
-func newInstallBuiltinCommand(workspace string) *cobra.Command {
+func newInstallBuiltinCommand(workspaceFn func() (string, error)) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:     "install-builtin",
 		Short:   "Install all builtin skills to workspace",
 		Example: `picoclaw skills install-builtin`,
-		Run: func(_ *cobra.Command, _ []string) {
+		RunE: func(_ *cobra.Command, _ []string) error {
+			workspace, err := workspaceFn()
+			if err != nil {
+				return err
+			}
 			skillsInstallBuiltinCmd(workspace)
+			return nil
 		},
 	}
 

cmd/picoclaw/internal/skills/installbuiltin_test.go

@@ -8,14 +8,15 @@ import (
 )
 
 func TestNewInstallbuiltinSubcommand(t *testing.T) {
-	cmd := newInstallBuiltinCommand("")
+	cmd := newInstallBuiltinCommand(nil)
 
 	require.NotNil(t, cmd)
 
 	assert.Equal(t, "install-builtin", cmd.Use)
 	assert.Equal(t, "Install all builtin skills to workspace", cmd.Short)
 
-	assert.NotNil(t, cmd.Run)
+	assert.Nil(t, cmd.Run)
+	assert.NotNil(t, cmd.RunE)
 
 	assert.True(t, cmd.HasExample())
 	assert.False(t, cmd.HasSubCommands())

cmd/picoclaw/internal/skills/list.go

@@ -6,13 +6,18 @@ import (
 	"github.com/sipeed/picoclaw/pkg/skills"
 )
 
-func newListCommand(skillsLoader *skills.SkillsLoader) *cobra.Command {
+func newListCommand(loaderFn func() (*skills.SkillsLoader, error)) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:     "list",
 		Short:   "List installed skills",
 		Example: `picoclaw skills list`,
-		Run: func(_ *cobra.Command, _ []string) {
+		RunE: func(_ *cobra.Command, _ []string) error {
-			skillsListCmd(skillsLoader)
+			loader, err := loaderFn()
+			if err != nil {
+				return err
+			}
+			skillsListCmd(loader)
+			return nil
 		},
 	}
 

cmd/picoclaw/internal/skills/list_test.go

@@ -15,7 +15,8 @@ func TestNewListSubcommand(t *testing.T) {
 	assert.Equal(t, "list", cmd.Use)
 	assert.Equal(t, "List installed skills", cmd.Short)
 
-	assert.NotNil(t, cmd.Run)
+	assert.Nil(t, cmd.Run)
+	assert.NotNil(t, cmd.RunE)
 
 	assert.True(t, cmd.HasExample())
 	assert.False(t, cmd.HasSubCommands())

cmd/picoclaw/internal/skills/remove.go

@@ -6,15 +6,20 @@ import (
 	"github.com/sipeed/picoclaw/pkg/skills"
 )
 
-func newRemoveCommand(installer *skills.SkillInstaller) *cobra.Command {
+func newRemoveCommand(installerFn func() (*skills.SkillInstaller, error)) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:     "remove",
 		Aliases: []string{"rm", "uninstall"},
 		Short:   "Remove installed skill",
 		Args:    cobra.ExactArgs(1),
 		Example: `picoclaw skills remove weather`,
-		Run: func(_ *cobra.Command, args []string) {
+		RunE: func(_ *cobra.Command, args []string) error {
+			installer, err := installerFn()
+			if err != nil {
+				return err
+			}
 			skillsRemoveCmd(installer, args[0])
+			return nil
 		},
 	}
 

cmd/picoclaw/internal/skills/remove_test.go

@@ -15,7 +15,8 @@ func TestNewRemoveSubcommand(t *testing.T) {
 	assert.Equal(t, "remove", cmd.Use)
 	assert.Equal(t, "Remove installed skill", cmd.Short)
 
-	assert.NotNil(t, cmd.Run)
+	assert.Nil(t, cmd.Run)
+	assert.NotNil(t, cmd.RunE)
 
 	assert.True(t, cmd.HasExample())
 	assert.False(t, cmd.HasSubCommands())

cmd/picoclaw/internal/skills/search.go

@@ -6,12 +6,17 @@ import (
 	"github.com/sipeed/picoclaw/pkg/skills"
 )
 
-func newSearchCommand(installer *skills.SkillInstaller) *cobra.Command {
+func newSearchCommand(installerFn func() (*skills.SkillInstaller, error)) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "search",
 		Short: "Search available skills",
-		Run: func(_ *cobra.Command, _ []string) {
+		RunE: func(_ *cobra.Command, _ []string) error {
+			installer, err := installerFn()
+			if err != nil {
+				return err
+			}
 			skillsSearchCmd(installer)
+			return nil
 		},
 	}
 

cmd/picoclaw/internal/skills/search_test.go

@@ -15,7 +15,8 @@ func TestNewSearchSubcommand(t *testing.T) {
 	assert.Equal(t, "search", cmd.Use)
 	assert.Equal(t, "Search available skills", cmd.Short)
 
-	assert.NotNil(t, cmd.Run)
+	assert.Nil(t, cmd.Run)
+	assert.NotNil(t, cmd.RunE)
 
 	assert.False(t, cmd.HasSubCommands())
 	assert.False(t, cmd.HasFlags())

cmd/picoclaw/internal/skills/show.go

@@ -6,14 +6,19 @@ import (
 	"github.com/sipeed/picoclaw/pkg/skills"
 )
 
-func newShowCommand(skillsLoader *skills.SkillsLoader) *cobra.Command {
+func newShowCommand(loaderFn func() (*skills.SkillsLoader, error)) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:     "show",
 		Short:   "Show skill details",
 		Args:    cobra.ExactArgs(1),
 		Example: `picoclaw skills show weather`,
-		Run: func(_ *cobra.Command, args []string) {
+		RunE: func(_ *cobra.Command, args []string) error {
-			skillsShowCmd(skillsLoader, args[0])
+			loader, err := loaderFn()
+			if err != nil {
+				return err
+			}
+			skillsShowCmd(loader, args[0])
+			return nil
 		},
 	}
 

cmd/picoclaw/internal/skills/show_test.go

@@ -15,7 +15,8 @@ func TestNewShowSubcommand(t *testing.T) {
 	assert.Equal(t, "show", cmd.Use)
 	assert.Equal(t, "Show skill details", cmd.Short)
 
-	assert.NotNil(t, cmd.Run)
+	assert.Nil(t, cmd.Run)
+	assert.NotNil(t, cmd.RunE)
 
 	assert.True(t, cmd.HasExample())
 	assert.False(t, cmd.HasSubCommands())