feat: supports scan QR code to configure feishu / lark#8191
Conversation
- Add app registration functionality for Lark and Feishu platforms, including endpoints and request handling. - Introduce polling mechanism for app registration status. - Create bot info retrieval functionality to fetch bot details after successful registration. - Enhance dashboard with new UI components for one-click QR setup and manual setup options. - Update internationalization files to support new features and actions. - Add unit tests for app registration endpoint resolution and data handling.
dosubot
Bot
added
size:XXL
Soulter
changed the title
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- Changing all platform
enabledefaults fromFalsetoTrueincore/config/default.pysignificantly alters the out-of-the-box behavior; consider keeping them disabled by default or scoping this to only the newly supported platforms (Lark / Personal WeChat) to avoid unintentionally activating unused adapters in existing deployments.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Changing all platform `enable` defaults from `False` to `True` in `core/config/default.py` significantly alters the out-of-the-box behavior; consider keeping them disabled by default or scoping this to only the newly supported platforms (Lark / Personal WeChat) to avoid unintentionally activating unused adapters in existing deployments.
## Individual Comments
### Comment 1
<location path="astrbot/core/platform/sources/lark/app_registration.py" line_range="36-43" />
<code_context>
+ elif normalized in {"lark", DEFAULT_LARK_OPEN_DOMAIN}:
+ accounts_base = "https://accounts.larksuite.com"
+ open_base = DEFAULT_LARK_OPEN_DOMAIN
+ else:
+ open_base = normalized
+ accounts_base = normalized.replace("://open.", "://accounts.", 1)
+
+ return LarkAppRegistrationEndpoints(
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Custom domain handling for accounts_base may break when domain does not follow `https://open.*` pattern.
The fallback branch assumes `normalized` contains `://open.` and derives `accounts_base` via `replace`, so for domains that don’t follow this pattern (e.g. `https://feishu.example.com`, `https://open-feishu.example.com`) `accounts_base` will incorrectly equal `open_base`. Consider either validating/enforcing the `open` vs `accounts` convention and failing fast when it’s not met, or allowing `accounts_base` to be configured explicitly instead of inferred, to avoid subtle runtime issues on custom domains.
```suggestion
elif normalized in {"lark", DEFAULT_LARK_OPEN_DOMAIN}:
accounts_base = "https://accounts.larksuite.com"
open_base = DEFAULT_LARK_OPEN_DOMAIN
else:
open_base = normalized
if "://open." in normalized:
accounts_base = normalized.replace("://open.", "://accounts.", 1)
else:
raise ValueError(
f"Unsupported custom Lark/Feishu domain {normalized!r}: "
"expected an 'open' subdomain (e.g. 'https://open.example.com') "
"so 'accounts_base' can be derived. Consider adding explicit "
"configuration for 'accounts_base' if you need other patterns."
)
return LarkAppRegistrationEndpoints(
```
</issue_to_address>
### Comment 2
<location path="tests/test_lark_app_registration.py" line_range="9" />
<code_context>
+)
+
+
+def test_resolve_app_registration_endpoints_uses_feishu_accounts_domain():
+ endpoints = resolve_app_registration_endpoints(DEFAULT_FEISHU_OPEN_DOMAIN)
+
</code_context>
<issue_to_address>
**suggestion (testing):** Extend `resolve_app_registration_endpoints` tests to cover shorthand and custom domains.
Current tests only cover the default domains. Please also add coverage for:
- Shorthand values: `"feishu"` and `"lark"`.
- Custom domains where `accounts_base` is computed via `replace("://open.", "://accounts.", 1)`.
This will guard against regressions in how we distinguish `accounts` vs `open` domains for the registration flow.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| elif normalized in {"lark", DEFAULT_LARK_OPEN_DOMAIN}: | ||
| accounts_base = "https://accounts.larksuite.com" | ||
| open_base = DEFAULT_LARK_OPEN_DOMAIN | ||
| else: | ||
| open_base = normalized | ||
| accounts_base = normalized.replace("://open.", "://accounts.", 1) | ||
|
|
||
| return LarkAppRegistrationEndpoints( |
There was a problem hiding this comment.
suggestion (bug_risk): Custom domain handling for accounts_base may break when domain does not follow https://open.* pattern.
The fallback branch assumes normalized contains ://open. and derives accounts_base via replace, so for domains that don’t follow this pattern (e.g. https://feishu.example.com, https://open-feishu.example.com) accounts_base will incorrectly equal open_base. Consider either validating/enforcing the open vs accounts convention and failing fast when it’s not met, or allowing accounts_base to be configured explicitly instead of inferred, to avoid subtle runtime issues on custom domains.
| elif normalized in {"lark", DEFAULT_LARK_OPEN_DOMAIN}: | |
| accounts_base = "https://accounts.larksuite.com" | |
| open_base = DEFAULT_LARK_OPEN_DOMAIN | |
| else: | |
| open_base = normalized | |
| accounts_base = normalized.replace("://open.", "://accounts.", 1) | |
| return LarkAppRegistrationEndpoints( | |
| elif normalized in {"lark", DEFAULT_LARK_OPEN_DOMAIN}: | |
| accounts_base = "https://accounts.larksuite.com" | |
| open_base = DEFAULT_LARK_OPEN_DOMAIN | |
| else: | |
| open_base = normalized | |
| if "://open." in normalized: | |
| accounts_base = normalized.replace("://open.", "://accounts.", 1) | |
| else: | |
| raise ValueError( | |
| f"Unsupported custom Lark/Feishu domain {normalized!r}: " | |
| "expected an 'open' subdomain (e.g. 'https://open.example.com') " | |
| "so 'accounts_base' can be derived. Consider adding explicit " | |
| "configuration for 'accounts_base' if you need other patterns." | |
| ) | |
| return LarkAppRegistrationEndpoints( |
| ) | ||
|
|
||
|
|
||
| def test_resolve_app_registration_endpoints_uses_feishu_accounts_domain(): |
There was a problem hiding this comment.
suggestion (testing): Extend resolve_app_registration_endpoints tests to cover shorthand and custom domains.
Current tests only cover the default domains. Please also add coverage for:
- Shorthand values:
"feishu"and"lark". - Custom domains where
accounts_baseis computed viareplace("://open.", "://accounts.", 1).
This will guard against regressions in how we distinguish accounts vs open domains for the registration flow.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a one-click QR setup feature for Lark and Personal WeChat platforms, allowing users to register bots directly through the dashboard. It includes new backend services for app registration and bot info retrieval, updated dashboard components, and revised documentation. Feedback highlights a critical issue where multiple platforms were accidentally enabled by default in the configuration. Additionally, reviewers recommend retaining the lark_bot_name configuration as a fallback and ensuring the im:bot permission is explicitly requested and documented, as the new automatic identification logic depends on it.
| "id": "default", | ||
| "type": "qq_official", | ||
| "enable": False, | ||
| "enable": True, |
There was a problem hiding this comment.
Enabling platforms by default in DEFAULT_CONFIG is likely unintended. This will cause the bot to attempt to initialize every platform adapter on startup, leading to errors and log spam for users who haven't provided credentials (appid, secret, etc.). This change should be reverted for all platforms in this file (QQ, OneBot, WeChat, Lark, DingTalk, Telegram, Discord, Misskey, Slack, Line, Satori, KOOK, Mattermost).
| "enable": True, | |
| "enable": False, |
| "type": "string", | ||
| "hint": "请务必填写正确，否则 @ 机器人将无法唤醒，只能通过前缀唤醒。", | ||
| }, | ||
| "discord_token": { |
There was a problem hiding this comment.
Removing lark_bot_name from the metadata prevents users from manually configuring it. Since the new automatic retrieval depends on the im:bot permission which might not be available in all environments (e.g., restricted manual setups), it is recommended to keep this field as an optional fallback.
| "discord_token": { | |
| "lark_bot_name": { | |
| "description": "飞书机器人的名字", | |
| "type": "string", | |
| "hint": "请务必填写正确，否则 @ 机器人将无法唤醒，只能通过前缀唤醒。", | |
| }, | |
| "discord_token": { |
| self.bot_name = "astrbot" | ||
| self.bot_open_id = "" |
There was a problem hiding this comment.
Relying solely on _refresh_bot_info() to set the bot name and ID introduces a dependency on the im:bot ("获取机器人信息") permission. If this permission is missing or the API call fails, the bot defaults to "astrbot", which may prevent it from correctly identifying itself in mentions if its actual name is different. It is safer to keep lark_bot_name as an optional configuration field to serve as a fallback. Additionally, ensure this new functionality is accompanied by corresponding unit tests.
| self.bot_name = "astrbot" | |
| self.bot_open_id = "" | |
| self.bot_name = platform_config.get("lark_bot_name", "astrbot") | |
| self.bot_open_id = "" |
References
- New functionality, such as handling attachments, should be accompanied by corresponding unit tests.
| > App ID 获取方式：回到 AstrBot 的 `机器人` 页，找到刚刚创建的飞书机器人，点击 `编辑`，弹出的对话框中可以看到 App ID。 | ||
| > | ||
| > ```text | ||
| > https://open.feishu.cn/app/<APP_ID>/auth?q=contact:contact.base:readonly,im:message.p2p_msg:readonly,im:message.group_at_msg:readonly,im:message:send,im:message,im:message:send_as_bot,im:resource:upload,im:resource,cardkit:card:write,im:message.group_at_msg:readonly,im:message.group_msg&op_from=openapi&token_type=tenant |
There was a problem hiding this comment.
The automatic bot info retrieval requires the im:bot permission. It should be added to this quick-auth URL to ensure users grant it when setting up permissions.
| > https://open.feishu.cn/app/<APP_ID>/auth?q=contact:contact.base:readonly,im:message.p2p_msg:readonly,im:message.group_at_msg:readonly,im:message:send,im:message,im:message:send_as_bot,im:resource:upload,im:resource,cardkit:card:write,im:message.group_at_msg:readonly,im:message.group_msg&op_from=openapi&token_type=tenant | |
| https://open.feishu.cn/app/<APP_ID>/auth?q=contact:contact.base:readonly,im:message.p2p_msg:readonly,im:message.group_at_msg:readonly,im:message:send,im:message,im:message:send_as_bot,im:resource:upload,im:resource,cardkit:card:write,im:message.group_at_msg:readonly,im:message.group_msg,im:bot&op_from=openapi&token_type=tenant |
| @@ -94,6 +119,9 @@ | |||
|
|
|||
| 如果需要在群聊里使用，请额外开通 `im:message.group_at_msg:readonly` 和 `im:message.group_msg` 权限。 | |||
There was a problem hiding this comment.
The im:bot ("获取机器人信息") permission is now required for the bot to correctly identify its name and ID. Please add it to the list of required permissions for manual creation.
| > To find the App ID, go back to AstrBot's `Bots` page, find the Lark bot you just created, click `Edit`, and check the dialog that opens. | ||
| > | ||
| > ```text | ||
| > https://open.feishu.cn/app/<APP_ID>/auth?q=contact:contact.base:readonly,im:message.p2p_msg:readonly,im:message.group_at_msg:readonly,im:message:send,im:message,im:message:send_as_bot,im:resource:upload,im:resource,cardkit:card:write,im:message.group_at_msg:readonly,im:message.group_msg&op_from=openapi&token_type=tenant |
There was a problem hiding this comment.
The automatic bot info retrieval requires the im:bot permission. It should be added to this quick-auth URL to ensure users grant it when setting up permissions.
| > https://open.feishu.cn/app/<APP_ID>/auth?q=contact:contact.base:readonly,im:message.p2p_msg:readonly,im:message.group_at_msg:readonly,im:message:send,im:message,im:message:send_as_bot,im:resource:upload,im:resource,cardkit:card:write,im:message.group_at_msg:readonly,im:message.group_msg&op_from=openapi&token_type=tenant | |
| https://open.feishu.cn/app/<APP_ID>/auth?q=contact:contact.base:readonly,im:message.p2p_msg:readonly,im:message.group_at_msg:readonly,im:message:send,im:message,im:message:send_as_bot,im:resource:upload,im:resource,cardkit:card:write,im:message.group_at_msg:readonly,im:message.group_msg,im:bot&op_from=openapi&token_type=tenant |
|
|
||
| Enter `im:resource:upload,im:resource` again to enable image upload permissions. | ||
|
|
||
| If you want to use the bot in group chats, additionally enable `im:message.group_at_msg:readonly` and `im:message.group_msg`. |
There was a problem hiding this comment.
The im:bot ("Get bot info") permission is now required for the bot to correctly identify its name and ID. Please add it to the list of required permissions for manual creation.
1 participant
Modifications / 改动点
Screenshots or Test Results / 运行截图或测试结果
Checklist / 检查清单
😊 If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
/ 如果 PR 中有新加入的功能，已经通过 Issue / 邮件等方式和作者讨论过。
👀 My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
/ 我的更改经过了良好的测试，并已在上方提供了“验证步骤”和“运行截图”。
🤓 I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in
requirements.txtandpyproject.toml./ 我确保没有引入新依赖库，或者引入了新依赖库的同时将其添加到
requirements.txt和pyproject.toml文件相应位置。😮 My changes do not introduce malicious code.
/ 我的更改没有引入恶意代码。
Summary by Sourcery
Add one-click QR-based registration flows for Lark and Personal WeChat platforms and integrate them into the dashboard and core platform adapters.
New Features:
Enhancements:
Documentation:
Tests: