fix(matrix): propagate sender MXID + reply context to MessageEvent
The Matrix adapter built MessageEvent from inbound room events but dropped
the sender's MXID and display name on the event itself -- only 'source'
carried them. Other adapters (signal/slack/telegram/discord/mattermost/irc)
have the same gap; this PR fixes matrix and adds the supporting
top-level MessageEvent fields so the rest can follow.
Downstream effects for matrix specifically:
- gateway prompt assembly can now read event.user_name (or source)
without having to dig into source per platform
- reply context (reply_to_text / reply_to_author_id /
reply_to_author_name) is parsed from the inline > <@user:server> ...
Matrix fallback format before stripping, instead of discarded
- the gateway's existing [Replying to: "..."] renderer can now show
who the user was replying to (was always anonymous for matrix)
MessageEvent gains two optional top-level fields (user_id, user_name,
both default None) so non-IM producers (cron/webhook/autonomous) remain
unaffected. Source still carries the same values for callers that
already read from there.
Tests cover:
- non-reply message carries sender user_id/user_name on MessageEvent
- different senders (alice, bob) both propagate
- reply message carries reply_to_message_id + reply_to_text +
reply_to_author_id + reply_to_author_name, parsed from the
> <@carol:example.org> original question\n\nactual reply shape
- non-reply message does NOT spuriously set reply_to_* fields
Sibling matrix tests (148 across test_matrix*.py) remain green.
Authored by WintleChoung <cwt@users.noreply.github.com>
Salvaged from PR #80293.
This commit is contained in:
WintleChoung
kshitij
parent
87086bc5d7
commit
e245a98781
|
|
@ -2060,7 +2060,16 @@ class MessageEvent:
|
|||
# Message content
|
||||
text: str
|
||||
message_type: MessageType = MessageType.TEXT
|
||||
|
||||
|
||||
# Author of this inbound message. Carried on the event itself (not
|
||||
# only on ``source``) so prompt builders that build per-message text
|
||||
# can resolve "who said this" without having to dig into ``source``.
|
||||
# ``source`` still carries the same values for callers that already
|
||||
# read from there. Adapters that produce events from non-IM sources
|
||||
# (cron, webhook, autonomous) may leave these as ``None``.
|
||||
user_id: Optional[str] = None
|
||||
user_name: Optional[str] = None
|
||||
|
||||
# Source information
|
||||
source: SessionSource = None
|
||||
|
||||
|
|
|
|||
|
|
@ -345,6 +345,49 @@ def _normalize_matrix_bang_command(text: str) -> str:
|
|||
return f"/{resolved}{match.group(2) or ''}"
|
||||
|
||||
|
||||
# Matrix reply fallback prefix looks like:
|
||||
# > <@alice:example.org> quoted text
|
||||
# > continuation of quoted text
|
||||
# <empty line>
|
||||
# actual reply text
|
||||
# Capture the original quoted text and the quoted author's MXID so the
|
||||
# gateway prompt layer can render "[Replying to: \"...\"]" with the author
|
||||
# attached. Returns (text, author_id) — text is the joined quoted body,
|
||||
# author_id is the MXID parsed from the leading "<@user:server>" pill or
|
||||
# ``None`` if the fallback uses an unsupported shape.
|
||||
_MATRIX_REPLY_FALLBACK_PILL_RE = re.compile(r"^>\s*<(@[^>]+)>\s*(.*)$")
|
||||
|
||||
|
||||
def _extract_reply_fallback(body: str) -> tuple[Optional[str], Optional[str]]:
|
||||
"""Return (reply_to_text, reply_to_author_id) parsed from a Matrix reply body.
|
||||
|
||||
Matrix stores reply text inline as ``> <@user:server> <line>\\n> <line>...``
|
||||
followed by a blank line and the actual reply. The first line carries an
|
||||
optional ``<@user:server>`` mention pill naming the original author.
|
||||
"""
|
||||
if not body or not body.startswith("> "):
|
||||
return None, None
|
||||
|
||||
quoted_lines: list[str] = []
|
||||
author_id: Optional[str] = None
|
||||
for line in body.split("\n"):
|
||||
if not line.startswith("> "):
|
||||
# Blank line or the start of the actual reply — stop accumulating.
|
||||
break
|
||||
content = line[2:]
|
||||
if author_id is None:
|
||||
pill_match = _MATRIX_REPLY_FALLBACK_PILL_RE.match(line)
|
||||
if pill_match:
|
||||
author_id = pill_match.group(1)
|
||||
# Drop the pill from the visible quoted text so "[Replying
|
||||
# to: ...]" in the LLM prompt reads naturally.
|
||||
content = pill_match.group(2)
|
||||
quoted_lines.append(content)
|
||||
|
||||
quoted_text = "\n".join(quoted_lines).strip() or None
|
||||
return quoted_text, author_id
|
||||
|
||||
|
||||
class _MatrixHtmlSanitizer(HTMLParser):
|
||||
"""Allowlist sanitizer for Matrix-compatible formatted HTML."""
|
||||
|
||||
|
|
@ -3394,8 +3437,17 @@ class MatrixAdapter(BasePlatformAdapter):
|
|||
if in_reply_to:
|
||||
reply_to = in_reply_to.get("event_id")
|
||||
|
||||
# Strip reply fallback from body.
|
||||
# Capture the reply fallback BEFORE stripping it from body, so the
|
||||
# gateway prompt layer can render "[Replying to: \"<original>\"]".
|
||||
# Other adapters (Signal, Slack, Telegram) populate reply_to_text
|
||||
# from their quote payload; Matrix stores it inline as `> <@user:srv>
|
||||
# <text>\n\n<actual reply>` and discards it after stripping.
|
||||
reply_to_text: Optional[str] = None
|
||||
reply_to_author_id: Optional[str] = None
|
||||
reply_to_author_name: Optional[str] = None
|
||||
if reply_to and body.startswith("> "):
|
||||
reply_to_text, reply_to_author_id = _extract_reply_fallback(body)
|
||||
|
||||
lines = body.split("\n")
|
||||
stripped = []
|
||||
past_fallback = False
|
||||
|
|
@ -3410,6 +3462,13 @@ class MatrixAdapter(BasePlatformAdapter):
|
|||
stripped.append(line)
|
||||
body = "\n".join(stripped) if stripped else body
|
||||
|
||||
# Resolve the replied-to author's display name when we have the
|
||||
# state_store available — falls back to the localpart otherwise.
|
||||
if reply_to_author_id:
|
||||
reply_to_author_name = await self._get_display_name(
|
||||
room_id, reply_to_author_id
|
||||
)
|
||||
|
||||
# Re-run bang normalization after reply-fallback stripping so a quoted
|
||||
# reply whose actual content is a bang command (e.g. ``> quoted\n\n!model``)
|
||||
# is treated as a command, matching how ``/command`` is recognized below.
|
||||
|
|
@ -3426,6 +3485,15 @@ class MatrixAdapter(BasePlatformAdapter):
|
|||
raw_message=source_content,
|
||||
message_id=event_id,
|
||||
reply_to_message_id=reply_to,
|
||||
reply_to_text=reply_to_text,
|
||||
reply_to_author_id=reply_to_author_id,
|
||||
reply_to_author_name=reply_to_author_name,
|
||||
# Sender metadata at MessageEvent level — `source.user_name`
|
||||
# already carries this, but downstream code (e.g. the prompt
|
||||
# layer, ghost-context rendering) historically reads the
|
||||
# top-level fields. Mirror them so matrix matches signal/slack.
|
||||
user_id=sender,
|
||||
user_name=display_name,
|
||||
)
|
||||
|
||||
if msg_type == MessageType.TEXT and self._text_batch_delay_seconds > 0:
|
||||
|
|
|
|||
|
|
@ -0,0 +1,199 @@
|
|||
"""Tests for Matrix MessageEvent metadata (sender, reply context).
|
||||
|
||||
The matrix adapter builds MessageEvent from inbound room events. Other adapters
|
||||
(Signal, Slack, Telegram, Discord, Mattermost, IRC) populate the sender /
|
||||
reply_to_* fields on MessageEvent so the gateway can:
|
||||
- prepend "[Name] message" to user prompt text in shared-multi-user sessions
|
||||
- render "[Replying to: ...]" with the replied-to author's name
|
||||
- render "[Replying to your previous message: ...]" when the reply target
|
||||
is the bot itself
|
||||
|
||||
Matrix historically dropped these fields, leaving the LLM unable to tell
|
||||
who said what in a shared room. Agents saw only bare text strings, so a
|
||||
self-emitted phantom interruption (e.g. "[This response was interrupted by
|
||||
a user correction.]") looked identical to a real user message and triggered
|
||||
endless reply loops.
|
||||
|
||||
These tests assert the invariant: every inbound Matrix text/media message
|
||||
that the adapter dispatches via handle_message() must carry the sender's
|
||||
MXID and display name on the MessageEvent (not buried in `source`), and
|
||||
reply-targeted messages must carry the replied-to message's text and author.
|
||||
"""
|
||||
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import AsyncMock
|
||||
|
||||
import time
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
def _make_adapter(require_mention=False, auto_thread=False, monkeypatch=None):
|
||||
"""Create a MatrixAdapter with mocked config and bypassed display-name lookup."""
|
||||
# MATRIX_REQUIRE_MENTION and MATRIX_AUTO_THREAD are read once at __init__,
|
||||
# so they must be set in the environment before constructing the adapter.
|
||||
if monkeypatch is not None:
|
||||
monkeypatch.setenv("MATRIX_REQUIRE_MENTION", "true" if require_mention else "false")
|
||||
monkeypatch.setenv("MATRIX_AUTO_THREAD", "true" if auto_thread else "false")
|
||||
else:
|
||||
import os
|
||||
os.environ["MATRIX_REQUIRE_MENTION"] = "true" if require_mention else "false"
|
||||
os.environ["MATRIX_AUTO_THREAD"] = "true" if auto_thread else "false"
|
||||
|
||||
from plugins.platforms.matrix.adapter import MatrixAdapter
|
||||
|
||||
from gateway.config import PlatformConfig
|
||||
|
||||
config = PlatformConfig(
|
||||
enabled=True,
|
||||
token="syt_test_token",
|
||||
extra={
|
||||
"homeserver": "https://matrix.example.org",
|
||||
"user_id": "@hermes:example.org",
|
||||
},
|
||||
)
|
||||
adapter = MatrixAdapter(config)
|
||||
adapter._text_batch_delay_seconds = 0
|
||||
adapter.handle_message = AsyncMock()
|
||||
# Bypass mautrix state_store lookup — fall back to localpart.
|
||||
adapter._client = None
|
||||
# Stub the DM/identity lookup chain so we don't need a real mautrix
|
||||
# client. _is_allowed_matrix_room_event -> _is_dm_room ->
|
||||
# _resolve_room_identity, and _resolve_message_context ->
|
||||
# _resolve_room_identity (used to fetch chat_type).
|
||||
identity = SimpleNamespace(
|
||||
display_name="Test Room",
|
||||
room_topic=None,
|
||||
server_name="example.org",
|
||||
chat_type="dm", # DM shortcut so we bypass MATRIX_ALLOWED_ROOMS
|
||||
)
|
||||
adapter._resolve_room_identity = AsyncMock(return_value=identity)
|
||||
return adapter
|
||||
|
||||
|
||||
def _make_event(
|
||||
body,
|
||||
sender="@alice:example.org",
|
||||
event_id="$evt1",
|
||||
room_id="!room1:example.org",
|
||||
thread_id=None,
|
||||
in_reply_to_event_id=None,
|
||||
):
|
||||
"""Build a fake Matrix room message event with optional reply context."""
|
||||
content = {"body": body, "msgtype": "m.text"}
|
||||
|
||||
relates_to = {}
|
||||
if thread_id:
|
||||
relates_to["rel_type"] = "m.thread"
|
||||
relates_to["event_id"] = thread_id
|
||||
if in_reply_to_event_id:
|
||||
relates_to["m.in_reply_to"] = {"event_id": in_reply_to_event_id}
|
||||
if relates_to:
|
||||
content["m.relates_to"] = relates_to
|
||||
|
||||
return SimpleNamespace(
|
||||
sender=sender,
|
||||
event_id=event_id,
|
||||
room_id=room_id,
|
||||
# Use *recent* timestamp so we don't fall into the startup-grace
|
||||
# filter (which drops events older than `_startup_ts - 5s`). The
|
||||
# production adapter ignores real events that pre-date gateway
|
||||
# start; tests must use "now-ish" timestamps.
|
||||
timestamp=int(time.time() * 1000),
|
||||
content=content,
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Sender metadata on MessageEvent
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_text_message_carries_sender_mxid(monkeypatch):
|
||||
"""The MXID of the message author must reach MessageEvent, not just source.
|
||||
|
||||
Without this, downstream consumers cannot tell who said what.
|
||||
"""
|
||||
adapter = _make_adapter(monkeypatch=monkeypatch)
|
||||
adapter._startup_ts = time.time() - 10
|
||||
|
||||
event = _make_event("hello world", sender="@alice:example.org")
|
||||
await adapter._on_room_message(event)
|
||||
|
||||
adapter.handle_message.assert_awaited_once()
|
||||
msg = adapter.handle_message.await_args.args[0]
|
||||
assert msg.user_id == "@alice:example.org"
|
||||
# Display name fallback to localpart when no state_store is available.
|
||||
assert msg.user_name == "alice"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_text_message_carries_sender_for_different_user(monkeypatch):
|
||||
"""MXID propagation must work for arbitrary senders, not just alice."""
|
||||
adapter = _make_adapter(monkeypatch=monkeypatch)
|
||||
adapter._startup_ts = time.time() - 10
|
||||
|
||||
event = _make_event("hi from bob", sender="@bob:chat.example.org")
|
||||
await adapter._on_room_message(event)
|
||||
|
||||
msg = adapter.handle_message.await_args.args[0]
|
||||
assert msg.user_id == "@bob:chat.example.org"
|
||||
assert msg.user_name == "bob"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Reply context on MessageEvent
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_reply_carries_target_text_and_author(monkeypatch):
|
||||
"""Reply messages must carry the replied-to message's text + author MXID/name.
|
||||
|
||||
Other adapters (Signal, Slack, Telegram) already populate these fields.
|
||||
Matrix historically did not, so "[Replying to: ...]" rendered only the
|
||||
quoted text without any indicator of *who* the user was replying to.
|
||||
"""
|
||||
adapter = _make_adapter(monkeypatch=monkeypatch)
|
||||
adapter._startup_ts = time.time() - 10
|
||||
|
||||
# Reply body in Matrix is: "> <@user:server> quoted body\n\nactual reply"
|
||||
reply_body = "> <@carol:example.org> original question\n\nbecause reasons"
|
||||
event = _make_event(
|
||||
reply_body,
|
||||
sender="@dave:example.org",
|
||||
in_reply_to_event_id="$target1",
|
||||
)
|
||||
await adapter._on_room_message(event)
|
||||
|
||||
adapter.handle_message.assert_awaited_once()
|
||||
msg = adapter.handle_message.await_args.args[0]
|
||||
|
||||
# The reply target pointer must reach MessageEvent.
|
||||
assert msg.reply_to_message_id == "$target1"
|
||||
# The replied-to message's body must reach MessageEvent (stripped of
|
||||
# the "> " quote prefix). Used by gateway/run.py:16015 to render
|
||||
# [Replying to: "..."] in the LLM prompt.
|
||||
assert msg.reply_to_text is not None
|
||||
assert "original question" in msg.reply_to_text
|
||||
# The replied-to author's MXID must reach MessageEvent. Used to detect
|
||||
# "Replying to your previous message" vs "Replying to another user's message".
|
||||
assert msg.reply_to_author_id == "@carol:example.org"
|
||||
assert msg.reply_to_author_name == "carol"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_non_reply_message_has_no_reply_context(monkeypatch):
|
||||
"""A non-reply message must not spuriously set reply_to_* fields."""
|
||||
adapter = _make_adapter(monkeypatch=monkeypatch)
|
||||
adapter._startup_ts = time.time() - 10
|
||||
|
||||
event = _make_event("plain message, no reply")
|
||||
await adapter._on_room_message(event)
|
||||
|
||||
msg = adapter.handle_message.await_args.args[0]
|
||||
assert msg.reply_to_message_id is None
|
||||
assert msg.reply_to_text is None
|
||||
assert msg.reply_to_author_id is None
|
||||
assert msg.reply_to_author_name is None
|
||||
Loading…
Reference in New Issue