From e01b795d5d54e6e34e9d91078baed4fa7203f117 Mon Sep 17 00:00:00 2001 From: damocles Date: Mon, 2 Nov 2026 21:50:21 +0100 Subject: [PATCH] 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 | 6 ++ hive-matrix-mcp/src/handlers.rs | 171 ++++++++++++++++++++++++++++---- 2 files changed, 157 insertions(+), 20 deletions(-) diff --git a/docs/tools/matrix.md b/docs/tools/matrix.md index 1d60e6d4..f5b87644 100644 --- a/docs/tools/matrix.md +++ b/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. diff --git a/hive-matrix-mcp/src/handlers.rs b/hive-matrix-mcp/src/handlers.rs index 0ba507f9..368a8ef1 100644 --- a/hive-matrix-mcp/src/handlers.rs +++ b/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, + 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 { +) -> 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")) + ); + } +}