Skip to content

Commit ff5fe2e

Browse files
authored
fix(roomlog): self means this device, not this account (#94)
Closes #93, found during the first live session of an agent following a room. An agent and the person it works for share one Matrix account. `self` compared the sender to the configured `user_id`, so it was true for both of them: ```json {"seq": 6, "type": "m.text", "sender": "@user:example.org", "self": true, "text": "… hey, claude? liest du mit?"} {"seq": 7, "type": "reaction", "sender": "@user:example.org", "self": true, "text": "… reacted 👀"} ``` One of those is the human, one is the agent, and the flag that exists to tell them apart says the same thing about both. ## Why it is not cosmetic The flag is what an agent uses to avoid answering itself. As it stood: - trusting it means ignoring the person addressing you - ignoring it means answering yourself, which with autonomous replies enabled is a loop with a homeserver in it Neither shows up in a quiet room. ## The fix A decrypted event carries `sender_key`, the curve25519 key of the *device* that encrypted it, and the daemon knows its own via `client.olm.account.identity_keys["curve25519"]`. `self` is now that comparison. An unencrypted room carries no `sender_key`. There the account comparison is the only answer available, so the record also carries `self_basis` — `"device"` or `"account"` — and a caller can tell which question was actually answered instead of assuming the stronger one. Own-device lines render as `Name (agent)`. The log is read by the agent that wrote part of it, and same account means same display name. ## Verification 6 new tests, 41 in the file, including the two that carry the change: the human's device on the same account is **not** self, and a missing `sender_key` falls back to the account while saying so. Live, against a running daemon in `#test`: ```json {"seq": 3, "self": true, "self_basis": "device", "text": "[11:25] Sebastian Mendel (agent): 🤖 Prüfung: erkennt der Log jetzt sein eigenes Gerät?"} ``` **The negative case is covered by tests only.** Confirming it live needs a message typed from the human's own device while the fixed daemon runs, which this session could not produce on its own. Saying so rather than implying the live check covered both.
2 parents b0dbbf1 + e23b482 commit ff5fe2e

3 files changed

Lines changed: 121 additions & 2 deletions

File tree

skills/matrix-communication/scripts/_lib/roomlog.py

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,11 @@ def render_text(record: dict) -> str:
5757
when = datetime.fromtimestamp(record["ts"] / 1000, tz=timezone.utc).astimezone()
5858
stamp = when.strftime("%H:%M")
5959
who = record.get("sender_display") or record["sender"]
60+
if record.get("self") and record.get("self_basis") == "device":
61+
# The log is read by the agent that wrote part of it. Without this the
62+
# agent's own lines are indistinguishable from its human's, because the
63+
# account and therefore the display name are the same.
64+
who = f"{who} (agent)"
6065
body = " ".join((record.get("body") or "").split())
6166

6267
if record["type"] == "encrypted":
@@ -74,8 +79,34 @@ def render_text(record: dict) -> str:
7479
return f"[{stamp}] {who}: {prefix}{body}"
7580

7681

77-
def build_record(*, seq: int, event: dict, own_user_id: str, own_display_name) -> dict:
82+
def _is_own_device(event: dict, own_user_id: str, own_sender_key) -> tuple[bool, str]:
83+
"""Did THIS device send the event, and on what evidence.
84+
85+
An agent and the person it works for share one Matrix account, so comparing
86+
the sender is not enough: it marks the human's messages as the agent's own.
87+
An agent that then skips "its own" messages skips the person addressing it,
88+
and one that does not can answer itself.
89+
90+
A decrypted event carries `sender_key`, the curve25519 key of the device
91+
that encrypted it, and that is the answer. An unencrypted room has no such
92+
key; the account comparison is then all there is, and the second return
93+
value says which of the two was possible so a caller never mistakes one for
94+
the other.
95+
"""
96+
if event["sender"] != own_user_id:
97+
return False, "device" if event.get(
98+
"sender_key"
99+
) and own_sender_key else "account"
100+
if event.get("sender_key") and own_sender_key:
101+
return event["sender_key"] == own_sender_key, "device"
102+
return True, "account"
103+
104+
105+
def build_record(
106+
*, seq: int, event: dict, own_user_id: str, own_display_name, own_sender_key=None
107+
) -> dict:
78108
"""One log record from one event."""
109+
is_self, basis = _is_own_device(event, own_user_id, own_sender_key)
79110
record = {
80111
"seq": seq,
81112
"ts": event["ts"],
@@ -87,7 +118,8 @@ def build_record(*, seq: int, event: dict, own_user_id: str, own_display_name) -
87118
"body": event.get("body"),
88119
"reply_to": event.get("reply_to"),
89120
"thread_root": event.get("thread_root"),
90-
"self": event["sender"] == own_user_id,
121+
"self": is_self,
122+
"self_basis": basis,
91123
"mentions_me": _mentions(event.get("body"), own_user_id, own_display_name),
92124
}
93125
if event.get("session_id"):

skills/matrix-communication/scripts/_lib/test_roomlog.py

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,5 +301,80 @@ def test_rewriting_the_bundle_does_not_duplicate_entries(self):
301301
self.assertNotIn("(b_example.org.md)", text)
302302

303303

304+
class SelfIsTheDeviceTests(unittest.TestCase):
305+
"""`self` has to mean "this device", not "this account".
306+
307+
Regression for #93: an agent and the person it works for share one Matrix
308+
account, so comparing the sender alone marks the human's messages as the
309+
agent's own. An agent that skips its own messages then skips the person
310+
addressing it; one that does not risks answering itself.
311+
"""
312+
313+
MINE = "curve25519-of-my-device"
314+
THEIRS = "curve25519-of-their-element"
315+
USER = "@shared:example.org"
316+
317+
def _record(self, sender_key, **extra):
318+
return build_record(
319+
seq=1,
320+
event={**EVENT, "sender": self.USER, "sender_key": sender_key, **extra},
321+
own_user_id=self.USER,
322+
own_display_name=None,
323+
own_sender_key=self.MINE,
324+
)
325+
326+
def test_our_own_device_is_self(self):
327+
rec = self._record(self.MINE)
328+
self.assertTrue(rec["self"])
329+
self.assertEqual(rec["self_basis"], "device")
330+
331+
def test_the_humans_device_on_the_same_account_is_not_self(self):
332+
rec = self._record(self.THEIRS)
333+
self.assertFalse(rec["self"])
334+
self.assertEqual(rec["self_basis"], "device")
335+
336+
def test_another_account_is_never_self(self):
337+
rec = build_record(
338+
seq=1,
339+
event={**EVENT, "sender": "@someone:example.org", "sender_key": self.MINE},
340+
own_user_id=self.USER,
341+
own_display_name=None,
342+
own_sender_key=self.MINE,
343+
)
344+
self.assertFalse(rec["self"])
345+
346+
def test_without_a_sender_key_it_falls_back_to_the_account_and_says_so(self):
347+
"""An unencrypted room carries no sender_key. The account comparison is
348+
then the only answer available, and the record must not pretend it is
349+
the device one."""
350+
rec = build_record(
351+
seq=1,
352+
event={**EVENT, "sender": self.USER},
353+
own_user_id=self.USER,
354+
own_display_name=None,
355+
own_sender_key=self.MINE,
356+
)
357+
self.assertTrue(rec["self"])
358+
self.assertEqual(rec["self_basis"], "account")
359+
360+
def test_without_our_own_key_it_falls_back_too(self):
361+
rec = build_record(
362+
seq=1,
363+
event={**EVENT, "sender": self.USER, "sender_key": self.THEIRS},
364+
own_user_id=self.USER,
365+
own_display_name=None,
366+
own_sender_key=None,
367+
)
368+
self.assertTrue(rec["self"])
369+
self.assertEqual(rec["self_basis"], "account")
370+
371+
def test_own_device_lines_are_marked_in_the_text(self):
372+
"""The log is read by the agent that wrote half of it."""
373+
mine = self._record(self.MINE)
374+
theirs = self._record(self.THEIRS)
375+
self.assertIn("(agent)", mine["text"])
376+
self.assertNotIn("(agent)", theirs["text"])
377+
378+
304379
if __name__ == "__main__":
305380
unittest.main(verbosity=2)

skills/matrix-communication/scripts/matrix-watchd.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,10 @@ def event_to_dict(room, event) -> dict | None:
122122
else None,
123123
"ts": getattr(event, "server_timestamp", None) or int(time.time() * 1000),
124124
"room_id": room.room_id,
125+
# The curve25519 key of the device that encrypted this, which is the
126+
# only thing that distinguishes our own messages from our human's -
127+
# they share the account. Absent in an unencrypted room.
128+
"sender_key": getattr(event, "sender_key", None),
125129
}
126130
if not base["event_id"] or not base["sender"]:
127131
return None
@@ -167,6 +171,7 @@ def __init__(self, config: dict, credentials: dict):
167171
self.started = time.time()
168172
self.last_sync = None
169173
self.display_name = None
174+
self.own_sender_key = None
170175
self.stopping = asyncio.Event()
171176
self.next_seq_by_room = {}
172177

@@ -194,6 +199,7 @@ def record_event(self, room_id: str, event_dict: dict) -> None:
194199
event=event_dict,
195200
own_user_id=self.credentials["user_id"],
196201
own_display_name=self.display_name,
202+
own_sender_key=self.own_sender_key,
197203
)
198204
append_record(path, record)
199205
self.next_seq_by_room[room_id] = seq + 1
@@ -377,6 +383,12 @@ async def run(self) -> int:
377383
name = await self.client.get_displayname()
378384
self.display_name = getattr(name, "displayname", None)
379385

386+
# Our own device's curve25519 key, read once. Without it every record
387+
# falls back to the account comparison, which cannot tell this device
388+
# from the human's on the same account.
389+
if self.client.olm:
390+
self.own_sender_key = self.client.olm.account.identity_keys["curve25519"]
391+
380392
await self.resolve_rooms()
381393
self.client.add_event_callback(self.on_event, object)
382394

0 commit comments

Comments
 (0)