fix(gateway): keep the Matrix reply fallback pill out of the mention strip
The Matrix adapter strips the bot's mention from the inbound body before
_extract_reply_context parses the inline reply fallback, and that strip is a
blind whole-body replace. A reply to the bot names the bot in the fallback
pill ("> <@bot:server> quoted"), which is exactly what makes the message
count as a mention under the default MATRIX_REQUIRE_MENTION=true -- so the
strip runs on every reply-to-the-bot and rewrites the pill to "> <>", after
which the pill regex no longer matches. reply_to_author_id is lost,
reply_to_text becomes the mangled "<> quoted" remnant, and the prompt
renders "[Replying to: "<> ..."]" instead of "[Replying to your previous
message: ...]".
Split the body into (quote block, reply text) and strip the mention from the
reply text only. The mention gate still sees the raw body, so a reply to the
bot keeps waking the bot; the visible reply text and the quote-block strip
are unchanged.
Fixes #111233
This commit is contained in:
@@ -228,6 +228,26 @@ def _is_permanent_matrix_auth_error(exc: BaseException) -> bool:
|
||||
return isinstance(status, int) and status in (401, 403)
|
||||
|
||||
|
||||
def _split_reply_fallback(body: str) -> tuple[str, str]:
|
||||
"""Split ``> quote\\n\\nreply`` into ``(quote_block, reply_text)``; ``("", body)`` when absent.
|
||||
|
||||
The two halves always concatenate back to *body* verbatim (``quote + reply == body``), so
|
||||
callers can transform one half and rebuild the body without disturbing the other. The blank
|
||||
separator line belongs to the quote block. Used to keep the ``> <@user:srv>`` reply pill —
|
||||
the only mention text in a reply-to-the-bot — out of whole-body rewrites.
|
||||
"""
|
||||
if not body or not body.startswith("> "):
|
||||
return "", body
|
||||
lines = body.split("\n")
|
||||
idx = 0
|
||||
while idx < len(lines) and (lines[idx].startswith("> ") or lines[idx] == ">"):
|
||||
idx += 1
|
||||
if idx < len(lines) and lines[idx] == "":
|
||||
idx += 1 # the blank line separating the quote from the reply belongs to the quote
|
||||
head = "\n".join(lines[:idx])
|
||||
return (head, "") if idx >= len(lines) else (head + "\n", "\n".join(lines[idx:]))
|
||||
|
||||
|
||||
class _MatrixHtmlSanitizer(HTMLParser):
|
||||
"""Allowlist sanitizer for Matrix-compatible formatted HTML."""
|
||||
|
||||
@@ -2002,7 +2022,12 @@ class MatrixAdapter(BasePlatformAdapter):
|
||||
event_id, thread_id)
|
||||
return None
|
||||
if is_mentioned and self._require_mention:
|
||||
body = self._strip_mention(body)
|
||||
# Strip the mention from the reply text only: the quote block carries the
|
||||
# ``> <@bot:srv> ...`` reply pill, which _extract_reply_context parses later
|
||||
# for reply_to_author_id. A whole-body replace rewrote the pill to ``> <>``
|
||||
# and silently dropped the replied-to author (#111233).
|
||||
quote_block, reply_text = _split_reply_fallback(body)
|
||||
body = quote_block + self._strip_mention(reply_text)
|
||||
# Real thread roots are preserved above; synthetic roots (this event) follow policy: DM
|
||||
# @mention threads / DM auto-thread, or room auto-thread unless session_scope pins the room.
|
||||
if not thread_id:
|
||||
|
||||
@@ -246,3 +246,106 @@ async def test_media_message_carries_sender_and_reply_context(monkeypatch):
|
||||
assert "nice photo" in msg.reply_to_text
|
||||
assert msg.reply_to_author_id == "@erin:example.org"
|
||||
assert msg.reply_to_author_name == "erin"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Reply fallback vs. the mention strip (#111233)
|
||||
#
|
||||
# A reply to the bot names the bot in the ``> <@bot:srv> ...`` pill, which is
|
||||
# what makes the message count as a mention under the default
|
||||
# MATRIX_REQUIRE_MENTION=true. The mention strip therefore runs on exactly the
|
||||
# messages that carry a reply pill, and it must not rewrite the pill before
|
||||
# _extract_reply_context parses it.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_reply_to_bot_under_require_mention_keeps_reply_author(monkeypatch):
|
||||
"""Replying to the bot must keep the replied-to author under the default mention gate.
|
||||
|
||||
The pill is the mention, so the strip runs and used to rewrite ``> <@bot>`` to
|
||||
``> <>`` before the fallback parse — losing reply_to_author_id and mangling
|
||||
reply_to_text, so the prompt rendered "[Replying to: "<> did you check the logs?"]"
|
||||
instead of "[Replying to your previous message: ...]".
|
||||
"""
|
||||
adapter = _make_adapter(require_mention=True, monkeypatch=monkeypatch)
|
||||
adapter._startup_ts = time.time() - 10
|
||||
|
||||
# No explicit "@hermes" in the reply text: the pill is the only mention.
|
||||
assert adapter._user_id not in "hello there"
|
||||
body = "> <@hermes:example.org> did you check the logs?\n\nhello there"
|
||||
event = _make_event(body, in_reply_to_event_id="$bot_msg", event_id="$evt_reply_bot")
|
||||
await adapter._on_room_message(event)
|
||||
|
||||
adapter.handle_message.assert_awaited_once()
|
||||
msg = adapter.handle_message.await_args.args[0]
|
||||
|
||||
assert msg.reply_to_message_id == "$bot_msg"
|
||||
assert msg.reply_to_author_id == "@hermes:example.org"
|
||||
assert msg.reply_to_author_name == "hermes"
|
||||
# The quoted text must be the reply target's text, never the mangled "<> ..." pill remnant.
|
||||
assert msg.reply_to_text == "did you check the logs?"
|
||||
# The user's actual reply is still delivered, with the quote block stripped.
|
||||
assert msg.text == "hello there"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_reply_with_explicit_mention_still_strips_it_from_reply_text(monkeypatch):
|
||||
"""The narrower strip must still remove an explicit @bot from the reply text.
|
||||
|
||||
Only the quote block is exempt; a typed mention in the reply itself is stripped as
|
||||
before, so the model does not see the addressing token.
|
||||
"""
|
||||
adapter = _make_adapter(require_mention=True, monkeypatch=monkeypatch)
|
||||
adapter._startup_ts = time.time() - 10
|
||||
|
||||
body = "> <@carol:example.org> original question\n\n@hermes:example.org because reasons"
|
||||
event = _make_event(body, in_reply_to_event_id="$carol_msg", event_id="$evt_reply_carol")
|
||||
await adapter._on_room_message(event)
|
||||
|
||||
msg = adapter.handle_message.await_args.args[0]
|
||||
assert msg.reply_to_author_id == "@carol:example.org"
|
||||
assert msg.reply_to_text == "original question"
|
||||
assert msg.text == "because reasons"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_plain_mention_without_quote_is_still_stripped(monkeypatch):
|
||||
"""A non-quote mention keeps the previous whole-body strip behaviour."""
|
||||
adapter = _make_adapter(require_mention=True, monkeypatch=monkeypatch)
|
||||
adapter._startup_ts = time.time() - 10
|
||||
|
||||
event = _make_event("@hermes:example.org hello there", event_id="$evt_plain_mention")
|
||||
await adapter._on_room_message(event)
|
||||
|
||||
msg = adapter.handle_message.await_args.args[0]
|
||||
assert msg.text == "hello there"
|
||||
assert msg.reply_to_author_id is None
|
||||
|
||||
|
||||
def test_split_reply_fallback_reassembles_body():
|
||||
"""The split is lossless: the two halves rebuild the original body byte for byte.
|
||||
|
||||
Callers transform one half and rebuild the body, so any drift between the split and
|
||||
the original text would silently rewrite the message.
|
||||
"""
|
||||
from plugins.platforms.matrix.adapter import _split_reply_fallback
|
||||
|
||||
cases = [
|
||||
("> <@a:ex.org> q\n\nreply", ("> <@a:ex.org> q\n\n", "reply")),
|
||||
("> <@a:ex.org> q\nreply", ("> <@a:ex.org> q\n", "reply")),
|
||||
("> <@a:ex.org> q", ("> <@a:ex.org> q", "")),
|
||||
("> <@a:ex.org> q\n", ("> <@a:ex.org> q\n", "")),
|
||||
("> q\n> more\n\nreply", ("> q\n> more\n\n", "reply")),
|
||||
("> q\n>\n\nreply", ("> q\n>\n\n", "reply")),
|
||||
# CRLF: the "\r" line is not the empty separator, so the quote block keeps "\r\n"
|
||||
# and the reply text keeps the leading break — which the strip's .strip() removes.
|
||||
("> <@a:ex.org> q\r\n\r\nreply", ("> <@a:ex.org> q\r\n", "\r\nreply")),
|
||||
("no quote here", ("", "no quote here")),
|
||||
("", ("", "")),
|
||||
]
|
||||
for body, expected in cases:
|
||||
quote, reply = _split_reply_fallback(body)
|
||||
assert (quote, reply) == expected
|
||||
# The split must be lossless: callers rebuild the body from the two halves.
|
||||
assert quote + reply == body
|
||||
|
||||
Reference in New Issue
Block a user