fix(commands): unify command execution and harden parsing

This commit is contained in:
mingmxren 2026-03-01 02:51:20 +08:00
parent e06ae83c22
commit ec50a0a0c2
13 changed files with 376 additions and 80 deletions

View file

@ -48,3 +48,10 @@ type PlaceholderRecorder interface {
type CommandRegistrarCapable interface { type CommandRegistrarCapable interface {
RegisterCommands(ctx context.Context, defs []commands.Definition) error RegisterCommands(ctx context.Context, defs []commands.Definition) error
} }
// CommandParserCapable is implemented by channels that expose a command
// dispatch entrypoint backed by shared command definitions/dispatcher.
// It is optional and intended for cross-channel command handling features.
type CommandParserCapable interface {
DispatchCommand(ctx context.Context, req commands.Request) commands.Result
}

View file

@ -11,6 +11,16 @@ type mockRegistrar struct{}
func (mockRegistrar) RegisterCommands(context.Context, []commands.Definition) error { return nil } func (mockRegistrar) RegisterCommands(context.Context, []commands.Definition) error { return nil }
type mockParser struct{}
func (mockParser) DispatchCommand(context.Context, commands.Request) commands.Result {
return commands.Result{Matched: false}
}
func TestCommandRegistrarCapable_Compiles(t *testing.T) { func TestCommandRegistrarCapable_Compiles(t *testing.T) {
var _ CommandRegistrarCapable = mockRegistrar{} var _ CommandRegistrarCapable = mockRegistrar{}
} }
func TestCommandParserCapable_Compiles(t *testing.T) {
var _ CommandParserCapable = mockParser{}
}

View file

@ -3,6 +3,7 @@ package telegram
import ( import (
"context" "context"
"errors" "errors"
"sync/atomic"
"testing" "testing"
"time" "time"
@ -28,3 +29,68 @@ func TestStartCommandRegistration_DoesNotBlock(t *testing.T) {
t.Fatal("registration did not start asynchronously") t.Fatal("registration did not start asynchronously")
} }
} }
func TestStartCommandRegistration_RetriesUntilSuccessThenStops(t *testing.T) {
ch := &TelegramChannel{}
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
origBackoff := commandRegistrationBackoff
commandRegistrationBackoff = []time.Duration{5 * time.Millisecond}
defer func() { commandRegistrationBackoff = origBackoff }()
var attempts atomic.Int32
ch.registerFunc = func(context.Context, []commands.Definition) error {
n := attempts.Add(1)
if n < 3 {
return errors.New("temporary failure")
}
return nil
}
ch.startCommandRegistration(ctx, []commands.Definition{{Name: "help", Description: "Help"}})
deadline := time.Now().Add(250 * time.Millisecond)
for time.Now().Before(deadline) {
if attempts.Load() >= 3 {
break
}
time.Sleep(5 * time.Millisecond)
}
if attempts.Load() < 3 {
t.Fatalf("expected at least 3 attempts, got %d", attempts.Load())
}
stable := attempts.Load()
time.Sleep(30 * time.Millisecond)
if attempts.Load() != stable {
t.Fatalf("expected retries to stop after success, got %d -> %d", stable, attempts.Load())
}
}
func TestStartCommandRegistration_StopsAfterCancel(t *testing.T) {
ch := &TelegramChannel{}
ctx, cancel := context.WithCancel(context.Background())
origBackoff := commandRegistrationBackoff
commandRegistrationBackoff = []time.Duration{5 * time.Millisecond}
defer func() { commandRegistrationBackoff = origBackoff }()
defer cancel()
var attempts atomic.Int32
ch.registerFunc = func(context.Context, []commands.Definition) error {
attempts.Add(1)
return errors.New("always fail")
}
ch.startCommandRegistration(ctx, []commands.Definition{{Name: "help", Description: "Help"}})
time.Sleep(20 * time.Millisecond)
cancel()
time.Sleep(20 * time.Millisecond) // allow in-flight attempt to settle
stable := attempts.Load()
time.Sleep(30 * time.Millisecond)
if attempts.Load() != stable {
t.Fatalf("expected retries to quiesce after cancel, got %d -> %d", stable, attempts.Load())
}
}

View file

