Merge pull request 'Handle leading mentions in Matrix messages' (#60) from matrix-mentions into main

nisse-dev/src/nisse_dev/control.py

@@ -314,7 +314,10 @@ class Orchestrator:
 
     def _record(self, msg: InboundMessage) -> None:
         self._store.append_event(
-            UserMessage(text=msg.text.strip(), source=f"transport:{msg.sender}")
+            UserMessage(
+                text=(msg.original or msg.text).strip(),
+                source=f"transport:{msg.sender}",
+            )
         )
 
     def _finish_stop(self, msg: InboundMessage) -> None:
@@ -356,7 +359,10 @@ class Orchestrator:
         # turns see feedback and answers as conversation rather than as
         # out-of-band parameters (A10).
         self._store.append_event(
-            UserMessage(text=msg.text.strip(), source=f"transport:{msg.sender}")
+            UserMessage(
+                text=(msg.original or msg.text).strip(),
+                source=f"transport:{msg.sender}",
+            )
         )
         state = self._store.get_state()
         current = state.state if state else IDLE

nisse-dev/src/nisse_dev/matrix.py

@@ -11,15 +11,26 @@ loop backs off (capped) and retries indefinitely — an outage delays reactions
 but never fails anything.
 """
 
+import html
+import re
 import threading
 import uuid
 from collections.abc import Callable
 from dataclasses import dataclass
+from urllib.parse import unquote
 
 import httpx
 
 from nisse_dev.transport import InboundMessage
 
+# A mention pill at the start of an HTML body: the link holds the user ID,
+# the link text is what the plain body starts with.
+_LEADING_PILL = re.compile(
+    r'\s*<a href="https://matrix\.to/#/([^"?]+)[^"]*">(.*?)</a>', re.DOTALL
+)
+# A user ID typed at the start of a plain body, e.g. "@alice:example: ...".
+_LEADING_USER_ID = re.compile(r"\s*(@[^\s:]+:[^\s:]+)")
+
 
 class TransportError(RuntimeError):
     pass
@@ -50,6 +61,9 @@ class MatrixTransport:
             transport=http_transport,
         )
         self._callback: Callable[[InboundMessage], None] | None = None
+        # The agent's display name in the room, learnt from its own member
+        # event; clients write it in front of messages addressed to it.
+        self._display_name: str | None = None
         self._stop = threading.Event()
         self._thread: threading.Thread | None = None
 
@@ -119,24 +133,83 @@ class MatrixTransport:
                 continue
             backoff = 1.0
             data = response.json()
+            self._learn_display_name(data)
             if since is not None:
                 self._deliver(data)
             since = data.get("next_batch", since)
 
+    def _room(self, sync_data: dict) -> dict:
+        return sync_data.get("rooms", {}).get("join", {}).get(self._config.room_id, {})
+
+    def _learn_display_name(self, sync_data: dict) -> None:
+        room = self._room(sync_data)
+        events = room.get("state", {}).get("events", []) + room.get("timeline", {}).get(
+            "events", []
+        )
+        for event in events:
+            if (
+                event.get("type") == "m.room.member"
+                and event.get("state_key") == self._config.user_id
+            ):
+                name = event.get("content", {}).get("displayname")
+                if name:
+                    self._display_name = name
+
     def _deliver(self, sync_data: dict) -> None:
-        room = sync_data.get("rooms", {}).get("join", {}).get(self._config.room_id, {})
+        for event in self._room(sync_data).get("timeline", {}).get("events", []):
-        for event in room.get("timeline", {}).get("events", []):
             if event.get("type") != "m.room.message":
                 continue
             sender = event.get("sender", "")
             if sender == self._config.user_id:
                 continue
-            body = event.get("content", {}).get("body")
+            content = event.get("content", {})
+            body = content.get("body")
             if not body:
                 continue
+            addressee, text = self._addressing(content)
+            if addressee is not None and addressee != self._config.user_id:
+                # Addressed to someone else: in a shared room, "Nisse Ops:
+                # approve" must not reach this agent as rejection feedback.
+                continue
             assert self._callback is not None
-            self._callback(InboundMessage(sender=sender, text=body))
+            self._callback(
+                InboundMessage(
+                    sender=sender,
+                    text=text,
+                    original=body if text != body else None,
+                )
+            )
+
+    def _addressing(self, content: dict) -> tuple[str | None, str]:
+        """Who a leading mention addresses, and the body without a leading
+        mention of this agent. Only the start of the message counts: a
+        mention later on ("approve, cc @alice") is part of the text. The
+        sender is never read from here; it comes from the event (A4)."""
+        body: str = content["body"]
+        own = self._config.user_id
+        if content.get("format") == "org.matrix.custom.html":
+            pill = _LEADING_PILL.match(content.get("formatted_body", ""))
+            if pill:
+                addressee = unquote(pill.group(1))
+                label = html.unescape(re.sub(r"<[^>]+>", "", pill.group(2)))
+                if addressee == own and label and body.startswith(label):
+                    return own, _after_prefix(body, len(label))
+                return addressee, body
+        if body.startswith(own):
+            return own, _after_prefix(body, len(own))
+        name = self._display_name
+        if name and body.startswith(name) and body[len(name) :].startswith(":"):
+            return own, _after_prefix(body, len(name))
+        typed = _LEADING_USER_ID.match(body)
+        if typed and body[typed.end() :].startswith(":"):
+            return typed.group(1), body
+        return None, body
 
     def close(self) -> None:
         self.stop()
         self._http.close()
+
+
+def _after_prefix(body: str, length: int) -> str:
+    """The body past a leading mention and the ':' clients put after it."""
+    return body[length:].lstrip().removeprefix(":").strip()

nisse-dev/src/nisse_dev/transport.py

@@ -13,7 +13,11 @@ from typing import Protocol
 @dataclass(frozen=True)
 class InboundMessage:
     sender: str
+    # What commands are matched against: a leading mention of the agent,
+    # which only says who the message is for, is already removed.
     text: str
+    # The message as sent, when that differs from text; the record keeps it.
+    original: str | None = None
 
 
 @dataclass(frozen=True)
@@ -51,6 +55,6 @@ class InMemoryTransport:
     def on_message(self, callback: Callable[[InboundMessage], None]) -> None:
         self._callback = callback
 
-    def receive(self, sender: str, text: str) -> None:
+    def receive(self, sender: str, text: str, original: str | None = None) -> None:
         assert self._callback is not None, "no message handler registered"
-        self._callback(InboundMessage(sender, text))
+        self._callback(InboundMessage(sender, text, original))

nisse-dev/tests/test_control.py

@@ -4,6 +4,7 @@ from pathlib import Path
 
 import pytest
 
+from nisse_dev.canonical import UserMessage
 from nisse_dev.control import (
     AWAIT_ARCH_SIGNOFF,
     AWAIT_INPUT,
@@ -125,6 +126,19 @@ def test_start_milestone_requires_approval_first(h: Harness) -> None:
     assert h.state == EXECUTING
 
 
+def test_a_message_is_matched_as_stripped_but_recorded_as_sent(
+    h: Harness,
+) -> None:
+    h.transport.receive("don", "use demo")
+    h.transport.receive("don", "approve", original="Nisse Dev: approve")
+    assert h.state == AWAIT_MILESTONE_SIGNOFF
+    recorded = [
+        e.event.text for e in h.store.read_events() if isinstance(e.event, UserMessage)
+    ]
+    assert "Nisse Dev: approve" in recorded
+    assert "approve" not in recorded
+
+
 def test_status_reports_state_and_gate(h: Harness) -> None:
     h.transport.receive("don", "use demo")
     h.transport.receive("don", "status")

nisse-dev/tests/test_matrix.py

@@ -143,3 +143,97 @@ def test_start_without_handler_raises() -> None:
     )
     with pytest.raises(TransportError, match="no message handler"):
         transport.start()
+
+
+# --- leading mentions --------------------------------------------------------
+
+
+def own_member_state(name: str) -> dict:
+    """The first sync's room state, which carries the agent's display name."""
+    body = sync_body([], "b1")
+    body["rooms"]["join"]["!room:test"]["state"] = {
+        "events": [
+            {
+                "type": "m.room.member",
+                "state_key": "@nisse-dev:test",
+                "sender": "@nisse-dev:test",
+                "content": {"membership": "join", "displayname": name},
+            }
+        ]
+    }
+    return body
+
+
+def pill_event(sender: str, user_id: str, label: str, rest: str) -> dict:
+    """A message as Element sends one that starts by mentioning user_id."""
+    return {
+        "type": "m.room.message",
+        "sender": sender,
+        "content": {
+            "msgtype": "m.text",
+            "body": f"{label}: {rest}",
+            "format": "org.matrix.custom.html",
+            "formatted_body": (
+                f'<a href="https://matrix.to/#/{user_id}">{label}</a>: {rest}'
+            ),
+        },
+    }
+
+
+def test_a_leading_pill_for_the_agent_is_removed_before_matching() -> None:
+    responses = [
+        sync_body([], "b1"),
+        sync_body(
+            [pill_event("@don:test", "@nisse-dev:test", "Nisse Dev", "approve")],
+            "b2",
+        ),
+    ]
+    [msg] = collect_messages(responses, expected=1)
+    assert msg.sender == "@don:test"  # from the event, never from the text
+    assert msg.text == "approve"
+    assert msg.original == "Nisse Dev: approve"
+
+
+def test_the_agents_display_name_typed_in_front_is_removed() -> None:
+    responses = [
+        own_member_state("Nisse Dev"),
+        sync_body([message_event("@don:test", "Nisse Dev: re-plan")], "b2"),
+    ]
+    [msg] = collect_messages(responses, expected=1)
+    assert msg.text == "re-plan"
+
+
+def test_the_agents_user_id_typed_in_front_is_removed() -> None:
+    responses = [
+        sync_body([], "b1"),
+        sync_body([message_event("@don:test", "@nisse-dev:test: status")], "b2"),
+    ]
+    [msg] = collect_messages(responses, expected=1)
+    assert msg.text == "status"
+
+
+def test_messages_addressed_to_someone_else_are_skipped() -> None:
+    responses = [
+        sync_body([], "b1"),
+        sync_body(
+            [
+                pill_event("@don:test", "@nisse-ops:test", "Nisse Ops", "approve"),
+                message_event("@don:test", "@alice:test: looks good"),
+                message_event("@don:test", "status"),
+            ],
+            "b2",
+        ),
+    ]
+    received = collect_messages(responses, expected=1)
+    assert received == [InboundMessage(sender="@don:test", text="status")]
+
+
+def test_a_mention_later_in_the_message_is_left_alone() -> None:
+    responses = [
+        sync_body([], "b1"),
+        sync_body([message_event("@don:test", "approve, cc @alice:test")], "b2"),
+    ]
+    received = collect_messages(responses, expected=1)
+    assert received == [
+        InboundMessage(sender="@don:test", text="approve, cc @alice:test")
+    ]