-
Notifications
You must be signed in to change notification settings - Fork 47.4k
feat(feishu): add require_mention config option for group chats #4591
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
70d7369
a8cceec
bacc261
dc5dac6
e6c31fd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -270,6 +270,7 @@ class FeishuAdapterSettings: | |
| webhook_host: str | ||
| webhook_port: int | ||
| webhook_path: str | ||
| require_mention: bool # Require @mention in group chats (default: true) | ||
|
|
||
|
|
||
| @dataclass | ||
|
|
@@ -972,6 +973,25 @@ def __init__(self, config: PlatformConfig): | |
| self._pending_media_batch_tasks = self._media_batch_state.tasks | ||
| self._load_seen_message_ids() | ||
|
|
||
| @staticmethod | ||
| def _resolve_require_mention(extra: Dict[str, Any]) -> bool: | ||
| """Resolve require_mention setting from extra config or env var. | ||
|
|
||
| Uses explicit None check to handle falsy values correctly: | ||
| - If extra["require_mention"] exists, use it (even if False) | ||
| - Otherwise fallback to FEISHU_REQUIRE_MENTION env var (default: true) | ||
| """ | ||
| configured = extra.get("require_mention") | ||
| if configured is not None: | ||
| if isinstance(configured, str): | ||
| value = configured.strip().lower() | ||
| # Default-true semantics: only explicit false-like values disable mention gating. | ||
| return value not in ("false", "0", "no", "off") | ||
| return bool(configured) | ||
| env_value = os.getenv("FEISHU_REQUIRE_MENTION", "true").strip().lower() | ||
| # Default-true semantics for env var as well. | ||
| return env_value not in ("false", "0", "no", "off") | ||
|
|
||
| @staticmethod | ||
| def _load_settings(extra: Dict[str, Any]) -> FeishuAdapterSettings: | ||
| return FeishuAdapterSettings( | ||
|
|
@@ -1020,6 +1040,7 @@ def _load_settings(extra: Dict[str, Any]) -> FeishuAdapterSettings: | |
| str(extra.get("webhook_path") or os.getenv("FEISHU_WEBHOOK_PATH", _DEFAULT_WEBHOOK_PATH)).strip() | ||
| or _DEFAULT_WEBHOOK_PATH | ||
| ), | ||
| require_mention=FeishuAdapter._resolve_require_mention(extra), | ||
| ) | ||
|
|
||
| def _apply_settings(self, settings: FeishuAdapterSettings) -> None: | ||
|
|
@@ -1042,6 +1063,7 @@ def _apply_settings(self, settings: FeishuAdapterSettings) -> None: | |
| self._webhook_host = settings.webhook_host | ||
| self._webhook_port = settings.webhook_port | ||
| self._webhook_path = settings.webhook_path | ||
| self._require_mention = settings.require_mention | ||
|
|
||
| def _build_event_handler(self) -> Any: | ||
| if EventDispatcherHandler is None: | ||
|
|
@@ -2665,9 +2687,17 @@ def _allow_group_message(self, sender_id: Any) -> bool: | |
| return bool(sender_open_id and sender_open_id in self._allowed_group_users) | ||
|
|
||
| def _should_accept_group_message(self, message: Any, sender_id: Any) -> bool: | ||
| """Require an explicit @mention before group messages enter the agent.""" | ||
| """Check if a group message should be routed to the agent. | ||
|
|
||
| Uses require_mention setting to control whether @mention is needed: | ||
| - require_mention=true (default): Only route if bot is @mentioned or @_all | ||
| - require_mention=false: Route all allowed group messages without @mention | ||
| """ | ||
| if not self._allow_group_message(sender_id): | ||
| return False | ||
| # If require_mention is disabled, accept all allowed group messages | ||
| if not self._require_mention: | ||
| return True | ||
|
Comment on lines
2696
to
+2700
|
||
| # @_all is Feishu's @everyone placeholder — always route to the bot. | ||
| raw_content = getattr(message, "content", "") or "" | ||
| if "@_all" in raw_content: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2474,6 +2474,105 @@ def test_at_all_still_requires_policy_gate(self): | |
| self.assertTrue(adapter._should_accept_group_message(message, allowed_sender)) | ||
|
|
||
|
|
||
| class TestRequireMentionDisabled(unittest.TestCase): | ||
| """Tests for require_mention=false behavior in group chats.""" | ||
|
|
||
| @patch.dict( | ||
| os.environ, | ||
| {"FEISHU_GROUP_POLICY": "open", "FEISHU_REQUIRE_MENTION": "false"}, | ||
| clear=True, | ||
| ) | ||
| def test_group_message_accepted_without_mention_when_require_mention_false(self): | ||
| """When require_mention=false, group messages are accepted without @mention.""" | ||
| from gateway.config import PlatformConfig | ||
| from gateway.platforms.feishu import FeishuAdapter | ||
|
|
||
| adapter = FeishuAdapter(PlatformConfig()) | ||
| message = SimpleNamespace(content='{"text":"hello"}', mentions=[]) | ||
| sender_id = SimpleNamespace(open_id="ou_any", user_id=None) | ||
| self.assertTrue(adapter._should_accept_group_message(message, sender_id)) | ||
|
Comment on lines
+2485
to
+2493
|
||
|
|
||
| @patch.dict( | ||
| os.environ, | ||
| {"FEISHU_GROUP_POLICY": "open", "FEISHU_REQUIRE_MENTION": "true"}, | ||
| clear=True, | ||
| ) | ||
| def test_group_message_rejected_without_mention_when_require_mention_true(self): | ||
| """Default behavior: require_mention=true requires @mention.""" | ||
| from gateway.config import PlatformConfig | ||
| from gateway.platforms.feishu import FeishuAdapter | ||
|
|
||
| adapter = FeishuAdapter(PlatformConfig()) | ||
| message = SimpleNamespace(content='{"text":"hello"}', mentions=[]) | ||
| sender_id = SimpleNamespace(open_id="ou_any", user_id=None) | ||
| self.assertFalse(adapter._should_accept_group_message(message, sender_id)) | ||
|
|
||
| @patch.dict(os.environ, {"FEISHU_GROUP_POLICY": "open"}, clear=True) | ||
| def test_require_mention_defaults_to_true_when_unset(self): | ||
| """When FEISHU_REQUIRE_MENTION is unset, default is true (require mention).""" | ||
| from gateway.config import PlatformConfig | ||
| from gateway.platforms.feishu import FeishuAdapter | ||
|
|
||
| adapter = FeishuAdapter(PlatformConfig()) | ||
| message = SimpleNamespace(content='{"text":"hello"}', mentions=[]) | ||
| sender_id = SimpleNamespace(open_id="ou_any", user_id=None) | ||
| self.assertFalse(adapter._should_accept_group_message(message, sender_id)) | ||
|
|
||
| @patch.dict( | ||
| os.environ, | ||
| {"FEISHU_GROUP_POLICY": "allowlist", "FEISHU_ALLOWED_USERS": "ou_allowed", "FEISHU_REQUIRE_MENTION": "false"}, | ||
| clear=True, | ||
| ) | ||
| def test_require_mention_false_still_respects_allowlist_policy(self): | ||
| """require_mention=false bypasses mention check but NOT allowlist policy.""" | ||
| from gateway.config import PlatformConfig | ||
| from gateway.platforms.feishu import FeishuAdapter | ||
|
|
||
| adapter = FeishuAdapter(PlatformConfig()) | ||
| message = SimpleNamespace(content='{"text":"hello"}', mentions=[]) | ||
| # Allowlisted user — should pass. | ||
| allowed_sender = SimpleNamespace(open_id="ou_allowed", user_id=None) | ||
| self.assertTrue(adapter._should_accept_group_message(message, allowed_sender)) | ||
| # Non-allowlisted user — should be blocked. | ||
| blocked_sender = SimpleNamespace(open_id="ou_blocked", user_id=None) | ||
| self.assertFalse(adapter._should_accept_group_message(message, blocked_sender)) | ||
|
|
||
| @patch.dict(os.environ, {"FEISHU_GROUP_POLICY": "open"}, clear=True) | ||
| def test_require_mention_false_from_platform_config_extra(self): | ||
| """require_mention=False via PlatformConfig.extra should be honored (no falsy fallback bug).""" | ||
| from gateway.config import PlatformConfig | ||
| from gateway.platforms.feishu import FeishuAdapter | ||
|
|
||
| # Explicit False via extra should work even without env var | ||
| adapter = FeishuAdapter(PlatformConfig(extra={"require_mention": False})) | ||
| message = SimpleNamespace(content='{"text":"hello"}', mentions=[]) | ||
| sender_id = SimpleNamespace(open_id="ou_any", user_id=None) | ||
| self.assertTrue(adapter._should_accept_group_message(message, sender_id)) | ||
|
|
||
| @patch.dict(os.environ, {"FEISHU_GROUP_POLICY": "open"}, clear=True) | ||
| def test_require_mention_true_from_platform_config_extra(self): | ||
| """require_mention=True via PlatformConfig.extra should be honored.""" | ||
| from gateway.config import PlatformConfig | ||
| from gateway.platforms.feishu import FeishuAdapter | ||
|
|
||
| adapter = FeishuAdapter(PlatformConfig(extra={"require_mention": True})) | ||
| message = SimpleNamespace(content='{"text":"hello"}', mentions=[]) | ||
| sender_id = SimpleNamespace(open_id="ou_any", user_id=None) | ||
| self.assertFalse(adapter._should_accept_group_message(message, sender_id)) | ||
|
|
||
| @patch.dict(os.environ, {"FEISHU_GROUP_POLICY": "open", "FEISHU_REQUIRE_MENTION": "true"}, clear=True) | ||
| def test_require_mention_env_var_overridden_by_explicit_false_in_extra(self): | ||
| """extra.get("require_mention") takes precedence over env var (even when False).""" | ||
| from gateway.config import PlatformConfig | ||
| from gateway.platforms.feishu import FeishuAdapter | ||
|
|
||
| # Env var says true, but extra explicitly says False — should use False | ||
| adapter = FeishuAdapter(PlatformConfig(extra={"require_mention": False})) | ||
| message = SimpleNamespace(content='{"text":"hello"}', mentions=[]) | ||
| sender_id = SimpleNamespace(open_id="ou_any", user_id=None) | ||
| self.assertTrue(adapter._should_accept_group_message(message, sender_id)) | ||
|
|
||
|
|
||
| @unittest.skipUnless(_HAS_LARK_OAPI, "lark-oapi not installed") | ||
| class TestSenderNameResolution(unittest.TestCase): | ||
| """Tests for _resolve_sender_name_from_api.""" | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
require_mentionis currently only read fromFEISHU_REQUIRE_MENTION. Since_load_settings()already acceptsextra(and other Feishu settings likeapp_id,connection_mode,webhook_*can be configured viaconfig.extra/gateway.json), it would be more consistent to also honorextra.get("require_mention")when present (e.g., bool or truthy/falsey string) and fall back to the env var otherwise. As-is,platforms.feishu.extra.require_mention(including values bridged intoPlatformConfig.extra) has no effect.