diff --git a/pkg/channels/dingtalk/dingtalk.go b/pkg/channels/dingtalk/dingtalk.go index 7ac2c073f..b23975d1a 100644 --- a/pkg/channels/dingtalk/dingtalk.go +++ b/pkg/channels/dingtalk/dingtalk.go @@ -6,6 +6,7 @@ package dingtalk import ( "context" "fmt" + "strings" "sync" "github.com/open-dingtalk/dingtalk-stream-sdk-go/chatbot" @@ -135,13 +136,17 @@ func (c *DingTalkChannel) onChatBotMessageReceived( ctx context.Context, data *chatbot.BotCallbackDataModel, ) ([]byte, error) { + if data == nil { + return nil, nil + } + // Extract message content from Text field - content := data.Text.Content + content := strings.TrimSpace(data.Text.Content) if content == "" { // Try to extract from Content interface{} if Text is empty if contentMap, ok := data.Content.(map[string]any); ok { if textContent, ok := contentMap["content"].(string); ok { - content = textContent + content = strings.TrimSpace(textContent) } } } @@ -150,12 +155,19 @@ func (c *DingTalkChannel) onChatBotMessageReceived( return nil, nil // Ignore empty messages } - senderID := data.SenderStaffId - senderNick := data.SenderNick - chatID := senderID - if data.ConversationType != "1" { - // For group chats - chatID = data.ConversationId + senderID := strings.TrimSpace(data.SenderStaffId) + if senderID == "" { + senderID = strings.TrimSpace(data.SenderId) + } + senderNick := strings.TrimSpace(data.SenderNick) + + chatID := strings.TrimSpace(data.ConversationId) + if chatID == "" && data.ConversationType == "1" { + // Fallback for direct chats when conversation_id is absent. + chatID = senderID + } + if chatID == "" { + return nil, nil } // Store the session webhook for this chat so we can reply later @@ -171,11 +183,15 @@ func (c *DingTalkChannel) onChatBotMessageReceived( var peer bus.Peer if data.ConversationType == "1" { - peer = bus.Peer{Kind: "direct", ID: senderID} + peerID := senderID + if peerID == "" { + peerID = chatID + } + peer = bus.Peer{Kind: "direct", ID: peerID} } else { peer = bus.Peer{Kind: "group", ID: data.ConversationId} // In group chats, apply unified group trigger filtering - respond, cleaned := c.ShouldRespondInGroup(false, content) + respond, cleaned := c.ShouldRespondInGroup(data.IsInAtList, content) if !respond { return nil, nil } @@ -189,10 +205,18 @@ func (c *DingTalkChannel) onChatBotMessageReceived( }) // Build sender info + platformID := senderID + if platformID == "" { + platformID = chatID + } + resolvedSenderID := senderID + if resolvedSenderID == "" { + resolvedSenderID = platformID + } sender := bus.SenderInfo{ Platform: "dingtalk", - PlatformID: senderID, - CanonicalID: identity.BuildCanonicalID("dingtalk", senderID), + PlatformID: platformID, + CanonicalID: identity.BuildCanonicalID("dingtalk", platformID), DisplayName: senderNick, } @@ -201,7 +225,7 @@ func (c *DingTalkChannel) onChatBotMessageReceived( } // Handle the message through the base channel - c.HandleMessage(ctx, peer, "", senderID, chatID, content, nil, metadata, sender) + c.HandleMessage(ctx, peer, "", resolvedSenderID, chatID, content, nil, metadata, sender) // Return nil to indicate we've handled the message asynchronously // The response will be sent through the message bus diff --git a/pkg/channels/dingtalk/dingtalk_test.go b/pkg/channels/dingtalk/dingtalk_test.go new file mode 100644 index 000000000..d2ab6a3cc --- /dev/null +++ b/pkg/channels/dingtalk/dingtalk_test.go @@ -0,0 +1,109 @@ +package dingtalk + +import ( + "context" + "testing" + "time" + + "github.com/open-dingtalk/dingtalk-stream-sdk-go/chatbot" + + "github.com/sipeed/picoclaw/pkg/bus" + "github.com/sipeed/picoclaw/pkg/config" +) + +func newTestDingTalkChannel(t *testing.T, cfg config.DingTalkConfig) (*DingTalkChannel, *bus.MessageBus) { + t.Helper() + + if cfg.ClientID == "" { + cfg.ClientID = "test-client-id" + } + if cfg.ClientSecret() == "" { + cfg.SetClientSecret("test-client-secret") + } + + msgBus := bus.NewMessageBus() + ch, err := NewDingTalkChannel(cfg, msgBus) + if err != nil { + t.Fatalf("new channel: %v", err) + } + return ch, msgBus +} + +func mustReceiveInbound(t *testing.T, msgBus *bus.MessageBus) bus.InboundMessage { + t.Helper() + select { + case msg := <-msgBus.InboundChan(): + return msg + case <-time.After(time.Second): + t.Fatal("expected inbound message") + return bus.InboundMessage{} + } +} + +func TestOnChatBotMessageReceived_GroupMentionOnlyUsesIsInAtList(t *testing.T) { + ch, msgBus := newTestDingTalkChannel(t, config.DingTalkConfig{ + GroupTrigger: config.GroupTriggerConfig{MentionOnly: true}, + }) + + _, err := ch.onChatBotMessageReceived(context.Background(), &chatbot.BotCallbackDataModel{ + Text: chatbot.BotCallbackDataTextModel{Content: " @bot hello "}, + SenderStaffId: "staff-123", + SenderNick: "Alice", + ConversationType: "2", + ConversationId: "group-abc", + SessionWebhook: "https://example.com/webhook", + IsInAtList: true, + }) + if err != nil { + t.Fatalf("handler returned error: %v", err) + } + + inbound := mustReceiveInbound(t, msgBus) + if inbound.Channel != "dingtalk" { + t.Fatalf("channel=%q", inbound.Channel) + } + if inbound.ChatID != "group-abc" { + t.Fatalf("chat_id=%q", inbound.ChatID) + } + if inbound.Peer.Kind != "group" || inbound.Peer.ID != "group-abc" { + t.Fatalf("peer=%+v", inbound.Peer) + } + if inbound.Content != "@bot hello" { + t.Fatalf("content=%q", inbound.Content) + } +} + +func TestOnChatBotMessageReceived_DirectFallbackSenderIDUsesConversationID(t *testing.T) { + ch, msgBus := newTestDingTalkChannel(t, config.DingTalkConfig{}) + + _, err := ch.onChatBotMessageReceived(context.Background(), &chatbot.BotCallbackDataModel{ + Text: chatbot.BotCallbackDataTextModel{Content: "ping"}, + SenderStaffId: "", + SenderId: "openid-user-42", + SenderNick: "Bob", + ConversationType: "1", + ConversationId: "conv-direct-42", + SessionWebhook: "https://example.com/webhook-direct", + }) + if err != nil { + t.Fatalf("handler returned error: %v", err) + } + + inbound := mustReceiveInbound(t, msgBus) + if inbound.ChatID != "conv-direct-42" { + t.Fatalf("chat_id=%q", inbound.ChatID) + } + if inbound.Peer.Kind != "direct" || inbound.Peer.ID != "openid-user-42" { + t.Fatalf("peer=%+v", inbound.Peer) + } + if inbound.SenderID != "dingtalk:openid-user-42" { + t.Fatalf("sender_id=%q", inbound.SenderID) + } + + if _, ok := ch.sessionWebhooks.Load("conv-direct-42"); !ok { + t.Fatal("expected session webhook keyed by conversation_id") + } + if _, ok := ch.sessionWebhooks.Load(""); ok { + t.Fatal("unexpected empty chat_id webhook key") + } +}