Skip to content

fix(qqofficial): preserve proactive message routing across restarts - #10152

Open
cyone123 wants to merge 2 commits into
AstrBotDevs:masterfrom
cyone123:fix/qqofficial-proactive-routing
Open

cyone123 wants to merge 2 commits into
AstrBotDevs:masterfrom
cyone123:fix/qqofficial-proactive-routing

Conversation

@cyone123

@cyone123 cyone123 commented Sep 20, 2026

Copy link
Copy Markdown

Problem

QQ Official proactive group messages can be silently skipped after AstrBot restarts. The adapter currently keeps the delivery scene only in the in-memory _session_scene mapping, so a saved UMO cannot determine whether a GROUP_MESSAGE destination is a QQ group or a guild text channel after restart.

This is the second problem described in #9831. The first problem in that issue, concerning the semantics of /sid under unique_session, is outside this PR's scope.

Changes

  • Persist QQ Official group and channel delivery scenes through the existing SharedPreferences store.
  • Scope persisted routes by platform instance ID and AppID.
  • Restore routes on demand when sending through a saved UMO.
  • Support proactive guild text-channel sends without requiring a cached reply msg_id.
  • Avoid reusing stale or bot-generated message IDs for proactive sends.
  • Raise an explicit error when the delivery scene cannot be resolved instead of silently dropping the message.
  • Share the persistence behavior between WebSocket and Webhook adapters.
  • Add regression coverage for restart recovery, route isolation, unique-session destination handling, stale reply IDs, unknown scenes, and persistence retries.

Verification

  • 90 automated tests passed, including QQ Official adapter tests and SharedPreferences tests.
  • ruff format . passed.
  • ruff check . passed.
  • git diff --check passed.
  • Live WebSocket verification with a real QQ bot and test group passed:
    • started a fresh process with empty in-memory caches;
    • recovered the group route from SQLite without receiving a new group message;
    • sent two consecutive messages through the actual Context.send_message() path;
    • both requests omitted msg_id, returned new QQ message IDs, and were confirmed received in the QQ client.

This PR is related to #9831 and addresses its second reported problem.

Summary by Sourcery

Persist QQ Official delivery routes so saved sessions can reliably send proactive group and guild-channel messages after restarts.

Bug Fixes:

  • Preserve QQ Official group and guild-channel proactive message routing across adapter restarts.
  • Fail explicitly when a group message destination has no resolvable delivery scene instead of silently dropping it.
  • Prevent proactive sends from reusing stale or bot-generated reply message IDs.

Enhancements:

  • Share persisted route handling across WebSocket and Webhook adapters, scoped to each platform instance and AppID.

Tests:

  • Add regression coverage for route persistence and restart recovery, destination isolation, unique-session handling, proactive message IDs, unknown scenes, and persistence failures.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hey - I've found 1 issue

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="582-595" />
<code_context>
+        if self._session_scene.get(session_id) == scene:
+            return
+        if scene in ("group", "channel"):
+            try:
+                await sp.put_async(
+                    "qqofficial",
+                    f"{self.meta().id}:{self.appid}",
+                    f"group_scene:{session_id}",
+                    scene,
+                )
+            except Exception:
+                # Leave the cache unchanged so the next event retries persistence.
+                logger.exception(
+                    "[QQOfficial] Failed to persist delivery scene for session %s",
+                    session_id,
+                )
+                return
         self._session_scene[session_id] = scene

</code_context>
<issue_to_address>
**issue (bug_risk):** When `sp.put_async` fails, `remember_session_scene` catches the exception and leaves only the in-memory route unchanged, but the caller still commits the incoming event. If the process restarts before another event for that destination arrives, the route is lost and proactive messages are again unable to resolve their delivery scene.

**Triggers:** When the SharedPreferences write fails and no subsequent message arrives for the same group or channel before a restart.

**Suggested fix:** Do not commit the event until route persistence succeeds, or enqueue a durable retry so a failed route write is retried independently of future incoming events.

```suggestion
            try:
                await sp.put_async(
                    "qqofficial",
                    f"{self.meta().id}:{self.appid}",
                    f"group_scene:{session_id}",
                    scene,
                )
            except Exception:
                logger.exception(
                    "[QQOfficial] Failed to persist delivery scene for session %s",
                    session_id,
                )
                raise
```
</issue_to_address>

Sourcery assessment

Needs a human reviewer. 1 finding to address first, and a persisted group or channel route now determines which QQ API receives proactive notifications after a restart, and cached reply IDs are no longer used. If the route is wrong or stale, messages may be sent through the wrong delivery path or fail; reverting stops future misrouting but cannot recall notifications already sent.

Blocking findings: astrbot/core/platform/sources/qqofficial/qqofficial_platform_adapter.py:595


Sourcery is free for open source - if you like our reviews please consider sharing them ✨

Comment thread astrbot/core/platform/sources/qqofficial/qqofficial_platform_adapter.py Outdated
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant