From eb4ce56100d8f3475ef482d6863e58bc926fcf39 Mon Sep 17 00:00:00 2001
From: damocles <damocles@hyperhive.local>
Date: Sat, 7 Nov 2026 07:39:35 +0100
Subject: [PATCH] matrix-mcp: name a downloaded attachment by its file name,
 not its caption

---
 hive-matrix-mcp/src/handlers.rs | 68 +++++++++++++++++++++++++++------
 1 file changed, 57 insertions(+), 11 deletions(-)

diff --git a/hive-matrix-mcp/src/handlers.rs b/hive-matrix-mcp/src/handlers.rs
index a1720458..2f789ba4 100644
--- a/hive-matrix-mcp/src/handlers.rs
+++ b/hive-matrix-mcp/src/handlers.rs
@@ -1106,6 +1106,28 @@ pub async fn read_room(
     DaemonResponse::ok(&collapse_edits(events))
 }
 
+/// The media source, file name and caption of an `m.file` / `m.image` /
+/// `m.audio` / `m.video` message; `None` for any other type. The event's
+/// `body` is the caption when the sender gave one, so the name comes from
+/// `filename()` (which falls back to `body` for senders that set no
+/// `filename`).
+fn media_parts(
+    msgtype: &MessageType,
+) -> Option<(
+    matrix_sdk::ruma::events::room::MediaSource,
+    String,
+    Option<String>,
+)> {
+    let (source, name, caption) = match msgtype {
+        MessageType::File(f) => (&f.source, f.filename(), f.caption()),
+        MessageType::Image(i) => (&i.source, i.filename(), i.caption()),
+        MessageType::Audio(a) => (&a.source, a.filename(), a.caption()),
+        MessageType::Video(v) => (&v.source, v.filename(), v.caption()),
+        _ => return None,
+    };
+    Some((source.clone(), name.to_owned(), caption.map(str::to_owned)))
+}
+
 /// Download the media attachment carried by `event_id` in `room` and
 /// write it to a local file, returning the path. Resolves the event,
 /// extracts the `MediaSource` from its `m.file` / `m.image` / `m.audio`
@@ -1145,16 +1167,10 @@ pub async fn download_file(
     let Some(orig) = msg.as_original() else {
         return DaemonResponse::error(format!("event {eid} is redacted"));
     };
-    let (source, filename) = match &orig.content.msgtype {
-        MessageType::File(f) => (f.source.clone(), f.body.clone()),
-        MessageType::Image(i) => (i.source.clone(), i.body.clone()),
-        MessageType::Audio(a) => (a.source.clone(), a.body.clone()),
-        MessageType::Video(v) => (v.source.clone(), v.body.clone()),
-        _ => {
-            return DaemonResponse::error(format!(
-                "event {eid} carries no file/image/audio/video attachment"
-            ));
-        }
+    let Some((source, filename, caption)) = media_parts(&orig.content.msgtype) else {
+        return DaemonResponse::error(format!(
+            "event {eid} carries no file/image/audio/video attachment"
+        ));
     };
     let req = MediaRequestParameters {
         source,
@@ -1180,6 +1196,7 @@ pub async fn download_file(
     DaemonResponse::ok(&serde_json::json!({
         "path": dest.display().to_string(),
         "filename": filename,
+        "caption": caption,
         "bytes": bytes.len(),
         "room_id": room.room_id().to_string(),
     }))
@@ -1298,7 +1315,7 @@ mod utd_body_tests {
 mod unread_tests {
     use super::{
         ParsedEvent, TimelineEvent, attachment_marker, collapse_edits, extract_body_sync,
-        extract_in_reply_to, is_intentional, replacement_of, unread_verdict,
+        extract_in_reply_to, is_intentional, media_parts, replacement_of, unread_verdict,
     };
     use matrix_sdk::ruma::events::AnySyncTimelineEvent;
     use matrix_sdk::ruma::events::room::message::MessageType;
@@ -1659,6 +1676,35 @@ mod unread_tests {
         .expect("known msgtype")
     }
 
+    #[test]
+    fn a_captioned_attachment_is_named_by_its_file_not_its_caption() {
+        for kind in ["m.file", "m.image", "m.audio", "m.video"] {
+            let captioned = msgtype(&json!({
+                "msgtype": kind,
+                "body": "here's the screenshot of the failing step",
+                "filename": "a.png",
+                "url": "mxc://example.org/a",
+            }));
+            let (_, name, caption) = media_parts(&captioned).expect(kind);
+            assert_eq!(name, "a.png", "{kind}");
+            assert_eq!(
+                caption.as_deref(),
+                Some("here's the screenshot of the failing step"),
+                "{kind}"
+            );
+
+            // A sender that sets no `filename` puts the name in `body`.
+            let plain = msgtype(&json!({
+                "msgtype": kind,
+                "body": "b.png",
+                "url": "mxc://example.org/a",
+            }));
+            let (_, name, caption) = media_parts(&plain).expect(kind);
+            assert_eq!((name.as_str(), caption), ("b.png", None), "{kind}");
+        }
+        assert!(media_parts(&msgtype(&json!({"msgtype": "m.text", "body": "x"}))).is_none());
+    }
+
     #[test]
     fn attachments_render_as_typed_markers_and_text_as_itself() {
         let marker = |kind: &str, extra: serde_json::Value| {