refactor: module command system handling

- Create `pkg/command` package to host command interfaces and registry.
- Implement `Command` and `Registry` interfaces for extensible command parsing.
- Move basic commands (`/show`, `/list`, `/switch`) and add `/start`, `/help` to `pkg/command/basic.go`.
- Update `AgentLoop` to use `command.Registry` instead of hardcoded switch-case logic.
- Inject `ChannelManager` into `AgentLoop` to support channel-related commands.
- Resolve code duplication in `pkg/agent/loop.go`.
- Refactor Telegram channel to use the generic command system, removing duplicated command handling logic from `pkg/channels/telegram.go` and deleting `pkg/channels/telegram_commands.go`.

pkg/agent/loop.go

@@ -20,6 +20,7 @@ import (
 
 	"github.com/sipeed/picoclaw/pkg/bus"
 	"github.com/sipeed/picoclaw/pkg/channels"
+	"github.com/sipeed/picoclaw/pkg/command"
 	"github.com/sipeed/picoclaw/pkg/config"
 	"github.com/sipeed/picoclaw/pkg/constants"
 	"github.com/sipeed/picoclaw/pkg/logger"
@@ -44,6 +45,7 @@ type AgentLoop struct {
 	running         atomic.Bool
 	summarizing     sync.Map // Tracks which sessions are currently being summarized
 	channelManager  *channels.Manager
+	commandRegistry *command.Registry
 }
 
 // processOptions configures how a message is processed
@@ -136,6 +138,14 @@ func NewAgentLoop(cfg *config.Config, msgBus *bus.MessageBus, provider providers
 	contextBuilder := NewContextBuilder(workspace)
 	contextBuilder.SetToolsRegistry(toolsRegistry)
 
+	// Create command registry
+	cmdRegistry := command.NewRegistry()
+	cmdRegistry.Register(&command.ShowCommand{})
+	cmdRegistry.Register(&command.ListCommand{})
+	cmdRegistry.Register(&command.SwitchCommand{})
+	cmdRegistry.Register(&command.StartCommand{})
+	cmdRegistry.Register(&command.HelpCommand{Registry: cmdRegistry})
+
 	return &AgentLoop{
 		bus:             msgBus,
 		provider:        provider,
@@ -148,6 +158,7 @@ func NewAgentLoop(cfg *config.Config, msgBus *bus.MessageBus, provider providers
 		contextBuilder:  contextBuilder,
 		tools:           toolsRegistry,
 		summarizing:     sync.Map{},
+		commandRegistry: cmdRegistry,
 	}
 }
 
@@ -205,6 +216,19 @@ func (al *AgentLoop) SetChannelManager(cm *channels.Manager) {
 	al.channelManager = cm
 }
 
+// Implement AgentState interface
+func (al *AgentLoop) GetModel() string {
+	return al.model
+}
+
+func (al *AgentLoop) SetModel(model string) {
+	al.model = model
+}
+
+func (al *AgentLoop) GetChannelManager() interface{} {
+	return al.channelManager
+}
+
 // RecordLastChannel records the last active channel for this workspace.
 // This uses the atomic state save mechanism to prevent data loss on crash.
 func (al *AgentLoop) RecordLastChannel(channel string) error {
@@ -980,167 +1004,14 @@ func (al *AgentLoop) estimateTokens(messages []providers.Message) int {
 }
 
 func (al *AgentLoop) handleCommand(ctx context.Context, msg bus.InboundMessage) (string, bool) {
-	content := strings.TrimSpace(msg.Content)
+	name, args, ok := al.commandRegistry.Parse(msg.Content)
-	if !strings.HasPrefix(content, "/") {
+	if !ok {
 		return "", false
 	}
 
-	parts := strings.Fields(content)
+	response, handled, err := al.commandRegistry.Execute(ctx, al, name, args, msg)
-	if len(parts) == 0 {
+	if err != nil {
-		return "", false
+		return fmt.Sprintf("Error executing command: %v", err), true
 	}
-
+	return response, handled
-	cmd := parts[0]
-	args := parts[1:]
-
-	switch cmd {
-	case "/show":
-		if len(args) < 1 {
-			return "Usage: /show [model|channel]", true
-		}
-		switch args[0] {
-		case "model":
-			return fmt.Sprintf("Current model: %s", al.model), true
-		case "channel":
-			return fmt.Sprintf("Current channel: %s", msg.Channel), true
-		default:
-			return fmt.Sprintf("Unknown show target: %s", args[0]), true
-		}
-
-	case "/list":
-		if len(args) < 1 {
-			return "Usage: /list [models|channels]", true
-		}
-		switch args[0] {
-		case "models":
-			// TODO: Fetch available models dynamically if possible
-			return "Available models: glm-4.7, claude-3-5-sonnet, gpt-4o (configured in config.json/env)", true
-		case "channels":
-			if al.channelManager == nil {
-				return "Channel manager not initialized", true
-			}
-			channels := al.channelManager.GetEnabledChannels()
-			if len(channels) == 0 {
-				return "No channels enabled", true
-			}
-			return fmt.Sprintf("Enabled channels: %s", strings.Join(channels, ", ")), true
-		default:
-			return fmt.Sprintf("Unknown list target: %s", args[0]), true
-		}
-
-	case "/switch":
-		if len(args) < 3 || args[1] != "to" {
-			return "Usage: /switch [model|channel] to <name>", true
-		}
-		target := args[0]
-		value := args[2]
-
-		switch target {
-		case "model":
-			oldModel := al.model
-			al.model = value
-			return fmt.Sprintf("Switched model from %s to %s", oldModel, value), true
-		case "channel":
-			// This changes the 'default' channel for some operations, or effectively redirects output?
-			// For now, let's just validate if the channel exists
-			if al.channelManager == nil {
-				return "Channel manager not initialized", true
-			}
-			if _, exists := al.channelManager.GetChannel(value); !exists && value != "cli" {
-				return fmt.Sprintf("Channel '%s' not found or not enabled", value), true
-			}
-
-			// If message came from CLI, maybe we want to redirect CLI output to this channel?
-			// That would require state persistence about "redirected channel"
-			// For now, just acknowledged.
-			return fmt.Sprintf("Switched target channel to %s (Note: this currently only validates existence)", value), true
-		default:
-			return fmt.Sprintf("Unknown switch target: %s", target), true
-		}
-	}
-
-	return "", false
-}
-
-func (al *AgentLoop) handleCommand(ctx context.Context, msg bus.InboundMessage) (string, bool) {
-	content := strings.TrimSpace(msg.Content)
-	if !strings.HasPrefix(content, "/") {
-		return "", false
-	}
-
-	parts := strings.Fields(content)
-	if len(parts) == 0 {
-		return "", false
-	}
-
-	cmd := parts[0]
-	args := parts[1:]
-
-	switch cmd {
-	case "/show":
-		if len(args) < 1 {
-			return "Usage: /show [model|channel]", true
-		}
-		switch args[0] {
-		case "model":
-			return fmt.Sprintf("Current model: %s", al.model), true
-		case "channel":
-			return fmt.Sprintf("Current channel: %s", msg.Channel), true
-		default:
-			return fmt.Sprintf("Unknown show target: %s", args[0]), true
-		}
-
-	case "/list":
-		if len(args) < 1 {
-			return "Usage: /list [models|channels]", true
-		}
-		switch args[0] {
-		case "models":
-			// TODO: Fetch available models dynamically if possible
-			return "Available models: glm-4.7, claude-3-5-sonnet, gpt-4o (configured in config.json/env)", true
-		case "channels":
-			if al.channelManager == nil {
-				return "Channel manager not initialized", true
-			}
-			channels := al.channelManager.GetEnabledChannels()
-			if len(channels) == 0 {
-				return "No channels enabled", true
-			}
-			return fmt.Sprintf("Enabled channels: %s", strings.Join(channels, ", ")), true
-		default:
-			return fmt.Sprintf("Unknown list target: %s", args[0]), true
-		}
-
-	case "/switch":
-		if len(args) < 3 || args[1] != "to" {
-			return "Usage: /switch [model|channel] to <name>", true
-		}
-		target := args[0]
-		value := args[2]
-
-		switch target {
-		case "model":
-			oldModel := al.model
-			al.model = value
-			return fmt.Sprintf("Switched model from %s to %s", oldModel, value), true
-		case "channel":
-			// This changes the 'default' channel for some operations, or effectively redirects output?
-			// For now, let's just validate if the channel exists
-			if al.channelManager == nil {
-				return "Channel manager not initialized", true
-			}
-			if _, exists := al.channelManager.GetChannel(value); !exists && value != "cli" {
-				return fmt.Sprintf("Channel '%s' not found or not enabled", value), true
-			}
-
-			// If message came from CLI, maybe we want to redirect CLI output to this channel?
-			// That would require state persistence about "redirected channel"
-			// For now, just acknowledged.
-			return fmt.Sprintf("Switched target channel to %s (Note: this currently only validates existence)", value), true
-		default:
-			return fmt.Sprintf("Unknown switch target: %s", target), true
-		}
-	}
-
-	return "", false
 }

pkg/channels/telegram.go

@@ -27,7 +27,6 @@ import (
 type TelegramChannel struct {
 	*BaseChannel
 	bot          *telego.Bot
-	commands     TelegramCommander
 	config       *config.Config
 	chatIDs      map[string]int64
 	transcriber  *voice.GroqTranscriber
@@ -70,7 +69,6 @@ func NewTelegramChannel(cfg *config.Config, bus *bus.MessageBus) (*TelegramChann
 
 	return &TelegramChannel{
 		BaseChannel:  base,
-		commands:     NewTelegramCommands(bot, cfg),
 		bot:          bot,
 		config:       cfg,
 		chatIDs:      make(map[string]int64),
@@ -99,22 +97,6 @@ func (c *TelegramChannel) Start(ctx context.Context) error {
 		return fmt.Errorf("failed to create bot handler: %w", err)
 	}
 
-	bh.HandleMessage(func(ctx *th.Context, message telego.Message) error {
-		c.commands.Help(ctx, message)
-		return nil
-	}, th.CommandEqual("help"))
-	bh.HandleMessage(func(ctx *th.Context, message telego.Message) error {
-		return c.commands.Start(ctx, message)
-	}, th.CommandEqual("start"))
-
-	bh.HandleMessage(func(ctx *th.Context, message telego.Message) error {
-		return c.commands.Show(ctx, message)
-	}, th.CommandEqual("show"))
-
-	bh.HandleMessage(func(ctx *th.Context, message telego.Message) error {
-		return c.commands.List(ctx, message)
-	}, th.CommandEqual("list"))
-
 	bh.HandleMessage(func(ctx *th.Context, message telego.Message) error {
 		return c.handleMessage(ctx, &message)
 	}, th.AnyMessage())

pkg/channels/telegram_commands.go

@@ -1,153 +0,0 @@
-package channels
-
-import (
-	"context"
-	"fmt"
-	"strings"
-
-	"github.com/mymmrac/telego"
-	"github.com/sipeed/picoclaw/pkg/config"
-)
-
-type TelegramCommander interface {
-	Help(ctx context.Context, message telego.Message) error
-	Start(ctx context.Context, message telego.Message) error
-	Show(ctx context.Context, message telego.Message) error
-	List(ctx context.Context, message telego.Message) error
-}
-
-type cmd struct {
-	bot    *telego.Bot
-	config *config.Config
-}
-
-func NewTelegramCommands(bot *telego.Bot, cfg *config.Config) TelegramCommander {
-	return &cmd{
-		bot:    bot,
-		config: cfg,
-	}
-}
-
-func commandArgs(text string) string {
-	parts := strings.SplitN(text, " ", 2)
-	if len(parts) < 2 {
-		return ""
-	}
-	return strings.TrimSpace(parts[1])
-}
-func (c *cmd) Help(ctx context.Context, message telego.Message) error {
-	msg := `/start - Start the bot
-/help - Show this help message
-/show [model|channel] - Show current configuration
-/list [models|channels] - List available options
-	`
-	_, err := c.bot.SendMessage(ctx, &telego.SendMessageParams{
-		ChatID: telego.ChatID{ID: message.Chat.ID},
-		Text:   msg,
-		ReplyParameters: &telego.ReplyParameters{
-			MessageID: message.MessageID,
-		},
-	})
-	return err
-}
-
-func (c *cmd) Start(ctx context.Context, message telego.Message) error {
-	_, err := c.bot.SendMessage(ctx, &telego.SendMessageParams{
-		ChatID: telego.ChatID{ID: message.Chat.ID},
-		Text:   "Hello! I am PicoClaw 🦞",
-		ReplyParameters: &telego.ReplyParameters{
-			MessageID: message.MessageID,
-		},
-	})
-	return err
-}
-
-func (c *cmd) Show(ctx context.Context, message telego.Message) error {
-	args := commandArgs(message.Text)
-	if args == "" {
-		_, err := c.bot.SendMessage(ctx, &telego.SendMessageParams{
-			ChatID: telego.ChatID{ID: message.Chat.ID},
-			Text:   "Usage: /show [model|channel]",
-			ReplyParameters: &telego.ReplyParameters{
-				MessageID: message.MessageID,
-			},
-		})
-		return err
-	}
-
-	var response string
-	switch args {
-	case "model":
-		response = fmt.Sprintf("Current Model: %s (Provider: %s)",
-			c.config.Agents.Defaults.Model,
-			c.config.Agents.Defaults.Provider)
-	case "channel":
-		response = "Current Channel: telegram"
-	default:
-		response = fmt.Sprintf("Unknown parameter: %s. Try 'model' or 'channel'.", args)
-	}
-
-	_, err := c.bot.SendMessage(ctx, &telego.SendMessageParams{
-		ChatID: telego.ChatID{ID: message.Chat.ID},
-		Text:   response,
-		ReplyParameters: &telego.ReplyParameters{
-			MessageID: message.MessageID,
-		},
-	})
-	return err
-}
-func (c *cmd) List(ctx context.Context, message telego.Message) error {
-	args := commandArgs(message.Text)
-	if args == "" {
-		_, err := c.bot.SendMessage(ctx, &telego.SendMessageParams{
-			ChatID: telego.ChatID{ID: message.Chat.ID},
-			Text:   "Usage: /list [models|channels]",
-			ReplyParameters: &telego.ReplyParameters{
-				MessageID: message.MessageID,
-			},
-		})
-		return err
-	}
-
-	var response string
-	switch args {
-	case "models":
-		provider := c.config.Agents.Defaults.Provider
-		if provider == "" {
-			provider = "configured default"
-		}
-		response = fmt.Sprintf("Configured Model: %s\nProvider: %s\n\nTo change models, update config.yaml",
-			c.config.Agents.Defaults.Model, provider)
-
-	case "channels":
-		var enabled []string
-		if c.config.Channels.Telegram.Enabled {
-			enabled = append(enabled, "telegram")
-		}
-		if c.config.Channels.WhatsApp.Enabled {
-			enabled = append(enabled, "whatsapp")
-		}
-		if c.config.Channels.Feishu.Enabled {
-			enabled = append(enabled, "feishu")
-		}
-		if c.config.Channels.Discord.Enabled {
-			enabled = append(enabled, "discord")
-		}
-		if c.config.Channels.Slack.Enabled {
-			enabled = append(enabled, "slack")
-		}
-		response = fmt.Sprintf("Enabled Channels:\n- %s", strings.Join(enabled, "\n- "))
-
-	default:
-		response = fmt.Sprintf("Unknown parameter: %s. Try 'models' or 'channels'.", args)
-	}
-
-	_, err := c.bot.SendMessage(ctx, &telego.SendMessageParams{
-		ChatID: telego.ChatID{ID: message.Chat.ID},
-		Text:   response,
-		ReplyParameters: &telego.ReplyParameters{
-			MessageID: message.MessageID,
-		},
-	})
-	return err
-}

pkg/command/basic.go

@@ -0,0 +1,164 @@
+package command
+
+import (
+	"context"
+	"fmt"
+	"strings"
+
+	"github.com/sipeed/picoclaw/pkg/bus"
+)
+
+type ShowCommand struct{}
+
+func (c *ShowCommand) Name() string {
+	return "/show"
+}
+
+func (c *ShowCommand) Description() string {
+	return "Show current configuration (model, channel)"
+}
+
+func (c *ShowCommand) Execute(ctx context.Context, agent AgentState, args []string, msg bus.InboundMessage) (string, error) {
+	if len(args) < 1 {
+		return "Usage: /show [model|channel]", nil
+	}
+	switch args[0] {
+	case "model":
+		return fmt.Sprintf("Current model: %s", agent.GetModel()), nil
+	case "channel":
+		return fmt.Sprintf("Current channel: %s", msg.Channel), nil
+	default:
+		return fmt.Sprintf("Unknown show target: %s", args[0]), nil
+	}
+}
+
+type ListCommand struct{}
+
+func (c *ListCommand) Name() string {
+	return "/list"
+}
+
+func (c *ListCommand) Description() string {
+	return "List available resources (models, channels)"
+}
+
+func (c *ListCommand) Execute(ctx context.Context, agent AgentState, args []string, msg bus.InboundMessage) (string, error) {
+	if len(args) < 1 {
+		return "Usage: /list [models|channels]", nil
+	}
+	switch args[0] {
+	case "models":
+		// TODO: Fetch available models dynamically if possible
+		return "Available models: glm-4.7, claude-3-5-sonnet, gpt-4o (configured in config.json/env)", nil
+	case "channels":
+		cm := agent.GetChannelManager()
+		if cm == nil {
+			return "Channel manager not initialized", nil
+		}
+
+		// Use reflection or interface assertion to access GetEnabledChannels
+		// Since we use interface{} to avoid circular deps, we need to assert a local interface or use reflection
+		// For simplicity, let's assume the caller injected something that has GetEnabledChannels
+		type ChannelLister interface {
+			GetEnabledChannels() []string
+		}
+
+		if lister, ok := cm.(ChannelLister); ok {
+			channels := lister.GetEnabledChannels()
+			if len(channels) == 0 {
+				return "No channels enabled", nil
+			}
+			return fmt.Sprintf("Enabled channels: %s", strings.Join(channels, ", ")), nil
+		}
+		return "Channel manager does not support listing channels", nil
+
+	default:
+		return fmt.Sprintf("Unknown list target: %s", args[0]), nil
+	}
+}
+
+type SwitchCommand struct{}
+
+func (c *SwitchCommand) Name() string {
+	return "/switch"
+}
+
+func (c *SwitchCommand) Description() string {
+	return "Switch configuration context"
+}
+
+func (c *SwitchCommand) Execute(ctx context.Context, agent AgentState, args []string, msg bus.InboundMessage) (string, error) {
+	if len(args) < 3 || args[1] != "to" {
+		return "Usage: /switch [model|channel] to <name>", nil
+	}
+	target := args[0]
+	value := args[2]
+
+	switch target {
+	case "model":
+		oldModel := agent.GetModel()
+		agent.SetModel(value)
+		return fmt.Sprintf("Switched model from %s to %s", oldModel, value), nil
+	case "channel":
+		cm := agent.GetChannelManager()
+		if cm == nil {
+			return "Channel manager not initialized", nil
+		}
+
+		type ChannelGetter interface {
+			GetChannel(name string) (interface{}, bool)
+		}
+
+		if getter, ok := cm.(ChannelGetter); ok {
+			if _, exists := getter.GetChannel(value); !exists && value != "cli" {
+				return fmt.Sprintf("Channel '%s' not found or not enabled", value), nil
+			}
+			return fmt.Sprintf("Switched target channel to %s (Note: this currently only validates existence)", value), nil
+		}
+		return "Channel manager check failed", nil
+
+	default:
+		return fmt.Sprintf("Unknown switch target: %s", target), nil
+	}
+}
+
+type StartCommand struct{}
+
+func (c *StartCommand) Name() string {
+	return "/start"
+}
+
+func (c *StartCommand) Description() string {
+	return "Start the bot and get a welcome message"
+}
+
+func (c *StartCommand) Execute(ctx context.Context, agent AgentState, args []string, msg bus.InboundMessage) (string, error) {
+	return "Hello! I am PicoClaw 🦞\nI am your personal AI agent. Type /help to see what I can do.", nil
+}
+
+type HelpCommand struct {
+	Registry *Registry
+}
+
+func (c *HelpCommand) Name() string {
+	return "/help"
+}
+
+func (c *HelpCommand) Description() string {
+	return "Show available commands"
+}
+
+func (c *HelpCommand) Execute(ctx context.Context, agent AgentState, args []string, msg bus.InboundMessage) (string, error) {
+	if c.Registry == nil {
+		return "Error: Command registry not available", nil
+	}
+
+	var sb strings.Builder
+	sb.WriteString("Available commands:\n")
+
+	for name, cmd := range c.Registry.ListCommands() {
+		sb.WriteString(fmt.Sprintf("%s - %s\n", name, cmd.Description()))
+	}
+
+	return sb.String(), nil
+}

pkg/command/registry.go

@@ -0,0 +1,61 @@
+package command
+
+import (
+	"context"
+	"strings"
+
+	"github.com/sipeed/picoclaw/pkg/bus"
+)
+
+// Registry manages the set of available commands.
+type Registry struct {
+	commands map[string]Command
+}
+
+// NewRegistry creates a new command registry.
+func NewRegistry() *Registry {
+	return &Registry{
+		commands: make(map[string]Command),
+	}
+}
+
+// Register adds a command to the registry.
+func (r *Registry) Register(cmd Command) {
+	r.commands[cmd.Name()] = cmd
+}
+
+// Execute attempts to execute a command by name.
+// Returns response string, handled boolean, and error.
+func (r *Registry) Execute(ctx context.Context, agent AgentState, name string, args []string, msg bus.InboundMessage) (string, bool, error) {
+	cmd, ok := r.commands[name]
+	if !ok {
+		return "", false, nil
+	}
+	resp, err := cmd.Execute(ctx, agent, args, msg)
+	return resp, true, err
+}
+
+// ListCommands returns a map of registered commands.
+func (r *Registry) ListCommands() map[string]Command {
+	return r.commands
+}
+
+// Parse extracts command and arguments from a message string.
+// Returns command name, arguments, and true if it looks like a command.
+func (r *Registry) Parse(content string) (string, []string, bool) {
+	content = strings.TrimSpace(content)
+	if !strings.HasPrefix(content, "/") {
+		return "", nil, false
+	}
+
+	parts := strings.Fields(content)
+	if len(parts) == 0 {
+		return "", nil, false
+	}
+
+	// Command name includes the slash, e.g., "/show"
+	cmdName := parts[0]
+	args := parts[1:]
+
+	return cmdName, args, true
+}

pkg/command/types.go

@@ -0,0 +1,21 @@
+package command
+
+import (
+	"context"
+
+	"github.com/sipeed/picoclaw/pkg/bus"
+)
+
+// AgentState provides access to agent internals needed by commands.
+type AgentState interface {
+	GetModel() string
+	SetModel(model string)
+	GetChannelManager() interface{} // Returns channels.Manager (interface to avoid circular dep)
+}
+
+// Command represents an executable command.
+type Command interface {
+	Name() string
+	Description() string
+	Execute(ctx context.Context, agent AgentState, args []string, msg bus.InboundMessage) (string, error)
+}