refactor(channels): Implement DingTalk client with card messaging cap…#1251
Closed
zhaoyunxing92 wants to merge 8 commits intosipeed:mainfrom
Closed
refactor(channels): Implement DingTalk client with card messaging cap…#1251zhaoyunxing92 wants to merge 8 commits intosipeed:mainfrom
zhaoyunxing92 wants to merge 8 commits intosipeed:mainfrom
Conversation
|
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a new DingTalk API client and integrates “card” (rich/updatable) messaging into the DingTalk channel, making card delivery configurable via new DingTalk config fields.
Changes:
- Added a new
pkg/channels/dingtalkHTTP client for token management, card creation/delivery, and card streaming updates. - Updated the DingTalk channel send path to prefer card replies when configured, with fallback to direct replies.
- Extended
DingTalkConfigwithRobotCode,CardTemplateID, andCardTemplateContentKey, plus client option helpers.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/config/config.go | Adds new DingTalk configuration fields for card messaging and robot code. |
| pkg/channels/dingtalk/options.go | Introduces client option setters for card template and robot code. |
| pkg/channels/dingtalk/dingtalk.go | Integrates the new client and implements card-vs-direct reply routing. |
| pkg/channels/dingtalk/client.go | New DingTalk HTTP client implementing token, card, and messaging APIs. |
| go.mod | Promotes github.com/ergochat/irc-go to a direct dependency. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
sipeed-bot
bot
added
type: enhancement
This was referenced Mar 9, 2026
- add mutex to Client to protect token/expires from concurrent refresh - pass context through GetToken so cancellation/deadlines propagate - rename Id/Ids identifiers to ID/IDs per Go conventions - fall back to direct reply instead of dropping message on card delivery failure - use Load instead of LoadAndDelete for sessionWebhooks and cardInstanceIDs to support multiple outbound messages per turn
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
alexhoshina
requested changes
Mar 12, 2026
alexhoshina
requested changes
Mar 12, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| // Try to create and deliver card (optional feature) | ||
| // If it fails, log the error but continue with normal message handling | ||
| if cardID, err := c.tryCardCreateAndDeliver(ctx, data); err != nil { | ||
| logger.WarnC("dingtalk", "Failed to create or deliver card, falling back to direct reply") |
Comment on lines
248
to
250
| if err != nil { | ||
| return fmt.Errorf("dingtalk send: %w", channels.ErrTemporary) | ||
| } |
Comment on lines
+225
to
+237
| func (c *Client) buildSendMessages(msgType MessageType, content string) string { | ||
| data := map[string]any{} | ||
| switch msgType { | ||
| case Markdown: | ||
| data = map[string]any{ | ||
| "title": "PicoClaw", | ||
| "text": content, | ||
| } | ||
| case Text: | ||
| data = map[string]any{ | ||
| "content": content, | ||
| } | ||
| } |
Comment on lines
+259
to
+280
| if body != nil { | ||
| data, _ := json.Marshal(body) | ||
| req, err = http.NewRequestWithContext(ctx, method, url, bytes.NewReader(data)) | ||
| } else { | ||
| req, err = http.NewRequestWithContext(ctx, method, url, nil) | ||
| } | ||
| if err != nil { | ||
| return err | ||
| } | ||
| req.Header.Set("Content-Type", "application/json") | ||
| if token != "" { | ||
| req.Header.Set("X-Acs-Dingtalk-Access-Token", token) | ||
| } | ||
| res, err = hc.Do(req) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| defer res.Body.Close() | ||
| data, err := io.ReadAll(res.Body) | ||
| if res.StatusCode != http.StatusOK || err != nil { | ||
| return fmt.Errorf("API request failed:\n Status: %d\n Body: %s", res.StatusCode, string(data)) | ||
| } |
Comment on lines
+119
to
+128
| // Check if we have a card instance ID for this chat (indicating we can send a card reply) | ||
| cardInstanceIDRaw, ok := c.cardInstanceIDs.LoadAndDelete(msg.ChatID) | ||
| if !ok { | ||
| return fmt.Errorf("no session_webhook found for chat %s, cannot send message", msg.ChatID) | ||
| return c.SendDirectReply(ctx, msg) | ||
| } | ||
|
|
||
| sessionWebhook, ok := sessionWebhookRaw.(string) | ||
| cardInstanceID, ok := cardInstanceIDRaw.(string) | ||
| if !ok { | ||
| return fmt.Errorf("invalid session_webhook type for chat %s", msg.ChatID) | ||
| return c.SendDirectReply(ctx, msg) | ||
| } | ||
|
|
||
| logger.DebugCF("dingtalk", "Sending message", map[string]any{ | ||
| "chat_id": msg.ChatID, | ||
| "preview": utils.Truncate(msg.Content, 100), | ||
| }) | ||
|
|
||
| // Use the session webhook to send the reply | ||
| return c.SendDirectReply(ctx, sessionWebhook, msg.Content) | ||
| return c.SendCardReply(ctx, cardInstanceID, msg.Content) |
| // Store the session webhook for this chat so we can reply later | ||
| c.sessionWebhooks.Store(chatID, data.SessionWebhook) | ||
| } else { | ||
| chatID = data.MsgId |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
+119
to
+128
| // Check if we have a card instance ID for this chat (indicating we can send a card reply) | ||
| cardInstanceIDRaw, ok := c.cardInstanceIDs.LoadAndDelete(msg.ChatID) | ||
| if !ok { | ||
| return fmt.Errorf("no session_webhook found for chat %s, cannot send message", msg.ChatID) | ||
| return c.SendDirectReply(ctx, msg) | ||
| } | ||
|
|
||
| sessionWebhook, ok := sessionWebhookRaw.(string) | ||
| cardInstanceID, ok := cardInstanceIDRaw.(string) | ||
| if !ok { | ||
| return fmt.Errorf("invalid session_webhook type for chat %s", msg.ChatID) | ||
| return c.SendDirectReply(ctx, msg) | ||
| } | ||
|
|
||
| logger.DebugCF("dingtalk", "Sending message", map[string]any{ | ||
| "chat_id": msg.ChatID, | ||
| "preview": utils.Truncate(msg.Content, 100), | ||
| }) | ||
|
|
||
| // Use the session webhook to send the reply | ||
| return c.SendDirectReply(ctx, sessionWebhook, msg.Content) | ||
| return c.SendCardReply(ctx, cardInstanceID, msg.Content) |
Comment on lines
+209
to
+210
| chatID = data.MsgId | ||
| c.cardInstanceIDs.Store(chatID, cardID) |
Collaborator
There was a problem hiding this comment.
I have the same opinion as Copilot #1251 (comment)
Comment on lines
+113
to
+119
| id, err := uuid.NewUUID() | ||
| if err != nil { | ||
| return err | ||
| } | ||
| body := map[string]any{ | ||
| "outTrackId": cardInstanceID, | ||
| "guid": id.String(), |
| } | ||
| ) | ||
|
|
||
| id, err := uuid.NewUUID() |
Comment on lines
+274
to
+278
| return err | ||
| } | ||
| defer res.Body.Close() | ||
| data, err := io.ReadAll(res.Body) | ||
| if res.StatusCode != http.StatusOK || err != nil { |
| } | ||
|
|
||
| // Handle the message through the base channel | ||
| c.HandleMessage(ctx, peer, "", senderID, chatID, content, nil, metadata, sender) |
| if c.config.CardTemplateID != "" { | ||
| // If it fails, log the error but continue with normal message handling | ||
| if cardID, err := c.tryCardCreateAndDeliver(ctx, data); err != nil { | ||
| logger.WarnC("dingtalk", "Failed to create or deliver card, falling back to direct reply") |
alexhoshina
reviewed
Mar 18, 2026
Comment on lines
+209
to
+210
| chatID = data.MsgId | ||
| c.cardInstanceIDs.Store(chatID, cardID) |
Collaborator
There was a problem hiding this comment.
I have the same opinion as Copilot #1251 (comment)
Comment on lines
+224
to
+225
| c.cardInstanceIDs.Store(chatID, cardID) | ||
| c.sessionWebhooks.Store(chatID, data.SessionWebhook) |
Collaborator
There was a problem hiding this comment.
cardInstanceIDs and sessionWebhooks will grow indefinitely during prolonged operation, as these two maps are only cleared when Stop() is called
|
@zhaoyunxing92 Hi! This PR has had no activity for over 2 weeks, so I'm closing it for now to keep things tidy. If it's still relevant, feel free to reopen it anytime and we'll pick it back up. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces a new, feature-rich Dingtalk API client and integrates support for sending "card" messages (rich, updatable messages) in addition to the existing direct reply method. The changes include adding a new client implementation, updating the Dingtalk channel to use this client for card delivery, and extending configuration options to support card templates and robot codes.
Key changes:
Dingtalk card messaging support:
Clientimplementation inpkg/channels/dingtalk/client.gowith methods for authentication, sending batch messages, creating and delivering cards, updating card content, and sending private messages. This enables richer, updatable card-based interactions in Dingtalk.DingTalkChannelstruct and initialization, enabling card-based replies when a card template is configured. [1] [2]DingTalkChannelto create and deliver a card when handling incoming messages, store the card instance ID, and use it for subsequent replies. Falls back to direct replies if card delivery is not available. [1] [2] [3] [4]Configuration and options:
DingTalkConfigto includeRobotCode,CardTemplateID, andCardTemplateContentKeyfields, making card messaging configurable via environment variables or JSON.ClientOptionfunctions inpkg/channels/dingtalk/options.goto support flexible client configuration for robot code and card template details.📝 Description
🗣️ Type of Change
🤖 AI Code Generation
🔗 Related Issue
📚 Technical Context (Skip for Docs)
🧪 Test Environment
📸 Evidence (Optional)
Click to view Logs/Screenshots
☑️ Checklist