feat(feishu): add require_mention config option for group chats - #4591
feat(feishu): add require_mention config option for group chats#4591Leegenux wants to merge 5 commits into
Conversation
Add feishu.require_mention config option (default: true) to control
whether @mention is required for bot to respond in group chats.
Previously, group_policy="open" would bypass @mention check, but this
was hardcoded behavior. Now users can configure this independently:
- require_mention=true (default): Only respond when bot is @mentioned or @_all
- require_mention=false: Respond to all allowed group messages without @mention
Configuration via config.yaml:
feishu:
require_mention: false
Or via environment variable:
FEISHU_REQUIRE_MENTION=false
This matches the pattern used by Discord's require_mention setting.
Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds a Feishu/Lark configuration switch to control whether the bot requires an @mention to respond in group chats, aligning the behavior with other gateway adapters that support mention-gating.
Changes:
- Add
require_mentionto Feishu adapter settings and apply it during adapter initialization. - Bridge
feishu.require_mentionfromconfig.yamlintoFEISHU_REQUIRE_MENTION(env var takes precedence). - Update Feishu group-message routing gate to respect
require_mention(defaulting to enabled).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
hermes_cli/config.py |
Adds a default feishu.require_mention: True entry to the generated/example config. |
gateway/platforms/feishu.py |
Introduces require_mention setting and uses it to bypass mention checks when disabled. |
gateway/config.py |
Maps config.yaml feishu.require_mention to FEISHU_REQUIRE_MENTION when the env var isn’t set. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| str(extra.get("webhook_path") or os.getenv("FEISHU_WEBHOOK_PATH", _DEFAULT_WEBHOOK_PATH)).strip() | ||
| or _DEFAULT_WEBHOOK_PATH | ||
| ), | ||
| require_mention=os.getenv("FEISHU_REQUIRE_MENTION", "true").strip().lower() not in ("false", "0", "no"), | ||
| ) |
There was a problem hiding this comment.
require_mention is currently only read from FEISHU_REQUIRE_MENTION. Since _load_settings() already accepts extra (and other Feishu settings like app_id, connection_mode, webhook_* can be configured via config.extra / gateway.json), it would be more consistent to also honor extra.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 into PlatformConfig.extra) has no effect.
| 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 |
There was a problem hiding this comment.
There are existing unit tests for _should_accept_group_message() in tests/gateway/test_feishu.py, but none currently cover the new require_mention=false behavior. Please add a test case asserting that when FEISHU_REQUIRE_MENTION=false (and group policy allows the sender), a group message without mentions is accepted, and that the default behavior remains unchanged when the env var is unset.
- Allow require_mention from extra.get() (platforms.feishu.extra) with env var fallback - Add TestRequireMentionDisabled test class covering: - require_mention=false accepts messages without @mention - require_mention=true requires @mention (default) - Default behavior when env var unset - Allowlist policy still respected when require_mention=false Addresses Copilot PR NousResearch#4591 review comments. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
There was a problem hiding this comment.
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.
| require_mention=( | ||
| str(extra.get("require_mention") or os.getenv("FEISHU_REQUIRE_MENTION", "true")).strip().lower() | ||
| not in ("false", "0", "no") | ||
| ), |
There was a problem hiding this comment.
Bug: extra.get("require_mention") or os.getenv(...) treats False / 0 / empty values from extra as “not provided” and falls back to the env var/default. This makes it impossible to disable mention gating via PlatformConfig.extra (and via platforms.feishu.extra.require_mention config paths) unless an env var is also set. Use an explicit is not None / key-exists check (similar to telegram._telegram_require_mention) before falling back to the env var.
| 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)) |
There was a problem hiding this comment.
Test coverage gap: the new behavior is only tested via FEISHU_REQUIRE_MENTION env var. Since _load_settings() also reads config.extra["require_mention"], add a test that sets PlatformConfig(extra={"require_mention": False}) (with FEISHU_REQUIRE_MENTION unset) to ensure the adapter honors an explicit False value from config and doesn’t regress due to falsy or fallbacks.
Use explicit None check instead of 'or' fallback to correctly handle require_mention=False from PlatformConfig.extra. Previously the 'or' operator treated False/0/empty string as 'not provided' and fell back to env var, making it impossible to disable mention gating via config. - Add _resolve_require_mention() helper with explicit None check - Add tests for require_mention via PlatformConfig.extra (True/False) - Add test for extra precedence over env var when explicitly False Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
There was a problem hiding this comment.
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.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Allow require_mention from extra.get() (platforms.feishu.extra) with env var fallback - Add TestRequireMentionDisabled test class covering: - require_mention=false accepts messages without @mention - require_mention=true requires @mention (default) - Default behavior when env var unset - Allowlist policy still respected when require_mention=false Addresses Copilot PR NousResearch#4591 review comments. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
- Allow require_mention from extra.get() (platforms.feishu.extra) with env var fallback - Add TestRequireMentionDisabled test class covering: - require_mention=false accepts messages without @mention - require_mention=true requires @mention (default) - Default behavior when env var unset - Allowlist policy still respected when require_mention=false Addresses Copilot PR NousResearch#4591 review comments. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
- Allow require_mention from extra.get() (platforms.feishu.extra) with env var fallback - Add TestRequireMentionDisabled test class covering: - require_mention=false accepts messages without @mention - require_mention=true requires @mention (default) - Default behavior when env var unset - Allowlist policy still respected when require_mention=false Addresses Copilot PR NousResearch#4591 review comments. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
1 similar comment
|
Thanks for the Feishu configuration work. This is an automated hermes-sweeper review; current
|
Summary
Adds a
require_mentionconfig option for Feishu/Lark group chats, allowing users to control whether @mention is required for the bot to respond.Changes
require_mentionfield toFeishuAdapterSettingsdataclassFEISHU_REQUIRE_MENTIONenvironment variable (default: true)feishu.require_mentionconfig.yaml option (mapped to env var)_should_accept_group_message()to respect the settingUsage
Users can disable @mention requirement via:
Or via environment variable:
export FEISHU_REQUIRE_MENTION=falseBehavior
require_mention=true(default): Only respond when bot is @mentioned or @_allrequire_mention=false: Respond to all allowed group messagesThis matches the existing Discord
require_mentionpattern for consistency.