refactor(channels): data-driven init — factories self-manage enabled check

initChannels was a manual dispatch table; adding a new channel required
a human to remember to add an if-enabled block there, separate from
writing the factory. Email was wired to the registry but never to the
dispatch table, so it was silently skipped at runtime.

- getAllFactories() added to registry so initChannels can iterate all
  registered factories in a single loop
- initChannel now skips silently when factory returns (nil, nil) instead
  of storing nil in m.channels
- Every factory closure gains an Enabled guard (return nil, nil when
  disabled); WhatsApp routing logic (UseNative) moved from initChannels
  into the two WhatsApp factories
- hiddenValues/updateKeys in manager_channel.go now handle email
  SMTPPassword/IMAPPassword so hot-reload doesn't lose credentials
- manager_sushi30_test.go: integration tests verify email channel is
  present after NewManager with Email.Enabled=true, and absent when
  disabled

Adding a channel in future requires only init.go + blank import in
gateway.go — the dispatch is automatic.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
github-actions[bot] 2026-04-12 18:15:55 +02:00
parent 3d3904e4fc
commit fd24364220
21 changed files with 132 additions and 89 deletions

View file

@ -8,6 +8,9 @@ import (
func init() { func init() {
channels.RegisterFactory("dingtalk", func(cfg *config.Config, b *bus.MessageBus) (channels.Channel, error) { channels.RegisterFactory("dingtalk", func(cfg *config.Config, b *bus.MessageBus) (channels.Channel, error) {
if !cfg.Channels.DingTalk.Enabled {
return nil, nil
}
return NewDingTalkChannel(cfg.Channels.DingTalk, b) return NewDingTalkChannel(cfg.Channels.DingTalk, b)
}) })
} }

View file

@ -9,6 +9,9 @@ import (
func init() { func init() {
channels.RegisterFactory("discord", func(cfg *config.Config, b *bus.MessageBus) (channels.Channel, error) { channels.RegisterFactory("discord", func(cfg *config.Config, b *bus.MessageBus) (channels.Channel, error) {
if !cfg.Channels.Discord.Enabled {
return nil, nil
}
ch, err := NewDiscordChannel(cfg.Channels.Discord, b) ch, err := NewDiscordChannel(cfg.Channels.Discord, b)
if err == nil { if err == nil {
ch.tts = tts.DetectTTS(cfg) ch.tts = tts.DetectTTS(cfg)

View file

@ -8,6 +8,9 @@ import (
func init() { func init() {
channels.RegisterFactory("feishu", func(cfg *config.Config, b *bus.MessageBus) (channels.Channel, error) { channels.RegisterFactory("feishu", func(cfg *config.Config, b *bus.MessageBus) (channels.Channel, error) {
if !cfg.Channels.Feishu.Enabled {
return nil, nil
}
return NewFeishuChannel(cfg.Channels.Feishu, b) return NewFeishuChannel(cfg.Channels.Feishu, b)
}) })
} }

View file

@ -8,6 +8,9 @@ import (
func init() { func init() {
channels.RegisterFactory("line", func(cfg *config.Config, b *bus.MessageBus) (channels.Channel, error) { channels.RegisterFactory("line", func(cfg *config.Config, b *bus.MessageBus) (channels.Channel, error) {
if !cfg.Channels.LINE.Enabled {
return nil, nil
}
return NewLINEChannel(cfg.Channels.LINE, b) return NewLINEChannel(cfg.Channels.LINE, b)
}) })
} }

View file

@ -8,6 +8,9 @@ import (
func init() { func init() {
channels.RegisterFactory("maixcam", func(cfg *config.Config, b *bus.MessageBus) (channels.Channel, error) { channels.RegisterFactory("maixcam", func(cfg *config.Config, b *bus.MessageBus) (channels.Channel, error) {
if !cfg.Channels.MaixCam.Enabled {
return nil, nil
}
return NewMaixCamChannel(cfg.Channels.MaixCam, b) return NewMaixCamChannel(cfg.Channels.MaixCam, b)
}) })
} }

View file

@ -329,6 +329,8 @@ func (m *Manager) initChannel(name, displayName string) {
"channel": displayName, "channel": displayName,
"error": err.Error(), "error": err.Error(),
}) })
} else if ch == nil {
// factory returned nil — channel disabled, skip silently
} else { } else {
// Inject MediaStore if channel supports it // Inject MediaStore if channel supports it
if m.mediaStore != nil { if m.mediaStore != nil {
@ -351,96 +353,11 @@ func (m *Manager) initChannel(name, displayName string) {
} }
} }
func (m *Manager) initChannels(channels *config.ChannelsConfig) error { func (m *Manager) initChannels(_ *config.ChannelsConfig) error {
logger.InfoC("channels", "Initializing channel manager") logger.InfoC("channels", "Initializing channel manager")
if channels.Telegram.Enabled && channels.Telegram.Token.String() != "" { for name := range getAllFactories() {
m.initChannel("telegram", "Telegram") m.initChannel(name, name)
}
if channels.WhatsApp.Enabled {
waCfg := channels.WhatsApp
if waCfg.UseNative {
m.initChannel("whatsapp_native", "WhatsApp Native")
} else if waCfg.BridgeURL != "" {
m.initChannel("whatsapp", "WhatsApp")
}
}
if channels.Feishu.Enabled {
m.initChannel("feishu", "Feishu")
}
if channels.Discord.Enabled && channels.Discord.Token.String() != "" {
m.initChannel("discord", "Discord")
}
if channels.MaixCam.Enabled {
m.initChannel("maixcam", "MaixCam")
}
if channels.QQ.Enabled {
m.initChannel("qq", "QQ")
}
if channels.DingTalk.Enabled && channels.DingTalk.ClientID != "" {
m.initChannel("dingtalk", "DingTalk")
}
if channels.Slack.Enabled && channels.Slack.BotToken.String() != "" {
m.initChannel("slack", "Slack")
}
if channels.Matrix.Enabled &&
m.config.Channels.Matrix.Homeserver != "" &&
m.config.Channels.Matrix.UserID != "" &&
m.config.Channels.Matrix.AccessToken.String() != "" {
m.initChannel("matrix", "Matrix")
}
if channels.LINE.Enabled && channels.LINE.ChannelAccessToken.String() != "" {
m.initChannel("line", "LINE")
}
if channels.OneBot.Enabled && channels.OneBot.WSUrl != "" {
m.initChannel("onebot", "OneBot")
}
if channels.WeCom.Enabled && channels.WeCom.BotID != "" && channels.WeCom.Secret.String() != "" {
m.initChannel("wecom", "WeCom")
}
if channels.Weixin.Enabled && channels.Weixin.Token.String() != "" {
m.initChannel("weixin", "Weixin")
}
if channels.Pico.Enabled && channels.Pico.Token.String() != "" {
m.initChannel("pico", "Pico")
}
if channels.PicoClient.Enabled && channels.PicoClient.URL != "" {
m.initChannel("pico_client", "Pico Client")
}
if channels.IRC.Enabled && channels.IRC.Server != "" {
m.initChannel("irc", "IRC")
}
if channels.VK.Enabled && channels.VK.Token.String() != "" && channels.VK.GroupID != 0 {
m.initChannel("vk", "VK")
}
if channels.TeamsWebhook.Enabled && len(channels.TeamsWebhook.Webhooks) > 0 {
hasValidTarget := false
for _, target := range channels.TeamsWebhook.Webhooks {
if target.WebhookURL.String() != "" {
hasValidTarget = true
break
}
}
if hasValidTarget {
m.initChannel("teams_webhook", "Teams Webhook")
}
} }
logger.InfoCF("channels", "Channel initialization completed", map[string]any{ logger.InfoCF("channels", "Channel initialization completed", map[string]any{

View file

@ -62,6 +62,9 @@ func hiddenValues(key string, value map[string]any, ch config.ChannelsConfig) {
value["app_secret"] = ch.Feishu.AppSecret.String() value["app_secret"] = ch.Feishu.AppSecret.String()
value["encrypt_key"] = ch.Feishu.EncryptKey.String() value["encrypt_key"] = ch.Feishu.EncryptKey.String()
value["verification_token"] = ch.Feishu.VerificationToken.String() value["verification_token"] = ch.Feishu.VerificationToken.String()
case "email":
value["smtp_password"] = ch.Email.SMTPPassword.String()
value["imap_password"] = ch.Email.IMAPPassword.String()
case "teams_webhook": case "teams_webhook":
// Expose webhook URLs for hash computation (they contain secrets) // Expose webhook URLs for hash computation (they contain secrets)
webhooks := make(map[string]string) webhooks := make(map[string]string)
@ -173,6 +176,10 @@ func updateKeys(newcfg, old *config.ChannelsConfig) {
newcfg.Feishu.EncryptKey = old.Feishu.EncryptKey newcfg.Feishu.EncryptKey = old.Feishu.EncryptKey
newcfg.Feishu.VerificationToken = old.Feishu.VerificationToken newcfg.Feishu.VerificationToken = old.Feishu.VerificationToken
} }
if newcfg.Email.Enabled {
newcfg.Email.SMTPPassword = old.Email.SMTPPassword
newcfg.Email.IMAPPassword = old.Email.IMAPPassword
}
if newcfg.TeamsWebhook.Enabled { if newcfg.TeamsWebhook.Enabled {
// Copy SecureString webhook URLs from old config // Copy SecureString webhook URLs from old config
for name, oldTarget := range old.TeamsWebhook.Webhooks { for name, oldTarget := range old.TeamsWebhook.Webhooks {

View file

@ -0,0 +1,53 @@
package channels_test
import (
"testing"
"github.com/sipeed/picoclaw/pkg/bus"
"github.com/sipeed/picoclaw/pkg/channels"
"github.com/sipeed/picoclaw/pkg/config"
_ "github.com/sipeed/picoclaw/pkg/channels/email"
)
// TestInitChannels_EmailPickedUp verifies that a channel with Enabled=true is
// actually registered in the manager after initialization. This guards against
// the class of bug where a factory is registered in init() but the manager's
// dispatch logic is never wired up.
func TestInitChannels_EmailPickedUp(t *testing.T) {
cfg := config.DefaultConfig()
cfg.Channels.Email = config.EmailConfig{
Enabled: true,
SMTPHost: "smtp.example.com",
SMTPFrom: *config.NewSecureString("bot@example.com"),
IMAPHost: "imap.example.com",
IMAPUser: *config.NewSecureString("bot@example.com"),
}
m, err := channels.NewManager(cfg, bus.NewMessageBus(), nil)
if err != nil {
t.Fatalf("NewManager returned error: %v", err)
}
ch, ok := m.GetChannel("email")
if !ok || ch == nil {
t.Error("email channel should be present in manager after NewManager with Email.Enabled=true")
}
}
// TestInitChannels_DisabledChannelAbsent verifies that a factory returning
// (nil, nil) — i.e. the channel is disabled — does not populate the manager.
func TestInitChannels_DisabledChannelAbsent(t *testing.T) {
cfg := config.DefaultConfig()
// Email disabled (default)
m, err := channels.NewManager(cfg, bus.NewMessageBus(), nil)
if err != nil {
t.Fatalf("NewManager returned error: %v", err)
}
ch, ok := m.GetChannel("email")
if ok && ch != nil {
t.Error("email channel should not be present in manager when Email.Enabled=false")
}
}

View file

@ -10,6 +10,9 @@ import (
func init() { func init() {
channels.RegisterFactory("matrix", func(cfg *config.Config, b *bus.MessageBus) (channels.Channel, error) { channels.RegisterFactory("matrix", func(cfg *config.Config, b *bus.MessageBus) (channels.Channel, error) {
if !cfg.Channels.Matrix.Enabled {
return nil, nil
}
matrixCfg := cfg.Channels.Matrix matrixCfg := cfg.Channels.Matrix
cryptoDatabasePath := matrixCfg.CryptoDatabasePath cryptoDatabasePath := matrixCfg.CryptoDatabasePath
if cryptoDatabasePath == "" { if cryptoDatabasePath == "" {

View file

@ -8,6 +8,9 @@ import (
func init() { func init() {
channels.RegisterFactory("onebot", func(cfg *config.Config, b *bus.MessageBus) (channels.Channel, error) { channels.RegisterFactory("onebot", func(cfg *config.Config, b *bus.MessageBus) (channels.Channel, error) {
if !cfg.Channels.OneBot.Enabled {
return nil, nil
}
return NewOneBotChannel(cfg.Channels.OneBot, b) return NewOneBotChannel(cfg.Channels.OneBot, b)
}) })
} }

View file

@ -8,9 +8,15 @@ import (
func init() { func init() {
channels.RegisterFactory("pico", func(cfg *config.Config, b *bus.MessageBus) (channels.Channel, error) { channels.RegisterFactory("pico", func(cfg *config.Config, b *bus.MessageBus) (channels.Channel, error) {
if !cfg.Channels.Pico.Enabled {
return nil, nil
}
return NewPicoChannel(cfg.Channels.Pico, b) return NewPicoChannel(cfg.Channels.Pico, b)
}) })
channels.RegisterFactory("pico_client", func(cfg *config.Config, b *bus.MessageBus) (channels.Channel, error) { channels.RegisterFactory("pico_client", func(cfg *config.Config, b *bus.MessageBus) (channels.Channel, error) {
if !cfg.Channels.PicoClient.Enabled {
return nil, nil
}
return NewPicoClientChannel(cfg.Channels.PicoClient, b) return NewPicoClientChannel(cfg.Channels.PicoClient, b)
}) })
} }

View file

@ -8,6 +8,9 @@ import (
func init() { func init() {
channels.RegisterFactory("qq", func(cfg *config.Config, b *bus.MessageBus) (channels.Channel, error) { channels.RegisterFactory("qq", func(cfg *config.Config, b *bus.MessageBus) (channels.Channel, error) {
if !cfg.Channels.QQ.Enabled {
return nil, nil
}
return NewQQChannel(cfg.Channels.QQ, b) return NewQQChannel(cfg.Channels.QQ, b)
}) })
} }

View file

@ -30,3 +30,14 @@ func getFactory(name string) (ChannelFactory, bool) {
f, ok := factories[name] f, ok := factories[name]
return f, ok return f, ok
} }
// getAllFactories returns a shallow copy of all registered channel factories.
func getAllFactories() map[string]ChannelFactory {
factoriesMu.RLock()
defer factoriesMu.RUnlock()
result := make(map[string]ChannelFactory, len(factories))
for k, v := range factories {
result[k] = v
}
return result
}

View file

@ -8,6 +8,9 @@ import (
func init() { func init() {
channels.RegisterFactory("slack", func(cfg *config.Config, b *bus.MessageBus) (channels.Channel, error) { channels.RegisterFactory("slack", func(cfg *config.Config, b *bus.MessageBus) (channels.Channel, error) {
if !cfg.Channels.Slack.Enabled {
return nil, nil
}
return NewSlackChannel(cfg.Channels.Slack, b) return NewSlackChannel(cfg.Channels.Slack, b)
}) })
} }

View file

@ -8,6 +8,9 @@ import (
func init() { func init() {
channels.RegisterFactory("teams_webhook", func(cfg *config.Config, b *bus.MessageBus) (channels.Channel, error) { channels.RegisterFactory("teams_webhook", func(cfg *config.Config, b *bus.MessageBus) (channels.Channel, error) {
if !cfg.Channels.TeamsWebhook.Enabled {
return nil, nil
}
return NewTeamsWebhookChannel(cfg.Channels.TeamsWebhook, b) return NewTeamsWebhookChannel(cfg.Channels.TeamsWebhook, b)
}) })
} }

View file

@ -8,6 +8,9 @@ import (
func init() { func init() {
channels.RegisterFactory("telegram", func(cfg *config.Config, b *bus.MessageBus) (channels.Channel, error) { channels.RegisterFactory("telegram", func(cfg *config.Config, b *bus.MessageBus) (channels.Channel, error) {
if !cfg.Channels.Telegram.Enabled {
return nil, nil
}
return NewTelegramChannel(cfg, b) return NewTelegramChannel(cfg, b)
}) })
} }

View file

@ -8,6 +8,9 @@ import (
func init() { func init() {
channels.RegisterFactory("vk", func(cfg *config.Config, b *bus.MessageBus) (channels.Channel, error) { channels.RegisterFactory("vk", func(cfg *config.Config, b *bus.MessageBus) (channels.Channel, error) {
if !cfg.Channels.VK.Enabled {
return nil, nil
}
return NewVKChannel(cfg, b) return NewVKChannel(cfg, b)
}) })
} }

View file

@ -8,6 +8,9 @@ import (
func init() { func init() {
channels.RegisterFactory("wecom", func(cfg *config.Config, b *bus.MessageBus) (channels.Channel, error) { channels.RegisterFactory("wecom", func(cfg *config.Config, b *bus.MessageBus) (channels.Channel, error) {
if !cfg.Channels.WeCom.Enabled {
return nil, nil
}
return NewChannel(cfg.Channels.WeCom, b) return NewChannel(cfg.Channels.WeCom, b)
}) })
} }

View file

@ -37,6 +37,9 @@ type WeixinChannel struct {
func init() { func init() {
channels.RegisterFactory("weixin", func(cfg *config.Config, bus *bus.MessageBus) (channels.Channel, error) { channels.RegisterFactory("weixin", func(cfg *config.Config, bus *bus.MessageBus) (channels.Channel, error) {
if !cfg.Channels.Weixin.Enabled {
return nil, nil
}
return NewWeixinChannel(cfg.Channels.Weixin, bus) return NewWeixinChannel(cfg.Channels.Weixin, bus)
}) })
} }

View file

@ -8,6 +8,10 @@ import (
func init() { func init() {
channels.RegisterFactory("whatsapp", func(cfg *config.Config, b *bus.MessageBus) (channels.Channel, error) { channels.RegisterFactory("whatsapp", func(cfg *config.Config, b *bus.MessageBus) (channels.Channel, error) {
return NewWhatsAppChannel(cfg.Channels.WhatsApp, b) waCfg := cfg.Channels.WhatsApp
if !waCfg.Enabled || waCfg.UseNative || waCfg.BridgeURL == "" {
return nil, nil
}
return NewWhatsAppChannel(waCfg, b)
}) })
} }

View file

@ -11,6 +11,9 @@ import (
func init() { func init() {
channels.RegisterFactory("whatsapp_native", func(cfg *config.Config, b *bus.MessageBus) (channels.Channel, error) { channels.RegisterFactory("whatsapp_native", func(cfg *config.Config, b *bus.MessageBus) (channels.Channel, error) {
waCfg := cfg.Channels.WhatsApp waCfg := cfg.Channels.WhatsApp
if !waCfg.Enabled || !waCfg.UseNative {
return nil, nil
}
storePath := waCfg.SessionStorePath storePath := waCfg.SessionStorePath
if storePath == "" { if storePath == "" {
storePath = filepath.Join(cfg.WorkspacePath(), "whatsapp") storePath = filepath.Join(cfg.WorkspacePath(), "whatsapp")