@ -40,7 +40,7 @@ func commandArgs(text string) string {
func (c *cmd) Help(ctx context.Context, message telego.Message) error { func (c *cmd) Help(ctx context.Context, message telego.Message) error {
defs := commands.NewRegistry(commands.BuiltinDefinitions(c.config)).ForChannel("telegram") defs := commands.NewRegistry(commands.BuiltinDefinitions(c.config)).ForChannel("telegram")
msg := formatHelpMessage(defs) msg := commands.FormatHelpMessage(defs)
_, err := c.bot.SendMessage(ctx, &telego.SendMessageParams{ _, err := c.bot.SendMessage(ctx, &telego.SendMessageParams{
ChatID: telego.ChatID{ID: message.Chat.ID}, ChatID: telego.ChatID{ID: message.Chat.ID},
Text: msg, Text: msg,
@ -51,26 +51,6 @@ func (c *cmd) Help(ctx context.Context, message telego.Message) error {
return err return err
} }
func formatHelpMessage(defs []commands.Definition) string {
if len(defs) == 0 {
return "No commands available."
}
lines := make([]string, 0, len(defs))
for _, def := range defs {
usage := def.Usage
if usage == "" {
usage = "/" + def.Name
}
desc := def.Description
if desc == "" {
desc = "No description"
}
lines = append(lines, fmt.Sprintf("%s - %s", usage, desc))
}
return strings.Join(lines, "\n")
}
func (c *cmd) Start(ctx context.Context, message telego.Message) error { func (c *cmd) Start(ctx context.Context, message telego.Message) error {
_, err := c.bot.SendMessage(ctx, &telego.SendMessageParams{ _, err := c.bot.SendMessage(ctx, &telego.SendMessageParams{
ChatID: telego.ChatID{ID: message.Chat.ID}, ChatID: telego.ChatID{ID: message.Chat.ID},

View file

@ -10,56 +10,45 @@ import (
"github.com/sipeed/picoclaw/pkg/logger" "github.com/sipeed/picoclaw/pkg/logger"
) )
func (c *TelegramChannel) dispatchCommand(ctx context.Context, message telego.Message) bool { func (c *TelegramChannel) DispatchCommand(ctx context.Context, req commands.Request) commands.Result {
if c.dispatcher == nil { if c.dispatcher == nil {
return false return commands.Result{Matched: false}
} }
return c.dispatcher.Dispatch(ctx, req)
}
func (c *TelegramChannel) dispatchCommand(ctx context.Context, message telego.Message) bool {
senderID := "" senderID := ""
if message.From != nil { if message.From != nil {
senderID = strconv.FormatInt(message.From.ID, 10) senderID = strconv.FormatInt(message.From.ID, 10)
} }
res := c.dispatcher.Dispatch(ctx, commands.Request{ res := c.DispatchCommand(ctx, commands.Request{
Channel: "telegram", Channel: "telegram",
ChatID: strconv.FormatInt(message.Chat.ID, 10), ChatID: strconv.FormatInt(message.Chat.ID, 10),
SenderID: senderID, SenderID: senderID,
Text: message.Text, Text: message.Text,
MessageID: strconv.Itoa(message.MessageID), MessageID: strconv.Itoa(message.MessageID),
Reply: func(text string) error {
_, err := c.bot.SendMessage(ctx, &telego.SendMessageParams{
ChatID: telego.ChatID{ID: message.Chat.ID},
Text: text,
ReplyParameters: &telego.ReplyParameters{
MessageID: message.MessageID,
},
})
return err
},
}) })
if !res.Matched { if !res.Matched {
return false return false
} }
switch res.Command { if res.Err != nil {
case "help": logger.ErrorCF("telegram", "Command execution failed", map[string]any{
if err := c.commands.Help(ctx, message); err != nil { "command": res.Command,
logger.ErrorCF("telegram", "Command execution failed", map[string]any{ "error": res.Err.Error(),
"command": "help", })
"error": err.Error(),
})
}
case "start":
if err := c.commands.Start(ctx, message); err != nil {
logger.ErrorCF("telegram", "Command execution failed", map[string]any{
"command": "start",
"error": err.Error(),
})
}
case "show":
if err := c.commands.Show(ctx, message); err != nil {
logger.ErrorCF("telegram", "Command execution failed", map[string]any{
"command": "show",
"error": err.Error(),
})
}
case "list":
if err := c.commands.List(ctx, message); err != nil {
logger.ErrorCF("telegram", "Command execution failed", map[string]any{
"command": "list",
"error": err.Error(),
})
}
} }
return true return true

View file

@ -20,14 +20,14 @@ import (
type WhatsAppChannel struct { type WhatsAppChannel struct {
*channels.BaseChannel *channels.BaseChannel
conn *websocket.Conn conn *websocket.Conn
config config.WhatsAppConfig config config.WhatsAppConfig
url string url string
dispatcher commands.Dispatching dispatcher commands.Dispatching
ctx context.Context ctx context.Context
cancel context.CancelFunc cancel context.CancelFunc
mu sync.Mutex mu sync.Mutex
connected bool connected bool
} }
func NewWhatsAppChannel(cfg config.WhatsAppConfig, bus *bus.MessageBus) (*WhatsAppChannel, error) { func NewWhatsAppChannel(cfg config.WhatsAppConfig, bus *bus.MessageBus) (*WhatsAppChannel, error) {
@ -262,16 +262,15 @@ func (c *WhatsAppChannel) tryHandleCommand(
ctx context.Context, ctx context.Context,
text, chatID, senderID, messageID string, text, chatID, senderID, messageID string,
) bool { ) bool {
if c.dispatcher == nil { res := c.DispatchCommand(ctx, commands.Request{
return false
}
res := c.dispatcher.Dispatch(ctx, commands.Request{
Channel: "whatsapp", Channel: "whatsapp",
ChatID: chatID, ChatID: chatID,
SenderID: senderID, SenderID: senderID,
Text: text, Text: text,
MessageID: messageID, MessageID: messageID,
Reply: func(text string) error {
return c.Send(ctx, bus.OutboundMessage{ChatID: chatID, Content: text})
},
}) })
if res.Err != nil { if res.Err != nil {
logger.WarnCF("whatsapp", "Command execution failed", map[string]any{ logger.WarnCF("whatsapp", "Command execution failed", map[string]any{
@ -279,6 +278,12 @@ func (c *WhatsAppChannel) tryHandleCommand(
"error": res.Err.Error(), "error": res.Err.Error(),
}) })
} }
return res.Matched
return res.Handled }
func (c *WhatsAppChannel) DispatchCommand(ctx context.Context, req commands.Request) commands.Result {
if c.dispatcher == nil {
return commands.Result{Matched: false}
}
return c.dispatcher.Dispatch(ctx, req)
} }

View file

@ -20,3 +20,15 @@ func TestTryHandleCommand_UsesDispatcher(t *testing.T) {
t.Fatalf("handled=%v called=%v", handled, called) t.Fatalf("handled=%v called=%v", handled, called)
} }
} }
func TestTryHandleCommand_MatchedWithoutHandler_DoesNotFallThrough(t *testing.T) {
ch := &WhatsAppChannel{}
ch.dispatcher = commands.DispatchFunc(func(context.Context, commands.Request) commands.Result {
return commands.Result{Matched: true, Handled: false, Command: "unknown"}
})
handled := ch.tryHandleCommand(context.Background(), "/unknown", "chat1", "user1", "mid1")
if !handled {
t.Fatal("expected matched command to be treated as handled")
}
}

View file

@ -22,3 +22,15 @@ func TestTryHandleCommand_UsesDispatcher(t *testing.T) {
t.Fatalf("handled=%v called=%v", handled, called) t.Fatalf("handled=%v called=%v", handled, called)
} }
} }
func TestTryHandleCommand_MatchedWithoutHandler_DoesNotFallThrough(t *testing.T) {
ch := &WhatsAppNativeChannel{}
ch.dispatcher = commands.DispatchFunc(func(context.Context, commands.Request) commands.Result {
return commands.Result{Matched: true, Handled: false, Command: "unknown"}
})
handled := ch.tryHandleCommand(context.Background(), "/unknown", "chat1", "user1", "mid1")
if !handled {
t.Fatal("expected matched command to be treated as handled")
}
}

View file

@ -406,15 +406,15 @@ func (c *WhatsAppNativeChannel) tryHandleCommand(
ctx context.Context, ctx context.Context,
text, chatID, senderID, messageID string, text, chatID, senderID, messageID string,
) bool { ) bool {
if c.dispatcher == nil { res := c.DispatchCommand(ctx, commands.Request{
return false
}
res := c.dispatcher.Dispatch(ctx, commands.Request{
Channel: "whatsapp_native", Channel: "whatsapp_native",
ChatID: chatID, ChatID: chatID,
SenderID: senderID, SenderID: senderID,
Text: text, Text: text,
MessageID: messageID, MessageID: messageID,
Reply: func(text string) error {
return c.Send(ctx, bus.OutboundMessage{ChatID: chatID, Content: text})
},
}) })
if res.Err != nil { if res.Err != nil {
logger.WarnCF("whatsapp", "Command execution failed", map[string]any{ logger.WarnCF("whatsapp", "Command execution failed", map[string]any{
@ -422,7 +422,14 @@ func (c *WhatsAppNativeChannel) tryHandleCommand(
"error": res.Err.Error(), "error": res.Err.Error(),
}) })
} }
return res.Handled return res.Matched
}
func (c *WhatsAppNativeChannel) DispatchCommand(ctx context.Context, req commands.Request) commands.Result {
if c.dispatcher == nil {
return commands.Result{Matched: false}
}
return c.dispatcher.Dispatch(ctx, req)
} }
func (c *WhatsAppNativeChannel) Send(ctx context.Context, msg bus.OutboundMessage) error { func (c *WhatsAppNativeChannel) Send(ctx context.Context, msg bus.OutboundMessage) error {

View file

@ -1,32 +1,167 @@
package commands package commands
import "github.com/sipeed/picoclaw/pkg/config" import (
"context"
"fmt"
"strings"
func BuiltinDefinitions(_ *config.Config) []Definition { "github.com/sipeed/picoclaw/pkg/config"
)
func BuiltinDefinitions(cfg *config.Config) []Definition {
return []Definition{ return []Definition{
{ {
Name: "start", Name: "start",
Description: "Start the bot", Description: "Start the bot",
Usage: "/start", Usage: "/start",
Channels: []string{"telegram", "whatsapp", "whatsapp_native"}, Channels: []string{"telegram", "whatsapp", "whatsapp_native"},
Handler: replyText("Hello! I am PicoClaw 🦞"),
}, },
{ {
Name: "help", Name: "help",
Description: "Show this help message", Description: "Show this help message",
Usage: "/help", Usage: "/help",
Channels: []string{"telegram", "whatsapp", "whatsapp_native"}, Channels: []string{"telegram", "whatsapp", "whatsapp_native"},
Handler: func(_ context.Context, req Request) error {
if req.Reply == nil {
return nil
}
defs := NewRegistry(BuiltinDefinitions(cfg)).ForChannel(req.Channel)
return req.Reply(FormatHelpMessage(defs))
},
}, },
{ {
Name: "show", Name: "show",
Description: "Show current configuration", Description: "Show current configuration",
Usage: "/show [model|channel]", Usage: "/show [model|channel]",
Channels: []string{"telegram", "whatsapp", "whatsapp_native"}, Channels: []string{"telegram"},
Handler: func(_ context.Context, req Request) error {
if req.Reply == nil {
return nil
}
if cfg == nil {
return req.Reply("Command unavailable in current context.")
}
args := commandArgs(req.Text)
if args == "" {
return req.Reply("Usage: /show [model|channel]")
}
switch args {
case "model":
return req.Reply(fmt.Sprintf(
"Current Model: %s (Provider: %s)",
cfg.Agents.Defaults.GetModelName(),
cfg.Agents.Defaults.Provider,
))
case "channel":
return req.Reply(fmt.Sprintf("Current Channel: %s", req.Channel))
default:
return req.Reply(fmt.Sprintf("Unknown parameter: %s. Try 'model' or 'channel'.", args))
}
},
}, },
{ {
Name: "list", Name: "list",
Description: "List available options", Description: "List available options",
Usage: "/list [models|channels]", Usage: "/list [models|channels]",
Channels: []string{"telegram", "whatsapp", "whatsapp_native"}, Channels: []string{"telegram"},
Handler: func(_ context.Context, req Request) error {
if req.Reply == nil {
return nil
}
if cfg == nil {
return req.Reply("Command unavailable in current context.")
}
args := commandArgs(req.Text)
if args == "" {
return req.Reply("Usage: /list [models|channels]")
}
switch args {
case "models":
provider := cfg.Agents.Defaults.Provider
if provider == "" {
provider = "configured default"
}
return req.Reply(fmt.Sprintf(
"Configured Model: %s\nProvider: %s\n\nTo change models, update config.json",
cfg.Agents.Defaults.GetModelName(),
provider,
))
case "channels":
enabled := enabledChannels(cfg)
return req.Reply(fmt.Sprintf("Enabled Channels:\n- %s", strings.Join(enabled, "\n- ")))
default:
return req.Reply(fmt.Sprintf("Unknown parameter: %s. Try 'models' or 'channels'.", args))
}
},
}, },
} }
} }
func FormatHelpMessage(defs []Definition) string {
if len(defs) == 0 {
return "No commands available."
}
lines := make([]string, 0, len(defs))
for _, def := range defs {
usage := def.Usage
if usage == "" {
usage = "/" + def.Name
}
desc := def.Description
if desc == "" {
desc = "No description"
}
lines = append(lines, fmt.Sprintf("%s - %s", usage, desc))
}
return strings.Join(lines, "\n")
}
func commandArgs(text string) string {
parts := strings.SplitN(text, " ", 2)
if len(parts) < 2 {
return ""
}
return strings.TrimSpace(parts[1])
}
func replyText(text string) Handler {
return func(_ context.Context, req Request) error {
if req.Reply == nil {
return nil
}
return req.Reply(text)
}
}
func enabledChannels(cfg *config.Config) []string {
enabled := make([]string, 0, 8)
if cfg.Channels.Telegram.Enabled {
enabled = append(enabled, "telegram")
}
if cfg.Channels.WhatsApp.Enabled {
enabled = append(enabled, "whatsapp")
}
if cfg.Channels.Feishu.Enabled {
enabled = append(enabled, "feishu")
}
if cfg.Channels.Discord.Enabled {
enabled = append(enabled, "discord")
}
if cfg.Channels.Slack.Enabled {
enabled = append(enabled, "slack")
}
if cfg.Channels.DingTalk.Enabled {
enabled = append(enabled, "dingtalk")
}
if cfg.Channels.LINE.Enabled {
enabled = append(enabled, "line")
}
if cfg.Channels.OneBot.Enabled {
enabled = append(enabled, "onebot")
}
return enabled
}

View file

@ -14,3 +14,17 @@ func TestBuiltinDefinitions_ContainsTelegramDefaults(t *testing.T) {
} }
} }
} }
func TestBuiltinDefinitions_WhatsAppOnlyHasBasicCommands(t *testing.T) {
defs := NewRegistry(BuiltinDefinitions(nil)).ForChannel("whatsapp")
names := map[string]bool{}
for _, d := range defs {
names[d.Name] = true
}
if !names["start"] || !names["help"] {
t.Fatalf("whatsapp should include start/help, got %+v", names)
}
if names["show"] || names["list"] {
t.Fatalf("whatsapp should not include show/list, got %+v", names)
}
}

View file

@ -13,6 +13,7 @@ type Request struct {
SenderID string SenderID string
Text string Text string
MessageID string MessageID string
Reply func(text string) error
} }
type Result struct { type Result struct {
@ -41,14 +42,13 @@ func NewDispatcher(reg *Registry) *Dispatcher {
} }
func (d *Dispatcher) Dispatch(ctx context.Context, req Request) Result { func (d *Dispatcher) Dispatch(ctx context.Context, req Request) Result {
token := firstToken(req.Text) cmdName, ok := parseCommandName(req.Text)
if token == "" { if !ok {
return Result{Matched: false} return Result{Matched: false}
} }
cmdName := strings.TrimPrefix(token, "/")
for _, def := range d.reg.ForChannel(req.Channel) { for _, def := range d.reg.ForChannel(req.Channel) {
if def.Name != cmdName { if def.Name != cmdName && !contains(def.Aliases, cmdName) {
continue continue
} }
if def.Handler == nil { if def.Handler == nil {
@ -68,3 +68,29 @@ func firstToken(input string) string {
} }
return parts[0] return parts[0]
} }
func parseCommandName(input string) (string, bool) {
token := firstToken(input)
if token == "" || !strings.HasPrefix(token, "/") {
return "", false
}
name := strings.TrimPrefix(token, "/")
if i := strings.Index(name, "@"); i >= 0 {
name = name[:i]
}
name = strings.TrimSpace(name)
if name == "" {
return "", false
}
return name, true
}
func contains(items []string, target string) bool {
for _, item := range items {
if item == target {
return true
}
}
return false
}

View file

@ -26,3 +26,36 @@ func TestDispatcher_MatchSlashCommand(t *testing.T) {
t.Fatalf("dispatch result = %+v, called=%v", res, called) t.Fatalf("dispatch result = %+v, called=%v", res, called)
} }
} }
func TestDispatcher_DoesNotMatchWithoutSlash(t *testing.T) {
d := NewDispatcher(NewRegistry([]Definition{{Name: "help"}}))
res := d.Dispatch(context.Background(), Request{
Channel: "telegram",
Text: "help",
})
if res.Matched {
t.Fatalf("expected unmatched for plain text, got %+v", res)
}
}
func TestDispatcher_MatchTelegramMentionSyntax(t *testing.T) {
called := false
d := NewDispatcher(NewRegistry([]Definition{
{
Name: "help",
Handler: func(context.Context, Request) error {
called = true
return nil
},
},
}))
res := d.Dispatch(context.Background(), Request{
Channel: "telegram",
Text: "/help@my_bot",
})
if !res.Matched || !res.Handled || !called || res.Err != nil {
t.Fatalf("dispatch result = %+v, called=%v", res, called)
}
}