From a2e2d132a43a7ed341d3eeb611451de9c9f975b8 Mon Sep 17 00:00:00 2001 From: Tzufucius <952105672@qq.com> Date: Tue, 17 Feb 2026 09:31:20 +0800 Subject: [PATCH] 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 | 237 +++++++----------------------- pkg/channels/telegram.go | 18 --- pkg/channels/telegram_commands.go | 153 ------------------- pkg/command/basic.go | 164 +++++++++++++++++++++ pkg/command/registry.go | 61 ++++++++ pkg/command/types.go | 21 +++ 6 files changed, 300 insertions(+), 354 deletions(-) delete mode 100644 pkg/channels/telegram_commands.go create mode 100644 pkg/command/basic.go create mode 100644 pkg/command/registry.go create mode 100644 pkg/command/types.go diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index b0efca4f3..e1f46e1b0 100644 --- a/pkg/agent/loop.go +++ b/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" @@ -31,19 +32,20 @@ import ( ) type AgentLoop struct { - bus *bus.MessageBus - provider providers.LLMProvider - workspace string - model string - contextWindow int // Maximum context window size in tokens - maxIterations int - sessions *session.SessionManager - state *state.Manager - contextBuilder *ContextBuilder - tools *tools.ToolRegistry - running atomic.Bool - summarizing sync.Map // Tracks which sessions are currently being summarized - channelManager *channels.Manager + bus *bus.MessageBus + provider providers.LLMProvider + workspace string + model string + contextWindow int // Maximum context window size in tokens + maxIterations int + sessions *session.SessionManager + state *state.Manager + contextBuilder *ContextBuilder + tools *tools.ToolRegistry + 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,18 +138,27 @@ 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, - workspace: workspace, - model: cfg.Agents.Defaults.Model, - contextWindow: cfg.Agents.Defaults.MaxTokens, // Restore context window for summarization - maxIterations: cfg.Agents.Defaults.MaxToolIterations, - sessions: sessionsManager, - state: stateManager, - contextBuilder: contextBuilder, - tools: toolsRegistry, - summarizing: sync.Map{}, + bus: msgBus, + provider: provider, + workspace: workspace, + model: cfg.Agents.Defaults.Model, + contextWindow: cfg.Agents.Defaults.MaxTokens, // Restore context window for summarization + maxIterations: cfg.Agents.Defaults.MaxToolIterations, + sessions: sessionsManager, + state: stateManager, + 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) - if !strings.HasPrefix(content, "/") { + name, args, ok := al.commandRegistry.Parse(msg.Content) + if !ok { return "", false } - parts := strings.Fields(content) - if len(parts) == 0 { - return "", false + response, handled, err := al.commandRegistry.Execute(ctx, al, name, args, msg) + if err != nil { + return fmt.Sprintf("Error executing command: %v", err), true } - - 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 ", 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 ", 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 + return response, handled } diff --git a/pkg/channels/telegram.go b/pkg/channels/telegram.go index 5601d508c..7f142c54f 100644 --- a/pkg/channels/telegram.go +++ b/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()) diff --git a/pkg/channels/telegram_commands.go b/pkg/channels/telegram_commands.go deleted file mode 100644 index df245e156..000000000 --- a/pkg/channels/telegram_commands.go +++ /dev/null @@ -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 -} diff --git a/pkg/command/basic.go b/pkg/command/basic.go new file mode 100644 index 000000000..f780deb75 --- /dev/null +++ b/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 ", 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 +} diff --git a/pkg/command/registry.go b/pkg/command/registry.go new file mode 100644 index 000000000..89d753569 --- /dev/null +++ b/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 +} diff --git a/pkg/command/types.go b/pkg/command/types.go new file mode 100644 index 000000000..db2a1c89f --- /dev/null +++ b/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) +}