feat(media): give audio and Slack video a URL the agent can actually fetch

crates/openab-core/src/discord.rs

@@ -27,6 +27,10 @@ use std::sync::LazyLock;
 use std::sync::{Arc, OnceLock};
 use tracing::{debug, error, info, warn};
 
+/// Named so a test can pin it: the agent fetches this link unaided, which is
+/// what makes a size or upload failure safe to report without a store note.
+pub(crate) const DISCORD_CDN_NOTE: &str = "Discord CDN URL, expires ~24h";
+
 /// Hard cap on consecutive bot messages in a channel or thread.
 /// Prevents runaway loops between multiple bots in "all" mode.
 const MAX_CONSECUTIVE_BOT_TURNS: u32 = 1000;
@@ -872,10 +876,12 @@ impl EventHandler for Handler {
         const TEXT_FILE_COUNT_CAP: u32 = 5;
 
         for attachment in &msg.attachments {
-            let mime = attachment.content_type.as_deref().unwrap_or("");
-            if media::is_audio_mime(mime) {
+            if let Some(mime_clean) =
+                media::audio_mime(&attachment.filename, attachment.content_type.as_deref())
+            {
+                let mime_clean = mime_clean.as_str();
+                let mut stt_line: Option<String> = None;
                 if self.stt_config.enabled {
-                    let mime_clean = mime.split(';').next().unwrap_or(mime).trim();
                     match media::download_and_transcribe(
                         &attachment.url,
                         &attachment.filename,
@@ -888,12 +894,7 @@ impl EventHandler for Handler {
                     {
                         Some(transcript) => {
                             debug!(filename = %attachment.filename, chars = transcript.len(), "voice transcript injected");
-                            extra_blocks.insert(
-                                0,
-                                ContentBlock::Text {
-                                    text: format!("[Voice message transcript]: {transcript}"),
-                                },
-                            );
+                            stt_line = Some(format!("[Voice message transcript]: {transcript}"));
                             echo_entries.push(crate::stt::EchoEntry::Success(transcript));
                         }
                         None => {
@@ -902,10 +903,47 @@ impl EventHandler for Handler {
                         }
                     }
                 } else {
-                    tracing::warn!(filename = %attachment.filename, "skipping audio attachment (STT disabled)");
+                    debug!(filename = %attachment.filename, "audio attachment not transcribed (STT disabled)");
                     let msg_ref = discord_msg_ref(&msg);
                     let _ = adapter.add_reaction(&msg_ref, "🎤").await;
                 }
+
+                // Passthrough runs whichever way STT went: a transcript is an
+                // extra block, never a substitute for the file itself.
+                #[cfg(feature = "filestore")]
+                let stored = match self.filestore {
+                    Some(ref fs) => Some(
+                        media::download_and_presign_attachment(
+                            &attachment.url,
+                            &attachment.filename,
+                            u64::from(attachment.size),
+                            Some(mime_clean),
+                            None,
+                            fs,
+                        )
+                        .await,
+                    ),
+                    None => None,
+                };
+                #[cfg(not(feature = "filestore"))]
+                let stored: Option<media::StoredAttachmentResult> = None;
+
+                let (url, note, size) = media::attachment_url_note_size(
+                    stored.as_ref(),
+                    attachment.url.as_str(),
+                    u64::from(attachment.size),
+                    media::PlatformUrl::Fetchable {
+                        note: DISCORD_CDN_NOTE,
+                    },
+                );
+                extra_blocks.extend(media::audio_attachment_blocks(
+                    &attachment.filename,
+                    mime_clean,
+                    size,
+                    Some(url),
+                    Some(&note),
+                    stt_line.as_deref(),
+                ));
             } else if media::is_text_file(&attachment.filename, attachment.content_type.as_deref())
             {
                 if text_file_count >= TEXT_FILE_COUNT_CAP {
@@ -961,11 +999,15 @@ impl EventHandler for Handler {
                     Ok(block) => {
                         debug!(url = %attachment.url, filename = %attachment.filename, "adding image attachment");
                         extra_blocks.push(block);
+                        let (safe_filename, safe_mime) = media::sanitize_attachment_meta(
+                            &attachment.filename,
+                            attachment.content_type.as_deref().unwrap_or("unknown"),
+                        );
                         extra_blocks.push(ContentBlock::Text {
                             text: format!(
                                 "[Image attachment]\nfilename: {}\ncontent_type: {}\nsize_bytes: {}\nurl: {} (expires ~24h)",
-                                attachment.filename,
-                                attachment.content_type.as_deref().unwrap_or("unknown"),
+                                safe_filename,
+                                safe_mime,
                                 attachment.size,
                                 attachment.url,
                             ),
@@ -977,11 +1019,13 @@ impl EventHandler for Handler {
                             attachment.content_type.as_deref(),
                         ) {
                             debug!(url = %attachment.url, filename = %attachment.filename, "adding video attachment link");
-                            extra_blocks.push(video_attachment_block(
+                            // Discord CDN links need no credentials, so no note is warranted.
+                            extra_blocks.push(media::video_attachment_block(
                                 &attachment.filename,
                                 attachment.content_type.as_deref(),
                                 u64::from(attachment.size),
                                 &attachment.url,
+                                None,
                             ));
                         }
                         // For all other unsupported formats (PDF, ZIP, binary, etc.):
@@ -3006,23 +3050,6 @@ fn resolve_mentions(content: &str, bot_id: UserId, allowed_role_ids: &HashSet<u6
     out.trim().to_string()
 }
 
-fn video_attachment_block(
-    filename: &str,
-    content_type: Option<&str>,
-    size: u64,
-    url: &str,
-) -> ContentBlock {
-    ContentBlock::Text {
-        text: format!(
-            "[Video attachment]\nfilename: {}\ncontent_type: {}\nsize_bytes: {}\nurl: {}",
-            filename,
-            content_type.unwrap_or("unknown"),
-            size,
-            url
-        ),
-    }
-}
-
 /// Build a `SenderContext` for Discord messages.
 ///
 /// Pure function extracted from `EventHandler::message` for testability.
@@ -3267,6 +3294,65 @@ fn truncate_to_utf16_budget(body: &str, prefix: &str, suffix: &str, limit: usize
 
 #[cfg(test)]
 mod tests {
+
+    /// The four store outcomes this adapter can hand the agent. Pins the note
+    /// constant too: an Accepted Residual Risk rests on its exact wording.
+    #[test]
+    fn discord_renders_every_store_outcome_and_only_hides_a_safe_failure() {
+        use crate::media::{AudioStoreError, PlatformUrl, StoredAttachment};
+        // Pinned literally: the rows below compare against the constant, so an
+        // emptied or repurposed value would satisfy them tautologically.
+        assert_eq!(super::DISCORD_CDN_NOTE, "Discord CDN URL, expires ~24h");
+        let platform = PlatformUrl::Fetchable {
+            note: super::DISCORD_CDN_NOTE,
+        };
+        let call = |stored: Option<&Result<StoredAttachment, AudioStoreError>>| {
+            let (u, n, sz) = crate::media::attachment_url_note_size(
+                stored,
+                "https://platform.example/file",
+                1_024,
+                platform,
+            );
+            (u.to_string(), n, sz)
+        };
+
+        // Stored: the presigned url wins, and so does the count measured while
+        // streaming, because Discord's reported size is advisory.
+        let ok = Ok(StoredAttachment {
+            url: "https://s3.example/presigned".into(),
+            note: "presigned URL, expires in 60 minutes".into(),
+            measured_bytes: 5_242_880,
+        });
+        let (u, n, sz) = call(Some(&ok));
+        assert_eq!(u, "https://s3.example/presigned");
+        assert_eq!(n, "presigned URL, expires in 60 minutes");
+        assert_eq!(sz, 5_242_880, "the measured count must win over 1_024");
+
+        // No filestore configured at all.
+        let (u, n, sz) = call(None);
+        assert_eq!(u, "https://platform.example/file");
+        assert_eq!(n, super::DISCORD_CDN_NOTE);
+        assert_eq!(sz, 1_024);
+
+        for err in [AudioStoreError::TooLarge, AudioStoreError::UploadFailed] {
+            let (u, n, _) = call(Some(&Err(err)));
+            assert_eq!(u, "https://platform.example/file", "{err:?}");
+            assert_eq!(
+                n,
+                super::DISCORD_CDN_NOTE,
+                "the CDN link still works, so no store note: {err:?}"
+            );
+        }
+
+        // DownloadFailed is the one outcome produced only after the bot fetched
+        // this very url and failed, so it may never read as a working url.
+        let (u, n, _) = call(Some(&Err(AudioStoreError::DownloadFailed)));
+        assert_eq!(u, "https://platform.example/file");
+        assert!(
+            n.contains("did not return the bytes"),
+            "DownloadFailed must say the platform withheld the bytes: {n}"
+        );
+    }
     use super::*;
     use crate::bot_turns::{TurnResult, HARD_BOT_TURN_LIMIT, BOT_TURN_LIMIT_WARNING_PREFIX};
 
@@ -3565,26 +3651,6 @@ mod tests {
         assert_eq!(result, "check @(role)");
     }
 
-    #[test]
-    fn video_attachment_block_includes_actionable_metadata() {
-        let block = video_attachment_block(
-            "demo.mp4",
-            Some("video/mp4"),
-            12345,
-            "https://cdn.discordapp.com/attachments/demo.mp4",
-        );
-
-        let ContentBlock::Text { text } = block else {
-            panic!("video attachments must be forwarded as text metadata");
-        };
-
-        assert!(text.contains("[Video attachment]"));
-        assert!(text.contains("filename: demo.mp4"));
-        assert!(text.contains("content_type: video/mp4"));
-        assert!(text.contains("size_bytes: 12345"));
-        assert!(text.contains("url: https://cdn.discordapp.com/attachments/demo.mp4"));
-    }
-
     #[test]
     fn image_attachment_block_includes_url_and_metadata() {
         // Simulates the format string used in the image attachment handler.