Handle leading mentions in Matrix messages
CI / checks (push) Skipped
CI / checks (pull_request) Successful in 1m24s
CI / checks (push) Skipped
CI / checks (pull_request) Successful in 1m24s
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) <noreply@anthropic.com>
This commit is contained in:
1 parent
9620404a2a
commit
b0c5f48be4
5 files changed
+199
-8
No files matched your search
@@ -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
|
||||
|
||||
@@ -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 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()
|
||||
@@ -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))
|
||||
@@ -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")
|
||||
|
||||
@@ -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")
|
||||
]
|
||||
Reference in new issue
Block a user