From ce9332cd0283e3f79d80299b11978f4ae1850121 Mon Sep 17 00:00:00 2001 From: damocles Date: Fri, 6 Nov 2026 19:40:37 +0100 Subject: [PATCH] hive-matrix-mcp: an edited message shows once, edited in place, and is not a second unread --- docs/2-handbook/tools/matrix.md | 5 +- hive-matrix-mcp/src/handlers.rs | 236 +++++++++++++++++++++++++++++++- hive-matrix-mcp/src/mcp.rs | 5 +- 3 files changed, 237 insertions(+), 9 deletions(-) diff --git a/docs/2-handbook/tools/matrix.md b/docs/2-handbook/tools/matrix.md index b1a1dc31..4ef3f834 100644 --- a/docs/2-handbook/tools/matrix.md +++ b/docs/2-handbook/tools/matrix.md @@ -27,7 +27,10 @@ is what an agent reads at call time. They fall into four groups: - **Reading:** `read_room`, `download_file`, `list_rooms` and `list_room_members`. `read_room` shows media as `[file: …]`, `[image: …]`, `[audio: …]` or `[video: …]` markers, and the event id of one goes to - `download_file`. + `download_file`. An edited message shows once, with its latest text and + `edited: true`. The edit event doesn't appear as a second message and doesn't + count as unread. When the original is outside the window read, the edit stays + as its own event, and its `replaces_event_id` names the message it replaces. - **Room membership:** `invite_user`, `list_invites`, `resolve_invite` and `join_room`. `resolve_invite` accepts or rejects a pending invite and is the path for invites; `join_room` is for a public room the agent wasn't invited to. diff --git a/hive-matrix-mcp/src/handlers.rs b/hive-matrix-mcp/src/handlers.rs index 368a8ef1..a1720458 100644 --- a/hive-matrix-mcp/src/handlers.rs +++ b/hive-matrix-mcp/src/handlers.rs @@ -77,6 +77,73 @@ pub struct TimelineEvent { /// carries. `None` for top-level (non-reply) messages. #[serde(skip_serializing_if = "Option::is_none")] pub in_reply_to_event_id: Option, + /// Set on a message that was edited: `body` then holds the latest + /// edited text, not the text first posted. + #[serde(skip_serializing_if = "std::ops::Not::not")] + pub edited: bool, + /// Set on an edit (`m.replace`) event whose original is outside the + /// window read: the id of the message it replaces, with `body` holding + /// the new text. Edits whose original is in the window are folded into + /// it by `collapse_edits` and never appear on their own. + #[serde(skip_serializing_if = "Option::is_none")] + pub replaces_event_id: Option, +} + +/// The `(replaced event id, new body)` an edit (`m.replace`) event +/// carries, or `None` for any other event. The top-level `body` of an edit +/// is a `* `-prefixed fallback for clients that cannot render edits; the +/// text to show is in `m.new_content`. +fn replacement_of( + event: &matrix_sdk::ruma::events::AnySyncTimelineEvent, +) -> Option<(String, String)> { + use matrix_sdk::ruma::events::room::message::Relation; + use matrix_sdk::ruma::events::{AnySyncMessageLikeEvent, AnySyncTimelineEvent}; + let AnySyncTimelineEvent::MessageLike(AnySyncMessageLikeEvent::RoomMessage(ev)) = event else { + return None; + }; + match ev.as_original()?.content.relates_to.as_ref()? { + Relation::Replacement(r) => Some(( + r.event_id.to_string(), + attachment_marker(&r.new_content.msgtype), + )), + _ => None, + } +} + +/// Fold each edit into the message it replaces, so a reader sees the +/// edited text once instead of the original plus a `* ` duplicate. +/// `events` is newest first, so the first edit met for a message is its +/// latest. An edit only counts when its sender also sent the original +/// (the spec's rule); one whose original is outside the window stays as an +/// event of its own with `replaces_event_id` set. +fn collapse_edits(events: Vec) -> Vec { + let mut latest: std::collections::HashMap = std::collections::HashMap::new(); + let mut folded: std::collections::HashSet = std::collections::HashSet::new(); + for edit in &events { + let Some(target) = &edit.replaces_event_id else { + continue; + }; + let original_by_same_sender = events + .iter() + .any(|e| &e.event_id == target && e.sender == edit.sender); + if original_by_same_sender { + folded.insert(edit.event_id.clone()); + latest + .entry(target.clone()) + .or_insert_with(|| edit.body.clone()); + } + } + events + .into_iter() + .filter(|e| !folded.contains(&e.event_id)) + .map(|mut e| { + if let Some(body) = latest.remove(&e.event_id) { + e.body = body; + e.edited = true; + } + e + }) + .collect() } /// Resolve a room reference (id `!abc:server` OR alias `#name:server`) @@ -231,18 +298,21 @@ async fn recent_events( /// (of every msgtype, not just `m.room.message`'s common ones), /// reactions, state changes (join/leave/topic/name/etc. are all real /// activity an agent should see, mara: don't lump them in with -/// housekeeping noise). The one deliberate exception is `m.space.child`: +/// housekeeping noise). The deliberate exceptions are `m.space.child`: /// a periodic re-apply of an *unchanged* value still appends a timeline /// event (see [`room_unread_state`]'s doc comment), and that specific -/// redundant re-emit is the one thing this function exists to filter — +/// redundant re-emit is the thing this function exists to filter — /// an earlier version of this filter over-corrected by excluding whole -/// event categories instead of just that one type; narrower is right. +/// event categories instead of just that one type; narrower is right — +/// and an edit (`m.replace`), which restates a message already counted. fn is_intentional(event: &matrix_sdk::ruma::events::AnySyncTimelineEvent) -> bool { use matrix_sdk::ruma::events::{AnySyncStateEvent, AnySyncTimelineEvent}; + // An edit (`m.replace`) is the same message again, not a new one: it + // must not count as a second unread (which would also hold up sends). !matches!( event, AnySyncTimelineEvent::State(AnySyncStateEvent::SpaceChild(_)) - ) + ) && replacement_of(event).is_none() } /// Whether `room` carries content the agent hasn't caught up on. @@ -890,8 +960,11 @@ fn to_timeline_event( is_encrypted: bool, ) -> Option { let parsed = ev.raw().deserialize().ok()?; + let replacement = replacement_of(&parsed); let body = if is_encrypted && ev.kind.is_utd() { "[unable to decrypt]".to_owned() + } else if let Some((_, new_body)) = &replacement { + new_body.clone() } else { extract_body_sync(&parsed) }; @@ -902,6 +975,8 @@ fn to_timeline_event( event_type: parsed.event_type().to_string(), body, in_reply_to_event_id: extract_in_reply_to(&parsed), + edited: false, + replaces_event_id: replacement.map(|(target, _)| target), }) } @@ -1028,7 +1103,7 @@ pub async fn read_room( .filter_map(|ev| to_timeline_event(ev, is_encrypted)) .collect() }; - DaemonResponse::ok(&events) + DaemonResponse::ok(&collapse_edits(events)) } /// Download the media attachment carried by `event_id` in `room` and @@ -1222,8 +1297,8 @@ mod utd_body_tests { #[cfg(test)] mod unread_tests { use super::{ - ParsedEvent, attachment_marker, extract_body_sync, extract_in_reply_to, is_intentional, - unread_verdict, + ParsedEvent, TimelineEvent, attachment_marker, collapse_edits, extract_body_sync, + extract_in_reply_to, is_intentional, replacement_of, unread_verdict, }; use matrix_sdk::ruma::events::AnySyncTimelineEvent; use matrix_sdk::ruma::events::room::message::MessageType; @@ -1311,6 +1386,153 @@ mod unread_tests { assert_eq!(verdict(None, &events), None); } + fn edit(id: &str, sender: &str, target: &str, new_body: &str) -> AnySyncTimelineEvent { + parse(json!({ + "type": "m.room.message", + "event_id": id, + "sender": sender, + "origin_server_ts": 2, + "content": { + "msgtype": "m.text", + "body": format!("* {new_body}"), + "m.new_content": {"msgtype": "m.text", "body": new_body}, + "m.relates_to": {"rel_type": "m.replace", "event_id": target}, + }, + })) + } + + fn timeline(id: &str, sender: &str, body: &str, replaces: Option<&str>) -> TimelineEvent { + TimelineEvent { + event_id: id.into(), + sender: sender.into(), + origin_server_ts: 1, + event_type: "m.room.message".into(), + body: body.into(), + in_reply_to_event_id: None, + edited: false, + replaces_event_id: replaces.map(Into::into), + } + } + + #[test] + fn an_edit_carries_the_new_text_and_the_id_it_replaces() { + let ev = edit( + "$e:example.org", + "@bob:example.org", + "$o:example.org", + "fixed", + ); + assert_eq!( + replacement_of(&ev), + Some(("$o:example.org".into(), "fixed".into())) + ); + assert_eq!( + replacement_of(&message("$m:example.org", "@bob:example.org", "plain")), + None + ); + } + + #[test] + fn an_edit_is_not_a_second_unread_message() { + assert!(!is_intentional(&edit( + "$e:example.org", + "@bob:example.org", + "$o:example.org", + "fixed" + ))); + // The edit sits newest; the walk skips it and lands on the + // original, so the room reports one unread message, not two. + let events = [ + Some(live(edit( + "$e:example.org", + "@bob:example.org", + "$o:example.org", + "fixed", + ))), + Some(live(message("$o:example.org", "@bob:example.org", "typo"))), + ]; + assert_eq!( + verdict(None, &events).map(|(id, _, _)| id), + Some("$o:example.org".into()) + ); + } + + #[test] + fn an_edit_folds_into_its_original_and_the_duplicate_disappears() { + let got = collapse_edits(vec![ + timeline( + "$e:example.org", + "@bob:example.org", + "fixed", + Some("$o:example.org"), + ), + timeline("$other:example.org", "@amy:example.org", "unrelated", None), + timeline("$o:example.org", "@bob:example.org", "typo", None), + ]); + let ids: Vec<&str> = got.iter().map(|e| e.event_id.as_str()).collect(); + assert_eq!(ids, ["$other:example.org", "$o:example.org"]); + assert_eq!(got[1].body, "fixed"); + assert!(got[1].edited); + assert!(!got[0].edited); + } + + #[test] + fn the_latest_of_several_edits_wins() { + let got = collapse_edits(vec![ + timeline( + "$e2:example.org", + "@bob:example.org", + "third", + Some("$o:example.org"), + ), + timeline( + "$e1:example.org", + "@bob:example.org", + "second", + Some("$o:example.org"), + ), + timeline("$o:example.org", "@bob:example.org", "first", None), + ]); + assert_eq!(got.len(), 1); + assert_eq!(got[0].body, "third"); + } + + #[test] + fn someone_elses_edit_does_not_rewrite_a_message() { + let got = collapse_edits(vec![ + timeline( + "$e:example.org", + "@mallory:example.org", + "forged", + Some("$o:example.org"), + ), + timeline("$o:example.org", "@bob:example.org", "real", None), + ]); + assert_eq!( + got.len(), + 2, + "the forged edit stays visible as its own event" + ); + assert_eq!(got[1].body, "real"); + assert!(!got[1].edited); + } + + #[test] + fn an_edit_whose_original_is_out_of_the_window_stays_and_says_what_it_replaces() { + let got = collapse_edits(vec![timeline( + "$e:example.org", + "@bob:example.org", + "fixed", + Some("$old:example.org"), + )]); + assert_eq!(got.len(), 1); + assert_eq!( + got[0].replaces_event_id.as_deref(), + Some("$old:example.org") + ); + assert_eq!(got[0].body, "fixed"); + } + #[test] fn a_space_child_re_emit_is_skipped_to_reach_the_real_message() { let events = [ diff --git a/hive-matrix-mcp/src/mcp.rs b/hive-matrix-mcp/src/mcp.rs index 2fd51907..ff5e0646 100644 --- a/hive-matrix-mcp/src/mcp.rs +++ b/hive-matrix-mcp/src/mcp.rs @@ -462,7 +462,10 @@ impl MatrixMcp { #[tool(description = "Read events from a matrix room (default 50, max 200), \ newest first. Returns each event's id, sender, timestamp, type, \ best-effort plain-text body, and the id of the event it replies to \ - (when any). By default returns the most recent events. Pass `from` \ + (when any). An edited message appears once with its latest text and \ + `edited: true`; the edit event itself is folded in, not listed (one \ + whose original is outside the window stays, with `replaces_event_id`). \ + By default returns the most recent events. Pass `from` \ = an event id to read forward from that event (anchor + newer \ events), or `until` = an event id to read backward to it (anchor + \ older events) — e.g. to fetch the context around a reply target \