Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,9 @@ async def _send_by_session_common(
)

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)
Comment on lines +241 to +243
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

无条件移除msg_id容易会出现无法预期的行为,有条件再移除吗,这样应该要记录msg_id的时间戳,过期丢弃这种写法,有点麻烦,遇到bug再修复吧

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

确实,要做成“有条件移除”而且判断 msg_id 是否过期的话,就得像你说的那样额外记时间戳、做过期逻辑,会复杂不少,尤其是现在只是为了解一个定时任务场景的 bug。

我这边的担心主要是两个点:

  1. 语义层面上的退化
    现在这一改动会让 QQ 订阅号的所有好友消息都走“主动推送”语义,而不再是真正意义上的“回复某条消息”,这在以后如果有人想做“精确回复/引用某条私聊消息”的功能时,可能会踩坑。

  2. 不可预期行为的来源
    不可预期的其实不是“删 msg_id”这一行为本身,而是“未来维护者/使用者不知道这里把所有好友消息都强制转成了主动推送”,导致排查问题时思路被带偏。

既然现在你更倾向于先用这一简单策略把 #6599 修掉、后面有需要再做精细化处理,那我有两个小建议,可以让后续的 debug 成本低一点:

  1. 把 trade-off 明确写在注释里(就在你现在这两行注释下面补一句 TODO 也行),类似:

    # NOTE: 当前实现会让所有好友消息都按主动推送发送,无法保证“回复某条消息”的语义。
    #      如果后续需要精确的 reply 语义,需要在会话/工具调用层面区分主动推送与普通回复。
  2. 在 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 的人也比较容易顺着线索找到这里。

总之,我这边不是强阻塞意见,只是把可能的后续影响说清楚,方便你和后面的维护者做权衡。

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

此改动将所有私聊消息都作为主动消息发送,以解决定时任务中 msg_id 过期的问题。然而,这也导致即时回复在 QQ 客户端中不会显示为对特定消息的“回复”,可能轻微影响用户体验。

一个更优的方案可能是仅在特定条件下(例如,距离上一条消息时间过长)才移除 msg_id,从而保留即时回复的引用关系。如果当前难以实现这种判断逻辑,目前的改动作为一种健壮性策略是可接受的。

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

同样的,推荐在有条件的时候才移除msg_id,比如离上一条消息很久才移除msg_id

payload["msg_seq"] = random.randint(1, 10000)
Comment on lines 240 to 244
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
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)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

注释写错了吗

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

对,你现在这段实现本身是没问题的,主要是注释的表述和实际行为有点对不上。

  • 代码: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)

你现在修的逻辑是对的,只要把注释再润色一下就更清晰了。

if image_base64:
media = await QQOfficialMessageEvent.upload_group_and_c2c_image(
Expand Down Expand Up @@ -268,9 +271,6 @@ async def _send_by_session_common(
if media:
payload["media"] = media
payload["msg_type"] = 7
# QQ API rejects msg_id for media (video/file) messages sent
# via the proactive tool-call path; remove it to avoid 越权 error.
payload.pop("msg_id", None)
if file_source:
media = await QQOfficialMessageEvent.upload_group_and_c2c_media(
send_helper, # type: ignore
Expand All @@ -282,7 +282,6 @@ async def _send_by_session_common(
if media:
payload["media"] = media
payload["msg_type"] = 7
payload.pop("msg_id", None)

ret = await QQOfficialMessageEvent.post_c2c_message(
send_helper, # type: ignore
Expand Down