From 9a65c703f65e111852f72c654c41794dca591f26 Mon Sep 17 00:00:00 2001
From: Ruslan Semagin <pixel.365.24@gmail.com>
Date: Tue, 24 Feb 2026 14:11:10 +0300
Subject: [PATCH] refactor(cli): address review feedback and improve command
 clarity

---
 cmd/picoclaw/internal/agent/command.go  |  5 -----
 cmd/picoclaw/internal/auth/helpers.go   | 22 +++++++++++-----------
 cmd/picoclaw/internal/cron/add.go       | 13 +------------
 cmd/picoclaw/internal/cron/add_test.go  | 14 ++++++++++++++
 cmd/picoclaw/internal/cron/command.go   |  2 ++
 cmd/picoclaw/internal/cron/disable.go   |  2 +-
 cmd/picoclaw/internal/cron/enable.go    |  2 +-
 cmd/picoclaw/internal/cron/helpers.go   | 10 ++--------
 cmd/picoclaw/internal/helpers.go        |  5 +++++
 cmd/picoclaw/internal/helpers_test.go   |  4 ++++
 cmd/picoclaw/internal/skills/command.go |  6 +++---
 cmd/picoclaw/main.go                    |  6 +++++-
 cmd/picoclaw/main_test.go               |  7 ++++++-
 13 files changed, 55 insertions(+), 43 deletions(-)

diff --git a/cmd/picoclaw/internal/agent/command.go b/cmd/picoclaw/internal/agent/command.go
index b9c06716a..47262fc85 100644
--- a/cmd/picoclaw/internal/agent/command.go
+++ b/cmd/picoclaw/internal/agent/command.go
@@ -17,11 +17,6 @@ func NewAgentCommand() *cobra.Command {
 		Short: "Interact with the agent directly",
 		Args:  cobra.NoArgs,
 		RunE: func(cmd *cobra.Command, _ []string) error {
-			debug, _ = cmd.Flags().GetBool("debug")
-			message, _ = cmd.Flags().GetString("message")
-			sessionKey, _ = cmd.Flags().GetString("session")
-			model, _ = cmd.Flags().GetString("model")
-
 			return agentCmd(message, sessionKey, model, debug)
 		},
 	}
diff --git a/cmd/picoclaw/internal/auth/helpers.go b/cmd/picoclaw/internal/auth/helpers.go
index 84e7319b3..5533d6857 100644
--- a/cmd/picoclaw/internal/auth/helpers.go
+++ b/cmd/picoclaw/internal/auth/helpers.go
@@ -24,12 +24,10 @@ func authLoginCmd(provider string, useDeviceCode bool) error {
 	case "anthropic":
 		return authLoginPasteToken(provider)
 	case "google-antigravity", "antigravity":
-		authLoginGoogleAntigravity()
+		return authLoginGoogleAntigravity()
 	default:
 		return fmt.Errorf("unsupported provider: %s (%s)", provider, supportedProvidersMsg)
 	}
-
-	return nil
 }
 
 func authLoginOpenAI(useDeviceCode bool) error {
@@ -80,7 +78,7 @@ func authLoginOpenAI(useDeviceCode bool) error {
 		appCfg.Agents.Defaults.Model = "gpt-5.2"
 
 		if err = config.SaveConfig(internal.GetConfigPath(), appCfg); err != nil {
-			return fmt.Errorf("warning: could not update config: %w", err)
+			return fmt.Errorf("could not update config: %w", err)
 		}
 	}
 
@@ -93,13 +91,12 @@ func authLoginOpenAI(useDeviceCode bool) error {
 	return nil
 }
 
-func authLoginGoogleAntigravity() {
+func authLoginGoogleAntigravity() error {
 	cfg := auth.GoogleAntigravityOAuthConfig()
 
 	cred, err := auth.LoginBrowser(cfg)
 	if err != nil {
-		fmt.Printf("Login failed: %v\n", err)
-		os.Exit(1)
+		return fmt.Errorf("login failed: %w", err)
 	}
 
 	cred.Provider = "google-antigravity"
@@ -124,8 +121,7 @@ func authLoginGoogleAntigravity() {
 	}
 
 	if err = auth.SetCredential("google-antigravity", cred); err != nil {
-		fmt.Printf("Failed to save credentials: %v\n", err)
-		os.Exit(1)
+		return fmt.Errorf("failed to save credentials: %w", err)
 	}
 
 	appCfg, err := internal.LoadConfig()
@@ -163,6 +159,8 @@ func authLoginGoogleAntigravity() {
 	fmt.Println("\n✓ Google Antigravity login successful!")
 	fmt.Println("Default model set to: gemini-flash")
 	fmt.Println("Try it: picoclaw agent -m \"Hello world\"")
+
+	return nil
 }
 
 func fetchGoogleUserEmail(accessToken string) (string, error) {
@@ -248,7 +246,7 @@ func authLoginPasteToken(provider string) error {
 			appCfg.Agents.Defaults.Model = "gpt-5.2"
 		}
 		if err := config.SaveConfig(internal.GetConfigPath(), appCfg); err != nil {
-			return fmt.Errorf("warning: could not update config: %w", err)
+			return fmt.Errorf("could not update config: %w", err)
 		}
 	}
 
@@ -330,7 +328,9 @@ func authStatusCmd() error {
 	}
 
 	if len(store.Credentials) == 0 {
-		return fmt.Errorf("no authenticated providers. run: picoclaw auth login --provider <name>")
+		fmt.Println("No authenticated providers.")
+		fmt.Println("Run: picoclaw auth login --provider <name>")
+		return nil
 	}
 
 	fmt.Println("\nAuthenticated Providers:")
diff --git a/cmd/picoclaw/internal/cron/add.go b/cmd/picoclaw/internal/cron/add.go
index e82865754..947557d5a 100644
--- a/cmd/picoclaw/internal/cron/add.go
+++ b/cmd/picoclaw/internal/cron/add.go
@@ -24,22 +24,10 @@ func newAddCommand(storePath func() string) *cobra.Command {
 		Short: "Add a new scheduled job",
 		Args:  cobra.NoArgs,
 		RunE: func(cmd *cobra.Command, _ []string) error {
-			if name == "" {
-				return fmt.Errorf("--name is required")
-			}
-
-			if message == "" {
-				return fmt.Errorf("--message is required")
-			}
-
 			if every <= 0 && cronExp == "" {
 				return fmt.Errorf("either --every or --cron must be specified")
 			}
 
-			if every > 0 && cronExp != "" {
-				return fmt.Errorf("--every and --cron are mutually exclusive")
-			}
-
 			var schedule cron.CronSchedule
 			if every > 0 {
 				everyMS := every * 1000
@@ -70,6 +58,7 @@ func newAddCommand(storePath func() string) *cobra.Command {
 
 	_ = cmd.MarkFlagRequired("name")
 	_ = cmd.MarkFlagRequired("message")
+	cmd.MarkFlagsMutuallyExclusive("every", "cron")
 
 	return cmd
 }
diff --git a/cmd/picoclaw/internal/cron/add_test.go b/cmd/picoclaw/internal/cron/add_test.go
index e190bb217..09701fab5 100644
--- a/cmd/picoclaw/internal/cron/add_test.go
+++ b/cmd/picoclaw/internal/cron/add_test.go
@@ -41,3 +41,17 @@ func TestNewAddSubcommand(t *testing.T) {
 	require.NotEmpty(t, val)
 	assert.Equal(t, "true", val[0])
 }
+
+func TestNewAddCommandEveryAndCronMutuallyExclusive(t *testing.T) {
+	cmd := newAddCommand(func() string { return "testing" })
+
+	cmd.SetArgs([]string{
+		"--name", "job",
+		"--message", "hello",
+		"--every", "10",
+		"--cron", "0 9 * * *",
+	})
+
+	err := cmd.Execute()
+	require.Error(t, err)
+}
diff --git a/cmd/picoclaw/internal/cron/command.go b/cmd/picoclaw/internal/cron/command.go
index 09b7c0a77..39f8ccf28 100644
--- a/cmd/picoclaw/internal/cron/command.go
+++ b/cmd/picoclaw/internal/cron/command.go
@@ -20,6 +20,8 @@ func NewCronCommand() *cobra.Command {
 		RunE: func(cmd *cobra.Command, _ []string) error {
 			return cmd.Help()
 		},
+		// Resolve storePath at execution time so it reflects the current config
+		// and is shared across all subcommands.
 		PersistentPreRunE: func(_ *cobra.Command, _ []string) error {
 			cfg, err := internal.LoadConfig()
 			if err != nil {
diff --git a/cmd/picoclaw/internal/cron/disable.go b/cmd/picoclaw/internal/cron/disable.go
index 047771ed6..a3670fd50 100644
--- a/cmd/picoclaw/internal/cron/disable.go
+++ b/cmd/picoclaw/internal/cron/disable.go
@@ -9,7 +9,7 @@ func newDisableCommand(storePath func() string) *cobra.Command {
 		Args:    cobra.ExactArgs(1),
 		Example: `picoclaw cron disable 1`,
 		RunE: func(_ *cobra.Command, args []string) error {
-			cronEnableCmd(storePath(), true, args[0])
+			cronSetJobEnabled(storePath(), args[0], false)
 			return nil
 		},
 	}
diff --git a/cmd/picoclaw/internal/cron/enable.go b/cmd/picoclaw/internal/cron/enable.go
index 5a61d58f3..7f8b05233 100644
--- a/cmd/picoclaw/internal/cron/enable.go
+++ b/cmd/picoclaw/internal/cron/enable.go
@@ -9,7 +9,7 @@ func newEnableCommand(storePath func() string) *cobra.Command {
 		Args:    cobra.ExactArgs(1),
 		Example: `picoclaw cron enable 1`,
 		RunE: func(_ *cobra.Command, args []string) error {
-			cronEnableCmd(storePath(), false, args[0])
+			cronSetJobEnabled(storePath(), args[0], true)
 			return nil
 		},
 	}
diff --git a/cmd/picoclaw/internal/cron/helpers.go b/cmd/picoclaw/internal/cron/helpers.go
index 6a293e69e..88bdf1bf7 100644
--- a/cmd/picoclaw/internal/cron/helpers.go
+++ b/cmd/picoclaw/internal/cron/helpers.go
@@ -55,17 +55,11 @@ func cronRemoveCmd(storePath, jobID string) {
 	}
 }
 
-func cronEnableCmd(storePath string, disable bool, jobID string) {
+func cronSetJobEnabled(storePath, jobID string, enabled bool) {
 	cs := cron.NewCronService(storePath, nil)
-	enabled := !disable
-
 	job := cs.EnableJob(jobID, enabled)
 	if job != nil {
-		status := "enabled"
-		if disable {
-			status = "disabled"
-		}
-		fmt.Printf("✓ Job '%s' %s\n", job.Name, status)
+		fmt.Printf("✓ Job '%s' enabled\n", job.Name)
 	} else {
 		fmt.Printf("✗ Job %s not found\n", jobID)
 	}
diff --git a/cmd/picoclaw/internal/helpers.go b/cmd/picoclaw/internal/helpers.go
index 4be249d86..a084dc1be 100644
--- a/cmd/picoclaw/internal/helpers.go
+++ b/cmd/picoclaw/internal/helpers.go
@@ -47,3 +47,8 @@ func FormatBuildInfo() (build string, goVer string) {
 	}
 	return
 }
+
+// GetVersion returns the version string
+func GetVersion() string {
+	return version
+}
diff --git a/cmd/picoclaw/internal/helpers_test.go b/cmd/picoclaw/internal/helpers_test.go
index e24558c2a..9342d141d 100644
--- a/cmd/picoclaw/internal/helpers_test.go
+++ b/cmd/picoclaw/internal/helpers_test.go
@@ -91,3 +91,7 @@ func TestGetConfigPath_Windows(t *testing.T) {
 
 	require.True(t, strings.EqualFold(got, want), "GetConfigPath() = %q, want %q", got, want)
 }
+
+func TestGetVersion(t *testing.T) {
+	assert.Equal(t, "dev", GetVersion())
+}
diff --git a/cmd/picoclaw/internal/skills/command.go b/cmd/picoclaw/internal/skills/command.go
index 53a418165..7f8bd011d 100644
--- a/cmd/picoclaw/internal/skills/command.go
+++ b/cmd/picoclaw/internal/skills/command.go
@@ -6,7 +6,7 @@ import (
 
 	"github.com/spf13/cobra"
 
-	internal2 "github.com/sipeed/picoclaw/cmd/picoclaw/internal"
+	"github.com/sipeed/picoclaw/cmd/picoclaw/internal"
 	"github.com/sipeed/picoclaw/pkg/skills"
 )
 
@@ -23,7 +23,7 @@ func NewSkillsCommand() *cobra.Command {
 		Use:   "skills",
 		Short: "Manage skills",
 		PersistentPreRunE: func(cmd *cobra.Command, _ []string) error {
-			cfg, err := internal2.LoadConfig()
+			cfg, err := internal.LoadConfig()
 			if err != nil {
 				return fmt.Errorf("error loading config: %w", err)
 			}
@@ -32,7 +32,7 @@ func NewSkillsCommand() *cobra.Command {
 			d.installer = skills.NewSkillInstaller(d.workspace)
 
 			// get global config directory and builtin skills directory
-			globalDir := filepath.Dir(internal2.GetConfigPath())
+			globalDir := filepath.Dir(internal.GetConfigPath())
 			globalSkillsDir := filepath.Join(globalDir, "skills")
 			builtinSkillsDir := filepath.Join(globalDir, "picoclaw", "skills")
 			d.skillsLoader = skills.NewSkillsLoader(d.workspace, globalSkillsDir, builtinSkillsDir)
diff --git a/cmd/picoclaw/main.go b/cmd/picoclaw/main.go
index d8eb09426..6db69c990 100644
--- a/cmd/picoclaw/main.go
+++ b/cmd/picoclaw/main.go
@@ -7,10 +7,12 @@
 package main
 
 import (
+	"fmt"
 	"os"
 
 	"github.com/spf13/cobra"
 
+	"github.com/sipeed/picoclaw/cmd/picoclaw/internal"
 	"github.com/sipeed/picoclaw/cmd/picoclaw/internal/agent"
 	"github.com/sipeed/picoclaw/cmd/picoclaw/internal/auth"
 	"github.com/sipeed/picoclaw/cmd/picoclaw/internal/cron"
@@ -23,9 +25,11 @@ import (
 )
 
 func NewPicoclawCommand() *cobra.Command {
+	short := fmt.Sprintf("%s picoclaw - Personal AI Assistant v%s\n\n", internal.Logo, internal.GetVersion())
+
 	cmd := &cobra.Command{
 		Use:     "picoclaw",
-		Short:   "picoclaw — Personal AI Assistant",
+		Short:   short,
 		Example: "picoclaw list",
 	}
 
diff --git a/cmd/picoclaw/main_test.go b/cmd/picoclaw/main_test.go
index 875ad8009..3740ba358 100644
--- a/cmd/picoclaw/main_test.go
+++ b/cmd/picoclaw/main_test.go
@@ -1,11 +1,14 @@
 package main
 
 import (
+	"fmt"
 	"slices"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+
+	"github.com/sipeed/picoclaw/cmd/picoclaw/internal"
 )
 
 func TestNewPicoclawCommand(t *testing.T) {
@@ -13,8 +16,10 @@ func TestNewPicoclawCommand(t *testing.T) {
 
 	require.NotNil(t, cmd)
 
+	short := fmt.Sprintf("%s picoclaw - Personal AI Assistant v%s\n\n", internal.Logo, internal.GetVersion())
+
 	assert.Equal(t, "picoclaw", cmd.Use)
-	assert.Equal(t, "picoclaw — Personal AI Assistant", cmd.Short)
+	assert.Equal(t, short, cmd.Short)
 
 	assert.True(t, cmd.HasSubCommands())
 	assert.True(t, cmd.HasAvailableSubCommands())