Skip to content

fix(qqbot): treat DM chat_type same as C2C for approval buttons and message delivery - #41801

Open
HongChenGG wants to merge 2 commits into
NousResearch:mainfrom
HongChenGG:fix/qqbot-dm-approval-auth
Open

fix(qqbot): treat DM chat_type same as C2C for approval buttons and message delivery#41801
HongChenGG wants to merge 2 commits into
NousResearch:mainfrom
HongChenGG:fix/qqbot-dm-approval-auth

Conversation

@HongChenGG

Copy link
Copy Markdown

Problem

QQ Bot adapter's only checks for , but QQ DM session keys are recorded as . This causes:

  1. Approval buttons rejected — users in QQ DMs see when trying to approve agent actions
  2. Message delivery failures — and don't handle chat_type, causing errors or wrong API paths

Fix

Treat the same as in 4 locations:

Location Function Change
Line 1095 == "c2c"in {"c2c", "dm"}
Line 2476 _send_chunk (text) same
Line 2603 _send_chunk (keyboard) same
Line 2928 _send_media same

Context

  • QQ Bot's C2C private messages use and the adapter stores chat_type="c2c" in _chat_type_map
  • But guild DM messages go through _handle_dm_message which stores chat_type="dm"
  • The MessageEvent itself always passes chat_type="dm" for both paths
  • This mismatch causes approval authorization to fail and message delivery to use wrong API paths

…essage delivery

QQ private-message session keys may be recorded as 'dm' while C2C button
events authorize against the same user_openid. Treat both spellings as
the same private chat in:

- _is_authorized_interaction_for_session: approval clicks from QQ DMs
  were incorrectly rejected as unauthorized
- _send_chunk: text/keyboard delivery to DM recipients
- _send_media: media delivery path selection

Without this fix, users in QQ DMs see 'Rejected unauthorized approval
click' when trying to approve agent actions, and bot replies to DMs may
fail with 'Unknown chat type'.
@alt-glitch alt-glitch added type/bug Something isn't working platform/qqbot QQ Bot adapter P2 Medium — degraded but workaround exists labels Jun 8, 2026
@liuhao1024

Copy link
Copy Markdown
Contributor

The dm alias is applied consistently in 4 places (authorization check, _send_chunk, send_with_keyboard, _send_media), but two other code paths still compare against "c2c" only:

  1. send_typing() (~line 3067)if chat_type != "c2c": return silently drops typing indicators for DM sessions. When the gateway sends a typing indicator for a DM message whose chat_type was recorded as "dm", the indicator is never delivered.

  2. _upload_media() (~line 2370)if target_type == "c2c" selects the API endpoint. A DM media upload where target_type is "dm" falls through to the group endpoint (/v2/groups/{id}/files), which will fail or route to the wrong recipient.

Both should use in {"c2c", "dm"} to match the rest of the PR.

- send_typing(): chat_type != 'c2c' → not in {'c2c', 'dm'}
- _upload_media(): target_type == 'c2c' → in {'c2c', 'dm'}

Addresses review feedback on NousResearch#41801.
@HongChenGG

Copy link
Copy Markdown
Author

Thanks for catching these! Fixed both:

  1. send_typing(): chat_type != "c2c"not in {"c2c", "dm"}
  2. _upload_media(): target_type == "c2c"in {"c2c", "dm"}

Also updated the docstring for send_typing to reflect DM support.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Medium — degraded but workaround exists platform/qqbot QQ Bot adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants