fix(#3072): stop waking every agent on a re-applied m.space.child state event

hive-matrix-mcp/src/handlers.rs

@@ -193,24 +193,52 @@ async fn unread_guard(client: &Client, room: &matrix_sdk::Room) -> Option<Daemon
     )))
 }
 
-/// Fetch the single most recent timeline event in `room`, of any type
-/// (redactions/state/reactions included — identity is what
-/// `room_unread_state` needs, not content). `None` for a genuinely
+/// Timeline events to scan backward, per room, when hunting for the
+/// latest event that carries actor intent (see [`is_intentional`])
+/// before giving up and answering conservatively. Bounds the walk a
+/// chatty periodic state re-emit (the prior `m.space.child` PUT-always-
+/// appends-an-event fix, since resolved on the forge tracker: `m.space.child`
+/// PUT'd unconditionally on an unchanged value, still appending a timeline
+/// event) would otherwise force — see [`room_unread_state`] for how the
+/// bound is used and why it doesn't need to be exact.
+const UNREAD_LOOKBACK: u32 = 50;
+
+/// Fetch up to [`UNREAD_LOOKBACK`] timeline events in `room`, newest
+/// first, of any type (redactions/state/reactions included —
+/// `room_unread_state` filters by type itself). Empty for a genuinely
 /// empty room or on any request failure.
-async fn latest_event(
+async fn recent_events(
     client: &Client,
     room: &matrix_sdk::Room,
-) -> Option<(OwnedEventId, OwnedUserId, String)> {
+) -> Vec<matrix_sdk::ruma::events::AnyTimelineEvent> {
     use matrix_sdk::ruma::api::Direction;
     use matrix_sdk::ruma::api::client::message::get_message_events;
     let mut req =
         get_message_events::v3::Request::new(room.room_id().to_owned(), Direction::Backward);
-    req.limit = matrix_sdk::ruma::UInt::from(1u32);
-    let resp = client.send(req).await.ok()?;
-    let raw = resp.chunk.first()?;
-    let ev = raw.deserialize().ok()?;
-    let body = extract_body(&ev);
-    Some((ev.event_id().to_owned(), ev.sender().to_owned(), body))
+    req.limit = matrix_sdk::ruma::UInt::from(UNREAD_LOOKBACK);
+    let Ok(resp) = client.send(req).await else {
+        return Vec::new();
+    };
+    resp.chunk
+        .iter()
+        .filter_map(|raw| raw.deserialize().ok())
+        .collect()
+}
+
+/// Whether `event` carries actor intent, as opposed to noise nobody
+/// "did" anything to produce. Everything counts by default — messages
+/// (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`:
+/// 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 —
+/// an earlier version of this filter over-corrected by excluding whole
+/// event categories instead of just that one type; narrower is right.
+fn is_intentional(event: &matrix_sdk::ruma::events::AnyTimelineEvent) -> bool {
+    use matrix_sdk::ruma::events::{AnyStateEvent, AnyTimelineEvent};
+    !matches!(event, AnyTimelineEvent::State(AnyStateEvent::SpaceChild(_)))
 }
 
 /// Whether `room` carries content the agent hasn't caught up on.
@@ -232,21 +260,22 @@ async fn latest_event(
 /// Self-authored latest events are never "unread" — after a daemon
 /// rebuild the read receipt can lag behind the agent's own just-sent
 /// message, which must not self-wake it.
+///
+/// Walks backward through up to [`UNREAD_LOOKBACK`] events (not just the
+/// newest one) looking for the first [`is_intentional`] hit, skipping a
+/// chatty state re-emit nobody "posted" instead of treating it as gospel.
+/// See the three branches below for what each outcome means.
 async fn room_unread_state(
     client: &Client,
     room: &matrix_sdk::Room,
 ) -> Option<(OwnedEventId, OwnedUserId, String)> {
     let own_user_id = client.user_id()?;
-    let (event_id, sender, body) = latest_event(client, room).await?;
-    if sender.as_str() == own_user_id.as_str() {
-        return None;
-    }
     // `load_user_receipt` takes `ruma::events::receipt::ReceiptType`
     // (imported here as `LocalReceiptType`), a distinct type from the
     // `ReceiptType` this module already imports for `send_single_receipt`
     // (`ruma::api::client::receipt::create_receipt::v3::ReceiptType`) — two
     // same-named enums from different ruma crates, not interchangeable.
-    let read = room
+    let read_event_id = room
         .load_user_receipt(
             LocalReceiptType::Read,
             ReceiptThread::Unthreaded,
@@ -254,11 +283,47 @@ async fn room_unread_state(
         )
         .await
         .ok()
-        .flatten();
-    if read.is_some_and(|(id, _)| id == event_id) {
+        .flatten()
+        .map(|(id, _)| id);
+
+    let events = recent_events(client, room).await;
+    for event in &events {
+        if read_event_id.as_deref() == Some(event.event_id()) {
+            // Scanned back to the agent's own read receipt without
+            // hitting anything intentional first — proven read, not
+            // guessed: there cannot be an unread intentional event in a
+            // range we've fully walked.
+            return None;
+        }
+        if !is_intentional(event) {
+            continue;
+        }
+        if event.sender().as_str() == own_user_id.as_str() {
+            return None;
+        }
+        return Some((
+            event.event_id().to_owned(),
+            event.sender().to_owned(),
+            extract_body(event),
+        ));
+    }
+    // Exhausted the lookback window without hitting the receipt or an
+    // intentional event — genuinely unknown (the receipt is further
+    // back than we scanned). Falls back to the pre-fix behaviour
+    // (surface the newest event, whatever its type) rather than
+    // guessing "read": a false "unread" just costs a wasted wake, a
+    // false "read" risks silently swallowing a real message. Expected
+    // to be rare now that the state-churn source this fix targets is
+    // itself bounded.
+    let newest = events.first()?;
+    if newest.sender().as_str() == own_user_id.as_str() {
         return None;
     }
-    Some((event_id, sender, body))
+    Some((
+        newest.event_id().to_owned(),
+        newest.sender().to_owned(),
+        extract_body(newest),
+    ))
 }
 
 pub async fn send_message(client: &Client, room_ref: &str, body: &str) -> DaemonResponse {