fix: 修复 aiocqhttp 管理员角色字段未透传的问题#5980
Conversation
There was a problem hiding this comment.
Hey - 我这边发现了 1 个问题,并给出了一些整体性的反馈:
- 允许的角色取值("admin"、"owner"、"member")在多处被硬编码;建议将它们集中到共享的常量或枚举中,以避免发生不一致,并让后续修改更安全。
- 在
aiocqhttp_message_event.get_group中,role被直接透传,而没有经过_convert_handle_message_event中所使用的校验;建议在这里也应用相同的归一化/校验逻辑,这样缓存成员就能与实时事件保持一致的行为。 AstrMessageEvent.role的文档字符串被改成了只有英文,但附近的文档字符串仍是中文;建议保持与周围注释在语言/风格上的一致性,以提升可读性。
给 AI 代理的提示词
Please address the comments from this code review:
## Overall Comments
- The allowed role values ("admin", "owner", "member") are hardcoded in multiple places; consider centralizing them into a shared constant or enum to avoid divergence and make future changes safer.
- In `aiocqhttp_message_event.get_group`, `role` is passed through without the validation used in `_convert_handle_message_event`; consider applying the same normalization/validation so cached members behave consistently with live events.
- The `AstrMessageEvent.role` docstring was changed to English only while nearby docstrings remain Chinese; consider keeping the language/style consistent with surrounding comments for readability.
## Individual Comments
### Comment 1
<location path="astrbot/core/platform/sources/aiocqhttp/aiocqhttp_message_event.py" line_range="244-247" />
<code_context>
MessageMember(
user_id=member["user_id"],
nickname=member.get("nickname") or member.get("card"),
+ role=member.get("role"),
)
for member in members
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Validate or normalize `member["role"]` to match the expected role set.
This path now passes `member.get("role")` through without the constraints used for `sender_role` ("admin"/"owner"/"member"/None). If CQHTTP ever returns an unexpected role string, it will bypass the checks in `AstrMessageEvent` and introduce inconsistent role values. Please reuse the same validation/normalization logic here so all sources produce a consistent, validated role set.
Suggested implementation:
```python
MessageMember(
user_id=member["user_id"],
nickname=member.get("nickname") or member.get("card"),
role=normalize_role(member.get("role")),
)
for member in members
],
```
1. Ensure there is a `normalize_role` (or similarly named) function in this module that implements the same validation/normalization logic currently used for `sender_role` (e.g. mapping only `"admin"`, `"owner"`, `"member"` to themselves and returning `None` or a default for any other value).
2. If the `sender_role` normalization is currently inline (e.g. `if role not in (...)`), extract that logic into `normalize_role(role: Optional[str]) -> Optional[str]` and replace the inline normalization used for `sender_role` with a call to `normalize_role` as well, to keep the behavior consistent between senders and members.
</issue_to_address>帮我变得更有用!请对每条评论点 👍 或 👎,我会根据你的反馈改进后续的代码审查。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- The allowed role values ("admin", "owner", "member") are hardcoded in multiple places; consider centralizing them into a shared constant or enum to avoid divergence and make future changes safer.
- In
aiocqhttp_message_event.get_group,roleis passed through without the validation used in_convert_handle_message_event; consider applying the same normalization/validation so cached members behave consistently with live events. - The
AstrMessageEvent.roledocstring was changed to English only while nearby docstrings remain Chinese; consider keeping the language/style consistent with surrounding comments for readability.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The allowed role values ("admin", "owner", "member") are hardcoded in multiple places; consider centralizing them into a shared constant or enum to avoid divergence and make future changes safer.
- In `aiocqhttp_message_event.get_group`, `role` is passed through without the validation used in `_convert_handle_message_event`; consider applying the same normalization/validation so cached members behave consistently with live events.
- The `AstrMessageEvent.role` docstring was changed to English only while nearby docstrings remain Chinese; consider keeping the language/style consistent with surrounding comments for readability.
## Individual Comments
### Comment 1
<location path="astrbot/core/platform/sources/aiocqhttp/aiocqhttp_message_event.py" line_range="244-247" />
<code_context>
MessageMember(
user_id=member["user_id"],
nickname=member.get("nickname") or member.get("card"),
+ role=member.get("role"),
)
for member in members
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Validate or normalize `member["role"]` to match the expected role set.
This path now passes `member.get("role")` through without the constraints used for `sender_role` ("admin"/"owner"/"member"/None). If CQHTTP ever returns an unexpected role string, it will bypass the checks in `AstrMessageEvent` and introduce inconsistent role values. Please reuse the same validation/normalization logic here so all sources produce a consistent, validated role set.
Suggested implementation:
```python
MessageMember(
user_id=member["user_id"],
nickname=member.get("nickname") or member.get("card"),
role=normalize_role(member.get("role")),
)
for member in members
],
```
1. Ensure there is a `normalize_role` (or similarly named) function in this module that implements the same validation/normalization logic currently used for `sender_role` (e.g. mapping only `"admin"`, `"owner"`, `"member"` to themselves and returning `None` or a default for any other value).
2. If the `sender_role` normalization is currently inline (e.g. `if role not in (...)`), extract that logic into `normalize_role(role: Optional[str]) -> Optional[str]` and replace the inline normalization used for `sender_role` with a call to `normalize_role` as well, to keep the behavior consistent between senders and members.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
astrbot/core/platform/sources/aiocqhttp/aiocqhttp_message_event.py
Outdated
Show resolved
Hide resolved
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 此拉取请求旨在解决 Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
修复 aiocqhttp / NapCat 群消息场景下发送者角色未正确透传到 AstrBot 事件对象的问题,导致群管理员和群主在插件侧被误判为普通成员,进而无法通过依赖
event.is_admin()的管理命令校验。Modifications / 改动点
在
astrbot/core/platform/astrbot_message.py的MessageMember中新增role字段,用于承载平台侧发送者角色。在
astrbot/core/platform/sources/aiocqhttp/aiocqhttp_platform_adapter.py中读取event.sender["role"],并在合法值为admin、owner、member时透传到MessageMember.role。在
astrbot/core/platform/astr_message_event.py中优先从message_obj.sender.role初始化事件角色,并将owner与admin都视为管理员。在
astrbot/core/platform/sources/aiocqhttp/aiocqhttp_message_event.py的群成员列表构建逻辑中补充role字段,保证成员缓存同步时也能拿到角色信息。在
tests/unit/test_astr_message_event.py和tests/unit/test_astrbot_message.py中补充对应单元测试,覆盖owner管理员判定和MessageMember.role默认值行为。This is NOT a breaking change. / 这不是一个破坏性变更。
Screenshots or Test Results / 运行截图或测试结果
执行结果:
Checklist / 检查清单
Summary by Sourcery
将来自 aiocqhttp/NapCat 事件的发送者角色信息传递到 AstrBot 的消息和事件对象中,使群管理员和群主能够被正确识别为管理员。
Bug Fixes:
Enhancements:
MessageMember,新增可选的role字段,并从 aiocqhttp 群消息和成员缓存中为其赋值。Tests:
is_admin中对群主角色的处理,以及MessageMember的默认角色行为。Original summary in English
Summary by Sourcery
Propagate sender role information from aiocqhttp/NapCat events into AstrBot message and event objects so group admins and owners are correctly recognized as administrators.
Bug Fixes:
Enhancements:
Tests: