Skip to content

feat(feishu): enhance channel with markdown cards, media, mentions, and editing#1000

Merged
yinwm merged 5 commits intosipeed:mainfrom
alexhoshina:main
Mar 3, 2026
Merged

feat(feishu): enhance channel with markdown cards, media, mentions, and editing#1000
yinwm merged 5 commits intosipeed:mainfrom
alexhoshina:main

Conversation

@alexhoshina
Copy link
Collaborator

@alexhoshina alexhoshina commented Mar 2, 2026

📝 Description

Upgrade the Feishu channel from basic text-only to full feature parity with Telegram/Discord: interactive card messages with markdown rendering, message editing (MessageEditor), placeholder messages (PlaceholderCapable), emoji reactions (ReactionCapable), and inbound/outbound media support (MediaSender).

Also add @mention detection with lazy bot open_id discovery, group trigger filtering with mention awareness, and multi-type inbound message parsing (text, post, image, file, audio, video).

Close: #978 #894

…nd editing

Upgrade the Feishu channel from basic text-only to full feature parity with
Telegram/Discord: interactive card messages with markdown rendering, message
editing (MessageEditor), placeholder messages (PlaceholderCapable), emoji
reactions (ReactionCapable), and inbound/outbound media support (MediaSender).

Also add @mention detection with lazy bot open_id discovery, group trigger
filtering with mention awareness, and multi-type inbound message parsing
(text, post, image, file, audio, video).
Copilot AI review requested due to automatic review settings March 2, 2026 16:50
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Enhances the Feishu channel implementation to support richer messaging capabilities (interactive markdown cards, editing/placeholders/reactions, and inbound/outbound media handling) and improves group-chat behavior with @mention awareness.

Changes:

  • Add placeholder configuration support for Feishu.
  • Implement interactive-card based sending, message editing, placeholder messages, reactions, and media send/upload.
  • Expand inbound parsing to support multiple message types, mention detection, and media downloading into the shared MediaStore flow.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File Description
pkg/config/config.go Adds Placeholder config to FeishuConfig to align with other channels’ placeholder support.
pkg/channels/feishu/common.go Adds shared helpers for card building, media key extraction, and stripping mention placeholders.
pkg/channels/feishu/feishu_64.go Main feature work: interactive card send/edit, placeholder/reaction/media support, mention-aware group filtering, and inbound media download/storage.
pkg/channels/feishu/feishu_32.go Adds stub methods to satisfy new optional channel capability interfaces on unsupported architectures.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- Remove stale "falls back to plain text" comment on Send
- Add empty ChatID validation in SendMedia to match Send
- Use messageID+fileKey as local filename to avoid write collisions
- Check allowlist before downloading inbound media to avoid wasted I/O
- Return errUnsupported consistently from all 32-bit stub methods
Copilot AI review requested due to automatic review settings March 2, 2026 17:28
@alexhoshina alexhoshina requested review from xiaket and yinwm March 2, 2026 17:31
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@xiaket
Copy link
Collaborator

xiaket commented Mar 2, 2026

1000! Such a milestone!

Copy link
Collaborator

@xiaket xiaket left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM, would benefit from some extra tests.

- Consolidate extractImageKey/extractFileKey/extractFileName into shared
  extractJSONStringField helper to reduce code duplication
- Move mentionPlaceholderRegex to package-level position after imports
- Rename feishuCfg field to config for clarity within FeishuChannel
- Replace @_user_1 heuristic with GET /open-apis/bot/v3/info API call
  at startup for reliable bot @mention detection
- Fix double close on file handle in downloadResource by removing defer
  and using explicit close in both success and error paths
- Add unit tests for common.go and feishu_64.go helpers (53 test cases)
Copilot AI review requested due to automatic review settings March 3, 2026 08:47
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +1 to +8
package feishu

import (
"encoding/json"
"testing"

larkim "github.com/larksuite/oapi-sdk-go/v3/service/im/v1"
)
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test file imports the Feishu SDK (larkim) but has no build tag, while the channel explicitly stubs out 32-bit builds because the SDK isn’t supported there. Add the same 64-bit-only build constraint used by feishu_64.go/feishu_64_test.go so go test ./... works on 32-bit architectures.

Copilot uses AI. Check for mistakes.
Comment on lines +192 to +194
return *resp.Data.MessageId, nil
}
return "", nil
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the placeholder message is successfully created but the API response doesn’t include MessageId, this returns "", nil. BaseChannel will then not record a placeholder ID, leaving an uneditable orphan placeholder message in the chat. Treat missing MessageId as an error (or attempt a fallback strategy) so placeholder sending is all-or-nothing.

Suggested change
return *resp.Data.MessageId, nil
}
return "", nil
if id := *resp.Data.MessageId; id != "" {
return id, nil
}
}
return "", fmt.Errorf("feishu placeholder: missing message ID in successful response")

Copilot uses AI. Check for mistakes.
Comment on lines +785 to +789
Body(larkim.NewCreateMessageReqBodyBuilder().
ReceiveId(chatID).
MsgType(larkim.MsgTypeFile).
Content(string(content)).
Build()).
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sendFile maps fileType to an upload file type (opus/mp4/stream) but always sends the message with MsgTypeFile. This means outbound MediaPart{Type:"audio"|"video"} will show up as a generic file message, losing type-specific behavior and diverging from how inbound audio/video are classified. Use MsgTypeAudio / MsgTypeMedia (video) when fileType indicates those types, or otherwise align the msg type with bus.MediaPart.Type.

Copilot uses AI. Check for mistakes.
Comment on lines 1 to +9
package feishu

import (
"encoding/json"
"regexp"
"strings"

larkim "github.com/larksuite/oapi-sdk-go/v3/service/im/v1"
)
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pkg/channels/feishu has a 32-bit stub (feishu_32.go) because the Feishu SDK isn’t supported on 32-bit, but this file is missing the same 64-bit-only build tag and imports the Feishu SDK (larkim). As-is, 32-bit builds will still try to compile this file and fail. Add the same //go:build amd64 || arm64 || riscv64 || mips64 || ppc64 constraint here (or split SDK-dependent helpers into a *_64.go file and provide a *_32.go stub).

Copilot uses AI. Check for mistakes.
Copy link
Collaborator

@yinwm yinwm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

This is a high-quality PR that successfully upgrades the Feishu channel to feature parity with Telegram/Discord.

✅ What's Good

  1. Complete interface implementation - , , , all properly implemented
  2. Good test coverage - 53 test cases added for helpers and content extraction
  3. Clean code organization - Helper functions like extractJSONStringField reduce duplication
  4. Correct resource management - MediaStore only records mappings, files must be preserved for downstream agent processing (not a leak!)
  5. Iterative improvements - Multiple commits show responsiveness to review feedback

💡 Minor Suggestions (non-blocking)

  • Consider extracting media-related code to feishu_media.go as feishu_64.go approaches 627 lines
  • The sendMediaPart silent error handling (returns nil on media resolve/open failure) is a design choice - might be intentional to skip corrupted media

🎯 Verdict

Approved

Great work @alexhoshina!

Copy link
Collaborator

@yinwm yinwm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

This is a high-quality PR that successfully upgrades the Feishu channel to feature parity with Telegram/Discord.

✅ What's Good

  1. Complete interface implementation - MessageEditor, PlaceholderCapable, ReactionCapable, MediaSender
  2. Good test coverage - 53 test cases added
  3. Clean code organization - Helper functions reduce duplication
  4. Correct resource management - MediaStore records mappings, files preserved for downstream processing
  5. Iterative improvements - Multiple commits show responsiveness to feedback

💡 Minor Suggestions (non-blocking)

  • Consider extracting media code to separate file as feishu_64.go approaches 627 lines
  • sendMediaPart silent error handling is a design choice worth noting

🎯 Verdict

Approved ✅ Great work!

@yinwm yinwm merged commit cf68166 into sipeed:main Mar 3, 2026
6 checks passed
hyperwd pushed a commit to hyperwd/picoclaw that referenced this pull request Mar 5, 2026
- Consolidate extractImageKey/extractFileKey/extractFileName into shared
  extractJSONStringField helper to reduce code duplication
- Move mentionPlaceholderRegex to package-level position after imports
- Rename feishuCfg field to config for clarity within FeishuChannel
- Replace @_user_1 heuristic with GET /open-apis/bot/v3/info API call
  at startup for reliable bot @mention detection
- Fix double close on file handle in downloadResource by removing defer
  and using explicit close in both success and error paths
- Add unit tests for common.go and feishu_64.go helpers (53 test cases)
hyperwd pushed a commit to hyperwd/picoclaw that referenced this pull request Mar 5, 2026
feat(feishu): enhance channel with markdown cards, media, mentions, and editing
Pluckypan pushed a commit to Pluckypan/picoclaw that referenced this pull request Mar 6, 2026
- Consolidate extractImageKey/extractFileKey/extractFileName into shared
  extractJSONStringField helper to reduce code duplication
- Move mentionPlaceholderRegex to package-level position after imports
- Rename feishuCfg field to config for clarity within FeishuChannel
- Replace @_user_1 heuristic with GET /open-apis/bot/v3/info API call
  at startup for reliable bot @mention detection
- Fix double close on file handle in downloadResource by removing defer
  and using explicit close in both success and error paths
- Add unit tests for common.go and feishu_64.go helpers (53 test cases)
Pluckypan pushed a commit to Pluckypan/picoclaw that referenced this pull request Mar 6, 2026
feat(feishu): enhance channel with markdown cards, media, mentions, and editing
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]配置飞书之后发消息没有反应

4 participants