From b0c5f48be42d55d4382e2af4db38930bc25ab28e Mon Sep 17 00:00:00 2001 From: Don Garrett Date: Tue, 29 Sep 2026 15:59:27 +0000 Subject: [PATCH] Handle leading mentions in Matrix messages Clients put the addressee's name in front of a message that mentions it ("Nisse Dev Testing: re-plan"), and the transport passed that text on whole, so no command matched: the live run's re-plan was asked again, and an addressed 'approve' would have counted as rejection feedback. - A leading mention of the agent (an HTML pill, its user ID, or its display name followed by ':') is removed before commands are matched. The display name is learnt from the agent's own member event. - A message whose leading mention is someone else is skipped: in a shared room it is addressed to another agent or person. - Only the start counts; a mention later in the text is left alone. - The transcript records the message as sent (InboundMessage.original). The sender is unaffected: it still comes from the event's homeserver-set sender field, never from the text. Co-Authored-By: Claude Opus 5.5 (1M context) --- nisse-dev/src/nisse_dev/control.py | 10 ++- nisse-dev/src/nisse_dev/matrix.py | 81 ++++++++++++++++++++++-- nisse-dev/src/nisse_dev/transport.py | 8 ++- nisse-dev/tests/test_control.py | 14 +++++ nisse-dev/tests/test_matrix.py | 94 ++++++++++++++++++++++++++++ 5 files changed, 199 insertions(+), 8 deletions(-) diff --git a/nisse-dev/src/nisse_dev/control.py b/nisse-dev/src/nisse_dev/control.py index 342dc14..92695ce 100644 --- a/nisse-dev/src/nisse_dev/control.py +++ b/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 diff --git a/nisse-dev/src/nisse_dev/matrix.py b/nisse-dev/src/nisse_dev/matrix.py index dde3e2b..a5f114a 100644 --- a/nisse-dev/src/nisse_dev/matrix.py +++ b/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*(.*?)', 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 room.get("timeline", {}).get("events", []): + for event in self._room(sync_data).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() diff --git a/nisse-dev/src/nisse_dev/transport.py b/nisse-dev/src/nisse_dev/transport.py index c52a99b..aca214d 100644 --- a/nisse-dev/src/nisse_dev/transport.py +++ b/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)) diff --git a/nisse-dev/tests/test_control.py b/nisse-dev/tests/test_control.py index df1dfc5..6dc5665 100644 --- a/nisse-dev/tests/test_control.py +++ b/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") diff --git a/nisse-dev/tests/test_matrix.py b/nisse-dev/tests/test_matrix.py index 049651b..f127e59 100644 --- a/nisse-dev/tests/test_matrix.py +++ b/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'{label}: {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") + ] -- 2.54.0