send_dm and open_dm report where the recipient stands and fail when they are not in the room. Refs #5982

docs/tools/matrix.md

@@ -35,6 +35,12 @@ is what an agent reads at call time. They fall into four groups:
   invite before it answers.
 - **Receipts:** `mark_read` advances the read receipt.
 
+**DM recipient check.** `send_dm` and `open_dm` report where the other user
+stands in the room through a `recipient` field. `joined` means they see the
+message. `invited` means the invite is pending, and the result carries a
+`warning`. When the user is in neither state, because the invite did not go
+through or they left, the call fails and sends nothing.
+
 **Unread guard.** `send_message`, `send_dm`, `send_file` and `send_reply` fail
 while the room has unread messages. The agent calls `read_room` and then
 `mark_read` on the latest event before sending to a room it hasn't read.

hive-matrix-mcp/src/handlers.rs

@@ -9,7 +9,7 @@
 //! `send_redact`, `list_rooms`, `list_room_members`, `read_room`.
 
 use matrix_sdk::{
-    Client,
+    Client, RoomMemberships,
     room::reply::{EnforceThread, Reply},
     ruma::{
         OwnedEventId, OwnedRoomId, OwnedServerName, OwnedUserId, RoomOrAliasId,
@@ -18,6 +18,7 @@ use matrix_sdk::{
             reaction::ReactionEventContent,
             receipt::{ReceiptThread, ReceiptType as LocalReceiptType},
             relation::Annotation,
+            room::member::MembershipState,
             room::message::{AddMentions, MessageType, RoomMessageEventContent},
         },
     },
@@ -403,31 +404,101 @@ pub async fn send_message(client: &Client, room_ref: &str, body: &str) -> Daemon
     }
 }
 
-/// Find the existing DM room with `user_id` or create one. Shared by
-/// `send_dm` and `open_dm`. `direct_targets()` (cached state) is
-/// used rather than the async `is_direct()`.
+/// Where the other side of a DM stands in the room.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+enum Recipient {
+    /// In the room: they see what is sent.
+    Joined,
+    /// Invited and not yet accepted: they see it only once they join.
+    Invited,
+    /// Neither (never invited, declined and left, or the invite failed):
+    /// anything sent goes to nobody.
+    Absent,
+}
+
+impl Recipient {
+    fn label(self) -> &'static str {
+        match self {
+            Self::Joined => "joined",
+            Self::Invited => "invited",
+            Self::Absent => "absent",
+        }
+    }
+}
+
+/// `uid`'s standing among `members` (the room's active members).
+fn recipient_among(
+    members: impl IntoIterator<Item = (OwnedUserId, MembershipState)>,
+    uid: &OwnedUserId,
+) -> Recipient {
+    members
+        .into_iter()
+        .find(|(member, _)| member == uid)
+        .map_or(Recipient::Absent, |(_, state)| match state {
+            MembershipState::Join => Recipient::Joined,
+            MembershipState::Invite => Recipient::Invited,
+            _ => Recipient::Absent,
+        })
+}
+
+/// Find the existing DM room with `user_id` or create one, and say where the
+/// recipient stands in it. Shared by `send_dm` and `open_dm`.
+/// `direct_targets()` (cached state) is used rather than the async
+/// `is_direct()`.
+///
+/// A DM nobody is in is an error, not a success: creating a room succeeds
+/// even when the invite to the other user did not, and a message sent there
+/// is delivered to no one.
 async fn resolve_or_create_dm(
     client: &Client,
     user_id: &str,
-) -> Result<matrix_sdk::Room, DaemonResponse> {
+) -> Result<(matrix_sdk::Room, Recipient), DaemonResponse> {
     let uid: OwnedUserId = user_id
         .parse()
         .map_err(|e| DaemonResponse::error(format!("invalid user_id {user_id}: {e}")))?;
-    if let Some(room) = client.joined_rooms().into_iter().find(|r| {
+    let room = match client.joined_rooms().into_iter().find(|r| {
         r.direct_targets()
             .iter()
             .any(|t| t.as_str() == uid.as_str())
     }) {
-        return Ok(room);
+        Some(room) => room,
+        None => client
+            .create_dm(&uid)
+            .await
+            .map_err(|e| DaemonResponse::error(format!("create_dm {uid}: {e}")))?,
+    };
+    let members = room.members(RoomMemberships::ACTIVE).await.map_err(|e| {
+        DaemonResponse::error(format!("list members of DM {}: {e}", room.room_id()))
+    })?;
+    let recipient = recipient_among(
+        members
+            .iter()
+            .map(|m| (m.user_id().to_owned(), m.membership().clone())),
+        &uid,
+    );
+    if recipient == Recipient::Absent {
+        return Err(DaemonResponse::error(format!(
+            "{uid} is not in DM room {}: the invite did not go through, or they left. Nothing was sent.",
+            room.room_id()
+        )));
     }
-    client
-        .create_dm(&uid)
-        .await
-        .map_err(|e| DaemonResponse::error(format!("create_dm {uid}: {e}")))
+    Ok((room, recipient))
+}
+
+/// The JSON fields that tell the caller where the recipient stands, with a
+/// `warning` while they have not joined yet.
+fn recipient_fields(recipient: Recipient) -> serde_json::Value {
+    let mut fields = serde_json::json!({ "recipient": recipient.label() });
+    if recipient == Recipient::Invited {
+        fields["warning"] = serde_json::json!(
+            "the recipient has been invited but has not joined; they will only see this once they accept"
+        );
+    }
+    fields
 }
 
 pub async fn send_dm(client: &Client, user_id: &str, body: &str) -> DaemonResponse {
-    let room = match resolve_or_create_dm(client, user_id).await {
+    let (room, recipient) = match resolve_or_create_dm(client, user_id).await {
         Ok(r) => r,
         Err(e) => return e,
     };
@@ -436,11 +507,15 @@ pub async fn send_dm(client: &Client, user_id: &str, body: &str) -> DaemonRespon
     }
     let content = RoomMessageEventContent::text_markdown(body);
     match room.send(content).await {
-        Ok(resp) => DaemonResponse::ok(&serde_json::json!({
-            "event_id": resp.response.event_id.to_string(),
-            "room_id": room.room_id().to_string(),
-            "user_id": user_id,
-        })),
+        Ok(resp) => {
+            let mut payload = serde_json::json!({
+                "event_id": resp.response.event_id.to_string(),
+                "room_id": room.room_id().to_string(),
+                "user_id": user_id,
+            });
+            merge(&mut payload, recipient_fields(recipient));
+            DaemonResponse::ok(&payload)
+        }
         Err(e) => DaemonResponse::error(format!("send DM to {user_id}: {e}")),
     }
 }
@@ -528,14 +603,23 @@ pub async fn send_file(
 /// tools (`send_file`, `send_message`, …) against that id — so there is
 /// no per-tool `_dm` variant.
 pub async fn open_dm(client: &Client, user_id: &str) -> DaemonResponse {
-    let room = match resolve_or_create_dm(client, user_id).await {
+    let (room, recipient) = match resolve_or_create_dm(client, user_id).await {
         Ok(r) => r,
         Err(e) => return e,
     };
-    DaemonResponse::ok(&serde_json::json!({
+    let mut payload = serde_json::json!({
         "room_id": room.room_id().to_string(),
         "user_id": user_id,
-    }))
+    });
+    merge(&mut payload, recipient_fields(recipient));
+    DaemonResponse::ok(&payload)
+}
+
+/// Copy `extra`'s keys into `target` (both JSON objects).
+fn merge(target: &mut serde_json::Value, extra: serde_json::Value) {
+    if let (Some(target), serde_json::Value::Object(extra)) = (target.as_object_mut(), extra) {
+        target.extend(extra);
+    }
 }
 
 pub async fn send_reaction(
@@ -1431,3 +1515,50 @@ mod unread_tests {
         );
     }
 }
+
+#[cfg(test)]
+mod recipient_tests {
+    use super::{Recipient, recipient_among, recipient_fields};
+    use matrix_sdk::ruma::OwnedUserId;
+    use matrix_sdk::ruma::events::room::member::MembershipState;
+
+    fn uid(id: &'static str) -> OwnedUserId {
+        OwnedUserId::try_from(id).expect("valid user id")
+    }
+
+    #[test]
+    fn a_joined_or_invited_recipient_is_reachable() {
+        let them = uid("@them:example.org");
+        let me = (uid("@me:example.org"), MembershipState::Join);
+        let joined = [me.clone(), (them.clone(), MembershipState::Join)];
+        let invited = [me, (them.clone(), MembershipState::Invite)];
+
+        assert_eq!(recipient_among(joined, &them), Recipient::Joined);
+        assert_eq!(recipient_among(invited, &them), Recipient::Invited);
+    }
+
+    #[test]
+    fn a_recipient_who_is_not_there_is_absent() {
+        let them = uid("@them:example.org");
+        let only_me = [(uid("@me:example.org"), MembershipState::Join)];
+        let left = [(them.clone(), MembershipState::Leave)];
+        let banned = [(them.clone(), MembershipState::Ban)];
+
+        assert_eq!(recipient_among(only_me, &them), Recipient::Absent);
+        assert_eq!(recipient_among(left, &them), Recipient::Absent);
+        assert_eq!(recipient_among(banned, &them), Recipient::Absent);
+    }
+
+    #[test]
+    fn only_a_pending_invite_carries_a_warning() {
+        assert_eq!(recipient_fields(Recipient::Joined)["recipient"], "joined");
+        assert!(recipient_fields(Recipient::Joined).get("warning").is_none());
+        let invited = recipient_fields(Recipient::Invited);
+        assert_eq!(invited["recipient"], "invited");
+        assert!(
+            invited["warning"]
+                .as_str()
+                .is_some_and(|w| w.contains("not joined"))
+        );
+    }
+}