Fix qq_offical 定时任务中 send_message_to_user 工具调用失败#6604
Fix qq_offical 定时任务中 send_message_to_user 工具调用失败#6604V-YOP wants to merge 1 commit intoAstrBotDevs:masterfrom
Conversation
Remove msg_id from payload to prevent errors with proactive tool-call path and avoid permission issues.
dosubot
bot
added
the
size:XS
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
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. Footnotes
|
![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
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="astrbot/core/platform/sources/qqofficial/qqofficial_platform_adapter.py" line_range="241-243" />
<code_context>
)
elif session.message_type == MessageType.FRIEND_MESSAGE:
+ # 参考 https://bot.q.qq.com/wiki/develop/pythonsdk/api/message/post_message.html
+ # msg_id 缺失时认为是主动推送，而似乎至少在私聊上主动推送是没有被限制的，这里直接移除 msg_id 可以避免越权或 msg_id 不可用的bug
+ payload.pop("msg_id", None)
payload["msg_seq"] = random.randint(1, 10000)
if image_base64:
</code_context>
<issue_to_address>
**issue (bug_risk):** Unconditionally dropping msg_id may break reply semantics for friend messages.
By always removing `msg_id` for `FRIEND_MESSAGE`, valid reply flows can no longer be treated as replies by the API. If the goal is only to avoid 越权 for proactive pushes when `msg_id` is missing or invalid, make this removal conditional (e.g., only when `msg_id` is absent, fails validation, or on specific proactive paths) so legitimate replies can still include `msg_id`.
</issue_to_address>
### Comment 2
<location path="astrbot/core/platform/sources/qqofficial/qqofficial_platform_adapter.py" line_range="240-244" />
<code_context>
elif session.message_type == MessageType.FRIEND_MESSAGE:
+ # 参考 https://bot.q.qq.com/wiki/develop/pythonsdk/api/message/post_message.html
+ # msg_id 缺失时认为是主动推送，而似乎至少在私聊上主动推送是没有被限制的，这里直接移除 msg_id 可以避免越权或 msg_id 不可用的bug
+ payload.pop("msg_id", None)
payload["msg_seq"] = random.randint(1, 10000)
if image_base64:
</code_context>
<issue_to_address>
**suggestion:** Behavior and comment are inconsistent regarding "msg_id 缺失时".
The comment describes special handling only when `msg_id` is missing, but the code unconditionally removes `msg_id`. Please either align the implementation with the described “missing/invalid msg_id” scenario or reword the comment to state that `msg_id` is intentionally removed for all friend messages.
```suggestion
elif session.message_type == MessageType.FRIEND_MESSAGE:
# 参考 https://bot.q.qq.com/wiki/develop/pythonsdk/api/message/post_message.html
# 对好友消息统一移除 msg_id，将其视为主动推送（官方文档对私聊主动推送暂无明显限制），
# 以避免可能的越权或 msg_id 不可用的问题
payload.pop("msg_id", None)
payload["msg_seq"] = random.randint(1, 10000)
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| # 参考 https://bot.q.qq.com/wiki/develop/pythonsdk/api/message/post_message.html | ||
| # msg_id 缺失时认为是主动推送，而似乎至少在私聊上主动推送是没有被限制的，这里直接移除 msg_id 可以避免越权或 msg_id 不可用的bug | ||
| payload.pop("msg_id", None) |
There was a problem hiding this comment.
issue (bug_risk): Unconditionally dropping msg_id may break reply semantics for friend messages.
By always removing msg_id for FRIEND_MESSAGE, valid reply flows can no longer be treated as replies by the API. If the goal is only to avoid 越权 for proactive pushes when msg_id is missing or invalid, make this removal conditional (e.g., only when msg_id is absent, fails validation, or on specific proactive paths) so legitimate replies can still include msg_id.
There was a problem hiding this comment.
无条件移除msg_id容易会出现无法预期的行为，有条件再移除吗，这样应该要记录msg_id的时间戳，过期丢弃这种写法，有点麻烦，遇到bug再修复吧
There was a problem hiding this comment.
确实，要做成“有条件移除”而且判断 msg_id 是否过期的话，就得像你说的那样额外记时间戳、做过期逻辑，会复杂不少，尤其是现在只是为了解一个定时任务场景的 bug。
我这边的担心主要是两个点：
-
语义层面上的退化：
现在这一改动会让 QQ 订阅号的所有好友消息都走“主动推送”语义，而不再是真正意义上的“回复某条消息”，这在以后如果有人想做“精确回复/引用某条私聊消息”的功能时，可能会踩坑。 -
不可预期行为的来源：
不可预期的其实不是“删 msg_id”这一行为本身，而是“未来维护者/使用者不知道这里把所有好友消息都强制转成了主动推送”，导致排查问题时思路被带偏。
既然现在你更倾向于先用这一简单策略把 #6599 修掉、后面有需要再做精细化处理，那我有两个小建议，可以让后续的 debug 成本低一点：
-
把 trade-off 明确写在注释里（就在你现在这两行注释下面补一句 TODO 也行），类似：
# NOTE: 当前实现会让所有好友消息都按主动推送发送，无法保证“回复某条消息”的语义。 # 如果后续需要精确的 reply 语义，需要在会话/工具调用层面区分主动推送与普通回复。
-
在 Issue 或后续 TODO 里记录这个限制：
比如在 [Bug] qq_offical 定时任务中 send_message_to_user 工具调用失败："请求参数msg_id无效或越权" #6599 或一个新的跟踪 issue 里写明：- 为了修复定时任务中的 send_message_to_user 失败问题，暂时对 QQ 好友消息统一移除了 msg_id；
- 已知影响：无法利用 msg_id 实现严格的“回复某条消息”语义；
- 如果后续有此需求，可以考虑：
- 在 session 中携带 msg_id + 时间戳，过期后自动丢弃；或
- 在工具调用路径上增加一个标志位（例如
is_proactive），仅在主动推送路径上移除 msg_id。
这样的话，现在这版改动既能立刻解决实际问题，也把潜在坑用注释/文档标出来了，后面真遇到相关 bug 的人也比较容易顺着线索找到这里。
总之，我这边不是强阻塞意见，只是把可能的后续影响说清楚，方便你和后面的维护者做权衡。
| elif session.message_type == MessageType.FRIEND_MESSAGE: | ||
| # 参考 https://bot.q.qq.com/wiki/develop/pythonsdk/api/message/post_message.html | ||
| # msg_id 缺失时认为是主动推送，而似乎至少在私聊上主动推送是没有被限制的，这里直接移除 msg_id 可以避免越权或 msg_id 不可用的bug | ||
| payload.pop("msg_id", None) | ||
| payload["msg_seq"] = random.randint(1, 10000) |
There was a problem hiding this comment.
suggestion: Behavior and comment are inconsistent regarding "msg_id 缺失时".
The comment describes special handling only when msg_id is missing, but the code unconditionally removes msg_id. Please either align the implementation with the described “missing/invalid msg_id” scenario or reword the comment to state that msg_id is intentionally removed for all friend messages.
| elif session.message_type == MessageType.FRIEND_MESSAGE: | |
| # 参考 https://bot.q.qq.com/wiki/develop/pythonsdk/api/message/post_message.html | |
| # msg_id 缺失时认为是主动推送，而似乎至少在私聊上主动推送是没有被限制的，这里直接移除 msg_id 可以避免越权或 msg_id 不可用的bug | |
| payload.pop("msg_id", None) | |
| payload["msg_seq"] = random.randint(1, 10000) | |
| elif session.message_type == MessageType.FRIEND_MESSAGE: | |
| # 参考 https://bot.q.qq.com/wiki/develop/pythonsdk/api/message/post_message.html | |
| # 对好友消息统一移除 msg_id，将其视为主动推送（官方文档对私聊主动推送暂无明显限制）， | |
| # 以避免可能的越权或 msg_id 不可用的问题 | |
| payload.pop("msg_id", None) | |
| payload["msg_seq"] = random.randint(1, 10000) |
There was a problem hiding this comment.
对，你现在这段实现本身是没问题的，主要是注释的表述和实际行为有点对不上。
- 代码：
payload.pop("msg_id", None)是无条件移除msg_id，也就是一律按“主动推送”来发好友私聊消息。 - 注释里写的是：
msg_id 缺失时认为是主动推送，读起来像是“只有在 msg_id 缺失的情况下才按主动推送处理”。
所以建议只是把注释改得更贴近现在的逻辑，比如：
elif session.message_type == MessageType.FRIEND_MESSAGE:
# 参考 https://bot.q.qq.com/wiki/develop/pythonsdk/api/message/post_message.html
# 好友私聊统一移除 msg_id，将其视为主动推送（官方文档中，私聊场景下主动推送目前未见明显限制），
# 以避免可能的越权或 msg_id 不可用问题
payload.pop("msg_id", None)
payload["msg_seq"] = random.randint(1, 10000)你现在修的逻辑是对的，只要把注释再润色一下就更清晰了。
dosubot
bot
added
the
area:platform
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
| elif session.message_type == MessageType.FRIEND_MESSAGE: | ||
| # 参考 https://bot.q.qq.com/wiki/develop/pythonsdk/api/message/post_message.html | ||
| # msg_id 缺失时认为是主动推送，而似乎至少在私聊上主动推送是没有被限制的，这里直接移除 msg_id 可以避免越权或 msg_id 不可用的bug | ||
| payload.pop("msg_id", None) |
There was a problem hiding this comment.
此改动将所有私聊消息都作为主动消息发送，以解决定时任务中 msg_id 过期的问题。然而，这也导致即时回复在 QQ 客户端中不会显示为对特定消息的“回复”，可能轻微影响用户体验。
一个更优的方案可能是仅在特定条件下（例如，距离上一条消息时间过长）才移除 msg_id，从而保留即时回复的引用关系。如果当前难以实现这种判断逻辑，目前的改动作为一种健壮性策略是可接受的。
There was a problem hiding this comment.
同样的，推荐在有条件的时候才移除msg_id，比如离上一条消息很久才移除msg_id
添加 7 个测试用例，覆盖 _send_by_session_common 中 FRIEND_MESSAGE 类型的 msg_id 处理逻辑： - 验证纯文本、图片、视频、文件私聊消息的 payload 中 msg_id 被正确移除 - 验证 GROUP_MESSAGE 类型的 payload 仍保留 msg_id - 验证 FRIEND_MESSAGE payload 中 msg_seq 正常存在 - 验证无缓存 msg_id 时提前返回的行为 Fixes AstrBotDevs#6599, AstrBotDevs#6670 Ref: AstrBotDevs#6604
2 participants
Remove msg_id from payload to prevent errors with proactive tool-call path and avoid permission issues.
修复了 #6599 （这是我第一次提PR，且我对原本的运行机制没有进行过学习研究，如果可能引入问题请拒绝该PR🙏）
Modifications / 改动点
在qq_offical适配器中私聊消息直接移除了msg_id字段以避免越权或 msg_id 不可用的问题
Screenshots or Test Results / 运行截图或测试结果
只做了简单测试，修改后带图片和纯文本的消息都能够正常回应，10分钟后的定时任务能正常响应
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
Adjust QQ Official private message sending to avoid msg_id-related errors in proactive tool-call scenarios.
Bug Fixes:
Enhancements: