Skip to content

feat(feishu): bind credentials through participants#204

Merged
RussellLuo merged 8 commits into
OpenCSGs:mainfrom
GatewayJ:feat/feishu-participant-bind
Jun 12, 2026
Merged

feat(feishu): bind credentials through participants#204
RussellLuo merged 8 commits into
OpenCSGs:mainfrom
GatewayJ:feat/feishu-participant-bind

Conversation

@GatewayJ

@GatewayJ GatewayJ commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Bind Feishu credentials through participants and remove the dedicated Feishu config store path.
  • Add participant binding CLI/API/client support and Feishu participant provider wiring.
  • Update Feishu setup docs, embedded skills, and runtime design notes.

@GatewayJ GatewayJ requested a review from RussellLuo June 12, 2026 08:24
Comment thread cli/participant/bind.go Outdated
type bindResult struct {
Status string `json:"status"`
Channel string `json:"channel"`
FeishuKind string `json:"feishu_kind"`

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Isn't FeishuKind too specific for a general structure? Propose ParticipantType or so to align with type in Participant.


func (s *Service) appConfigForMentionLocked(mention string) (AppConfig, error) {
if app, ok := s.apps[mention]; ok {
if app, ok := s.appConfigByIDLocked(mention); ok {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The new bind flow stores two different Feishu participant shapes: agent participants carry app credentials in channel_app_config, while the admin human carries only channel_user_kind=open_id and channel_user_ref=ou_xxx. Feishu can mention a human by open_id; the sender app config is required to send the message, but the human target should not need its own app config.

This path still treats every mention target as app-backed: SendMessage resolves mention_id through appConfigForMentionLocked, and defaultSendMessage then fetches a bot open_id from that target app. That works for agent participants, but mention_id=admin fails before send because the human participant has no app config.

Proposal: resolve mention targets through the participant model first. For type=human + channel_user_kind=open_id, use channel_user_ref as the Feishu at-user open_id. For type=agent, keep resolving the bot open_id from its app config. Please add focused coverage for agent -> admin human mention and keep the existing agent -> agent mention behavior.

h.handleParticipantEventsStream(w, r, participantID)
case "feishu":
h.handleFeishuParticipantEvents(w, r, h.resolveFeishuParticipantTargetID(id))
h.handleFeishuParticipantEvents(w, r, id, h.resolveFeishuParticipantTargetID(id))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This event target set includes the participant id and channel_user_ref, but app-backed Feishu agents have no channel_user_ref. Real Feishu mention events normally carry the bot open_id, so /channels/feishu/participants/{id}/events may miss real @agent events unless the target set also includes the resolved bot open_id or inbound Feishu mentions are normalized to participant IDs before filtering.

@GatewayJ GatewayJ force-pushed the feat/feishu-participant-bind branch from 850a6ec to 6dc8a28 Compare June 12, 2026 09:28
@@ -0,0 +1,184 @@
package feishuparticipant

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This package does not introduce a new channel; it implements a participant-backed provider for the existing Feishu channel. Keeping it under internal/channel/feishuparticipant makes the directory layout read like a new channel module. Would it be clearer to move this into internal/channel/feishu, for example as participant_provider.go or participant.go, so the package structure matches the actual responsibility?

@RussellLuo

Copy link
Copy Markdown
Collaborator

This now masks app_secret as the string present, which is fine locally, but the masking convention looks like an API boundary concern rather than a one-off handler detail. Since the same redaction semantics may need to stay consistent across API responses, CLI output, docs, and tests, would it be better to centralize this representation behind a shared constant or helper instead of hardcoding the literal here?

@RussellLuo RussellLuo merged commit 135ebcd into OpenCSGs:main Jun 12, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants