From bd501cce340c4704b7cae23e9ee1bd834af7c3f5 Mon Sep 17 00:00:00 2001 From: Ziheng Zhang <142805986+MuXinCG2004@users.noreply.github.com> Date: Sun, 19 Apr 2026 18:14:23 +0800 Subject: [PATCH] fix(channel/qq): deliver HITL approval prompts reliably (#166) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(channel/qq): deliver HITL approval prompts reliably QQ approval prompts were silently dropped when the markdown send hit a QQ server-side error (e.g. template not configured, content audit) because the fallback path only matched TypeError / specific string patterns, and the plain-text retry reused the already-consumed msg_seq which QQ then rejects as duplicate. - Consume a fresh msg_seq for the plain-text fallback send - Recognize QQ server error codes (304014/304023/304003/40034059) and CN fragments ("模版"/"审核") as markdown-fallback triggers - Promote send failure logs from debug to warning/error with chat_id/msg_id/seq so real-world errors can be diagnosed - Extend test_qq_channel with a server-error-code fallback case * style(channel/qq): apply ruff formatter to approval-delivery fix * Fix --- EvoScientist/channels/qq/channel.py | 73 ++++++++++++++++++++++++----- tests/test_qq_channel.py | 69 ++++++++++++++++++++++++++- 2 files changed, 128 insertions(+), 14 deletions(-) diff --git a/EvoScientist/channels/qq/channel.py b/EvoScientist/channels/qq/channel.py index eb8753b..b693c05 100644 --- a/EvoScientist/channels/qq/channel.py +++ b/EvoScientist/channels/qq/channel.py @@ -200,12 +200,56 @@ class QQChannel(Channel): return except Exception as exc: if not self._should_fallback_to_plain_text(exc): + logger.error( + "QQ markdown send failed with non-fallbackable error " + "(chat_id=%s, msg_id=%s, seq=%s): %r", + chat_id, + msg_id, + seq, + exc, + ) raise self._record_markdown_fallback(chat_id, raw_text, exc) - logger.debug("QQ markdown send failed, falling back to plain text: %s", exc) + logger.warning( + "QQ markdown send failed, falling back to plain text " + "(chat_id=%s, msg_id=%s, seq=%s): %r", + chat_id, + msg_id, + seq, + exc, + ) + # QQ may have already consumed `seq` server-side even on failure. + # Reusing it for the plain retry triggers "duplicate msg_seq", so + # always advance to a fresh seq before the fallback send. + fallback_seq = self._next_msg_seq(msg_id) plain_text = self._plain_formatter.format(raw_text) - await self._post_plain_message(chat_id, plain_text, msg_type, msg_id, seq) + try: + await self._post_plain_message( + chat_id, plain_text, msg_type, msg_id, fallback_seq + ) + except Exception as plain_exc: + logger.error( + "QQ plain fallback also failed (chat_id=%s, msg_id=%s, seq=%s): %r", + chat_id, + msg_id, + fallback_seq, + plain_exc, + ) + raise + + # QQ server-side error codes / fragments that indicate the markdown + # request itself is invalid (template not configured, format rejected, + # content audit, etc.). Seeing any of these means we should retry with + # plain text rather than re-raise. + _QQ_MARKDOWN_ERROR_MARKERS: ClassVar[tuple[str, ...]] = ( + "304014", # markdown template not configured + "304003", # invalid markdown params + "40034059", # generic send message failed (often markdown-related) + "模板", # CN: template (standard form) + "模版", # CN: template (variant form) + "审核", # CN: audit + ) def _should_fallback_to_plain_text(self, exc: Exception) -> bool: """Return True only for markdown compatibility/validation failures.""" @@ -214,17 +258,20 @@ class QQChannel(Channel): msg = str(exc).lower() compatibility_tokens = ("unsupported", "unexpected", "unknown", "invalid") - return ( - "unexpected keyword argument" in msg - or ( - "markdown" in msg - and any(token in msg for token in compatibility_tokens) - ) - or ( - "msg_type" in msg - and any(token in msg for token in compatibility_tokens) - ) - ) + + if "unexpected keyword argument" in msg: + return True + if "markdown" in msg and any(token in msg for token in compatibility_tokens): + return True + if "msg_type" in msg and any(token in msg for token in compatibility_tokens): + return True + + # QQ-specific server error codes returned by qq-botpy as strings. + raw = str(exc) + for marker in self._QQ_MARKDOWN_ERROR_MARKERS: + if marker in raw or marker.lower() in msg: + return True + return False def _record_markdown_fallback( self, diff --git a/tests/test_qq_channel.py b/tests/test_qq_channel.py index 6eaad75..3332915 100644 --- a/tests/test_qq_channel.py +++ b/tests/test_qq_channel.py @@ -71,7 +71,10 @@ class TestQQChannelSend: assert second["msg_type"] == 0 assert second["content"] == "Title\n\n• item" assert second["msg_id"] == "evt_2" - assert second["msg_seq"] == 1 + # Plain fallback consumes a fresh msg_seq — QQ may have already + # counted the failed markdown attempt, so re-using seq would + # trigger "duplicate msg_seq". + assert second["msg_seq"] == 2 def test_send_does_not_fallback_on_transport_error(self): channel = self._make_ready_channel() @@ -99,3 +102,67 @@ class TestQQChannelSend: sent = channel._client.api.post_c2c_message.await_args.kwargs assert sent["msg_type"] == 2 assert "content" not in sent + + def test_send_does_not_fallback_when_transport_error_mentions_markdown(self): + """A transport-layer error whose message incidentally contains the word + "markdown" must NOT be reclassified as a markdown compatibility failure, + otherwise genuine send failures get silently swallowed as plain-text.""" + channel = self._make_ready_channel() + + async def _send_once(coro_factory, max_retries=3): + return await coro_factory() + + channel._send_with_retry = _send_once + channel._client.api.post_c2c_message = AsyncMock( + side_effect=ConnectionError( + "failed to post markdown message: ConnectionReset" + ) + ) + msg = OutboundMessage( + channel="qq", + chat_id="openid", + content="## Title", + metadata={ + "chat_id": "openid", + "event_id": "evt_transport", + "msg_type": "c2c", + }, + ) + + assert _run(channel.send(msg)) is False + channel._client.api.post_c2c_message.assert_awaited_once() + + def test_send_falls_back_on_qq_server_error_code(self): + """QQ server-side markdown errors (e.g. 304014 template not configured) + should trigger plain-text fallback with a fresh msg_seq.""" + channel = self._make_ready_channel() + channel._client.api.post_c2c_message = AsyncMock( + side_effect=[ + RuntimeError( + '{"code": 304014, "message": "markdown template not configured"}' + ), + None, + ] + ) + msg = OutboundMessage( + channel="qq", + chat_id="openid", + content="## Title", + metadata={ + "chat_id": "openid", + "event_id": "evt_4", + "msg_type": "c2c", + }, + ) + + assert _run(channel.send(msg)) is True + + assert channel._client.api.post_c2c_message.await_count == 2 + first = channel._client.api.post_c2c_message.await_args_list[0].kwargs + second = channel._client.api.post_c2c_message.await_args_list[1].kwargs + + assert first["msg_type"] == 2 + assert first["msg_seq"] == 1 + assert second["msg_type"] == 0 + # Fresh seq on fallback — avoids QQ "duplicate msg_seq" rejection. + assert second["msg_seq"] == 2