From cd0ecff2e2ea6a44b29cb3089688823ce7277c4f Mon Sep 17 00:00:00 2001 From: seagochen Date: Thu, 26 Feb 2026 17:00:07 +0900 Subject: [PATCH] =?UTF-8?q?refactor:=20unify=20command=20prefix=20style=20?= =?UTF-8?q?(/=20=E2=86=92=20:)=20and=20merge=20handlers?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Merge handleCommand() and handleExtensionCommand() into a single function. Convert /show, /list, /switch to :show, :list, :switch. Remove duplicate Telegram-side /show and /list handlers. Co-Authored-By: Claude Opus 4.6 --- pkg/agent/loop.go | 169 +++++++++++++----------------- pkg/agent/loop_test.go | 37 ++++++- pkg/channels/telegram.go | 10 +- pkg/channels/telegram_commands.go | 118 ++------------------- 4 files changed, 115 insertions(+), 219 deletions(-) diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index 6d78e2e0e..0aff9b7f6 100644 --- a/pkg/agent/loop.go +++ b/pkg/agent/loop.go @@ -1151,12 +1151,7 @@ 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) - // Handle : prefixed extension commands (work across all channels) - if strings.HasPrefix(content, ":") { - return al.handleExtensionCommand(content) - } - - if !strings.HasPrefix(content, "/") { + if !strings.HasPrefix(content, ":") { return "", false } @@ -1168,91 +1163,6 @@ func (al *AgentLoop) handleCommand(ctx context.Context, msg bus.InboundMessage) cmd := parts[0] args := parts[1:] - switch cmd { - case "/show": - if len(args) < 1 { - return "Usage: /show [model|channel|agents]", true - } - switch args[0] { - case "model": - defaultAgent := al.registry.GetDefaultAgent() - if defaultAgent == nil { - return "No default agent configured", true - } - return fmt.Sprintf("Current model: %s", defaultAgent.Model), true - case "channel": - return fmt.Sprintf("Current channel: %s", msg.Channel), true - case "agents": - agentIDs := al.registry.ListAgentIDs() - return fmt.Sprintf("Registered agents: %s", strings.Join(agentIDs, ", ")), true - default: - return fmt.Sprintf("Unknown show target: %s", args[0]), true - } - - case "/list": - if len(args) < 1 { - return "Usage: /list [models|channels|agents]", true - } - switch args[0] { - case "models": - return "Available models: configured in config.json per agent", 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 - case "agents": - agentIDs := al.registry.ListAgentIDs() - return fmt.Sprintf("Registered agents: %s", strings.Join(agentIDs, ", ")), 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": - defaultAgent := al.registry.GetDefaultAgent() - if defaultAgent == nil { - return "No default agent configured", true - } - oldModel := defaultAgent.Model - defaultAgent.Model = value - return fmt.Sprintf("Switched model from %s to %s", oldModel, value), true - case "channel": - 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 - } - return fmt.Sprintf("Switched target channel to %s", value), true - default: - return fmt.Sprintf("Unknown switch target: %s", target), true - } - } - - return "", false -} - -// handleExtensionCommand handles : prefixed commands that work across all channels. -func (al *AgentLoop) handleExtensionCommand(content string) (string, bool) { - parts := strings.Fields(content) - if len(parts) == 0 { - return "", false - } - - cmd := parts[0] - switch cmd { case ":cmd", ":pico", ":hipico", ":edit": // Pass through to processMessage for mode handling (needs sessionKey from routing) @@ -1265,9 +1175,9 @@ func (al *AgentLoop) handleExtensionCommand(content string) (string, bool) { :pico - Switch to chat mode (default, AI conversation) :hipico - Ask AI for help (from command mode, one-shot) :edit - View/edit files (cmd mode) -/show [model|channel|agents] - Show current configuration -/list [models|channels|agents] - List available options -/switch [model|channel] to - Switch model or channel`, true +:show [model|channel|agents] - Show current configuration +:list [models|channels|agents] - List available options +:switch [model|channel] to - Switch model or channel`, true case ":usage": agent := al.registry.GetDefaultAgent() @@ -1294,6 +1204,77 @@ Token usage (this session): agent.TotalRequests.Load(), ), true + case ":show": + if len(args) < 1 { + return "Usage: :show [model|channel|agents]", true + } + switch args[0] { + case "model": + defaultAgent := al.registry.GetDefaultAgent() + if defaultAgent == nil { + return "No default agent configured", true + } + return fmt.Sprintf("Current model: %s", defaultAgent.Model), true + case "channel": + return fmt.Sprintf("Current channel: %s", msg.Channel), true + case "agents": + agentIDs := al.registry.ListAgentIDs() + return fmt.Sprintf("Registered agents: %s", strings.Join(agentIDs, ", ")), true + default: + return fmt.Sprintf("Unknown show target: %s", args[0]), true + } + + case ":list": + if len(args) < 1 { + return "Usage: :list [models|channels|agents]", true + } + switch args[0] { + case "models": + return "Available models: configured in config.json per agent", 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 + case "agents": + agentIDs := al.registry.ListAgentIDs() + return fmt.Sprintf("Registered agents: %s", strings.Join(agentIDs, ", ")), 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": + defaultAgent := al.registry.GetDefaultAgent() + if defaultAgent == nil { + return "No default agent configured", true + } + oldModel := defaultAgent.Model + defaultAgent.Model = value + return fmt.Sprintf("Switched model from %s to %s", oldModel, value), true + case "channel": + 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 + } + return fmt.Sprintf("Switched target channel to %s", value), true + default: + return fmt.Sprintf("Unknown switch target: %s", target), true + } + default: // Don't intercept unrecognized : prefixed messages (e.g. :) :D :thinking:) // Let them pass through as normal chat messages diff --git a/pkg/agent/loop_test.go b/pkg/agent/loop_test.go index 63c0754f2..6042ecf74 100644 --- a/pkg/agent/loop_test.go +++ b/pkg/agent/loop_test.go @@ -632,9 +632,9 @@ func TestAgentLoop_ContextExhaustionRetry(t *testing.T) { } } -// TestHandleExtensionCommand_EmojiPassthrough verifies that emoji-like +// TestHandleCommand_EmojiPassthrough verifies that emoji-like // messages starting with : are not intercepted as commands. -func TestHandleExtensionCommand_EmojiPassthrough(t *testing.T) { +func TestHandleCommand_EmojiPassthrough(t *testing.T) { cfg := &config.Config{ Agents: config.AgentsConfig{ Defaults: config.AgentDefaults{ @@ -647,10 +647,12 @@ func TestHandleExtensionCommand_EmojiPassthrough(t *testing.T) { msgBus := bus.NewMessageBus() provider := &mockProvider{} al := NewAgentLoop(cfg, msgBus, provider) + ctx := context.Background() emojiInputs := []string{":)", ":D", ":heart:", ":thinking:", ":-)", ":100:"} for _, input := range emojiInputs { - _, handled := al.handleExtensionCommand(input) + msg := bus.InboundMessage{Content: input} + _, handled := al.handleCommand(ctx, msg) if handled { t.Errorf("Expected %q to pass through (not handled), but it was handled", input) } @@ -659,9 +661,36 @@ func TestHandleExtensionCommand_EmojiPassthrough(t *testing.T) { // Known commands should still be handled knownCommands := []string{":help", ":usage"} for _, cmd := range knownCommands { - _, handled := al.handleExtensionCommand(cmd) + msg := bus.InboundMessage{Content: cmd} + _, handled := al.handleCommand(ctx, msg) if !handled { t.Errorf("Expected %q to be handled, but it was not", cmd) } } + + // :show, :list, :switch should be handled + showMsg := bus.InboundMessage{Content: ":show model", Channel: "cli"} + resp, handled := al.handleCommand(ctx, showMsg) + if !handled { + t.Error("Expected :show model to be handled") + } + if resp != "Current model: test-model" { + t.Errorf("Unexpected :show model response: %s", resp) + } + + listMsg := bus.InboundMessage{Content: ":list agents"} + _, handled = al.handleCommand(ctx, listMsg) + if !handled { + t.Error("Expected :list agents to be handled") + } + + // Non-: messages should not be handled + plainInputs := []string{"hello", "/show model", "ls -la"} + for _, input := range plainInputs { + msg := bus.InboundMessage{Content: input} + _, handled := al.handleCommand(ctx, msg) + if handled { + t.Errorf("Expected %q to not be handled, but it was", input) + } + } } diff --git a/pkg/channels/telegram.go b/pkg/channels/telegram.go index 524494849..edc1ef2e4 100644 --- a/pkg/channels/telegram.go +++ b/pkg/channels/telegram.go @@ -76,7 +76,7 @@ func NewTelegramChannel(cfg *config.Config, bus *bus.MessageBus) (*TelegramChann return &TelegramChannel{ BaseChannel: base, - commands: NewTelegramCommands(bot, cfg), + commands: NewTelegramCommands(bot), bot: bot, config: cfg, chatIDs: make(map[string]int64), @@ -113,14 +113,6 @@ func (c *TelegramChannel) Start(ctx context.Context) 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 index f28434f46..bc5b095b9 100644 --- a/pkg/channels/telegram_commands.go +++ b/pkg/channels/telegram_commands.go @@ -2,46 +2,31 @@ 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 + bot *telego.Bot } -func NewTelegramCommands(bot *telego.Bot, cfg *config.Config) TelegramCommander { +func NewTelegramCommands(bot *telego.Bot) TelegramCommander { return &cmd{ - bot: bot, - config: cfg, + bot: bot, } } -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 +:show [model|channel|agents] - Show current configuration +:list [models|channels|agents] - List available options +:switch [model|channel] to - Switch model or channel ` _, err := c.bot.SendMessage(ctx, &telego.SendMessageParams{ ChatID: telego.ChatID{ID: message.Chat.ID}, @@ -63,94 +48,3 @@ func (c *cmd) Start(ctx context.Context, message telego.Message) error { }) 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.GetModelName(), - 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.GetModelName(), 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 -}