fix(feishu): gate group pairing by target bot#1291
Merged
clark-cant merged 2 commits intoJun 27, 2026
Conversation
added 2 commits
June 27, 2026 23:48
- Previously, when bot_open_id was empty, ALL mentions were treated as bot mentions - This caused multiple agents in same group to all respond to any mention - Now requires bot_open_id to be set AND match exactly for mentionedBot=true - Fixes issue where CPPAI PM would respond even when other agent was mentioned
clark-cant
approved these changes
Jun 27, 2026
clark-cant
left a comment
Contributor
There was a problem hiding this comment.
Maintainer Review
Summary: Gates Feishu group pairing by target bot open_id, drops non-target mentions before media/policy/pairing/routing, adds app_id guard on WS/webhook dispatch, and re-checks pairing state before sending pair codes. 8 files, +728/−56.
Risk level: Medium — touches channel routing, pairing flow, and bot identity fail-closed behavior.
Mandatory gates:
- Duplicate/prior implementation: clear — no duplicate PR or issue for this Feishu multi-agent group fix
- Project standards: passed — scoped to
internal/channels/feishu/plus changelog; no UI/API/CLI parity concerns - Strategic necessity: clear value — multi-agent Lark groups currently have all bots responding to mentions not targeting them
- CI/checks: green (go ✅, web ✅, release-versioning ✅)
Findings:
Positive signals:
- Fail-closed bot identity change is correct and a security improvement: unknown
botOpenIDno longer treats any mention as bot mention - Non-target mention skip is correctly placed before media resolution, policy, pairing, and agent routing
canRecordUnmentionedGroupMessagecleanly separates history-recording eligibility from full policy acceptanceIsPairedpre-check insendPairingReplyprevents duplicate pairing requests — good defensive programming- App_id guard in
shouldProcessMessageEventprevents cross-app event injection - Excellent test coverage: 6 new tests with realistic mock infrastructure covering non-target mentions (requireMention true/false), target mention pairing, already-paired groups, and app_id match/mismatch
- Structured logging is consistent and includes channel/chat/tenant context throughout
Suggestion:
canRecordUnmentionedGroupMessageduplicates somecheckGroupPolicylogic. Minor — the separation of concerns (policy check vs history-recording eligibility) is reasonable and the shared switch structure is clear enough.
Verdict: APPROVE
Posted by github-maintain
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.
Summary
open_idmatches before treating a mention as targeting the current botheader.app_idto avoid cross-app channel handlingTest plan